Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xapian-bindings: Fix the build with slibtool #322

Closed
wants to merge 1 commit into from

Conversation

orbea
Copy link

@orbea orbea commented May 6, 2022

gentoo issue: https://bugs.gentoo.org/793428

Note: Tested against xapian-bindings-1.4.19.

When building xapian-bindings with slibtool instead of GNU libtool it will fail when trying to save the output of $(LIBTOOL) --config to libtoolconfig.tmp where slibtool does not have --config.

This all seems to not accomplish anything other than breaking slibtool so I suggest removing it so it works with both libtool implementations. The files installed with make install do not change.

The issue still exists in the python and python3 directories and I can fix it there too, but I want to start here to see if the solution is acceptable.

@ojwb
Copy link
Contributor

ojwb commented Jun 2, 2022

@orbea Did you miss that your patch breaks the perl bindings tests on macos?

More generally, we use $(LIBTOOL) --config to get values like objdir (which is usually .libs but _libs on some platforms) as it seems to be the documented way to do so. It's easy to eliminate such usage if you only want to make things work on Linux (e.g. simply hardcode the right value for Linux), but we're using libtool because we care about having things work portability.

I've not come across slibtool before, but https://github.com/midipix-project/slibtool seems to be it and says:

slibtool is designed to be a clean, fast, easy-to-use libtool drop-in replacement

Lack of support for --config isn't documented under "Differences from GNU libtool" so really this seems like a bug in slibtool to me.

@orbea
Copy link
Author

orbea commented Jun 2, 2022

I did not miss the osx tests failure, but it was not clear what was wrong with it from the log or how to fix it. It is not a platform I can test locally unfortunately.

This is documented indirectly in the automake documentation.

You should not assume anything about the structure of .la or .lo files and how libtool constructs them: this is libtool’s concern, and the last thing one wants is to learn about libtool’s guts.

https://www.gnu.org/software/automake/manual/html_node/Libtool-Concept.html

The --config exposes the "guts" of the libtool implementation and should not be used. That said slibtool does support it, but its usage is different than from GNU libtool. Additionally the .libs directory is internally used by the libtool implementation and should not be used by the user.

I do agree that slibtool should probably document the differences with --config better.

@ojwb
Copy link
Contributor

ojwb commented Jun 2, 2022

It's certainly not 100% clean, but in order to stop doing it we need an alternative approach to running tests using the built-but-uninstalled binary module that works on all platforms that the current approach supports. Not being able to run the testsuites on some platforms where we currently can would be an unhelpful regression.

@orbea orbea force-pushed the perl branch 7 times, most recently from d7d1d04 to 8e821e9 Compare July 21, 2022 01:03
@orbea orbea changed the title xapian-bindings/perl: Fix build with slibtool xapian-bindings: Fix build with slibtool Jul 21, 2022
@orbea orbea changed the title xapian-bindings: Fix build with slibtool xapian-bindings: Fix the build with slibtool Jul 21, 2022
@orbea
Copy link
Author

orbea commented Jul 21, 2022

@ojwb Apologies for the slow reply. I had a lot on my plate and lost track of this issue.

I updated the commit with a different hack that seems to work for the CI and locally using both GNU libtool and slibtool. Additionally I also fixed it for the java, python3 and tcl8 directories, these with the perl directory seem to be all of the places with this issue.

I'm not very happy with this, but the old hack doesn't work with slibtool and the new one doesn't work with GNU libtool. I think this is the best it can get without far less obvious changes to make these hacks unnecessary?

With slibtool the --config argument works differently than in GNU
libtool which results in 'auto/Xapian/Xapian$(PERL_SO)' failing to be
created. This can be worked around by using libtool --mode=install where
slibtool can copy the module to the intended location.

However GNU libtool still requires using --config otherwise it fails
during --mode=install with the following error.

  /bin/bash ../libtool --mode=install cp Xapian.la /home/runner/work/xapian/xapian/xapian-bindings/perl/auto/Xapian/Xapian.la
  libtool:   error: error: cannot install 'Xapian.la' to a directory not ending in /usr/local/lib/x86_64-linux-gnu/perl/5.26.1/auto/Xapian

gentoo issue: https://bugs.gentoo.org/793428
orbea added a commit to orbea/gentoo that referenced this pull request Jul 22, 2022
orbea added a commit to orbea/gentoo that referenced this pull request Jul 22, 2022
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Jul 22, 2022
Bug: https://bugs.gentoo.org/793428
Upstream-PR: xapian/xapian#322
Signed-off-by: orbea <orbea@riseup.net>
Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
@ojwb
Copy link
Contributor

ojwb commented Aug 4, 2022

Ick, that's ugly.

I still think slibtool should really implement a compatible --config or else stop claiming to be a drop-in replacement for libtool.

The --config option is documented in the libtool manual at https://www.gnu.org/software/libtool/manual/html_node/Invoking-libtool.html:

Display libtool configuration variables and exit.

And the list of configuration variables is also documented at https://www.gnu.org/software/libtool/manual/html_node/libtool-script-contents.html - for example objdir is documented as:

Variable: objdir

The name of the directory that contains temporary libtool files.

So this really does look like a documented interface to me.

The quote you give above from the automake docs that "You should not assume anything about the structure of .la or .lo files" doesn't seem relevant here since we aren't assuming anything about the structure of those files.

@orbea
Copy link
Author

orbea commented Aug 4, 2022

Ick, that's ugly.

Yea, agreed...

I still think slibtool should really implement a compatible --config or else stop claiming to be a drop-in replacement for libtool.

To be forward there are only a small handful of programs with this issue in Gentoo where in almost all if not all cases there is a better solution. Maybe @midipix might be able to offer more reasoning for this?

@midipix
Copy link

midipix commented Feb 12, 2024

Ick, that's ugly.

Yea, agreed...

I still think slibtool should really implement a compatible --config or else stop claiming to be a drop-in replacement for libtool.

To be forward there are only a small handful of programs with this issue in Gentoo where in almost all if not all cases there is a better solution. Maybe @midipix might be able to offer more reasoning for this?

You're both right; all was well intended but I should have known better ... will follow-up soon.

@ojwb
Copy link
Contributor

ojwb commented Feb 13, 2024

The next 1.4.x release won't actually use libtool --config since we now get the .libs or equivalent from $objdir at configure-time. This seems cleaner than having to mess around with saving output to a temporary file and then sourcing it. Therefore I'm going to close this PR as outdated.

I don't know what the status of slibtool working is now, but from my point of view either slibtool actually is a drop-in replacement and it just works and everything is groovy and there really is nothing more to do here, or else we document slibtool as unsupported and if gentoo still want to use it that's their decision, and they get to carry the full burden of extra work from making it. FOSS licensing allows anyone to modify the code, but there's no obligation on upstream to support them in doing so.

For upstream, aiming to support two libtool implementations which aren't fully compatible just creates more work and increases the number of scenarios for testing. Our source releases are set up to use GNU libtool so lack of support for slibtool is only a limitation for people who are determined to override that and force its use.

@ojwb ojwb closed this Feb 13, 2024
@midipix
Copy link

midipix commented Feb 13, 2024

I don't know what the status of slibtool working is now, but from my point of view either slibtool actually is a drop-in replacement and it just works and everything is groovy and there really is nothing more to do here, or else we document slibtool as unsupported and if gentoo still want to use it that's their decision, and they get to carry the full burden of extra work from making it. FOSS licensing allows anyone to modify the code, but there's no obligation on upstream to support them in doing so.

Hi! While there have been cases where slibtool exposed bugs in project that libtool simply let slide away, this clearly is not the case. For slibtool to have an incompatible --config is indeed wrong and will be fixed soon. Will follow up again once the relevant pending commits have been pushed.

For what it's worth, slibtool's primary aim remains to be serve as a gnu libtool drop-in replacement, and with the aforementioned commits in place building xapian-bindings with slibtool should indeed just work.

@midipix
Copy link

midipix commented Feb 13, 2024

Thanks again to all for catching this! As of slibtool commit c1f21665c7010c70b87f79e1b81dc7f2564f26c9, rlibtool --config now sends to stdout a perfectly compatible output.

@orbea orbea deleted the perl branch February 13, 2024 14:42
@orbea
Copy link
Author

orbea commented Feb 13, 2024

@ojwb I can confirm that xapian-bindings compiles with slibtool on Gentoo without changes now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants