cabal new-install silently fails if symlink target already exists

[hvr]

Currently, when you e.g. cabal new-install hsc2hs --symlink-bindir=/tmp/bin, and something exists already in /tmp/bin, new-install will claim to

Symlinking hsc2hs

even though it failed to update the symlink.

E.g.

$ cabal new-install hsc2hs-0.68.3 --symlink-bindir=/tmp/bin/
Resolving dependencies...
Up to date
Symlinking hsc2hs

$ ls -l /tmp/bin/
total 0
lrwxrwxrwx 1 hvr hvr 127 Aug  2 11:17 hsc2hs -> ../../home/hvr/.cabal/store/ghc-8.2.2/hsc2hs-0.68.3-098a68e230c98f65b69ba9c636ab16a4395deacc40aff03647d85d54a5ad7aeb/bin/hsc2hs

that's ok so far, as the end-result was in fact having a symlink pointing to hsc2hs-0.68.3; however, when I request to new-install again which is expected to results in a different symlink, new-install silently fails to:

$ cabal new-install hsc2hs-0.68.2 --symlink-bindir=/tmp/bin/
Resolving dependencies...
Build profile: -w ghc-8.2.2 -O1
In order, the following will be built (use -v for more details):
 - hsc2hs-0.68.2 (exe:hsc2hs) (requires download & build)
Downloading  hsc2hs-0.68.2
Downloaded   hsc2hs-0.68.2
Starting     hsc2hs-0.68.2 (exe:hsc2hs)
Building     hsc2hs-0.68.2 (exe:hsc2hs)
Installing   hsc2hs-0.68.2 (exe:hsc2hs)
Completed    hsc2hs-0.68.2 (exe:hsc2hs)
Symlinking hsc2hs

$ ls -l /tmp/bin/
total 0
lrwxrwxrwx 1 hvr hvr 127 Aug  2 11:17 hsc2hs -> ../../home/hvr/.cabal/store/ghc-8.2.2/hsc2hs-0.68.3-098a68e230c98f65b69ba9c636ab16a4395deacc40aff03647d85d54a5ad7aeb/bin/hsc2hs

Consequently, new-install ought to at very least warn if not even error (or even better yet overwrite), when it cannot produce the desired final state of the symlink; specifically,

I'd expect two subsequent invocations of e.g.

$ cabal new-install hsc2hs-0.68.3 --symlink-bindir=/tmp/bin/
$ cabal new-install hsc2hs-0.68.3 --symlink-bindir=/tmp/bin/

to be idempotent and the 2nd invocation not warn if the first one succeded, as the resulting end-state of the symlink would be unchanged.

---

As to whether we ought to

• warn (and don't overwrite existing entries)
• error (i.e. abort; don't do anything)
• force to overwrite (in which case I'd expect a warning/info message if an existing symlink was updated to a new target)

I'm not totally sure (I'd prefer the overwrite variant); I merely claim that the current situation is definitely bad/wrong, and any of the options above would be an improvement.

See also #5408 where old-build appears to have a warning

Warning: could not create a symlink in /home/me/.cabal/bin for hlint because the file exists there already but is not managed by cabal. You can create a symlink for this executable manually if you wish. The executable file has been installed at /home/me/.cabal/bin/hlint

[phadej]

I'd prefer error. overwrite is second best.

Would be nice to make an error even before building anything.

[hvr]

@phadej what'ya say about making error default, and introducing a --install-force-overwrite flag? Then the error could read something like "Target file exists already; use '--install-force-overwrite' if you want to overwrite/update install target files"

[chshersh]

to be idempotent and the 2nd invocation not warn if the first one succeded

I would like to observe this behaviour as well. If nothing changes then the second command should ideally do nothing and say something like Already has symlink to this version.

If versions are different and symlink already exist then I'm okay with having error by default and see message that @hvr suggested in this comment: #5491 (comment)

[int-index]

Overwriting sounds like a better default: I sometimes reinstall to upgrade to a newer version. After all, we're talking aboit a symlink. We don't lose any data.

It would be nice to print a warning and a path to the old symlink destination, so that the user could undo the overwrite easily.

[23Skidoo]

v1-install overwrites, so overwriting wouldn't be surprising, I think.

[dmwit]

I'll chime in with my preferred bikeshed color.

Symlinks should be checked during build-plan creation. If any symlinks would be modified, there should be an error (ergonomics of this error discussed below). A build-plan that involves creating or rewriting symlinks but having them point at the same thing they're already pointing at shouldn't trigger an error. There should be a flag that forces action, even if this would modify some symlinks. Anyway the goal here in to cause an error as quickly as possible -- it would be very painful to kick off a 20 minute build and go to do something else only to discover it failed 3 minutes in.

It could happen that the filesystem is modified after build-plan creation time but before symlink-creation time. Each symlink modification/creation should make an effort to detect this situation. In case it happens, I think a fatal and unrecoverable error is simplest, and this situation should be exceptional enough that this is not objectionable. The user probably knows what they did to cause it, anyway.

About the early error reporting. I expect by far the most common scenario is that each overwritten symlink is moving from one version or dependency hash of a package to another version or dependency hash of the same package. If this is the case for all the executables being overwritten, the error should list each package involved once, together with either the old/new version pair if that is different or the old/new dependency tree hash if the versions are the same. I think it's not necessary to list all the executables for each of these packages, but this is also my least strongly held opinion of this whole comment.

In exceptional circumstances, some executable is going to be provided by a different package than it used to be; then the error should list each executable that will be overwritten, together with the old/new package pair (if packages differ), the old/new version pair (if packages are the same but versions differ), or the old/new hash pair (if packages and versions are the same).

[KirinDave]

I've been asked to add my opinion, so I will.

From the perspective of configuring machines and/or docker images, I'd much rather have a default error condition on overwrite (even if it would converge to an identical state on overwrite). Usually when this happens, it's the sign of something wrong in automation or an operator error. I agree that a force-overwrite would be nice, and it would be excellent as a short option since the most likely case for such a flag would be human intervention (manually upgrading a dev environment or quickly producing a prototype docker image with updated deps for testing).

Notification of error is a lot easier than having a flag to require an error, because it's easier to search documentation (or I guess, all the of the internet) for a present event with a fixed error string rather than the alternative.

[gwils]

I'd prefer an error, with a flag to allow overwrite. It would also be good if the error message mentioned the flag.

Maximum bikeshedding:
I'd prefer that the flag name isn't too scary looking (ie. not --force). Upgrading to a newer version of a package is something I do all the time, as I imagine a lot of people do. I don't want it to happen without my knowing, but if I want to do it, it shouldn't feel like I'm straying into unsafe territory. Something like --upgrade or --allow-upgrade would be my preference.

[hvr]

@gwils btw, --allow-upgrade would imply that we'd only allow to update symlinks to newer versions; in that case we'd also need --allow-downgrade, and support the combination --allow-upgrade --allow-downgrade to just allow whatever is needed to achieve the requested end-state.

[phadej]

I didn't read properly, but --allow-upgrade is difficult. Somewhat contrived examples:

cabal new-install doctest --constraint='doctest==0.16.0' -w ghc-8.2.2

# what will happen?
cabal new-install doctest --constraint='doctest==0.15.0' -w ghc-8.4.3 --allow-upgrade

# or same versions of package & compiler but different flags
cabal new-install doctest --constraint="doctest -some-flag" -w ghc-8.2

# or, comparing install plans for "newer" is difficult:
cabal new-update
cabal new-install doctest --constraint='doctest==0.16.0' -w ghc-8.2.2

So I'd prefer a quick error by default (and no error if exactly same link), and a --force-overwrite flag.

I had following sequence way to often:

cabal new-install hlint
# builds new hlint just to tell me that I already had it :(

OTOH: this is more rare, but I need a manual step now

hlint ...
# Error, haskell-src-exts too old, cannot handle my code

# I'd like to
cabal new-install hlint --force-overwrite
# but I need to
rm -rf ~/.cabal/bin/hlint
cabal new-install hlint

# and now:
hlint ...
# works! :)

---

To bikeshed ever further, I'd like to have --force-overwrite-with-lease (or something like that),
where building / installing will happen only if, symlink exists, and points somewhere into the store.
If the file is there, but doesn't point to store (installed by stack? copied or linked manually?), it would error. If executable is not there, it will error with "what you wanna me to do".
And if the to-builded version is the same, then the new-install would be an no-op.

[hvr]

(marking high prio as users are reporting this and being confused)

[fgaz]

@hvr can we lower priority now that #5602 is merged?

[fommil]

It still makes it very difficult to upgrade cabal-install itself.

[tdammers]

I'm thinking the reason why opinions on the "replace silently" vs. "error out" matter clash here is because of conflicting use cases.

One use case is installing things for production use; in this case, you don't want to casually replace symlinks, so an error (with an -f override or the like) would be the right thing to have.

The other use case is a development loop: change code - build - install - run & test. In this scenario, we do want to silently overwrite the symlink, because the effect we are after is that whatever binary is on the $PATH reflects the most recently built state of the codebase.

[fgaz]

#5638 adds the error and the flag. I'm lowering the priority of this issue since it only remains to check for error/no-op as early as possible.

[fgaz]

Closing in favor of #6832 since this issue is actually resolved and keeping it open is misleading.