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.

Sign up for GitHub

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 packages to PATH #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add packages to PATH #25

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ inputs:
enable-cache: # 'true' or 'false'
description: 'Caching the entire Nix store in github based on your devbox.json'
default: 'false'
enable-packages-path: # 'true' or 'false'
description: 'Add devbox packages to $PATH'
default: 'false'
refresh-cli: # 'true' or 'false'
description: 'Specify whether the CLI should be redownloaded'
default: 'false'
Expand Down Expand Up @@ -152,6 +155,11 @@ runs:
run: |
devbox run --config=${{ inputs.project-path }} -- echo "Packages installed!"

- name: Adding devbox packages to $PATH
if: inputs.enable-packages-path == 'true'
shell: bash
run: echo '.devbox/virtenv/.wrappers/bin' >> $GITHUB_PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do this by default? Is there a downside to it? cc @savil thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we can do it by default because some packages may not work if the environment is not set.

A few alternatives:

  • Keep this PR as is with the caveat that some packages may not work.
  • Add new run field to the action that runs in a devbox shell.
  • export the entire devbox environment into github action environment? (not sure if this is possible)
  • Add eval "$(devbox shellenv)" before other normal run` commands. (@rcrowe this should work for you without any action changes)

If possible I think the best solution is exporting entire environment (PATH and other env vars), but this PR is OK as a middle step as well.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the comments.

I need to double check the eval suggestion as I thought I'd tried that, but each step is running in an isolated shell, it's very likely I just misconfigured something so will check.

My motivation for raising this PR was to find a way for a more out of the box experience. As you install things with the action but still have to go tweaking other things to make use of it.

Copy link
Contributor

@LucilleH LucilleH Oct 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if there's a way to add eval "$(devbox shellenv --config=<dir>)" in the rc file of the runner, so that each subsequent step gets the devbox environment automatically? But then, since the shells are all non-interactive, that wouldn't work? 🤔

Copy link

Choose a reason for hiding this comment

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

Hello! Thank you for researching this problem! Just want to share my solution:

- name: Configure devbox
  run: |
    eval "$(devbox shellenv)"
    printenv >> $GITHUB_ENV

Copy link
Contributor

Choose a reason for hiding this comment

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

Would simply doing run: devbox shellenv >> $GITHUB_ENV work as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, I'm 100% in favor of doing this. It's a breaking change, but since we're still on major version 0 that's ok?

Copy link

Choose a reason for hiding this comment

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

I think just setting the PATH will be a little fragile now that wrappers have been removed. Some packages depend on other environment variables and will fail in weird ways if they're missing. For example Python might need PYTHONPATH and the gcc wrapper will need NIX_LDFLAGS to find libraries.

Exporting everything to GITHUB_ENV sounds like the way to go, but we might need to do a bit of tweaking to get the syntax right. Namely:

  • If I recall, the lines in $GITHUB_ENV can't have quoted values because they don't get reinterpreted by the shell. So echo 'FOO="bar"' >> $GITHUB_ENV is like doing setenv("FOO", "\"bar\"").
  • Multi-line values need to be handled (I think Nix does set some of these).

Copy link

Choose a reason for hiding this comment

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

We'll also need to make sure that the devbox shellenv output doesn't have arbitrary shell commands.

Copy link

@altano altano Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

eval $(devbox shellenv) doesn't work with Devbox's corepack virtual environment. devbox shellenv doesn't do anything with the corepack virtenv directory, so your package manager won't be on your path. I think that somehow comes from devbox global shellenv (even though corepack is not global!).


- name: Save nix store cache
if: inputs.enable-cache == 'true' && steps.cache-devbox-nix-store.outputs.cache-hit != 'true'
uses: actions/cache/save@v3
Expand Down