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

@mmarchini request to join jenkins-admins #2209

Closed
mmarchini opened this issue Mar 7, 2020 · 5 comments
Closed

@mmarchini request to join jenkins-admins #2209

mmarchini opened this issue Mar 7, 2020 · 5 comments

Comments

@mmarchini
Copy link
Contributor

mmarchini commented Mar 7, 2020

I have several permissions missing on Jenkins: I can edit configuration of node-test-pull-request but not any other jobs (I checked node-test-commit-v8-linux, a few benchmark jobs and citgm-smoker). I also don't have permission to create new Jobs. Ok, I think something changed wrt how we manage access. I probably shouldn't have edit rights to node-test-pull-request.


I'm trying to workaround nodejs/node#30152, so we can restore V8 test on x64 platforms, but I need to change node-test-commit-v8-linux configuration to do that (just need to set one environment variable before calling ./configure).

I'm also planning on being more active on the WG as a whole again, and having Jenkins access would help.

@mmarchini mmarchini changed the title Update @mmarchini permissions on Jenkins @mmarchini request to join jenkins-admins Mar 7, 2020
@rvagg
Copy link
Member

rvagg commented Mar 9, 2020

Screenshot 2020-03-09 14 06 32

This is why you have node-test-pull-request access, everyone on Build does. tbh I don't know why this is so permissive—who did it and why. Most jobs use the global permissions which give nodejs/jenkins-admins the permissions to configure. My preference is to keep that setup and undo this custom permission. If you want to make a case for nodejs/jenkins-admins then that's a separate process and our main concern these days is that we limit it to people who have enough clue about what they're doing that Jenkins isn't gong to get too too messed up and that we have assurance from nodejs/jenkins-admins that we get a heads-up in nodejs/build about who/what/why changes are made (like the node-test-pull-request permissions change!).

Maybe for this particular instance you could just tell us what needs to be changed to get it working.

@mmarchini
Copy link
Contributor Author

My preference is to keep that setup and undo this custom permission.

I'm fine with that, but it's worth pinging the team before doing anything because people will lose access they have today, and might be confused by it.

Maybe for this particular instance you could just tell us what needs to be changed to get it working.

Sure. On node-test-commit-v8-linux we need to set the FORCE_PYTHON2 environment variable before calling ./configure, because gclient doesn't support Python 2 yet (we still need to merge nodejs/node#32140 for that variable to take effect, but we need to set it now to run the test there).

If you want to make a case for nodejs/jenkins-admins then that's a separate process

Let's keep it as is for now. If I need access a few more times in the future I'll request to join the team.

@rvagg
Copy link
Member

rvagg commented Mar 9, 2020

  ./configure
  make -j $JOBS test-v8 V=1 $DISABLE_V8_I18N_OPTION DESTCPU=$DESTCPU ARCH=$DESTCPU.release $ADDITIONAL_CLANG_OPTIONS ENABLE_V8_TAP=True V8_EXTRA_TEST_OPTIONS="--progress=dots --timeout=120"

is FORCE_PTHON2 a depot_tools things? I can't find it in our codebase. I've tried prefixing both of the above two commands with various conbinations of PYTHON=python2, FORCE_PYTHON2=true and even NODE_GYP_FORCE_PYTHON=python2 (which is a thing in node-gyp but not tools/gyp) but it still fails with depot tools saying "Warning: Running gclient on Python 3."

@mmarchini
Copy link
Contributor Author

No, it's introduced in the PR I linked above to force configure to use Python 2 instead of 3, and the Python version selected here is then used later for gclient.

@rvagg
Copy link
Member

rvagg commented Mar 9, 2020

ah ok, sorry!

FORCE_PYTHON2=true ./configure

https://ci.nodejs.org/job/node-test-commit-v8-linux/2928, looking good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants