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

Add RC files for ROOT #71

Merged
merged 1 commit into from
Aug 18, 2021
Merged

Add RC files for ROOT #71

merged 1 commit into from
Aug 18, 2021

Conversation

plexoos
Copy link
Member

@plexoos plexoos commented Jul 26, 2021

It turns out the rootlogon.C and rootlogoff.C scripts were never saved in the repository, although they are used by every root4star job executed on the farm nodes. In fact, the jobs cannot properly run out of the box without the commands executed in these scripts. For reproducibility and analysis preservation reasons the RC scripts must be provided along with the code.

I took the currently used scripts from the default ROOT5 prefix in /afs and clean them up to keep only the minimally necessary commands. This revision works for me, but of course comments and suggestions are welcome.

It is expected that modifications for ROOT6 will be necessary and tracking such changes is essential.

Copy link
Contributor

@genevb genevb left a comment

Choose a reason for hiding this comment

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

I expect you should discuss this proposal with Jerome.

@plexoos
Copy link
Member Author

plexoos commented Aug 17, 2021

We need these files to run test jobs in our CI. See #106

(Sent a note to Jerome as suggested by Gene but received no feedback so far)

@jlauret
Copy link

jlauret commented Aug 18, 2021

If I read this correctly, the proposed commits are simplified root logon / logoff and not the STAR standard scripts. If so, this should be fine.

@genevb
Copy link
Contributor

genevb commented Aug 18, 2021

It turns out the rootlogon.C and rootlogoff.C scripts were never saved in the repository

I'm a bit confused by this note. Is the above statement referring to the git repository? These scripts were in the CVS repository under root5/etc. Just trying to clarify what is being said here.

@genevb
Copy link
Contributor

genevb commented Aug 18, 2021 via email

@plexoos
Copy link
Member Author

plexoos commented Aug 18, 2021

Yes, missing from this (Git) repository because the root5 directory was not migrated

@plexoos
Copy link
Member Author

plexoos commented Aug 18, 2021

I'm also very confused by this email notification I received from github

I believe GitHub uses the full name of the user as recorded in the profile settings for email notifications. I suspect that the full "real" name for user @jlauret was set to "Cthulhu" as of this morning, however, it is now set to "JL". I don't think GitHub user cthulhu was involved in that email exchange/notification in any way.

@jlauret
Copy link

jlauret commented Aug 18, 2021

Are you sure you know user jlauret :-) ?? I reset to JL as Dmitri noted but kept the nice picture of myself and ... you are never sure what the real Cthulhu would be involved in :-D

As per my comment, I looked at what is proposed to be added. It is not the full rootlogon.C as appearing on disk at the SDCC. It is a stripped down version (minimal version) that seemed to be designed to support CI purpose (my interpretation). In that sense, we are good as no secrets are in - we just need to make sure one does not pull this and replace the real rootlogon from STAR.

@genevb
Copy link
Contributor

genevb commented Aug 18, 2021

I'm also very confused by this email notification I received from github

I believe GitHub uses the full name of the user as recorded in the profile settings for email notifications. I suspect that the full "real" name for user @jlauret was set to "Cthulhu" as of this morning, however, it is now set to "JL". I don't think GitHub user cthulhu was involved in that email exchange/notification in any way.

Thanks for the likely explanation, Dmitri. Authentication on github is apparently a guessing game. With RACF/SDCC systems, I have much higher trust/confidence in establishing who is whom with all the vetting that takes place before someone gets access to the systems.

Copy link
Contributor

@genevb genevb left a comment

Choose a reason for hiding this comment

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

Given @jlauret 's approval of these versions, I'll mark myself as approved.

@plexoos plexoos merged commit 58b8833 into main Aug 18, 2021
@plexoos plexoos deleted the ds-rootrc branch August 18, 2021 16:17
@plexoos plexoos mentioned this pull request Aug 18, 2021
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.

None yet

3 participants