[PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Fabrice Fontaine
The SVN_LIB_MACHO_ITERATE macro contains an AC_RUN_IFELSE test that
doesn't work when cross-compiling. However, this macro is related to
testing Mac OS X APIs, so in the context of Buildroot, we don't care,
and the test program is not even going to build. So we simply
workaround this by setting the action-if-cross-compiling to :.

Signed-off-by: Thomas Petazzoni <[hidden email]>
[Retrieved (and slightly updated following the suggestion of Nathan
Hartman) from:
https://git.buildroot.net/buildroot/tree/package/subversion/0002-workaround-ac-run-ifelse.patch]
Signed-off-by: Fabrice Fontaine <[hidden email]>
---
Changes v1 -> v2 (after review of Nathan Hartamn):
 - Set action-if-cross-compiling instead of replacing AC_RUN_IFELSE by
   AC_COMPILE_IFELSE

 build/ac-macros/macosx.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build/ac-macros/macosx.m4 b/build/ac-macros/macosx.m4
index 92fa58e0bc..1f2aac5370 100644
--- a/build/ac-macros/macosx.m4
+++ b/build/ac-macros/macosx.m4
@@ -38,7 +38,7 @@ AC_DEFUN(SVN_LIB_MACHO_ITERATE,
     AC_MSG_RESULT([yes])
   ],[
     AC_MSG_RESULT([no])
-  ])
+  ],[:])
 ])
 
 dnl SVN_LIB_MACOS_PLIST
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Daniel Shahaf-2
Fabrice Fontaine wrote on Sun, 30 Aug 2020 17:24 +0200:
> The SVN_LIB_MACHO_ITERATE macro contains an AC_RUN_IFELSE test that
> doesn't work when cross-compiling.

Don't say "doesn't work"; describe the symptoms, error messages, etc..

> Signed-off-by: Thomas Petazzoni <[hidden email]>
> [Retrieved (and slightly updated following the suggestion of Nathan Hartman) from:
> https://git.buildroot.net/buildroot/tree/package/subversion/0002-workaround-ac-run-ifelse.patch]

We use different rfc822esque header names.

The URL will break as soon as you package a Subversion release to
which this patch will have been merged.

The outer parenthetical implies the v2 patch is the v1 patch plus
a "slight update" courtesy of Nathan.  That implication is not accurate.

To address all these issues, how about:

[[[
Patch by: Fabrice Fontaine

Inspired by: Thomas Petazzoni
(See https://git.buildroot.net/buildroot/tree/package/subversion/0002-workaround-ac-run-ifelse.patch?h=2020.08-rc3)
]]]

Feel free to add "Suggested by" and "Review by" lines as you see fit.
Email addresses in the header values are supported, but not required.

("Inspired by" is supported by contribulyzer.)

> Signed-off-by: Fabrice Fontaine <[hidden email]>
> ---

> +++ b/build/ac-macros/macosx.m4
> @@ -38,7 +38,7 @@ AC_DEFUN(SVN_LIB_MACHO_ITERATE,
>      AC_MSG_RESULT([yes])
>    ],[
>      AC_MSG_RESULT([no])
> -  ])
> +  ],[:])

Isn't that going to DTWT when cross-compiling for a macosx target?  In
that case SVN_HAVE_MACHO_ITERATE should possibly be defined, but won't be.

>  ])
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Fabrice Fontaine
Le lun. 31 août 2020 à 04:40, Daniel Shahaf <[hidden email]> a écrit :
>
> Fabrice Fontaine wrote on Sun, 30 Aug 2020 17:24 +0200:
> > The SVN_LIB_MACHO_ITERATE macro contains an AC_RUN_IFELSE test that
> > doesn't work when cross-compiling.
>
> Don't say "doesn't work"; describe the symptoms, error messages, etc..
OK, I'll write the full error message.

>
> > Signed-off-by: Thomas Petazzoni <[hidden email]>
> > [Retrieved (and slightly updated following the suggestion of Nathan Hartman) from:
> > https://git.buildroot.net/buildroot/tree/package/subversion/0002-workaround-ac-run-ifelse.patch]
>
> We use different rfc822esque header names.
>
> The URL will break as soon as you package a Subversion release to
> which this patch will have been merged.
>
> The outer parenthetical implies the v2 patch is the v1 patch plus
> a "slight update" courtesy of Nathan.  That implication is not accurate.
>
> To address all these issues, how about:
>
> [[[
> Patch by: Fabrice Fontaine
>
> Inspired by: Thomas Petazzoni
> (See https://git.buildroot.net/buildroot/tree/package/subversion/0002-workaround-ac-run-ifelse.patch?h=2020.08-rc3)
> ]]]
>
> Feel free to add "Suggested by" and "Review by" lines as you see fit.
> Email addresses in the header values are supported, but not required.
OK will be in v3

>
> ("Inspired by" is supported by contribulyzer.)
>
> > Signed-off-by: Fabrice Fontaine <[hidden email]>
> > ---
>
> > +++ b/build/ac-macros/macosx.m4
> > @@ -38,7 +38,7 @@ AC_DEFUN(SVN_LIB_MACHO_ITERATE,
> >      AC_MSG_RESULT([yes])
> >    ],[
> >      AC_MSG_RESULT([no])
> > -  ])
> > +  ],[:])
>
> Isn't that going to DTWT when cross-compiling for a macosx target?  In
> that case SVN_HAVE_MACHO_ITERATE should possibly be defined, but won't be.
I can add an AC_COMPILE_IFELSE mimicking the AC_RUN_IFELSE in the
action-if-cross-compiling if you prefer this solution.
>
> >  ])
Best Regards,

Fabrice
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Daniel Shahaf-2
Fabrice Fontaine wrote on Mon, 31 Aug 2020 15:34 +0200:

> Le lun. 31 août 2020 à 04:40, Daniel Shahaf <[hidden email]> a écrit :
> > Fabrice Fontaine wrote on Sun, 30 Aug 2020 17:24 +0200:  
> > > +++ b/build/ac-macros/macosx.m4
> > > @@ -38,7 +38,7 @@ AC_DEFUN(SVN_LIB_MACHO_ITERATE,
> > >      AC_MSG_RESULT([yes])
> > >    ],[
> > >      AC_MSG_RESULT([no])
> > > -  ])
> > > +  ],[:])  
> >
> > Isn't that going to DTWT when cross-compiling for a macosx target?  In
> > that case SVN_HAVE_MACHO_ITERATE should possibly be defined, but won't be.  
> I can add an AC_COMPILE_IFELSE mimicking the AC_RUN_IFELSE in the
> action-if-cross-compiling if you prefer this solution.

I suspect this wouldn't be correct, since if it were correct, whoever
wrote the existing AC_RUN_IFELSE() call would have used
AC_COMPILE_IFELSE() instead.

I assumed we'd need to add a way for cross-builds to explicitly set the
result of this check (e.g., by a new --with-* option).  However, I'm
not an OS X user, so there may be another solution I'm unaware of.

Could an OS X user chime in, please?

Cheers,

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Thomas Petazzoni-2
Hello,

On Wed, 2 Sep 2020 02:01:11 +0000
Daniel Shahaf <[hidden email]> wrote:

> I suspect this wouldn't be correct, since if it were correct, whoever
> wrote the existing AC_RUN_IFELSE() call would have used
> AC_COMPILE_IFELSE() instead.
>
> I assumed we'd need to add a way for cross-builds to explicitly set the
> result of this check (e.g., by a new --with-* option).  However, I'm
> not an OS X user, so there may be another solution I'm unaware of.

Usually when an AC_RUN_IFELSE() test is needed, and you're
cross-compiling, the typical way to get around that is using an
autoconf cache variable, with which the user can tell the configure
script how the target is going to behave for that particular test.

You would typically enclose the AC_RUN_IFELSE() test inside an
AC_CACHE_CHECK(), see
https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Caching-Results.html.
The AC_RUN_IFELSE() test would therefore only be executed if the
autoconf cache variable isn't passed.

Best regards,

Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Nathan Hartman
In reply to this post by Daniel Shahaf-2
On Tue, Sep 1, 2020 at 10:01 PM Daniel Shahaf <[hidden email]> wrote:

>
> Fabrice Fontaine wrote on Mon, 31 Aug 2020 15:34 +0200:
> > Le lun. 31 août 2020 à 04:40, Daniel Shahaf <[hidden email]> a écrit :
> > > Fabrice Fontaine wrote on Sun, 30 Aug 2020 17:24 +0200:
> > > > +++ b/build/ac-macros/macosx.m4
> > > > @@ -38,7 +38,7 @@ AC_DEFUN(SVN_LIB_MACHO_ITERATE,
> > > >      AC_MSG_RESULT([yes])
> > > >    ],[
> > > >      AC_MSG_RESULT([no])
> > > > -  ])
> > > > +  ],[:])
> > >
> > > Isn't that going to DTWT when cross-compiling for a macosx target?  In
> > > that case SVN_HAVE_MACHO_ITERATE should possibly be defined, but won't be.
> > I can add an AC_COMPILE_IFELSE mimicking the AC_RUN_IFELSE in the
> > action-if-cross-compiling if you prefer this solution.
>
> I suspect this wouldn't be correct, since if it were correct, whoever
> wrote the existing AC_RUN_IFELSE() call would have used
> AC_COMPILE_IFELSE() instead.

That would be brane.

Looking at the history of build/ac-macros/macosx.m4, this dyld test
originally (in r1381880) used AC_COMPILE_IFELSE. In r1413467, it is
changed to actually run the test:

[[[
r1413467 | brane | 2012-11-25 22:04:27 -0500 (Sun, 25 Nov 2012) | 8 lines

Fix the MacOS-specific autoconf macros that just appeared to work sort of
by accident.

* build/ac-macros/macosx.m4 (SVN_LIB_MACHO_ITERATE): Actually run the test
   program, and use the IFELSE part to set results.
  (SVN_LIB_MACOS_PLIST, SVN_LIB_MACOS_KEYCHAIN): Make the tests independent,
   and use AC_COMPILE_IFELSE correctly.
]]]

Looks like compiling alone to test the existence of headers and APIs
'mach-o/dyld.h', 'mach-o/loader.h', _dyld_get_image_header(), and
_dyld_get_image_name() is insufficient. I haven't yet dug further into
these APIs to learn under what circumstances they exist but shouldn't
be called. SVN_HAVE_MACHO_ITERATE allows svn_sysinfo__loaded_libs() to
list loaded shared libraries for 'svn --version --verbose'. It's
important we don't break that, as it will be more difficult to help
users when this information is needed.

Thomas's suggestion to wrap it in an AC_CACHE_CHECK() sounds like an
approach that should fix cross-compilation for buildroot and address
Daniel's point when cross-compiling for a Mach-O target.

If ya'll could provide a patch, I'll verify I can build on Mac OS X
and the macro is defined as expected and 'svn --version --verbose'
shows the loaded shared libs.

Thanks,
Nathan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] build/ac-macros/macosx.m4: workaround AC_RUN_IFELSE

Daniel Shahaf-2
Nathan Hartman wrote on Wed, 02 Sep 2020 11:09 -0400:
> Thomas's suggestion to wrap it in an AC_CACHE_CHECK() sounds like an
> approach that should fix cross-compilation for buildroot and address
> Daniel's point when cross-compiling for a Mach-O target.

Agreed.  If a cache variable is considered the more idiomatic solution,
then let's do that.

> If ya'll could provide a patch, I'll verify I can build on Mac OS X
> and the macro is defined as expected and 'svn --version --verbose'
> shows the loaded shared libs.

Thanks,

Daniel