Skip to content

Conversation

@sandydoo
Copy link
Member

@sandydoo sandydoo commented Apr 21, 2024

Fixes #1153.

nix develop sets TMPDIR which changes the computed runtime dir between creating the shell and running devenv up.
DEVENV_RUNTIME changes from /tmp/devenv-<hash> to /tmp/nix-shell.<hash>/devenv-<hash>.

This makes the flake-based devenv up error out because process-compose expects the runtime dir to exists when creating the unix socket.
Non-flake devenv up is not affected by this, as it seems to create the new runtime dir before running the procfile.

On NixOS, this error doesn't show up because XDG_RUNTIME_DIR is set by default and is used as the tmpdir. This is not touched by nix develop.

Works around an issue where `nix develop` changes the `TMPDIR`.
@sandydoo sandydoo added the bug Something isn't working label Apr 21, 2024
@sandydoo sandydoo requested a review from domenkozar April 21, 2024 18:35
# - free to create as an unprivileged user across OSes
default =
let
runtimeEnv = builtins.getEnv "DEVENV_RUNTIME";
Copy link
Member

Choose a reason for hiding this comment

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

I'm not following, what sets $DEVENV_RUNTIME when evaluating using nix develop?

Copy link
Member Author

@sandydoo sandydoo Apr 22, 2024

Choose a reason for hiding this comment

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

We first eval the shell and compute the initial devenv.runtime based on the either the XDG_RUNTIME_DIR or TMPDIR. This then becomes $DEVENV_RUNTIME in the launched shell (set via env.DEVENV_RUNTIME).

In the shell, nix develop updates the TMPDIR env, but devenv.runtime has no way of picking this up retroactively.

Once inside the shell, we run devenv up, which re-evaluates the procfileScript and picks up the new TMPDIR. devenv.runtime is now different, but the enterShell script that creates the runtime dir is not run again.

This is a bit of a hack anyways. I think the right way to solve this is to eval everything once. Something like #1142 should fix this.

@domenkozar domenkozar marked this pull request as ready for review May 23, 2024 17:04
@domenkozar domenkozar merged commit 2837f49 into main May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Processes are failing to start in flake based config

3 participants