Add package revision and cabal file hash to plan.json #5695

arianvp · 2018-11-18T13:33:32Z

This is useful for external tools like plan2nix [1] which
can then create their own build plan based on the cabal
solver output, even in the presence of revised cabal files.

[1] - https://github.com/angerman/nix-tools/tree/master/plan2nix

Please include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.
If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

This is useful for external tools like plan2nix [1] which can then create their own build plan based on the cabal solver output, even in the presence of revised cabal files. [1] - https://github.com/angerman/nix-tools/tree/master/plan2nix

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-7.10.3" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-7.8.4" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-8.2.2" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-8.0.2" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-7.6.3" }

arianvp · 2018-11-18T14:13:58Z

By the way, feel free to bikeshed about the field names. I just arbitrarily came up with something that sort of looked appropriate

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-8.4.4" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-8.6.2" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"linux-8.4.4-fdebug-expensive-assertions" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"osx-7.8.4" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"osx-7.10.3" }

"url":"pull/5695", "account":"haskell", "repo":"cabal", "commit": "b9ed3a9c99b79fb8d56167db5185253c3145c730", "tag":"osx-8.0.2" }

hvr

Please drop the x-revision field -- it's misleading and generally unprincipled -- no tool should rely on it for anything; it was intentionally not included in plan.json for a reason

PS: and rename "pkg-revised-cabal-sha256" to "pkg-cabal-sha256"

hvr · 2018-11-19T11:05:38Z

cabal-install/Distribution/Client/ProjectPlanOutput.hs

@@ -141,6 +141,12 @@ encodePlanAsJson distDirLayout elaboratedInstallPlan elaboratedSharedConfig =
        , "style"      J..= J.String (style2str (elabLocalToProject elab) (elabBuildStyle elab))
        , "pkg-src"    J..= packageLocationToJ (elabPkgSourceLocation elab)
        ] ++
+        [ "pkg-revision" J..=  J.String x
+        | Just x <- [lookup "x-revision" (PD.customFieldsPD (elabPkgDescription elab))]


strong 👎 from me, sorry!

I'm strongly against adding the x-revision field to the plan.json file to infer the version; it's a completely unreliable field that is only used internally by Hackage (but other package repositories do not -- the x-revision field would be totally bogus there; notice the x--prefix which should have given this away already) for bookkeeping purposes and as a failsafe; any tooling which relies on it to be accurate is plain and simple broken!

Also, the revision number is a concept that breaks down completely for e.g. Hackage overlays; the only way to accurately identify a revision is via its SHA256. I plan to add support to cabal for pinning to revisions but I do not intend to support pinning by revision number; only by either SHA256 or by timestamp (the latter addressing scheme will likely be the first to be implemented as it's the one that's close to the --index-state design).

That indeed sounds like a saner approach to me. Let me just state that I would very much be interested for the plan.json to contain information about what revision is being used. The approach I implemented might be too fragile, so let's not merge it, but I would very much a less fragile approach to be available.

Adding the cabal revision sha256 could still be a useful addition but only of course if addressing revisions by sha is added first. So we could introduce that change once that is implemented.

hvr · 2018-11-19T11:27:54Z

cabal-install/Distribution/Client/ProjectPlanOutput.hs

+        [ "pkg-revision" J..=  J.String x
+        | Just x <- [lookup "x-revision" (PD.customFieldsPD (elabPkgDescription elab))]
+        ] ++
+        [ "pkg-revised-cabal-sha256" J..= J.String hash


I'd just call it

Suggested change

[ "pkg-revised-cabal-sha256" J..= J.String hash

[ "pkg-cabal-sha256" J..= J.String hash

It doesn't really add much value to qualify it as revised and might even be inaccurate/misleading in some cases. If anything it should be called "override" which is more general than the somewhat Hackage-specific "revision" term. But more generally I'd rather leave it off, and ideally I'd like to see this field be non-optional (because for the tooling I have in mind, I need to know the cabal sha256 always)!

The hash is currently calculated from the cabal file ByteString that is only present when a revised cabal file is present. I'm not sure how I would get access to the cabal file verbatim in the case no revised cabal file is present.

It's ok if we for now only have the SHA256 info for overridden .cabals included in plan.json; we might figure out a way to make it available always in a different PR; we definitely have access to the ByteString somewhere