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

hardware config: fix git add logic #424

Merged
merged 2 commits into from
Nov 16, 2024
Merged

Conversation

picnoir
Copy link
Member

@picnoir picnoir commented Nov 12, 2024

Fixing the git add hardware config logic in case the user provides a relative path. We're doing two things:

  1. Failing loudly if the git add command fails. At that point, we already know git is in $PATH. If the git add fails, the evaluation will likely fail in a flake setting. Better fail early with a helpful git error message.
  2. We're resolving the hardware config path in the CLI parsing layer to make sure this path has the meaning the user intented it to have.

@picnoir
Copy link
Member Author

picnoir commented Nov 12, 2024

The CI failure is related to a test case where we have git but the local repo is not git-tracked.

I guess we should check if the local repo is git-tracked before failing?

@picnoir picnoir force-pushed the pic/git branch 3 times, most recently from 188183a to 35f99c9 Compare November 12, 2024 15:04
@picnoir
Copy link
Member Author

picnoir commented Nov 12, 2024

CI fixed. This is ready for review.

pushd "$(dirname "$hardwareConfigPath")"
git add --intent-to-add --force -- "$hardwareConfigPath" >/dev/null 2>&1 || true
popd
if git rev-parse --is-inside-work-tree >/dev/null; then
Copy link
Member

@Mic92 Mic92 Nov 15, 2024

Choose a reason for hiding this comment

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

So does this mean users have to change into the repository for this to work?
I can imagine that users would do something like
nixos-anywhere --flake $HOME/git/my-config

Copy link
Member Author

@picnoir picnoir Nov 15, 2024

Choose a reason for hiding this comment

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

Riiight, I get the pushd/popd part now!

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@picnoir picnoir force-pushed the pic/git branch 2 times, most recently from b1d192d to bae44b6 Compare November 15, 2024 08:30
Fixing the git add hardware config logic in case the user provides a
relative path. We're doing two things:

1. Failing loudly if we're in a git repo and the git add command
   fails. At that point, we already know git is in $PATH. If the git add
   fails, the evaluation will likely fail in a flake setting. Better fail
   early with a helpful git error message.
2. We're resolving the hardware config path in the CLI parsing layer
   to make sure this path has the meaning the user intented it to have.
@Mic92
Copy link
Member

Mic92 commented Nov 16, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Nov 16, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 548b4a9

@mergify mergify bot merged commit 548b4a9 into nix-community:main Nov 16, 2024
5 checks passed
@picnoir picnoir deleted the pic/git branch December 24, 2024 08:04
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.

2 participants