Adds flake.nix, shell.nix & flake.lock for creating NixOS developer environments

I don't think we're inclined to take this - minimally it should be a separate PR. We don't want to imply nix support by including these files without actually committing to keeping it working, which we don't have the resources to do right now.

Could you drop these changes from the PR for now to keep the discussion focused on the other part?

Reimplements tilde_expansion as resolve_path. It now takes a &Path, returns an anyhow::Result, and looks in $PATH if input is a standalone string (e.g. true will now resolve to /bin/true)
Various edits to fix yaml files with absolute paths or relative paths that conflicted with the $PATH lookup.

I think it'd be good to have this or something like this. e.g. it'd also be nice if tor simulation configs didn't have to hard-code paths to tor, tgen, etc.

Locating binaries using the PATH environment variable is convenient, though opens the door to a little bit of confusion about what binary actually ran. I think this could be mitigated by one or both of:

• Include the fully resolved path in shadow's log and processed-config.yaml. @stevenengler do you know how difficult it'd be to do the latter?
• Maybe use a command-line option like --path or --binary-search-path instead of the environment variable, to make this behavior a bit more explicit.

@stevenengler and @robgjansen - thoughts?

Overall this seems like a reasonable improvement.

I agree with sporksmith's points:

• We don't want to commit to supporting and maintaining the NixOS files; thanks for removing those.
• We really should be logging the absolute path to the binaries we are executing. It's not fun spending hours debugging unexpected sim behavior only to realize it's running the wrong binary. (I swear that's never happened to me 😢)
• I lean toward making the behavior explicit and therefore user-predictable; searching PATH is a bit of automation that may not be immediately obvious to users and may lead to confusion. A --binary-search-path option would also allow setting a different path for the simulator than the one you might want for your terminal.

@stevenengler and @robgjansen - thoughts?

Back when we switched to YAML, we had a few discussions about whether to search PATH for the binary, or to require a hard-coded binary path. At the time we decided that we didn't want to search the PATH, use a flag like --binary-search-path, or use any kind of YAML templating to deduplicate paths. I'm okay with searching the PATH or using a flag like --binary-search-path, but I'm wondering why we want to change it now. Is it because we've decided that our previous decision was wrong?

I like the --binary-search-path/general.binary_search_path option, but does this solve what the PR is intended for? This change to search the PATH is meant to make it so that a single shadow config file will work in different environments (such as NixOS). Will our examples and tests just use --binary-search-path $PATH everywhere? Is there ever a reason to use something other than --binary-search-path $PATH? If not, why not just search PATH directly instead?

A --binary-search-path option would also allow setting a different path for the simulator than the one you might want for your terminal.

Why shadow --binary-search-path "/home/user/.local/bin" config.yaml rather than just something like PATH="/home/user/.local/bin" shadow config.yaml?

Include the fully resolved path in shadow's log and processed-config.yaml. @stevenengler do you know how difficult it'd be to do the latter?

It wouldn't be difficult, but the reason I added the processed-config.yaml was to output the configuration after only the very basic config processing steps (merge the defaults, the yaml, and the cli options). If we want to add additional runtime-generated things to this file, then we'll need to make the configuration object mutable, make sure to update the config object after resolving paths, and then make sure we do that before we've written the config to a file. So we can do this, but it makes it less clear about what the processed-config.yaml file represents. I'd personally be in favour of just logging the paths to the binaries, but we could do it either way.

We really should be logging the absolute path to the binaries we are executing.

The way it resolves the path and inserts it into args at [0] makes sure the program being used is logged as the full path 👍

I'm wondering why we want to change it now. Is it because we've decided that our previous decision was wrong?

Basically. I think we decided to kick the can on it and see if it was a worthwhile feature after more experience. After more experience with shadow I'm leaning towards this feature being worth it.

After @stevenengler 's comments and our offline chat about this, I think I've circled around to just using the PATH environment variable.

• It's what shells do
• We already do shell-like interprolation of ~
• If we were to add a command-line option like --binary-search-path, typical usage is likely to be to set it to the PATH environment variable anyway, whether explicitly or because we make that the default.
• Users can still override it like any other environment variable when running shadow. e.g. PATH="/path/to/experiment/bins" shadow shadow.yaml

@robgjansen @stevenengler any objections?

It would be great if the code is written such that it is really simple to remove the behavior that we merge here but want to remove in 3.0, or at the very least please make sure to add sufficient comments so we understand/remember what needs to be changed later when we release 3.0. Thanks!

Aight. I pretty much rewrote the entire commit, so I force-pushed. Should be a lot cleaner now

Codecov Report

Base: 42.69% // Head: 42.22% // Decreases project coverage by -0.46% ⚠️

Coverage data is based on head (394ff9a) compared to base (6fff91b).
Patch coverage: 48.00% of modified lines in pull request are covered.

❗ Current head 394ff9a differs from pull request most recent head 2e55b9a. Consider uploading reports for the commit 2e55b9a to get more accurate results

@@            Coverage Diff             @@
##             main    #2447      +/-   ##
==========================================
- Coverage   42.69%   42.22%   -0.47%     
==========================================
  Files         180      180              
  Lines       26479    26496      +17     
  Branches     5373     5377       +4     
==========================================
- Hits        11304    11189     -115     
- Misses      12538    12674     +136     
+ Partials     2637     2633       -4     

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.