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

ansible: install devtoolset-8 on centos7 #2262

Merged
merged 7 commits into from
Apr 9, 2020
Merged

ansible: install devtoolset-8 on centos7 #2262

merged 7 commits into from
Apr 9, 2020

Conversation

sam-github
Copy link
Contributor

Refs: #2242

@sam-github
Copy link
Contributor Author

I wish I could move this back to draft... I'm finding its a bit WIP as I work through the various rhel/centos systems and need to patch the ansible.

Comment on lines +24 to +25
[ /^centos[67]-(arm)?(64|32)-gcc48/,anyType, gte(10) ],
[ /^centos[67]-(arm)?(64|32)-gcc6/, anyType, lt(10) ],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added the ^ for consistency.

Copy link
Member

Choose a reason for hiding this comment

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

ok, but are you sure there are no labels used anywhere with some cruft in front? I don't think there but we've been caught with overly strict regexes in the past (in select-compler.sh trying to identify all the benchmark machines iirc was the latest).

Comment on lines +26 to +27
[ /^centos[67]-(arm)?(64|32)-gcc6/, anyType, gte(14) ], // 14.x: gcc6 builds stop
[ /^centos7-(arm)?(64)-gcc8/, anyType, lt(14) ], // 14.x: gcc8 builds start
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we are stopping using one set of labels after 14.x, and switching to a new set. I think its intentional to do anyType, its not intended that we test with gcc6/aka devtoolset-6, is it?

Copy link
Member

Choose a reason for hiding this comment

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

ack
Yes let's just leave it as a clean break even for testing. We can expand later if we see a need but we had had a problem with exploding permutation complexity just because we can in the past, and let's try to resist that urge.

Copy link
Member

Choose a reason for hiding this comment

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

I assume we still have some linux platforms building/testing on gcc6 thought right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For 14.x, specifically? I don't know. For earlier releases, yes.

If you want I can take can try to find the compiler levels for all machines 14.x are tested on, but I spent 10 minutes looking, and its not documented anywhere AFAICT. Figuring it out would take a combination of finding all linux builds in https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/1899/, reading the console logs, then sshing into the machines (if they don't explicitly set the version, many don't, I have to look... for example ubuntu 18.04 is using gcc7, I just checked). If nodejs/node#32715 (comment) lands, then the ssh won't be needed, just reading the top of all the console logs.

AIX builds on gcc6.

Copy link
Member

Choose a reason for hiding this comment

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

From the CI for nodejs/node#32715 (https://ci.nodejs.org/job/node-test-pull-request/30565/) the following Linux jobs are using gcc6:

@sam-github
Copy link
Contributor Author

@nodejs/build @nodejs/releasers Please take a careful look at this, I think I got it all right, see checklist in #2242. Some of the builds were kicked off recently as repeats of last night to make sure I build with default configs selected, so are ongoing, but went well last night.

I did builds on my job-forks for 12.x (to make sure I didn't break existing releases) and master (for 14.x).

I'm trying to be careful, and the changes aren't huge, but there is a non-zero chance of breaking things. Once I merge, I'll kick off builds for multiple branches in ci and i-release (obviously not real release builds, just test runs).

releasers: Is there any more releases planned for the next few days that this could impact?

build-wg and anyone: this reflects the 14.x plan? PTAL

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

lgtm, but we're going to need to make sure iojs+release is doing the right thing too and has all the right tags

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM with a question

When trying to ansible test-rackspace-centos7-x64-1 I found that it
can't install git, because that's 1.8, and it has git216 installed.
So, git can't be shared by all centos7 platforms, the name differs, move
into a more specific package list.
It isn't there now, so probably doesn't have to be, and it doesn't seem
to be available, so move it out of the common packages.
In addition, configuration changes were made in CI.
@sam-github sam-github merged commit a6dd558 into nodejs:master Apr 9, 2020
sam-github added a commit that referenced this pull request Apr 9, 2020
sam-github added a commit that referenced this pull request Apr 9, 2020
When trying to ansible test-rackspace-centos7-x64-1 I found that it
can't install git, because that's 1.8, and it has git216 installed.
So, git can't be shared by all centos7 platforms, the name differs, move
into a more specific package list.

PR-URL: #2262
sam-github added a commit that referenced this pull request Apr 9, 2020
It isn't there now, so probably doesn't have to be, and it doesn't seem
to be available, so move it out of the common packages.

PR-URL: #2262
sam-github added a commit that referenced this pull request Apr 9, 2020
sam-github added a commit that referenced this pull request Apr 9, 2020
In addition, configuration changes were made in CI.

PR-URL: #2262
sam-github added a commit that referenced this pull request Apr 9, 2020
@sam-github sam-github deleted the centos7-devtoolset-8 branch April 9, 2020 17:04
@sam-github
Copy link
Contributor Author

Landed in a6dd558...a042232

richardlau added a commit to richardlau/build that referenced this pull request Apr 10, 2020
nodejs#2262 mistakenly added a
duplicated case that prevented the previously existing logic from
being executed. Merge the two centos cases for ppcle Linux.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>
sam-github pushed a commit that referenced this pull request Apr 10, 2020
#2262 mistakenly added a
duplicated case that prevented the previously existing logic from
being executed. Merge the two centos cases for ppcle Linux.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>
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.

5 participants