autogen.sh: check directly for libtoolize instead of libtool [as libtool is not used here] #3886
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
balay
requested review from
lrknox,
derobins,
byrnHDF,
fortnern,
jhendersonHDF,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
…ool is not used here] On Ubuntu - libtoolize can be installed without libtool. So best to check directly for libtoolize - instead of indirectly via libtool $ ./autogen.sh ************************** * HDF5 autogen.sh script * ************************** Running trace script: Finished processing HDF5 API calls: instrumented 0 API functions and 0 argument lists Running error generation script: Generating 'H5Epubgen.h' Generating 'H5Einit.h' Generating 'H5Eterm.h' Generating 'H5Edefin.h' Running API version generation script: Generating 'H5version.h' Running overflow macro generation script: Generating 'H5overflow.h' ./libtoolize --copy --force ./autogen.sh: 202: ./libtoolize: not found
glennsong09
added
Priority - 3. Low 🔽
derobins
reviewed
Dec 9, 2023
| *) | ||
| HDF5_LIBTOOLIZE="${LIBTOOL_DIR}/libtoolize" | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Why are you removing the MacOS hack?
There was a problem hiding this comment.
Not sure I understand.
MacOS hack is already at line 116 above.
My change is removing the redirection from "libtool" to "libtoolize/glibtoolize" - i.e why not look directly for libtoolize/glibtoolize [as libtool is not really used in this script - only libtoolize/glibtoolize is used]?
My build on MacOS goes through fine with this PR change
balay@mpro git.hdf5 % sw_vers
ProductName: macOS
ProductVersion: 14.1.2
BuildVersion: 23B92
balay@mpro git.hdf5 % which libtool
/usr/bin/libtool
balay@mpro git.hdf5 % which libtoolize
libtoolize not found
balay@mpro git.hdf5 % which glibtool
/opt/homebrew/bin/glibtool
balay@mpro git.hdf5 % which glibtoolize
/opt/homebrew/bin/glibtoolize
balay@mpro git.hdf5 % git show -q --oneline
d86cb0da314 (HEAD) autogen.sh: check directly for libtoolize instead of libtool [as libtool is not used here]
balay@mpro git.hdf5 % ./autogen.sh
**************************
* HDF5 autogen.sh script *
**************************
Running trace script:
Finished processing HDF5 API calls:
instrumented 0 API functions and 0 argument lists
Running error generation script:
Generating 'H5Epubgen.h'
Generating 'H5Einit.h'
Generating 'H5Eterm.h'
Generating 'H5Edefin.h'
Running API version generation script:
Generating 'H5version.h'
Running overflow macro generation script:
Generating 'H5overflow.h'
/opt/homebrew/bin/glibtoolize --copy --force
glibtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'bin'.
glibtoolize: copying file 'bin/ltmain.sh'
glibtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
glibtoolize: copying file 'm4/libtool.m4'
glibtoolize: copying file 'm4/ltoptions.m4'
glibtoolize: copying file 'm4/ltsugar.m4'
glibtoolize: copying file 'm4/ltversion.m4'
glibtoolize: copying file 'm4/lt~obsolete.m4'
glibtoolize: Consider adding '-I m4' to ACLOCAL_AMFLAGS in Makefile.am.
NOTE: You can ignore the warning about adding -I m4.
We already do this in an included file.
/opt/homebrew/bin/aclocal --force -I m4 -I /opt/homebrew/bin/../share/aclocal
/opt/homebrew/bin/autoheader --force
/opt/homebrew/bin/automake --copy --add-missing --force-missing
configure.ac:506: installing 'bin/compile'
configure.ac:136: installing 'bin/config.guess'
configure.ac:136: installing 'bin/config.sub'
configure.ac:35: installing 'bin/install-sh'
configure.ac:35: installing 'bin/missing'
parallel-tests: installing 'bin/test-driver'
c++/src/Makefile.am: installing 'bin/depcomp'
/opt/homebrew/bin/autoconf --force --warnings=no-obsolete
*** SUCCESS ***
balay@mpro git.hdf5 %
Or perhaps I don't know the use case - where the build breaks without this extra re-direction [from libtool to libtoolize]
There was a problem hiding this comment.
Checking if there are any issues to resolve before this PR can be merged!
There was a problem hiding this comment.
Sorry - I didn't see that since I was looking at the PR on my phone.
There was a problem hiding this comment.
The previous MacOS code would check for both glibtoolize and libtoolize, but this change makes it only check for glibtoolize. If it's working, great, let's simplify, but it might need the previous hack if other devs need libtoolize instead of glibtoolize on the Mac.
derobins
approved these changes
Jan 12, 2024
qkoziol
requested changes
Jan 12, 2024
… - and then libtoolize
thanks, pushed! |
qkoziol
approved these changes
Jan 12, 2024
lrknox
pushed a commit
to lrknox/hdf5
that referenced
this pull request
Feb 15, 2024
…ool is not used here] (HDFGroup#3886) * autogen.sh: check directly for libtoolize instead of libtool [as libtool is not used here] * autogen.sh: simplify (g)libtoolize check - look for glibtoolize first - and then libtoolize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On ubuntu - libtoolize can be installed without libtool. So best to check directly for libtoolize - instead of indirectly via libtool