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

Workbench: make shell pure #5155

Closed
wants to merge 1 commit into from

Conversation

andreabedini
Copy link
Contributor

@andreabedini andreabedini commented Apr 26, 2023

Description

This change makes the workbench shell pure. Nix will do its best to strip off the current environment and will give the user a pristine shell, where the only environment variables are set by nix. This prevents unintentional mingling of system- and nix-provided libraries.

Tested with:

  • make shell
  • start-cluster
  • wb a
  • wb a std 2023-04-26-08-40-28344-default-bage-sup
  • wb a compare 2023-04-26-08-40-28344-default-bage-sup 2023-04-26-08-40-28344-default-bage-sup

@andreabedini andreabedini requested review from a team, deepfire, mgmeier and fmaste as code owners April 26, 2023 08:56
Copy link
Contributor

@mgmeier mgmeier left a comment

Choose a reason for hiding this comment

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

Great, thanks @andreabedini

@NadiaYvette
Copy link
Contributor

Great, thanks @andreabedini

This fixes the LD_LIBRARY_PATH build issue I was having. I see something unusual with ps not being found in my path when I try to run start-cluster, though I'm not sure if that's my end again or not.

@andreabedini andreabedini force-pushed the andreabedini/pure-workbench-shell branch from ea047e9 to 329c3ed Compare May 4, 2023 13:09
@andreabedini
Copy link
Contributor Author

@fmaste had some issues running the nomad backend with this change. Is it still a problem?

@andreabedini andreabedini self-assigned this May 11, 2023
@angerman
Copy link
Contributor

Great, thanks @andreabedini

This fixes the LD_LIBRARY_PATH build issue I was having. I see something unusual with ps not being found in my path when I try to run start-cluster, though I'm not sure if that's my end again or not.

This is probably related. --pure ensures that there is no leakage from the env prior to the nix-shell call into the shell. So it would ingore any LD_LIBRARY_PATH values, as well as PATH values.

Do check the PATH var inside the shell, and check outside of the shell where ps lives (e.g. with which ps).

@andreabedini andreabedini marked this pull request as draft June 12, 2023 08:26
@andreabedini
Copy link
Contributor Author

Closing for now, we wil revisit another time.

@andreabedini andreabedini deleted the andreabedini/pure-workbench-shell branch September 13, 2023 07:23
@NadiaYvette NadiaYvette restored the andreabedini/pure-workbench-shell branch August 7, 2024 17:19
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.

4 participants