Skip to content

hints/darwin.sh: skip ldflags in lddlflags #23227

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

Open
wants to merge 64 commits into
base: blead
Choose a base branch
from

Conversation

sevan
Copy link
Contributor

@sevan sevan commented Apr 27, 2025

It has not effect.
"adopting flags from ldflags is supposed to happen in Configure, not in hints." - leont

Configuring a build of perl with -Aappend:ldflags=" -L/var/empty" and checking the output of 'perl -V:ldflags -V:lddlflags' shows

ldflags=' -L/var/empty';
lddlflags='-bundle -undefined dynamic_lookup -L/var/empty';

Found when analysing issue #9437
Tested on OS X 10.4 aka Darwin 8.

sevan and others added 2 commits April 27, 2025 21:07
It has not effect.
"adopting flags from ldflags is supposed to happen in Configure, not in
hints." - leont

Configuring a build of perl with -Aappend:ldflags=" -L/var/empty"
and checking the output of 'perl -V:ldflags -V:lddlflags' shows

ldflags=' -L/var/empty';
lddlflags='-bundle -undefined dynamic_lookup -L/var/empty';

Found when analysing issue Perl#9437
Tested on OS X 10.4 aka Darwin 8.
These macros suffixed with 'x' are guaranteed to evaluate their
arguments just once.  Prior to this commit, they used PL_Sv to
accomplish that.  But since 1ef9039 in 5.37, the macros they call
only evaluate their arguments once, so the PL_Sv is superfluous.
@tonycoz
Copy link
Contributor

tonycoz commented Apr 28, 2025

Introduces a bunch of link warnings, from github builds:

ld: warning: object file (/Users/runner/work/perl5/perl5/cpan/Compress-Raw-Bzip2/Bzip2.o) was built for newer 'macOS' version (14.7) than being linked (14.0)
ld: warning: object file (/Users/runner/work/perl5/perl5/cpan/Compress-Raw-Bzip2/blocksort.o) was built for newer 'macOS' version (14.7) than being linked (14.0)

locally:

ld: warning: object file (/Users/tony/dev/perl/git/perl/dist/PathTools/Cwd.o) was built for newer 'macOS' version (15.4) than being linked (15.0)
ld: warning: object file (/Users/tony/dev/perl/git/perl/ext/B/B.o) was built for newer 'macOS' version (15.4) than being linked (15.0)

@sevan
Copy link
Contributor Author

sevan commented Apr 28, 2025

Just as a test, can you try the following patch locally?

--- a/hints/darwin.sh
+++ b/hints/darwin.sh
@@ -304,6 +304,7 @@ case "$osvers" in  # Note: osvers is the kernel version, not the 10.x
     [1-9][0-9].*)
       add_macosx_version_min ccflags $MACOSX_DEPLOYMENT_TARGET
       add_macosx_version_min ldflags $MACOSX_DEPLOYMENT_TARGET
+      add_macosx_version_min lddlflags $MACOSX_DEPLOYMENT_TARGET
       ;;
     '')
       # Empty MACOSX_DEPLOYMENT_TARGET is okay.

khwilliamson and others added 3 commits April 28, 2025 11:08
The latest MacOS release has locales that set this wrongly.  This isn't
the first time this has been a problem.  Add a debugging line that can
easily be enabled to check for problems that may arise in  the field.
v5.41.7-43-gcdbed2a40e introduced the OP_SUBSTR_LEFT op,
which is an optimised version of OP_SUBSTR for when the offset
is zero and the replacement string is missing or ''.

Unfortunately the deparsing for this OP missed out the closing
parenthesis when the replacement string wasn't present; i.e.:

    substr($lex, 0, 4);

deparsed as:

    substr($lex, 0, 4;

The fix is trivial.
Add a couple of recently-added test files to

    Porting/deparse-skips.txt

These two test files have

    use utf8;
    "some string with accented chars"

and the accented char gets deparsed wrongly at the moment.
So skip those test files under

    cd t; ./TEST -deparse
@tonycoz
Copy link
Contributor

tonycoz commented Apr 28, 2025

That didn't improve matters, nor did adding it further down as well:

diff --git a/hints/darwin.sh b/hints/darwin.sh
index e889824b7c..b766548e55 100644
--- a/hints/darwin.sh
+++ b/hints/darwin.sh
@@ -304,6 +304,7 @@ case "$osvers" in  # Note: osvers is the kernel version, not the 10.x
     [1-9][0-9].*)
       add_macosx_version_min ccflags $MACOSX_DEPLOYMENT_TARGET
       add_macosx_version_min ldflags $MACOSX_DEPLOYMENT_TARGET
+      add_macosx_version_min lddlflags $MACOSX_DEPLOYMENT_TARGET
       ;;
     '')
       # Empty MACOSX_DEPLOYMENT_TARGET is okay.
@@ -330,6 +331,7 @@ EOM
     [1-9][0-9].*)
       add_macosx_version_min ccflags $prodvers
       add_macosx_version_min ldflags $prodvers
+      add_macosx_version_min lddlflags $prodvers
       ;;
     *)
       cat <<EOM >&4

Configure reported it added it:

Which of these apply, if any? [darwin]  
Adding -mmacosx-version-min=15.4 to ccflags
Adding -mmacosx-version-min=15.4 to ldflags
Adding -mmacosx-version-min=15.4 to lddlflags
Operating system name? [darwin]  

but it didn't end up in config.sh:

tony@hermes perl % grep lddlflags config.sh 
lddlflags='-bundle -undefined dynamic_lookup -fstack-protector-strong'
tony@hermes perl % uname -a
Darwin hermes.local 24.4.0 Darwin Kernel Version 24.4.0: Wed Mar 19 21:17:25 PDT 2025; root:xnu-11417.101.15~1/RELEASE_ARM64_T6020 arm64
...
"../../miniperl" "-I../../lib" "../../lib/ExtUtils/xsubpp"  -typemap '/Users/tony/dev/perl/git/perl/dist/PathTools/../../lib/ExtUtils/typemap'  Cwd.xs > Cwd.xsc
mv Cwd.xsc Cwd.c
/Users/tony/dev/perl/git/perl/dist/PathTools/../../miniperl "-I../../lib" -MExtUtils::Command::MM -e 'cp_nonempty' -- Cwd.bs ../../lib/auto/Cwd/Cwd.bs 644
cc -c   -fno-common -DPERL_DARWIN -mmacosx-version-min=15.4 -DNO_THREAD_SAFE_QUERYLOCALE -DNO_POSIX_2008_LOCALE -DHAS_BROKEN_LANGINFO_CODESET -DNO_LOCALE_COLLATE -fno-strict-aliasing -pipe -fstack-protector-strong -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -Wno-error=implicit-function-declaration -O3   -DVERSION=\"3.94\" -DXS_VERSION=\"3.94\"  "-I../.."  -DDOUBLE_SLASHES_SPECIAL=0 Cwd.c
rm -f ../../lib/auto/Cwd/Cwd.bundle
cc  -bundle -undefined dynamic_lookup -fstack-protector-strong  Cwd.o  -o ../../lib/auto/Cwd/Cwd.bundle  \
              \

ld: warning: object file (/Users/tony/dev/perl/git/perl/dist/PathTools/Cwd.o) was built for newer 'macOS' version (15.4) than being linked (15.0)
chmod 755 ../../lib/auto/Cwd/Cwd.bundle

Tested with after git clean -dxfq

@sevan
Copy link
Contributor Author

sevan commented Apr 29, 2025

Thanks for checking. I'll dig a bit more over the next couple of days & report back.

sevan added 3 commits April 29, 2025 15:11
So it can be appended, otherwise it just hardcodes lddlflags="-bundle -undefined dynamic_lookup"
Make sure MACOSX_DEPLOYMENT_TARGET is added to $lddlflags.
Need to be explicit with OS' for macOS 11 and up here. Otherwise:
Unexpected MACOSX_DEPLOYMENT_TARGET=11
Unless MACOSX_DEPLOYMENT_TARGET is passed into the build, just ride
the defaults of the OS' toolchain.
This code assumes OS X versioning (10.15.x ) and tries to drop the patch version
from the product version so that you're targetting macOS 10.15, rather
than 10.15.1. Unfortunately with macOS 11 and up, it ends up targetting
the patch version and since we're no longer appending $ld to $lddlflags
we end up with the core targetting the major version and the modules
targetting the patch version. Resulting in warnings
ld: warning: object file (/Users/runner/work/perl5/perl5/cpan/Compress-Raw-Bzip2/Bzip2.o) was built for newer 'macOS' version (14.7) than being linked (14.0)
@sevan
Copy link
Contributor Author

sevan commented Apr 29, 2025

I think we're good now. The only warning from the linker is ld: warning: -force_flat_namespace is no longer supported, using -flat_namespace instead now in the latest build logs from the github runner.
I don't think this is related to my changes since it's added in via Makefile.SH, which I haven't touched.

I looked into into and it looks like the warning was added sometime after macOS 10.15 (macOS 11?) but I don't have anything running macOS 11 to 13 in order to point to the specific version.

@sevan
Copy link
Contributor Author

sevan commented Apr 29, 2025

Built with ./Configure -des -Dusethreads -Duseshrplib -Dusedevel -DDEBUGGING -Dprefix=somepath && make && make test

Tested this patch on OS X 10.4/i386 shouldn't have affected anything there since MACOSX_DEPLOYMENT_TARGET is set to 10.3 explicitly, but just in case. Built successfully & test passed. No warnings from linker.
Tested on OS X 10.5/i386 where this change does have an impact. Built successfully & test passed. No warnings from linker.
Unable to build on 10.5 & target 10.4 due to a longstanding issue (described here) unrelated to the changes here but suffice to say trying to target 10.4 when building on 10.5 failed due to
ld: -rpath can only be used when targetting Mac OS X 10.5 or later

Tested on macOS 10.15/x86_64. Built successfully & test passed. No warnings from linker.
otool -l perl shows among the output:

Load command 10
      cmd LC_BUILD_VERSION
  cmdsize 32
 platform MACOS
    minos 10.15
      sdk 10.15.6
   ntools 1
     tool LD
  version 609.8

Tested on macOS 10.15 with MACOSX_DEPLOYMENT_TARGET set to 10.14. Built successfully & test passed, but there are a couple of warnings during the test suite run.

env MACOSX_DEPLOYMENT_TARGET=10.14 ./Configure -des -Dusethreads -Duseshrplib -Dusedevel -Dprefix=somepath && make && make test

During the configure stage, Configure reports

Adding -mmacosx-version-min=10.14 to ccflags
Adding -mmacosx-version-min=10.14 to ldflags
Adding -mmacosx-version-min=10.14 to lddlflags

otools -l perl reported

Load command 9
      cmd LC_BUILD_VERSION
  cmdsize 32
 platform 1
    minos 10.14
      sdk 10.15.6
   ntools 1
     tool 3
  version 609.8

When running the test suite

dist/ExtUtils-CBuilder/t/03-cplusplus ............................ ld: warning: object file (compilet-dd7tK.o) was built for newer macOS version (10.15) than being linked (10.14)
ld: warning: object file (t/cplust.o) was built for newer macOS version (10.15) than being linked (10.14)
cpan/ExtUtils-MakeMaker/t/04-xs-rpath-darwin ..................... Warning: -L./mylib changed to -L/somepath/perl5/cpan/ExtUtils-MakeMaker/t/fmd4vh32U0/./mylib
ld: warning: dylib (/somepath/perl5/cpan/ExtUtils-MakeMaker/t/fmd4vh32U0/mylib/libmylib.dylib) was built for newer macOS version (10.15) than being linked (10.14)

I suspect these are bugs in the tests, which this change has brought to light, rather than fallout due to the change.

@tonycoz
Copy link
Contributor

tonycoz commented Apr 30, 2025

longstanding issue (described here) unrelated to the changes

Is there an open ticket for this?

Copy link
Contributor

@tonycoz tonycoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look reasonable to me:

  • it builds fine on current versions of macos (the most common case)
  • you've tested it on some older macos, and built successfully

I wouldn't mind some second opinions though

@sevan
Copy link
Contributor Author

sevan commented Apr 30, 2025

longstanding issue (described here) unrelated to the changes

Is there an open ticket for this?

There's mattn/p5-Devel-CheckLib#40

@sevan
Copy link
Contributor Author

sevan commented Apr 30, 2025

But ExtUtils::MakeMaker needs a bug report too. It's not just Devel::CheckLib which creates breaks targeting 10.4 and older.

mauke and others added 12 commits May 1, 2025 12:02
…ses"

- `use re 'strict` -> `use re 'strict'`
- delete/add some commas
- link `use locale` to locale.pm
0.035     2025-05-01 11:35:41+02:00 Europe/Berlin
          - regex_sets no longer experimental
          - switch/smartmatch were deprecated then undeprecated
          - Properly handle the win32_perlio warning
…mple.

Updates from PTS 2025 to clarify how the perl disclosure process will work.

* Clarify what an embargo period is as this is surprisingly not well
  documented on the internet.
* Provide a simple walkthrough to show a real example of the process.
This improves the code in two ways:

1. Considering strict::bits is used by B::Deparse and vars.pm as
   unofficial API, the function having this side effect is arguably
   a latent bug even if it doesn’t break those callers.

2. It is harder to follow the intended logic when the function modifies
   $^H itself but also returns a value for the caller to apply to $^H.
Blead version was updated in March.
These entries apply also to arrays.
It is not good form to have links in the middle of a section go back to
the top of that very section.  Someone clicks on it, thinking they are
going to get more detail or a different explanation, and all that
happens is they lose their place in the section.
The documentation for doing these operations on arrays was added at a later time to the documentation onf hashes.  This cleans up some awkwardness and adds an example.

Fixes Perl#17719
The desired value has already been cached under a different name.
Supercedes Perl#23268, which didn't really get to the core problem.

These were generating uninitialized variable warnings on OpenBSD, due to
the reference existing, but the array being empty.
@khwilliamson
Copy link
Contributor

Should this go in 5.42?

khwilliamson and others added 8 commits May 13, 2025 15:02
See Perl#18995

This commit just adds a simple test that IV is large enough to hold a
pointer; something we claim is true, but didn't actually guarantee.
This generates a few errors on EBCDIC, which could be fixed; but it is
not currently used there.
With 9e9b2f9, Configure guarantees an
IV can hold a pointer.  But it may be larger.  This commit takes Tony
Cook's suggestion about extra wording.

This fixes Perl#18995
This was going to cover more, but it turned out to already
be fairly well covered.
Pod::Simple's entry for $VERSION in Maintainers.pl
is now in synch with what's on CPAN.  We no longer need the
corrective applied in 6ea7dac.
@sevan
Copy link
Contributor Author

sevan commented May 14, 2025

Should this go in 5.42?

Yes please, but I suspect that question was not intended for me :)

@haarg
Copy link
Contributor

haarg commented May 14, 2025

I think this should go in to 5.42. We are going to have another dev release, and this seems low risk. Based on what I understand about the change, the new version looks more correct.

I would hope that an issue in ExtUtils::MakeMaker can also be filed and fixed.

@sevan
Copy link
Contributor Author

sevan commented May 15, 2025

I would hope that an issue in ExtUtils::MakeMaker can also be filed and fixed.

I'm trying put together an example Makefile for DBD::mysql which fixes its bundle by overriding what's in place by default, to call install_name_tool.
With that in place, we can shed all the rpath bits in Devel::Checklib and ExtUtils::MakeMaker. I will raised the bug report for ExtUtils::MakeMaker in the next couple of days.
Basically, handling rpath things should be on a case by case basis and not forced on everything by default by Devel::Checklib and ExtUtils::MakeMaker, which is what's currently happening. This came about because there were issues with linking to libmysqlclient & its included libtls & libcrypto when building DBD::mysql.

thibaultduponchelle and others added 13 commits May 15, 2025 07:18
The modern way to change UTF-8 to its component bytes is to use
utf8::encode

Some of the tests are making sure that those component bytes aren't
mistaken for being UTF-8.  What those component bytes are is not
actually relevant, but the tests were looking at the specific expected
values of them.  The problem is that these differ on EBCDIC vs ASCII
platforms.  Several commits had been added to try to get the correct
values on both types, but EBCDIC still was getting failures.  And, there
is no need to test for the specific values of these irrelevant bytes.
What is important is that they were not misinterpreted as if they were
UTF-8.

This commit goes back to the original tests before those other commits
were added, and changes the matching pattern to not look for the
specific irrelevant byte values.

Doing so makes the tests pass on both types of platforms.
Some test descriptions were split over input lines, leading to ragged
output (suppressed under harness testing).
Currently, a CONST OP's SV will not have the `IsCOW` flag set if
the PV buffer was truncated such that it is too small to be COWed.
In which case, not short circuting `Perl_op_convert_list` causes
the OP to undergo constant folding, resulting in a CONST OP with
an SV that can participate in COW.

This means that the commit which introduced the short-circuit for
CONST OPs accidentally introduced a regression:
Perl@a902d92

This commit adds an additional check to ensure that short circuiting
does not happen when the CONST OP SV cannot be COWed. This is a
short term workaround given the proximity to the next stable release.
Perl#23290 seems like the more
appropriate fix, but is a bigger change, so will be held until the
next development cycle.
Add example of how it handles numbers that aren't decimal positional.

I thought it would clarify things to expand and correct the flawed
example pointed out in GH Perl#23003.
I noticed that nowhere in our documentation do we introduce the concept
of sigils.  This is something that distinguishes Perl from most other
languages, so could easily be confusing to a newcomer.
It has not effect.
"adopting flags from ldflags is supposed to happen in Configure, not in
hints." - leont

Configuring a build of perl with -Aappend:ldflags=" -L/var/empty"
and checking the output of 'perl -V:ldflags -V:lddlflags' shows

ldflags=' -L/var/empty';
lddlflags='-bundle -undefined dynamic_lookup -L/var/empty';

Found when analysing issue Perl#9437
Tested on OS X 10.4 aka Darwin 8.
So it can be appended, otherwise it just hardcodes lddlflags="-bundle -undefined dynamic_lookup"
Make sure MACOSX_DEPLOYMENT_TARGET is added to $lddlflags.
Need to be explicit with OS' for macOS 11 and up here. Otherwise:
Unexpected MACOSX_DEPLOYMENT_TARGET=11
Unless MACOSX_DEPLOYMENT_TARGET is passed into the build, just ride
the defaults of the OS' toolchain.
This code assumes OS X versioning (10.15.x ) and tries to drop the patch version
from the product version so that you're targetting macOS 10.15, rather
than 10.15.1. Unfortunately with macOS 11 and up, it ends up targetting
the patch version and since we're no longer appending $ld to $lddlflags
we end up with the core targetting the major version and the modules
targetting the patch version. Resulting in warnings
ld: warning: object file (/Users/runner/work/perl5/perl5/cpan/Compress-Raw-Bzip2/Bzip2.o) was built for newer 'macOS' version (14.7) than being linked (14.0)
@sevan
Copy link
Contributor Author

sevan commented May 19, 2025

bleh, there goes a pull request.
Merged into the wrong branch, prepping for a smoke test :/

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.

None yet