-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
AixPb: Put /usr/bin to the front of the PATH #2057
Conversation
ping @aixtools |
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.
Perhaps a typo - missing '/' in string PATH=usr/bin:...
Also, why not use something like the PATH: statement on line 11:
+10 environment:
+11 PATH: "/opt/IBM/xlC/13.1.3/bin:/opt/freeware/bin:{{ ansible_env.PATH }}"
And, I wonder if it should not ALSO be changed on line 11?
I searched through the aix playbook. I dont think that PATH environment variable is used at all. Will need to make a more thorough search |
Before merging, I will have to test a build and test with this new PATH variable |
jdk11 hotspot build on test-ibm-aix71-ppc64-1 with the new PATH variable to test |
That passed. Running a jdk11 openjdk sanity test |
That too passed. A nightly openj9 jdk8 job is running on the machine. Just for safe measure it can be used as a final indicator as to whether this new PATH variable breaks anything |
The above job passed. Ping @sxa for review |
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.
While this works - have you tested it to be sure it works a second time?
i.e., when PATH=/usr/bin:/opt/IBM/xlC/13.1.3/bin:/opt/freeware/bin/:...
does not regexp to:
PATH=/usr/bin:/opt/IBM/xlC/13.1.3/bin:/opt/freeware/bin/:/opt/IBM/xlC/13.1.3/bin:/opt/freeware/bin/:...
How about including /etc in the regexp but replacing it - in a way that the regexp will not match a second time around?
@aixtools Youre right it does. The I was thinking the updated PATH could start with |
I can look at few more systems for a default. If needed - in the future
this could use an with: items: on the regexp. For now,
=/usr/bin:/etc:... seems to have been default PATH for many levels.
…On 24/03/2021 14:51, Haroon Khel wrote:
@aixtools <https://github.com/aixtools> Youre right it does.
The |/etc| idea is good, but the question is will all new machines
have a |/usr/bin:/etc| block in their PATH?
I was thinking the new PATH could be
|/opt/IBM/xlC/13.1.3/bin:/usr/bin:/opt/freeware/bin|? That would make
the change idempotent as there wouldnt be a |PATH=/usr/bin| to for
ansible to find
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2057 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACSZR5J3A4MVIPS25XO25OLTFHU43ANCNFSM4ZORVH4Q>.
|
That works. This regexp is now idempotent |
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.
Can we ensure that this does not run if the adoptopenjdk
tag is skipped - we shouldn't be modfiying system defaults like this if someone else is running them and skipping the adoptopenjdk ones, especially since it could remove things that they already have in there.
@sxa Just that task? Or the other
|
Most of those are additions rather than potentially breaking things, so while it's not as critical, I'd say that yes they should all have it, although I'm not sure why we need to add |
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.
Couple of things:
- Considering /bin/bash is a symbolic link outside of / I do not consider it a fantastic argument for a shell (for AIX admin accounts - /bin/sh and /bin/ksh used to be as login shells AND the AIX install makes sure 'shells' are installed as binary programs - not links - these may be in the default shells - BETTER - would be (even as a symbolic link) as more inline with AIX user administration to have the bash shell be /usr/bin/bash.
- Also, the regexp used to add
bash
as a shell leaves it open for recursive addition - Suggestion: replace
regexp
with 'shells = /bin' and the new string to be 'shells = /usr/bin/bash,/usr'
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.
- replace
/bin/bash
with/usr/bin/bash
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.
Urgh - my two - now comments - are meant as request changes - so this comment
is actually two requests for change
@aixtools instead of a regexp, why not something like
On ojdk06, the shells in /etc/security/login.cfg in the
So how about something tidier like
Would that suffice? This command is also idempotent |
|
@aixtools regarding your idempotency comment, would it not be more consistent for all new systems to simply have Also, would changing this list of shells affect the way jenkins behaves on the machine? |
|
FYI: jenkins userid does not need bash as shell. iirc - gmake is calling bash directly. |
Fixes: #1544 |
|
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.
For reference, the existing /etc/security/login.cfg
on aix72-2 (ojdk04) looks like this:
shells = /bin/bash,/bin/bash,/bin/bash,/bin/bash,/bin/bash,/bin/bash,/bin/bash,/bin/bash,/bin/sh,/bin/bsh,/bin/csh,/bin/ksh,/bin/tsh,/bin/ksh93,/usr/bin/sh,/usr/bin/bsh,/usr/bin/csh,/usr/bin/ksh,/usr/bin/tsh,/usr/bin/ksh93,/usr/bin/rksh,/usr/bin/rksh93,/usr/sbin/uucp/uucico,/usr/sbin/sliplogin,/usr/sbin/snappd
And the chsec command will wipe most of those out, which seems somewat undersirable. I feel this role needs to just add entries, not change the ones already on the system.
However I'm not sure what the relationship between these entries and /etc/shells
is - @aixtools do you know?
@sxa - not sure why or when
|
Closing due to pr not passing eclipse auth check. Reopened at #2288 |
Following on from a discussion with @sxa in slack, /usr/bin should be at the beginning of the PATH variable, ahead of /opt/freeware/bin