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

files: avoid surprises when linking files #3018

Merged
merged 1 commit into from
Jan 9, 2023

Conversation

ncfavier
Copy link
Member

@ncfavier ncfavier commented Jun 12, 2022

In the unlikely (but possible) event that a directory is created in place of a target file after checkLinkTargets but before linkGeneration, currently the link is created inside the directory, which is surprising (see #3017 for context).

To avoid this, use ln -T and bail if it fails.

Also bail if backing up the target fails.

Ideally, checkLinkTargets and linkGeneration should be more tightly coupled.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

Copy link
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I'm pretty sure I hit this situation at some point but I don't remember.

The -T flag does not seem to be POSIX and does not seem to be supported by BSD ln (so not supported by Darwin/Mac).

Do you know whether the GNU coreutil package is in the activation script's $PATH? If so, my previous comment can be ignored but a note should be left.

@ncfavier
Copy link
Member Author

ncfavier commented Jul 7, 2022

Do you know whether the GNU coreutil package is in the activation script's $PATH? If so, my previous comment can be ignored but a note should be left.

Yep. I'm not sure we want to add a comment in the code every time we rely on this though, it seems like a fine "ambient assumption".

@stale
Copy link

stale bot commented Oct 6, 2022

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Oct 6, 2022
@ncfavier ncfavier requested review from berbiche and removed request for rycee October 6, 2022 07:59
@ncfavier
Copy link
Member Author

ncfavier commented Dec 2, 2022

cc @rycee, I still think this change is good

@stale stale bot removed the status: stale label Dec 2, 2022
@sumnerevans sumnerevans requested a review from rycee December 2, 2022 15:04
In the unlikely but possible event that a directory is created in
place of a target file *after* `checkLinkTargets` but *before*
`linkGeneration`, currently the link is created *inside* the directory,
which is surprising.

To avoid this, use `ln -T` and bail if it fails.

Ideally, `checkLinkTargets` and `linkGeneration` should be more tightly
coupled.
@ncfavier ncfavier force-pushed the files-no-surprises branch from a77d089 to af4d6e7 Compare January 3, 2023 12:57
@ncfavier
Copy link
Member Author

ncfavier commented Jan 3, 2023

I've removed the other change, added a comment and rebased on master.

Also note that we're already using ln -T:

$DRY_RUN_CMD ln -Tsf $VERBOSE_ARG "$newGenPath" "$newGenGcPath"

@ncfavier ncfavier merged commit 69806e9 into nix-community:master Jan 9, 2023
@ncfavier ncfavier deleted the files-no-surprises branch January 9, 2023 12:00
15cm pushed a commit to 15cm/home-manager that referenced this pull request Feb 9, 2023
soywod added a commit to soywod/home-manager that referenced this pull request Feb 9, 2023
add none backend and sender

fix typos

run ./format -C

add notify service

add watch service

run ./format -C

add sync options

starship: fix nushell integration

Overwrite starship/init.nu if already exists, since this is a cache
file for sourcing in `init.nu`.

starship: re-add ion integration

which was apparently mistakenly removed in commit 7ae7250

broot: update test to match upstream changes

Fixes nix-community#3527

broot: simplify test slightly

flake.lock: Update

Flake lock file updates:

• Updated input 'nixpkgs':
    'github:nixos/nixpkgs/757b82211463dd5ba1475b6851d3731dfe14d377' (2022-12-16)
  → 'github:nixos/nixpkgs/fad51abd42ca17a60fc1d4cb9382e2d79ae31836' (2022-12-25)

direnv: enable nushell integration

This enables nushell integration by default for direnv, similar to
bash/zsh/fish. The slightly verbose way of setting this is to ensure
that peoples' existing nushell configuration isn't overwritten, only
appended to, as would be the case if we just used the integration
example from the nushell docs:

  https://www.nushell.sh/cookbook/direnv.html

Closes nix-community#3520

fish: set tmp $HOME to silence errors

parcellite: add extraOptions option

Even though `--no-icon` is currently the only viable option for both
parcellite and clipit, other options may be added to later releases.

parcellite: add basic test case

clipman: add module

gitui: update default theme to match upstream

Fixes nix-community#3506

neovim: fix extraLuaPackages type. (nix-community#3533)

Assigning to `programs.neovim.extraLuaPackages` a function taking a lua package set as input
and returning a list of packages, as described in the documentation,
threw an error because the rest of the code assumed that the value was always a plain list.
Using `lib.types.coercedTo`, we can accept such functions, as per the documentation,
as well as plain lists, which we then convert to a function ignoring its input argument.
We print a warning when a plain list is assigned, since the function
form is preferred, as it ensures that the right lua package set is used.

For the lua packages, we also get the lua package set from the
finalPackage, to make sure that we are always using the same package set
as the actual unwrapped neovim package being built.

For `programs.neovim.extraPythonPackages` I did the same.

I updated the test case so that we test both ways of setting these options.

kakoune: update hooks (nix-community#3418)

Some hooks were removed in Kakoune, and some were added. This PR updates the list so they are aligned with the latest version of Kakoune.
Removed

mawww/kakoune@e4fb70e

    NormalBegin
    NormalEnd
    InsertBegin
    InsertEnd

mawww/kakoune@78419bc

    InsertCompletionSelect

Added

mawww/kakoune@c8839e7

    ClientCreate
    ClientClose

mawww/kakoune@47ba36c

    RegisterModified

mawww/kakoune@f2cc7bc

    User

man: update database cache generation

Specifically, use the configured man-db package for generating the
database cache.

i3: Fix escaping in documentation

In nix `${` is escaped like this `''${` and not like this `\${`

home-environment: use `lazyAttrsOf` for `home.sessionVariables` (nix-community#3541)

* home-environment: use `lazyAttrsOf` for `home.sessionVariables`

`attrs` has unreasonable merge semantics and is deprecated. `attrsOf`
doesn't support variables depending on each other as is recommended in
the option's description.

* home-environment: restrict `sessionVariables` type

The consumer is `toString`, but we don't want to accept e.g. lists.

vim,neovim: add `defaultEditor` (nix-community#3496)

Also rename `vim_configurable` to `vim-full` per NixOS/nixpkgs#204438

feh: Add package option (nix-community#3552)

Co-authored-by: Naïm Favier <n@monade.li>
Co-authored-by: Sumner Evans <me@sumnerevans.com>

i3-sway: Use foot as default terminal on sway (nix-community#3490)

fluxbox: fix a typo, windowMenu -> windowmenu (nix-community#3544)

home.pointerCursor: use mkDefault to set XCURSOR_PATH (nix-community#3553)

fixes nix-community#3551

docs: bump nmd

- Simplifies DocBook build by generation version DocBook 5 files
  directly.

- Make DocBook generate consistent anchor IDs.

easyeffects: add package option (nix-community#3568)

docs: bump nmd

- Introduces support for Markdown documentation in modules.

ncmpcpp: Allow `str` type values for `mpdMusicDir` option (nix-community#3565)

The default value of `programs.ncmpcpp.mpdMusicDir` is taken from
`services.mpd.musicDirectory` if the mpd module is enabled, which has
type `either path str`. `programs.ncmpcpp.mpdMusicDir` did not accept
`str` values, though, so an error was raised when the default value was
used and `services.mpd.musicDirectory` was set to a value of type `str`.

This commit changes the type of `programs.ncmpcpp.mpdMusicDir` to also
accept `str` to reflect the type of `services.mpd.musicDirectory`.

Fixes nix-community#3560

docs: bump nmd

Fixes nix-community#3573

i3: remove i3/i3-gaps distinction (nix-community#3563)

firefox: remove https-everywhere from example

See https://gitlab.com/rycee/nur-expressions/-/commit/b70c44e8575e65ff63c5f52eb726aa3fc9a86af7

files: avoid surprises when linking files (nix-community#3018)

docs: bump nmd

Fixes nix-community#3579

Translate using Weblate (Norwegian Bokmål)

Currently translated at 87.5% (28 of 32 strings)

Co-authored-by: Petter K <petterkarlsrud@me.com>
Translate-URL: https://hosted.weblate.org/projects/home-manager/cli/nb_NO/
Translation: Home Manager/Home Manager CLI

Translate using Weblate (Chinese (Traditional))

Currently translated at 92.8% (13 of 14 strings)

Co-authored-by: Eric Ho <eric913@gmail.com>
Translate-URL: https://hosted.weblate.org/projects/home-manager/modules/zh_Hant/
Translation: Home Manager/Home Manager Modules

Add translation using Weblate (Lithuanian)

Co-authored-by: Kornelijus Tvarijanavičius <kornelitvari@protonmail.com>

Add translation using Weblate (Lithuanian)

Co-authored-by: Weblate <noreply@weblate.org>

Translate using Weblate (Lithuanian)

Currently translated at 84.3% (27 of 32 strings)

Co-authored-by: Kornelijus Tvarijanavičius <kornelijus@tvaria.com>
Translate-URL: https://hosted.weblate.org/projects/home-manager/cli/lt/
Translation: Home Manager/Home Manager CLI

i3: Do not set i3 package (nix-community#3585)

Resolves nix-community#3584

home-manager: Add home-manager search path to EXTRA_NIX_PATH (nix-community#3241)

docs: bump nmd

- Fix paragraphs in option field texts containing Markdown.

- Fix generation of documentation for options with Markdown in
  multiple fields.

- Support AsciiDoc in module option documentation.

i3-sway: config.focus.wrapping deprecates forceWrapping (nix-community#3467)

Stop using the legacy syntax described in the i3 documentation:
https://i3wm.org/docs/userguide.html#_focus_wrapping

opam: fix enableFishIntegration (nix-community#3597)

Fish shell doesn't require arguments to `eval` to be double quoted
like in a bash shell. At the moment doing so gives us the following
error:

~/.config/fish/config.fish (line 12): $(...) is not supported. In fish, please use '(/nix/store/8asq…)'.
eval "$(/nix/store/8asqgnhs89wzyjvs8p1n5hvxn7lkn9wa-opam-2.1.3/bin/opam env --shell=fish)"
      ^
from sourcing file ~/.config/fish/config.fish
	called during startup
source: Error while reading file “/home/user/.config/fish/config.fish”

This commit fixes the above error.

docs: mention how to override the home-manager flake input (nix-community#3556)

docs: fetch nmd from sourcehut

docs: bump nmd

firefox: refactor search.json.mozlz4 generation

The new setup should be easier to read.

gpg-agent: fix SSH support for fish

docs: bump nmd

Fixes nix-community#3605

flake.lock: Update (nix-community#3549)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

docs: bump nmd

Fixes nix-community#3612

treewide: fix typos (nix-community#3618)

files: allow disabling creation of a file

PR nix-community#3622

ci: bump DeterminateSystems/update-flake-lock from 15 to 16

Bumps [DeterminateSystems/update-flake-lock](https://github.com/DeterminateSystems/update-flake-lock) from 15 to 16.
- [Release notes](https://github.com/DeterminateSystems/update-flake-lock/releases)
- [Commits](DeterminateSystems/update-flake-lock@v15...v16)

---
updated-dependencies:
- dependency-name: DeterminateSystems/update-flake-lock
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

bash: format using nixfmt

PR nix-community#3609

papis: add module

wlogout: add module (nix-community#3629)

rbenv: add module

pass-secret-service: various improvements

Allow setting the application package and storePath used by the
config. Since the `programs.password-store` Home Manager module sets
config values via global environment variables, the default behavior
of the module should continue to behave as before for the user.

Additionally,

- Adds a few tests.

- Use "escapeShellArg" function call to the path parameter call to
  ensure paths with spaces work.

- Allow not setting storePath, which will cause `pass_secret_service`
  to default to using `~/.password-store`.

- If `pass-secret-service` is enabled, set its store path to default
  to the one defined in our password-store environment settings.

- Add myself (houstdav000) as maintainer.

home-manager: edit `homeManagerConfiguration` err (nix-community#3632)

* home-manager: edit `homeManagerConfiguration` err

This adds https://nix-community.github.io/home-manager/release-notes.html#sec-release-22.11-highlights
to the homeManagerConfiguration arguments error.

* home-manager: `homeManagerConfiguration` indent

Moved URL of the release notes for 22.11 to the same line so that
./format doesn't add extra whitespace

---------

Co-authored-by: zaporter <opensource@zackporter.com>

git: unset `pager` while using difftastic (nix-community#3633)

flake.lock: Update (nix-community#3619)

Flake lock file updates:

• Updated input 'nixpkgs':
    'github:nixos/nixpkgs/5ed481943351e9fd354aeb557679624224de38d5' (2023-01-20)
  → 'github:nixos/nixpkgs/2caf4ef5005ecc68141ecb4aac271079f7371c44' (2023-01-30)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

services.autorandr: add module

mbsync: make passwordCommand escaping consistent (nix-community#3630)

* Fix mbsync passwordCommand escaping

* email: use lib.escapeShellArgs

Co-authored-by: Naïm Favier <n@monade.li>

* mbsync: update tests

---------

Co-authored-by: Naïm Favier <n@monade.li>

tmux: mouse support (nix-community#3642)

Co-authored-by: Sleroq <sleroq@sleroq.link>

modules: java: fix setting JAVA_HOME (nix-community#3638)

Some JVMs pass through `home` as a derivation rather than as a string, as `openjdk` does. Since the module option for session variables expects a string, this is a type error. I suspect that this incorrect, and have changed the assignment here to coerce the `cfg.package.home` attribute to a string to be safe.

After discussing with @NobbZ, we have decided it is best to mitigate this problem in HM rather than to make potentially breaking changes to Nixpkgs.

Please do mention if you think we ought to propose a change to Nixpkgs instead.

vim-vint: add module (nix-community#3604)

* vim-vint: add module

* vim-vint: add tests

* maintainers: add tomodachi94

* vim-vint: fix tests

---------

Co-authored-by: Naïm Favier <n@monade.li>

borgmatic: Do not inhibit idle in service (nix-community#3637)

This reflects a systemd service sample file change made in borgmatic
1.7.6, commit 2e9f70d49647d47fb4ca05f428c592b0e4319544:

    When backing up a machine with a monitor using logind to control
    idle timeout and things like DPMS, borgmatic can block the screen
    from turning on/off with systemd-inhibit. This is because by
    default systemd-inhibit will block
    "idle:sleep:shutdown". Borgmatic does not need to care about idle,
    only about suspend and shutdown. So, add an explicit `--what` flag
    for what borgmatic should inhibit.

    For more information see systemd-inhibit(1).

home-manager: pass --refresh to nix (nix-community#3624)

This flag is useful to force Nix to re-fetch cached flakes. Without it,
you cannot deploy from a non-local flake in quick succession, since the
caching causes the flake to not be re-fetched.

tmux: fix secureSocket environment variable (nix-community#3593)

firefox: support passing any json value to settings (nix-community#3580)

Firefox internally only supports bool, int, and string types for
preferences, but often stores objects, arrays and floats as strings.

This change makes it nicer to specify those type of preferences in
Nix, and it also makes it possible to merge objects & arrays across
multiple modules.

programs.neovim: add extraLuaConfig (nix-community#3639)

* programs.neovim: add extraLuaConfig

Add a configuration option to add custom lua configuration lines to
`lua.init`.

* apply review: formatting

* apply review: fix test

firefox: manage add-ons per-profile

Internally we already managed them per-profile but exposed a global
option to maintain backwards compatibility. The benefit to having
per-profile extensions is quite large though, so it is time to switch.

Users of the global extensions option will get an error message that
indicates how to edit their configuration to work again.

nix: fix package assertion

Sync it with the condition for generating nix.conf so that it triggers
if `extraOptions` is set but `package` isn't.

activation-init.sh: remove shebang, execute bit

This is not a standalone script, it's only meant to be included from the
activation script.

modules: add platform assertions

systemd: remove platform assertion

Allow modules to define systemd services on macOS. It won't actually
have any effect, but it would allow modules to define both systemd
services and launchd agents without boilerplate conditionals.

As a consequence of this change, each module would have to check for
compatibility with the OS target instead.

scmpuff: clean up tests

Stub the scmpuff package. Also remove unnecessary `config` wrapping.

docs: bump nmd

Fixes cross-compilation.

xsuspender: fix typo that made debug option a noop (nix-community#3653)

vscode: add extensions.json file in extensions dir (nix-community#3588)

* vscode: add extensions.json file in extensions dir

This change generates an 'extensions.json` file the same way that
nixpkgs' vscode-with-extensions does, and makes sure it is placed in the
directory with the extensions.

* vscode: remove leftover trace

Co-authored-by: Naïm Favier <n@monade.li>

* vscode: fix adding extensions.json with mutable extension dir

Co-authored-by: Naïm Favier <n@monade.li>

* vscode: let vscode regenerate the mutable extensions.json

* Remove nixpkgs duplication; only apply on vscodes new enough to need it

* Use lib.versionAtLeast

Co-authored-by: Naïm Favier <n@monade.li>

* Format vscode.nix

---------

Co-authored-by: Naïm Favier <n@monade.li>

vscode: fix erroneous application of lib.optional (nix-community#3655)

Revert "mbsync: make passwordCommand escaping consistent" (nix-community#3657)

This reverts commit e2c1756.

himalaya: add missing service options

Also improved doc in general.
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
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