Skip to content

VMS build update #474

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

Closed
wants to merge 3 commits into from
Closed

Conversation

craigberry
Copy link
Contributor

No description provided.

@craigberry
Copy link
Contributor Author

The CI test that failed was freebsd 13.2, and the relevant error was:

pkg: No packages available to install matching 'openssl30' have been found in the repositories

so it never actually got set up to build much less run a test. Sounds like that combination needs to be taken out of the list.

Makefile.PL was initializing MakeMaker's CCFLAGS argument to
$Config{ccflags}, then manually appending -D switches to it. That
is not portable since not all compilers use that syntax for defines.
Luckily, MakeMaker knows how to parse compiler flags and inject
defines passed in -D format via its DEFINE argument.  So use DEFINE
instead of CCFLAGS.
This test was failing on VMS because list assignment to %ENV is
not allowed.  So just override the slice of %ENV that the test is
concerned with.
Comment on lines -114 to -116
# CCFLAGS is used internally by Makefile.PL to define various C preprocessor
# macros (as opposed to DEFINE, which is user-facing).
$eumm_args{CCFLAGS} = $Config{ccflags};
Copy link
Contributor

Choose a reason for hiding this comment

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

SSLeay.xs currently says:

p5-net-ssleay/SSLeay.xs

Lines 209 to 214 in 420b801

/* Debugging output - to enable use:
*
* perl Makefile.PL DEFINE=-DSHOW_XS_DEBUG
* make
*
*/

It would be good to leave DEFINE for command line use, as shown above, but I couldn't find an easy way to do this. In other words, passing defines to the compiler directly from Makefile.PL and also allowing additional defines defined on the command line. Would you happen to know a way to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean. The MakeMaker docs seem pretty clear that you can send an attribute to WriteMakefile or you can make it an argument on the command line -- not both. The only alternative I can think of would be to parse off any DEFINEs found in @argv and add the concatenated result to the DEFINE that gets passed to WriteMakefile. That's not pretty but I can't think of anything better at the moment. Maybe inspiration will strike.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! It seems your suggestion of looping through the ARGV is the easiest way. More exactly, loop through the ARGV in reverse order and push the Makefile.PL DEFINEs to the first found entry in the ARGV. Reverse it's a bit safer because even the latest MakeMaker seems to only consider the last DEFINE from the command line.

In case the command line has no DEFINE, pass the Makefile.PL DEFINEs directly to WriteMakefile().

@h-vn h-vn self-assigned this May 1, 2025
@h-vn h-vn added the enhancement New feature or request label May 1, 2025
@h-vn
Copy link
Contributor

h-vn commented May 1, 2025

These three commits are now merged. My intention was to merge them differently, but for some reason the branch protection did not hold and they went directly to the master branch. I'll need to check why this happens.

remote: Bypassed rule violations for refs/heads/master:
remote: 
remote: - Changes must be made through a pull request.

I also added a commit that handles the case where DEFINE is also given as a command line argument when running perl Makefile.PL ....

@h-vn h-vn closed this May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants