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

libtool: update to 2.5.2 #4897

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

libtool: update to 2.5.2 #4897

wants to merge 1 commit into from

Conversation

ikspress
Copy link
Contributor

@ikspress ikspress commented Sep 12, 2024

Based on my previous PR (#4826).

The propose of this PR is as follows:

  • Refresh some patches
  • Rephrase PKGBUILD following mingw-w64-libtool
  • Run autoconf -f to regenerate some files with msys target support

Here is the difference between the original patches and the refreshed ones.

Original name Refreshed name Comments
0002-cygwin-mingw-Create-UAC-manifest-files.mingw.patch 0001-cygwin-mingw-Create-UAC-manifest-files.patch Refresh without any changes
0005-Fix-seems-to-be-moved.patch 0002-Fix-seems-to-be-moved.patch Refresh without any changes
0006-Fix-strict-ansi-vs-posix.patch 0003-Fix-STRICT_ANSI-vs-POSIX.patch Refresh without any changes
0013-Allow-statically-linking-compiler-support-libraries-.patch 0004-Allow-statically-linking-Flang-support-libraries.patch Remove '-' in file name
0010-libtool-2.4.2-include-process-h.patch 0005-libtool-include-process.h.patch Remove changes in ltmain.sh
0009-libtool-2.4.2.418-msysize.patch 0006-msysize.patch Remove changes in automatically generated files
0011-Pick-up-clang_rt-static-archives-compiler-internal-l.patch 0011-Pick-up-clang_rt-static-archives-compiler-internal.patch Refresh and remove "-l" in file name
0012-Prefer-response-files-over-linker-scripts-for-mingw-.patch 0012-Prefer-response-files-over-linker-scripts-for-mingw.patch Refresh and remove '-' in file name
0013-Allow-statically-linking-compiler-support-libraries-.patch 0013-Allow-statically-linking-compiler-support-libraries.patch Remove '-' in file name

@lazka
Copy link
Member

lazka commented Sep 12, 2024

Thanks, these patches were all removed because they are not relevant for cygwin? Or any other reason?

libtool/0003-Pass-various-runtime-library-flags-to-GCC.mingw.patch
libtool/0006-Fix-strict-ansi-vs-posix.patch
libtool/0007-fix-cr-for-awk-in-configure.all.patch
libtool/0011-Pick-up-clang_rt-static-archives-compiler-internal-l.patch
libtool/0015-Allow-statically-linking-Flang-support-libraries.patch

the rest looks good to me.

libtool/PKGBUILD Outdated Show resolved Hide resolved
@ikspress
Copy link
Contributor Author

ikspress commented Sep 13, 2024

libtool/0003-Pass-various-runtime-library-flags-to-GCC.mingw.patch

Actually, I don't know how to refresh it.

build-aux/ltmain.in

      -specs=*|-fsanitize=*|-fno-sanitize*|-shared-libsan|-static-libsan| \
      -ffile-prefix-map=*|-fdebug-prefix-map=*|-fmacro-prefix-map=*|-fprofile-prefix-map=*| \
      -fdiagnostics-color*|-frecord-gcc-switches| \
      -fuse-ld=*|-static-*|-fcilkplus|-Wa,*|-Werror|-Werror=*)

libtool had added most of these flags (except -shared-libgcc, -ftree-parallelize-loops=*, -fgnu-tm etc).

Additionally, I built binutils and gcc several times, they both works properly for me.

If this patch really needed, please tell me.

libtool/0007-fix-cr-for-awk-in-configure.all.patch

This patch seems fixes nothing. sed handles \r properly. \\r just prevent bash convert it to 0x0D.

libtool/0006-Fix-strict-ansi-vs-posix.patch

libtool/0011-Pick-up-clang_rt-static-archives-compiler-internal-l.patch

libtool/0015-Allow-statically-linking-Flang-support-libraries.patch

I revert them back in 7fdd2a9.

@mmuetzel
Copy link
Collaborator

Imho, it is a good thing that libtool is the same in MSYS2 and in the MinGW environments. Is there a good reason to no longer keep the patches in sync between the two?

@jeremyd2019
Copy link
Member

it at least used to be the case that the msys2 libtool script was frequently used while generating autoreconf for packages being built from MINGW-packages.

@ikspress
Copy link
Contributor Author

ikspress commented Sep 14, 2024

Imho, it is a good thing that libtool is the same in MSYS2 and in the MinGW environments. Is there a good reason to no longer keep the patches in sync between the two?

it at least used to be the case that the msys2 libtool script was frequently used while generating autoreconf for packages being built from MINGW-packages.

Oh, yes. Maybe I'm wrong. My removal of these patches causes inconsistent behavior between the two libtool. autoconf always add msys target support, but mingw-w64-libtool broken it without 0006-msysize.patch.

@mmuetzel
Copy link
Collaborator

Afaict, the 0003-patch hasn't been upstreamed (at least some parts of it haven't). See:
https://git.savannah.gnu.org/cgit/libtool.git/tree/build-aux/ltmain.in#n5441
Please, keep adding the flags that are handled by the existing patch. I.e., -ftree-parallelize-loops=*, -fgnu-tm, -ffast-math' , '-funsafe-math-optimizations, -shared-libgcc. (The remaining flags are already covered by the new upstream "glob" expression iiuc.)
If you think (some) of these flags no longer need to be handled, please be specific about that here and/or in the commit message.

IIUC, the current patch names are the ones that are generated by a git format-patch command. Is there a reason why you renamed these files?

@lazka
Copy link
Member

lazka commented Sep 17, 2024

it at least used to be the case that the msys2 libtool script was frequently used while generating autoreconf for packages being built from MINGW-packages.

it used to be a case for some times, but was reverted via msys2/MINGW-packages@31cf2ca (see commit message for details)

In theory this package could skip mingw specific packages, but you need to rebase them anyway, so it's probably easier to just keep both libtool packages in sync.

@ikspress
Copy link
Contributor Author

I'm sorry for my late reply. I was too busy that have no time to working on libtool update.

IIUC, the current patch names are the ones that are generated by a git format-patch command. Is there a reason why you renamed these files?

Hi, mmuetzel. Thank you for pointing out my mistake directly, and I apologize for these "smart-ass" changes.

I'll revert it back later.

Afaict, the 0003-patch hasn't been upstreamed (at least some parts of it haven't). See: https://git.savannah.gnu.org/cgit/libtool.git/tree/build-aux/ltmain.in#n5441 Please, keep adding the flags that are handled by the existing patch. I.e., -ftree-parallelize-loops=*, -fgnu-tm, -ffast-math' , '-funsafe-math-optimizations, -shared-libgcc. (The remaining flags are already covered by the new upstream "glob" expression iiuc.) If you think (some) of these flags no longer need to be handled, please be specific about that here and/or in the commit message.

libtool/0003-Pass-various-runtime-library-flags-to-GCC.mingw.patch

Actually, I don't know how to refresh it.

It took me some time to get the drift in 0003-Pass-various-runtime-library-flags-to-GCC.mingw.patch and libtool.

Here's a guess as to why the maintainers of libtool didn't added these flags.

Please run the following codes in your bash with MSYS2 environment. (I think they noticed this patch, but some parts not upstreamed)

libtool --version | head -1
echo "void foo(void) {}" >foo.c
libtool --mode=compile --tag=CC cc -c foo.c  -o foo.lo    -O3 -ffast-math
libtool --mode=link    --tag=CC cc    foo.lo -o libfoo.la -O3 -ffast-math -rpath /usr/lib -version-info 0 -no-undefined

Here is the output of v2.5.2.

libtool (GNU libtool) 2.5.2
libtool: compile:  cc -c foo.c -O3 -ffast-math  -DDLL_EXPORT -DPIC -o .libs/foo.o
libtool: compile:  cc -c foo.c -O3 -ffast-math -o foo.o >/dev/null 2>&1
libtool: link: gcc -shared  .libs/foo.o    -O3   -o .libs/msys-foo-0.dll -Wl,--enable-auto-image-base -Xlinker --out-implib -Xlinker .libs/libfoo.dll.a
libtool: link: ar cr .libs/libfoo.a  foo.o
libtool: link: ranlib .libs/libfoo.a
libtool: link: ( cd ".libs" && rm -f "libfoo.la" && cp -pR "../libfoo.la" "libfoo.la" )

Here is the output of v2.4.7.

libtool (GNU libtool) 2.4.7
libtool: compile:  cc -c foo.c -O3 -ffast-math  -DDLL_EXPORT -DPIC -o .libs/foo.o
libtool: compile:  cc -c foo.c -O3 -ffast-math -o foo.o >/dev/null 2>&1
libtool: link: gcc -shared  .libs/foo.o    -O3 -ffast-math   -o .libs/msys-foo-0.dll -Wl,--enable-auto-image-base -Xlinker --out-implib -Xlinker .libs/libfoo.dll.a
libtool: link: ar cr .libs/libfoo.a  foo.o
libtool: link: ranlib .libs/libfoo.a
libtool: link: ( cd ".libs" && rm -f "libfoo.la" && cp -pR "../libfoo.la" "libfoo.la" )

As you can see, -ffast-math is ignored by libtool v2.5.2 when linking.

According to the ltmain.in in libtool source tree, -ftree-parallelize-loops=* -fgnu-tm -ffast-math -funsafe-math-optimizations -fvtable-verify* and -shared-libgcc were still ignored by libtool. I think these flags just used in compiling not linking except -shared-libgcc. If -shared-libgcc or others is still needed, feel free to tell me.

I've noticed the following patches related to it also applied in Debian's libtool without above flags (you can download libtool-bin package from packages.debian.org and open usr/bin/libtool in your editor to see it).

0001-libtool-fix-GCC-linking-with-specs.patch

0020-libtool-fix-GCC-clang-linking-with-fsanitize.patch

0025-libtool-pass-use-ld.patch

0055-pass-flags-unchanged.patch

They built the whole Debian system without these flags, so I think they are not necessary.

Thanks for your continued attention for this PR. :D

@ikspress
Copy link
Contributor Author

IIUC, the current patch names are the ones that are generated by a git format-patch command. Is there a reason why you renamed these files?

Just I think "-l" "-whe" represents nothing and nosense, so I removed it manually.

@mmuetzel
Copy link
Collaborator

mmuetzel commented Sep 23, 2024

The omission of -ffast-math (and the other flags) on Linux might be because of platform-specific differences to Windows.

See, e.g., this page:
https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
In the description of -shared:

For predictable results, you must also specify the same set of options used for compilation [...]

And in the footnote:

On some systems, ‘gcc -shared’ needs to build supplementary stub code for constructors to work. On multi-libbed systems, ‘gcc -shared’ must select the correct support libraries to link against. Failing to supply the correct flags may lead to subtle defects. Supplying them in cases where they are not necessary is innocuous. -shared suppresses the addition of startup code to alter the floating-point environment as done with -ffast-math, -Ofast or -funsafe-math-optimizations on some targets.

To be honest, I don't understand the intricacies involved here.
To be on the safe side: Please, only remove exemption for these flags if you are absolutely sure that it is safe to remove them now. If you are sure, please make it clear in the commit message that these flags have been removed and describe the reason why.

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.

5 participants