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

nxcomp: Restore old 'VERSION' move behavior #27066

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

Conversation

arjanvandervelde
Copy link
Contributor

Description

This restores old behavior where VERSION file was move at "pre-build" time, rather than "post-patch". VERSION is used to fill in version number. It's currently broken and version ends up being "0.0.0.0".

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 14.7.2 23H311 arm64

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@Ionic for port nxcomp.

@macportsbot macportsbot added type: bugfix maintainer: open Affects an openmaintainer port labels Dec 13, 2024
@ryandesign
Copy link
Contributor

The guidelines have not been followed. The commit message should begin with the port name, a colon, and a space.

If this changes the files the port installs, the revision must be increased.

I'm surprised the build goes wrong when VERSION is moved before configure time. If it was reading the VERSION file at configure time, I would have expected an error message when it is not found. But indeed I do see sh: VERSION: No such file or directory while autoreconf is running, and unfortunately it does not cause the build to exit with an error code. Maybe we can fix it so that it does exit with an error when something goes wrong.

I changed it from pre-build to post-patch for a reason: to stop an unexpected error if the build is interrupted and then restarted. This PR will reintroduce that problem. Instead of pre-build, post-configure would be a better place that would not reintroduce the problem.

Or we can rename the VERSION file to something else, like VERSION.txt.

The problem (the incompatibility with C++20 compilers that necessitates renaming this file, or removing the -I flag pointing to the directory it's in) needs to be reported to the developers so they can fix it properly so we can remove our workarounds.

@ryandesign
Copy link
Contributor

The […] incompatibility with C++20 compilers […] needs to be reported to the developers

ArcticaProject/nx-libs#1086

@arjanvandervelde
Copy link
Contributor Author

Thanks for looking into this. I've updated to commit message, apologies for that. I think post-configure sounds reasonable. I'll update that and the revision, and test the build.

@ryandesign ryandesign changed the title Restore old 'VERSION' move behavior nxcomp: Restore old 'VERSION' move behavior Dec 13, 2024
@arjanvandervelde
Copy link
Contributor Author

@ryandesign, done. thank you.

@ryandesign
Copy link
Contributor

The revision of the nxproxy subport also needs to be increased, because the nxproxy program it installs links with the libXcomp dynamic library installed by the nxcomp port, and this fix to the version of nxcomp changes the install name of libXcomp, so nxproxy needs to rebuild to link with the new version. Otherwise a user may see:

dyld[78953]: Library not loaded: '/opt/local/lib/libXcomp.0.dylib'
  Referenced from: '/opt/local/bin/nxproxy'
  Reason: tried: '/opt/local/lib/libXcomp.0.dylib' (no such file), '/usr/local/lib/libXcomp.0.dylib' (no such file), '/usr/lib/libXcomp.0.dylib' (no such file)
zsh: abort      nxproxy

because after the fix the library is called libXcomp.3.dylib.

@arjanvandervelde
Copy link
Contributor Author

done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: bugfix
Development

Successfully merging this pull request may close these issues.

4 participants