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

makefile.shared: Don't use libtool #594

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

orbea
Copy link
Contributor

@orbea orbea commented Jul 16, 2022

Gentoo Bug: https://bugs.gentoo.org/777084

This changes the makefile.shared build to not require libtool during the build instead of the existing hacks. This is helpful for avoiding the already existing hacks for slibtool. I am not sure this is the direction the libtomcrypt / libtommath projects want to go, but I am proposing it as an option.

Note: I tested this on Gentoo and Slackware linux, but there may be issues for other platforms I can't personally test and they should be tested first.

@orbea orbea force-pushed the shared branch 8 times, most recently from 5344722 to 597fe4b Compare July 17, 2022 00:07
@orbea
Copy link
Contributor Author

orbea commented Jul 17, 2022

I'm not sure what is up with the failing test? Seems to be happening with the makefile which I have not modified?

@orbea orbea force-pushed the shared branch 6 times, most recently from 79375a5 to 5a67f1a Compare July 17, 2022 16:25
@orbea
Copy link
Contributor Author

orbea commented Jul 17, 2022

I'm not sure what is up with the failing test? Seems to be happening with the makefile which I have not modified?

There appears to be some kind of indeterminate failure that often hits the PTHREAD test on CI, I was able to reproduce it only once locally. Regardless I don't think its the result of anything done here.

@orbea orbea force-pushed the shared branch 3 times, most recently from 6e2e14b to 7ee2ff5 Compare July 18, 2022 00:37
@orbea orbea changed the title build: Don't use libtool makefile.shared: Don't use libtool Jul 18, 2022
@orbea orbea force-pushed the shared branch 2 times, most recently from 36bc8fc to 859f545 Compare July 18, 2022 20:15
@sjaeckel
Copy link
Member

sjaeckel commented Sep 2, 2022

There appears to be some kind of indeterminate failure that often hits the PTHREAD test on CI, I was able to reproduce it only once locally. Regardless I don't think its the result of anything done here.

No, that issue was fixed by 038db7e

@orbea
Copy link
Contributor Author

orbea commented Sep 2, 2022

No, that issue was fixed by 038db7e

I rebased this PR against the develop branch.

@sjaeckel
Copy link
Member

sjaeckel commented Sep 5, 2022

but there may be issues for other platforms I can't personally test and they should be tested first.

after having a discussion in #autotools I'm pretty sure that we'll be breaking something on other platforms than GNU/Linux ... but I'm also not sure how we should tackle this resp. if it's really required ...

17:08 < jaeckel> ... https://www.gnu.org/software/libtool/manual/libtool.html#Versioning only specifies the `current:revision:age` semantics and not how the files in the end are called
17:11 < anyone> Oh. Well, I'd argue that is system-specific.
17:13 < anyone> On Linux you get libfoo.so.x.y.z, on Cygwin/MinGW, you get libfoo-x.dll. On *BSD, it even depends on which libtool implementation (GNU vs BSD) you use ;-)
17:14 < anyone> Since libtool is supposed to be treated like a blackbox, all you are supposed to know if "libfoo.la".
17:14 < anyone> ...is libfoo.la

The only non-Linux I know of who packages ltc is @gahr for FreeBSD and @karel-m for perl-cryptX which also has Windows(&Mac?) support.

I'll keep this open until I get feedback from them whether this change would be fine for them resp. what needs to change.

@gahr
Copy link
Collaborator

gahr commented Sep 5, 2022

Hi, thanks! I can't comment on the goodness / reasonableness of the approach, but I can build just fine on FreeBSD :)

@gahr
Copy link
Collaborator

gahr commented Sep 5, 2022

One difference, though, is that before makefile.shared used to produce both shared and static libraries. Now I have to use makefile.unix to produce the static lib. Is that intended?

@orbea
Copy link
Contributor Author

orbea commented Sep 5, 2022

One difference, though, is that before makefile.shared used to produce both shared and static libraries. Now I have to use makefile.unix to produce the static lib. Is that intended?

From the README.md it says this which I did not previously notice: "builds a shared (and static) library (GNU Make required)"

https://github.com/libtom/libtomcrypt#building-the-library

I can add the static library back or the README.md can be changed, I'm not sure which is preferred? I was thinking using Makefile would create the static library.

after having a discussion in #autotools I'm pretty sure that we'll be breaking something on other platforms than GNU/Linux ... but I'm also not sure how we should tackle this resp.

I was scared of this, but the current usage of libtool is not ideal. Build systems should depend on the libtool script generated in the build directory and not the one installed on the system which will then cause problems with for instance rlibtool which depends on this generated script to determine if its building shared, static or both.

However libtomcrypt and libtommath unfortunately do not use autotools so it is missing the LT_INIT macro which generates the libtool script.

I think this leaves two obvious choices, either use a standard autotools build which would be more robust, but would be a little slower and would be disruptive to downstream and developers when at least Makefile and Makefile.shared are entirely replaced with autoreconf -fi && ./configure && make. Otherwise the less disruptive option would be to open the black box of libtool and do it manually, but this risks regressions for some platforms.

The former option is also a lot of effort given the way the build system is currently written, but I can try to do it if that is the preferred approach.

@sjaeckel
Copy link
Member

sjaeckel commented Sep 5, 2022

The former option is also a lot of effort given the way the build system is currently written, but I can try to do it if that is the preferred approach.

Meh... if we'll merge this PR it'll come with a change to the README. We're not going to implement our even worse version of libtool ;)

Users who want static and shared libraries then would have to run two builds. Would this be acceptable @gahr ?

Also there's still #586 which will be the "fancy new $h17"(c) instead of going for autotools, since ltm also already has support for cmake (and apparently I get the feeling "it's not that bad at all"^TM). I've some refinements of #586 locally which I will finalize soon'ish, so we can maybe close that one pretty soon.

So in the end we'll have makefile[.msvc|.unix] for static libraries, makefile.shared for shared libraries, makefile.mingw and cmake which will be able to build both. cmake is also there for all the persons who want a "more modern way" of building ltc.

@gahr
Copy link
Collaborator

gahr commented Sep 5, 2022

Users who want static and shared libraries then would have to run two builds. Would this be acceptable @gahr ?

I guess so, yeah, as long as we make sure that CFLAGS are the same in the two makefiles! What I think I'll do is make -f makefile.shared library to produce the shared library folllowed by make -f makefile.unix.
The targets for both libraries will depend on the same object files, so makefile.unix will find the object files compiled via makefile.shared and will only ar r .... them.
Anyway, this is what used to happen with the old makefile.shared, so I wonder.. why don't we keep supporting it?

@orbea
Copy link
Contributor Author

orbea commented Sep 5, 2022

Anyway, this is what used to happen with the old makefile.shared, so I wonder.. why don't we keep supporting it?

I think this was a consequence of not having the generated libtool which results with the system version of libtool not knowing if its static or shared so it builds both.

It would not be hard to add this to my PR, but there should be some consensus if its a good idea or not first. :)

@ryancog
Copy link

ryancog commented Aug 4, 2024

This seems like it might have lost some traction, but I just wanted to say many thanks to @orbea for this commit. Even if it isn't merged yet, these changes worked a charm to make cross-compiling for Windows (from Linux) possible. libtool was a pain point in trying to do that and applying your changes works perfectly!

Just needed to set the CROSS_COMPILE variable.

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

Successfully merging this pull request may close these issues.

4 participants