Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

don't run .bashrc for any commands fed from .hcbuild #2108

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

pdaoust
Copy link
Collaborator

@pdaoust pdaoust commented Feb 12, 2020

PR summary

Resolves #2097

testing/benchmarking notes

Profound apologies; I can't get the silly thing to compile (missing build deps for newrelic-sys and it's late). But the means for testing would be as follows:

  1. Add the line echo "hello" somewhere in your .bashrc file.
  2. Compile a zome with hc package.

On current develop this should fail with a message like:

Error: Couldn't traverse DNA in directory "/home/alice/Holochain/stub-project": artifact path /home/alice/Holochain/stub-project/zomes/stub-zome/code/***hello***
/home/alice/target/wasm32-unknown-unknown/release/stub-zome.wasm either doesn't point to a file or doesn't exist

With this fix it should succeed.

changelog

  • if this is a code change that effects some consumer (e.g. zome developers) of holochain core, then it has been added to our between-release changelog with the format
- summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)

Copy link

@samrose samrose left a comment

Choose a reason for hiding this comment

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

@pdaoust it looks like the nixpkgs project merged in the option when running nix-shell --pure that the .bashrc file would not be sourced.

NixOS/nix#3131

Is there anything that prevents or precludes someone from having the nix-shell invocation run as pure?

@pdaoust
Copy link
Collaborator Author

pdaoust commented Feb 12, 2020

@samrose Thanks for taking the time to review and recmmend! We certainly could tell devs to run nix-shell --pure in the install guide (although it is nice to have most of your comfy environment still available to you). But the concern here is that a new bash -c is spawned for every step of the zome build process, and at least one of the steps captures STDOUT and tries to do meaningful things with it. If any command in .bashrc outputs to STDOUT it borks the build process. So it's less about the host environment leaking into those build steps.

Do you know if --pure causes 'inner' invocations of bash to be pure as well, or is it just the immediate shell created by nix-shell?

@pdaoust
Copy link
Collaborator Author

pdaoust commented Feb 12, 2020

I presume the app-spec-tests-sim2h failure is a red herring... looks like it all happens after app-spec is already built.

@pdaoust pdaoust requested a review from samrose February 13, 2020 17:40
@pdaoust
Copy link
Collaborator Author

pdaoust commented Mar 5, 2020

Hey @samrose just checking in on your change request -- given the extra details I shared, does this seem like a decent solution?

@pdaoust
Copy link
Collaborator Author

pdaoust commented Mar 27, 2020

@zippy bumping this PR -- I think it was languishing for a while because it was blocked by some requested changes. I don't know if those requested changes pertain to the issue this is trying to fix though.

I've updated this branch to master and the diff is still clean. It should be an easy merge. The only thing I don't know is whether this causes unintended consequences. Are there times when the inner bash that hc package spawns should actually re-run the .bashrc? Are there times when not running it could break things for the dev? Nothing comes to mind, but hey, these are computers we're talking about -- you never know.

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

Successfully merging this pull request may close these issues.

writing to STDOUT in .bashrc causes hc package to fail
4 participants