snowleopard / hadrian

Hadrian: a new build system for the Glasgow Haskell Compiler. Now merged into the GHC tree!
https://gitlab.haskell.org/ghc/ghc/tree/master/hadrian
MIT License
208 stars 39 forks source link

ghc-pkg register failed with "already installed" #543

Closed izgzhen closed 6 years ago

izgzhen commented 6 years ago
| Run GhcPkg Dependencies Stage0: integer-gmp
rts-1.0: package(s) with this id already exist: rts
rts-1.0: package rts-1.0 is already installed
rts-1.0: Package names may be treated case-insensitively in the future.
Package rts-1.0 overlaps with: rts-1.0 (use --force to override)
shakeArgsWith   0.000s    0%                           
Function shake  8.289s   50%  =========================
Database read   0.003s    0%                           
With database   0.000s    0%                           
Running rules   8.154s   49%  ======================== 
Total          16.446s  100%                           
Error when running Shake build system:
* _build/stage1/bin/unlit
* OracleQ (PackageDataFile (Context {stage = Stage1, package = Package {pkgLanguage = Haskell, pkgType = Program, pkgName = "unlit", pkgPath = "utils/unlit"}, way = v}))
* _build/stage1/utils/unlit/setup-config
* _build/stage0/bin/ghc
* OracleQ (PackageDataFile (Context {stage = Stage0, package = Package {pkgLanguage = Haskell, pkgType = Program, pkgName = "ghc-bin", pkgPath = "ghc"}, way = v}))
* _build/stage0/ghc/setup-config
* _build/stage0/lib/package.conf.d/ghc-8.5.conf
* OracleQ (PackageDataFile (Context {stage = Stage0, package = Package {pkgLanguage = Haskell, pkgType = Library, pkgName = "ghc", pkgPath = "compiler"}, way = v}))
* _build/stage0/compiler/setup-config
* _build/stage0/lib/package.conf.d/transformers-0.5.5.0.conf
* _build/stage0/lib/package.conf.d/base-4.10.1.0.conf
* _build/stage0/lib/package.conf.d/rts.conf
user error (Development.Shake.cmd, system command failed
Command: /usr/local/bin/ghc-pkg --global-package-db _build/stage0/lib/package.conf.d register -v0 -
Exit code: 1
Stderr:
rts-1.0: package(s) with this id already exist: rts
rts-1.0: package rts-1.0 is already installed
rts-1.0: Package names may be treated case-insensitively in the future.
Package rts-1.0 overlaps with: rts-1.0 (use --force to override)
)

probably one of #540

izgzhen commented 6 years ago

there are current three solutions as I see:

  1. try to unregister and then register
  2. try to register
  3. register if we can say it is safe (through describe or latest subcommands).

The try to here means a fallible cmd action (@snowleopard BTW, how to retrieve the exit code without aborting?).

snowleopard commented 6 years ago

I think we can get the exit code using this:

https://hackage.haskell.org/package/shake-0.16.3/docs/Development-Shake.html#t:Exit

izgzhen commented 6 years ago

@snowleopard Thanks! also, I am curious if there is any existing case of "if A failed then run B" in hadrian's code base?

izgzhen commented 6 years ago

it looks like a "no":

> git grep -n '<- cmd '
src/Builder.hs:207:            Stdout stdout <- cmd [path] ["--no-user-package-db", "field", input, "depends"]
src/Builder.hs:224:                    Stdout stdout <- cmd [path] buildArgs
src/Builder.hs:246:                    Stdout stdout <- cmd (Stdin stdin) [path] buildArgs
src/Builder.hs:260:                    Stdout pkgDesc <- cmd [path]
snowleopard commented 6 years ago

Indeed, there are no cases like this. I'm interested what @ndmitchell thinks about using the Exit feature for solving this issue.

izgzhen commented 6 years ago

Looks like the solution 3 also requires a "try-to" mechanism, since running latest or describe on non-existing package will throw as well...

snowleopard commented 6 years ago

@izgzhen We didn't have to deal with this previously. Why do we need now? What has changed?

izgzhen commented 6 years ago
diff --git a/src/Rules/Register.hs b/src/Rules/Register.hs
index 7c0a3e0..14b085d 100644
--- a/src/Rules/Register.hs
+++ b/src/Rules/Register.hs
-registerPackage :: [(Resource, Int)] -> Context -> Rules ()
-registerPackage rs context@Context {..} = do
-    when (stage == Stage0) $ do
-            ....
-            buildConf rs context
+registerPackages :: [(Resource, Int)] -> Context -> Rules ()
+registerPackages rs context@Context {..} = do
+  root <- buildRootRules
+  root -/- relativePackageDbPath stage %>
+    buildStamp rs context

-        when (package == ghc) $ "//" ++ stage0PackageDbDir -/- packageDbStamp %>
-            buildStamp rs context
+  root -/- relativePackageDbPath stage -/- packageDbStamp %> \stamp ->
+    writeFileLines stamp []

-    when (stage == Stage1) $ do
-        inplacePackageDbPath -/- pkgName package ++ "*.conf" %%>
-            buildConf rs context
+  root -/- relativePackageDbPath stage -/- "*.conf" %> \conf -> do
+    ....
+    case stage of
+      Stage0 | pkg `notElem` bootLibs -> copyConf rs (context { package = pkg }) conf
+      _                               -> buildConf rs (context { package = pkg }) conf

-        when (package == ghc) $ inplacePackageDbPath -/- packageDbStamp %>
-            buildStamp rs context

 buildConf :: [(Resource, Int)] -> Context -> FilePath -> Action ()
-buildConf rs context@Context {..} conf = do
-    confIn <- pkgInplaceConfig context
-    need [confIn]
-    buildWithResources rs $ target context (GhcPkg Update stage) [confIn] [conf]
+buildConf _ context@Context {..} _conf = do
+    ....
+    copyPackage context
+    registerPackage context
+
+copyConf :: [(Resource, Int)] -> Context -> FilePath -> Action ()
+copyConf rs context@Context {..} conf = do
+      ....
+      target context (GhcPkg Clone stage) [pkgName package] [conf]
izgzhen commented 6 years ago

an edited except above might help illustrate: before #531, we never call ghc-pkg register, but now we need.

I used to call ghc-cabal's register in my old install implementation, but I am not sure if it is relevant. Look like we register locally now somehow. Maybe @alpmestan @angerman can help on this issue?

angerman commented 6 years ago

Yes. This is new. We now build properly populated package databases that can stand on their own. (An alternative approach would be package db chaining).

Quickfix: pass --force to register. I'm not sure this is the right solution though under all circumstances.

What we do, is we query the bootstrap compilers package database for the relevant package, and then clone it (ghc-pkg dump + ghc-pkg register) it into the local package database. This only happens for packages that we do not build ourselves. Notably anything that's not a bootstrap-package.

We do this so that we have a consistent package database together with the bootstrap packages when building the first set of executables. Building GHC: The package database and Building GHC: The Stages maybe helpful.

izgzhen commented 6 years ago

Quickfix: pass --force to register. I'm not sure this is the right solution though under all circumstances.

this doesn't work for me :(

-> % ghc-pkg --version
GHC package manager version 8.2.2
angerman commented 6 years ago

@izgzhen so, adding --force to /usr/local/bin/ghc-pkg --global-package-db _build/stage0/lib/package.conf.d register -v0 does not work? Alright, I guess we could check if the .conf file exists, and just not try to clone if it does.

izgzhen commented 6 years ago

there are actually two possibilities:

  1. [pkg-id].conf doesn't exist AND register failed with id already exist. This will happen if the [pkg-id].conf is deleted manually.
  2. [pkg-id].conf exists and id already exists is thrown.

Both cases need us to unregister first.