Opened 4 years ago

Closed 12 months ago

#5442 closed bug (fixed)

ghc-pkg unregister --user/--global/--package-conf not normative

Reported by: guest Owned by: ezyang
Priority: normal Milestone: 7.10.1
Component: ghc-pkg Version: 7.2.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions: Phab:D84

Description

For package P,

"ghc-pkg unregister --user P" drops the global package if P is not in user.

"ghc-pkg unregister --global P" drops the user package if P is in user.

"ghc-pkg unregister --package-conf=<dir> P" drops the user or the global package, preferring the user one, if P is not found in <dir>.

Same with "ghc-pkg --user unregister P", "ghc-pkg unregister P --user", etc.

GHC versions 6.10, 6.12, 7.0, 7.2.

Change History (11)

comment:1 Changed 4 years ago by igloo

  • Milestone set to 7.6.1
  • Priority changed from normal to high

comment:2 Changed 3 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:3 Changed 3 years ago by igloo

  • difficulty set to Unknown
  • Milestone changed from 7.6.2 to 7.8.1

comment:4 Changed 22 months ago by bgamari

Is this actually true? Reading through the source it seems to me that this should be doing the right thing. Namely, getPkgDatabases only returns a stack containing the databases indicated by the user if any flags are specified.

comment:5 Changed 22 months ago by bgamari

  • Owner set to bgamari

comment:6 Changed 14 months ago by thoughtpolice

  • Milestone changed from 7.8.3 to 7.10.1

Bumping priority down (these tickets haven't been closely followed or fixed in 7.4), and moving out to 7.10 and out of 7.8.3.

comment:7 Changed 14 months ago by thoughtpolice

  • Priority changed from high to normal

Actually dropping priority. :)

comment:8 Changed 12 months ago by ezyang

  • Owner changed from bgamari to ezyang

OK, I can confirm that there is something fishy going on here. Here is the matrix of behaviors that ghc-pkg implements right now:

  • If P is in the user and global, unregister --user removes user, unregister --global removes USER, unregister removes user
  • If P is in the user but not global, unregister --user removes user, unregister --global removes USER, unregister removes user
  • If P is in the global but not user, unregister --user removes GLOBAL, unregister --global removes global, unregister removes global

This seems clearly wrong. The bug seems to me that ghc-pkg calculates the database to modify fine, but then ignores it in modifyPackage, always using the full database stack rather than the database it calculated to use. My suggested fix is to switch unregister to use the flag package stack, since it is implicitly a read, and then a delete. But I don't know if there is any existing tooling that would be broken by this fix. I have a patch and test-cases.

comment:9 Changed 12 months ago by ezyang

  • Differential Revisions set to Phab:D84
  • Status changed from new to patch

comment:10 Changed 12 months ago by Edward Z. Yang <ezyang@…>

In d7c807f7975c13444e1ce79e4c36dd802321cf40/ghc:

[ghc-pkg] Fix #5442 by using the flag db stack to modify packages.

Summary:
Previously, the full database stack was used for ghc-pkg to
modify packages, which meant that commands like
'ghc-pkg unregister --user' worked the same as 'ghc-pkg unregister'.
Since package modification is a "read and write" operation, we
should use the flag db stack (which is currently used for reads)
to determine which database to update.

There is also a new flag --user-package-db, which lets you explicitly
set the user database (as seen by --user).  This was mostly added
to aid in testing, but could be useful for end users as well.

Signed-off-by: Edward Z. Yang <[email protected]>

Test Plan: validate

Reviewers: simonmar, hvr, austin

Subscribers: simonmar, relrod, carter

Differential Revision: https://phabricator.haskell.org/D84

comment:11 Changed 12 months ago by ezyang

  • Resolution set to fixed
  • Status changed from patch to closed
Note: See TracTickets for help on using tickets.