-
Notifications
You must be signed in to change notification settings - Fork 701
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
{toolchain} GCC 10.3.0 #12521
{toolchain} GCC 10.3.0 #12521
Conversation
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 813321070 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 816025310 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot edit (by @boegel): sources failed to download because mirrors haven't caught up yet, I'll seed in the sources on |
@boegelbot: I noticed your comment, but I only dance when @akesandgren or @bartoldeman or @boegel or @branfosj or @casparvl or @Micket or @migueldiascosta or @smoors or @verdurin tells me (for now), I'm sorry... - notification for comment with ID 816029120 processed Message to humans: this is just bookkeeping information for me, |
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 816038162 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegel |
Test report by @Micket |
Test report by @boegel |
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.
About binutils
: Is there a reason to use version 2.35
and version 2.36.1
?
easybuild/easyconfigs/h/help2man/help2man-1.48.3-GCCcore-10.3.0.eb
Outdated
Show resolved
Hide resolved
…th GCCcore/10.3.0
@boegelbot please test @ generoso |
Test report by @boegel |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 816107035 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegel |
Test report by @akesandgren |
Test report by @Micket |
GCCcore itself requires perl during install phase !!
So we do need Perl-base at SYSTEM level Or a patch that removes the perl usage there. It simply wants to make relative symlinks, and we know what they should be instead... |
@akesandgren Is it worth blocking this PR over that? Is this a problem new to GCC 10.3? If not, I suggest we open an issue to follow up on this... |
@boegel If perl is already required, I'd suggest to include makeinfo again to avoid the "hack". In which case I'd say this is a blocker. Alternatively: Add perl to the system-dependencies as we do with other packages already. And to be blunt: What does Spack do? Always good to see how others handle the problem ;) |
Hmm, yeah, I guess we can add back
Probably not difficult, but Perl being the very first thing that gets built is going to be annoying...
That makes most sense to me, since in practice it shouldn't make much difference (because
No trace of |
Actually, since |
I'm for just treating it as a OS dep, at least for now, so that we can get things started with GCCcore 10.3.0. And then let Åke sort this out for all the versions of GCC where we enable nvptx :) I'm also for just keeping makeinfo out of system level packages if possible. It really doesn't add any benefit to the stuff we use to just boostrap the real stuff. In fact, I don't think the manuals/documentation in modules really serve much purpose at all in a module system. I just would look up documentation online anyway for any software, and I think users do that as well. |
I think people are having trouble with including an EB-built Perl, and I certainly did not intend to try to force installation of one. I was trying to ask whether it should be done, or whether the system dependency should be documented. I was purposely starting from a Red Hat 8 'minimal' installation to see what was required from the system packages, and this came up. For what my opinion is worth, I think it would be fine to make the two Perl libraries a system dependency, so long as the error is detected and can be clearly stated along with the solution, or, preferably, just listed so that people can have an OS equivalent to a So far as I know, there isn't a list of what is needed from the system, and I was probably using an artificially small installation, but such a list could (should?) be made, which would render this moot. Regardless of the outcome of this, a list of system dependencies would be worth having, wouldn't it? Sorry for creating a storm in the teacup. |
makeinfo at the basic level does not require extra perl modules, see my comment in #12067 |
I'm still in favor of not including a makeinfo at SYSTEM level if we can avoid it, even if we have a perl. It just doesn't add anything of value for bootstrapping the toolchain. And there is plenty of time to resolve the perl-dependency for GCCcore before 4.3.5 (since the problem isn't new with 10.3, it is surely also present in 10.2 and probably several earlier versions as well). |
I've added |
Actually, I guess that doesn't matter much, since an OS dependency is really only checking during the installation, there's no runtime check added to the module file for these... |
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.
As far as I understood perl
is only need when compiling GCCcore
with nvptx
support.
'0e135e1cc7cec701beea9d7d17a61bab34cfd496b4b555930016b98db99f922e', # GCCcore-9.3.0_gmp-c99.patch | ||
] | ||
|
||
osdependencies = ['perl'] |
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.
osdependencies = ['perl'] | |
# needed for nvptx | |
if withnvptx: | |
osdependencies = ['perl'] |
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.
Isn't osdeps a () list?
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.
Just looked it up: I can be a STRING_OR_TUPLE_LIST
.
Do you prefer a tuple list?
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.
We use []
elsewhere, so should be []
here too, for consistency.
The if withnvptx
is not going to work though, unless it's moved down below the withnvptx = True
...
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.
Yes, my idea was to move this if withnvptx
below withnvptx = True
. In that way it's more clear why we need perl
when someone just looks at the easyconfig in the future.
Also this could make it more clear that people can choose to disable nvptx
in case perl
is it not available (or if they don't want to install it). Although personally I think nvptx
offload support is a really nice feature 😉
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.
@boegel This is what I actually had in mind.
'0e135e1cc7cec701beea9d7d17a61bab34cfd496b4b555930016b98db99f922e', # GCCcore-9.3.0_gmp-c99.patch | ||
] | ||
|
||
osdependencies = ['perl'] |
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.
osdependencies = ['perl'] |
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 817982484 processed Message to humans: this is just bookkeeping information for me, |
Test report by @SebastianAchilles |
Test report by @boegelbot |
Test report by @Micket |
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.
lgtm
@akesandgren said he had no complaints about the current state of the PR.
Going in, thanks @boegel! |
(created using
eb --new-pr
)WIP because final GCC 10.3.0 is not available yet, using first release candidate (10.2.1) for now; final 10.3 release expected later this week (cfr. https://gcc.gnu.org/pipermail/gcc/2021-April/235257.html)(all good now, final 10.3.0 release is there)