-
Notifications
You must be signed in to change notification settings - Fork 695
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
add RISC-v support to recent LAME easyconfigs by removing workaround for finding libncurses #20970
add RISC-v support to recent LAME easyconfigs by removing workaround for finding libncurses #20970
Conversation
Test report by @bedroge Only tried the GCCcore 13.2.0 version on RISC-V, as it doesn't really support older GCC versions. |
Test report by @bedroge |
@boegelbot please test @ generoso |
@bedroge: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2210993808 processed Message to humans: this is just bookkeeping information for me, |
Test report by @bedroge |
Test report by @boegelbot |
@boegelbot please test @ jsc-zen3 |
@bedroge: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2211084066 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
Test report by @SebastianAchilles |
Test report by @verdurin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine.
Going in, thanks @bedroge! |
This workaround would set an environment variable with
configure_cmd_prefix
, and this is somewhat ugly. It also breaks the config.guess update functionality in the configuremake easyblock, as this will check ifconfigure_command
exists, and it will think it doesn't exist if the command includes this environment variable (as it's basically doingos.path.exists("FRONTEND_LDADD='-L${EBROOTNCURSES}/lib' ./configure"
). See https://github.com/easybuilders/easybuild-easyblocks/blob/develop/easybuild/easyblocks/generic/configuremake.py#L303. As LAME includes a very old config.guess, it doesn't build on RISC-V this way.I couldn't reproduce any issues with the workaround removed, and without it LAME would indeed build fine on RISC-V, as the easyblock then does update the config.guess. So I'm proposing that we remove this workaround. If it would still be needed for any reason, we should just set it in
(pre)configopts
.