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

Regression: cabal-3.12.1.0 install ignores --program-suffix when checking whether exe is already installed #10290

Closed
andreasabel opened this issue Aug 28, 2024 · 4 comments

Comments

@andreasabel
Copy link
Member

andreasabel commented Aug 28, 2024

cabal-3.12.1.0 install wrongly ignores --program-suffix when checking whether exe is already installed:

$ cabal-3.12.1.0 install hello
...
Symlinking 'hello' to '/Users/test/.local/bin/hello'

$ cabal-3.12.1.0 install hello --program-suffix=-new
...
Error: [Cabal-7149]
Path '/Users/test/.local/bin/hello' already exists. Use --overwrite-policy=always to overwrite.

cabal-3.10.3.0 can still do this correctly:

$ cabal-3.10.3.0 install hello --program-suffix=-new
...
Symlinking 'hello' to '/Users/test/.local/bin/hello-new'

Regression possibly introduced by:

The --program-suffix feature likely needs more systematic regression tests.

@ulysses4ever
Copy link
Collaborator

hey @wismill. As we're nearing 3.14.1.0 cabal-install, any chance you could take a look at this? It looks like in #10056 you modified the config flags inside the install action to "forget" about affixes in a big chunk of the action implementation. I suspect it should be limited...

@ulysses4ever
Copy link
Collaborator

I bisected it to 09c04e9, cc @philderbeast Sorry @wismill!

@philderbeast any chance you could look into this regression from your patch?

@wismill
Copy link
Collaborator

wismill commented Oct 26, 2024

Sorry, I have no time at the moment to work on this. But the key is maybe from this comment:

Fixed by making the name of the executable in the store canonical, i.e.
always ignoring the program affix options.

I wonder it the following reported error:

Path '/Users/test/.local/bin/hello' already exists. Use --overwrite-policy=always to overwrite.

merely reports an overwrite that would happen in the store, not in ~/.local/bin.

It definitely needs a fix and a test for the use case of the current issue.

@philderbeast
Copy link
Collaborator

philderbeast commented Oct 26, 2024

@philderbeast any chance you could look into this regression from your patch?

I've had a quick look but this looks too big for me to handle within my schedule for the next month.

The --program-suffix feature likely needs more systematic regression tests.

I could only find one test1 for --program-suffix, a test that includes that option when checking "whether config options are commented or not".

$ grep -R 'program-suffix' ./**/*.hs
./cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs:        , "program-suffix"
./cabal-install/tests/IntegrationTests2.hs:  "-- program-suffix" @=? findLineWith True "program-suffix" defaultConfigFile
./Cabal/src/Distribution/Simple/Setup/Config.hs:          ["program-suffix"]

-- Tests whether config options are commented or not
testConfigOptionComments :: Assertion

"-- program-suffix" `assertHasCommentLine` "program-suffix"

Footnotes

  1. I found more tests when checking files other than *.hs files. These seem to be lightweight configure and nix config tests.

ulysses4ever added a commit that referenced this issue Oct 28, 2024
When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

fixup
ulysses4ever added a commit that referenced this issue Oct 28, 2024
When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.
geekosaur pushed a commit that referenced this issue Nov 2, 2024
When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.
@mergify mergify bot closed this as completed in ee3c313 Nov 2, 2024
mergify bot pushed a commit that referenced this issue Nov 3, 2024
…10483)

* fix cabal install --program-suffix/prefix (fix #10290 and #10476)

When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

* Add regression tests for overwrite policies and porgram-affixes

Add regression tests for the `program-prefix` and `program-suffix` flags
combined with the overwrite-policy.
In short, the overwrite-policy needs to take potential program affixes
into account when deciding whether it will need to overwrite a program
path during installation.

---------

Co-authored-by: Fendor <[email protected]>
(cherry picked from commit ee3c313)
mergify bot pushed a commit that referenced this issue Nov 3, 2024
…10483)

* fix cabal install --program-suffix/prefix (fix #10290 and #10476)

When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

* Add regression tests for overwrite policies and porgram-affixes

Add regression tests for the `program-prefix` and `program-suffix` flags
combined with the overwrite-policy.
In short, the overwrite-policy needs to take potential program affixes
into account when deciding whether it will need to overwrite a program
path during installation.

---------

Co-authored-by: Fendor <[email protected]>
(cherry picked from commit ee3c313)
geekosaur pushed a commit that referenced this issue Nov 3, 2024
…10483)

* fix cabal install --program-suffix/prefix (fix #10290 and #10476)

When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

* Add regression tests for overwrite policies and porgram-affixes

Add regression tests for the `program-prefix` and `program-suffix` flags
combined with the overwrite-policy.
In short, the overwrite-policy needs to take potential program affixes
into account when deciding whether it will need to overwrite a program
path during installation.

---------

Co-authored-by: Fendor <[email protected]>
(cherry picked from commit ee3c313)
geekosaur pushed a commit that referenced this issue Nov 3, 2024
…10483)

* fix cabal install --program-suffix/prefix (fix #10290 and #10476)

When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

* Add regression tests for overwrite policies and porgram-affixes

Add regression tests for the `program-prefix` and `program-suffix` flags
combined with the overwrite-policy.
In short, the overwrite-policy needs to take potential program affixes
into account when deciding whether it will need to overwrite a program
path during installation.

---------

Co-authored-by: Fendor <[email protected]>
(cherry picked from commit ee3c313)
geekosaur pushed a commit that referenced this issue Nov 3, 2024
…10483)

* fix cabal install --program-suffix/prefix (fix #10290 and #10476)

When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

* Add regression tests for overwrite policies and porgram-affixes

Add regression tests for the `program-prefix` and `program-suffix` flags
combined with the overwrite-policy.
In short, the overwrite-policy needs to take potential program affixes
into account when deciding whether it will need to overwrite a program
path during installation.

---------

Co-authored-by: Fendor <[email protected]>
(cherry picked from commit ee3c313)
mergify bot added a commit that referenced this issue Nov 5, 2024
…10483) (#10510)

* fix cabal install --program-suffix/prefix (fix #10290 and #10476)

When checking for existing installations, cabal would not account for
an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a
second one with a non-empty affix (a perfectly legal operation) would
fail.

The reason seemed to be a typo in
09c04e9, which passed the arguments to
the Symlink structure in a wrong order.

When failing to install a binary because of an existing one, cabal would
report suffix-less existing target even if a suffix was set.

* Add regression tests for overwrite policies and porgram-affixes

Add regression tests for the `program-prefix` and `program-suffix` flags
combined with the overwrite-policy.
In short, the overwrite-policy needs to take potential program affixes
into account when deciding whether it will need to overwrite a program
path during installation.

---------

Co-authored-by: Fendor <[email protected]>
(cherry picked from commit ee3c313)

Co-authored-by: Artem Pelenitsyn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants
@philderbeast @andreasabel @wismill @ulysses4ever @Kleidukos and others