-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Remove /etc as hardcoded sysconfdir from Nix build #1777
Conversation
Our static binaries behaved differently than our regular relocatable installations: They were looking for the global configuration in `/etc/vast/vast.yaml` rather than the expected `<prefix>/etc/vast/vast.yaml`. This change makes them behave just like regular relocatable installations. For users, this means that configuration files at `/etc/vast/vast.yaml` need to be moved accordingly.
670bfe2
to
f5e52e5
Compare
Why is the example config all commented out? |
That's intentional: It's an example config file, and if we install it, then we don't want it to mesh with the actual defaults. Unless we find a way to guarantee that defaults and example config are in sync (which we don't right now), I'd rather have everything commented out. |
I would agree if it's an actual The UX of reading a properly syntax-highlighted file and separate the tuning knobs from the content is not to be dismissed, I think. |
Take a look at the changed CMake code in this PR: We install this file to |
I wonder whether we should simply keep the uncommented example file in the repo and install a transformation (e.g., through |
e6c9358
to
a0e697c
Compare
Also, avoid running black on vast.yaml.example; the CI was a bit overzealous here with black thinking the vast.yaml.example was valid Python code.
a0e697c
to
1ff9dc8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you can get awk
installed in Nix, we're in business in CI.
9bf1548
to
0de237e
Compare
0de237e
to
3fd3e57
Compare
📔 Description
This PR contains two changes:
/etc/vast/vast.yaml
rather than the expected<prefix>/etc/vast/vast.yaml
. This change makes them behave just like regular relocatable installations.For users, this means that configuration files at
/etc/vast/vast.yaml
need to be moved accordingly.📝 Checklist
🎯 Review Instructions
Read the code, try the static binary from CI.