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

Update Hackage to use Cabal 3 #862

Merged
merged 18 commits into from
Apr 5, 2020
Merged

Update Hackage to use Cabal 3 #862

merged 18 commits into from
Apr 5, 2020

Conversation

alexbiehl
Copy link
Member

@alexbiehl alexbiehl commented Mar 11, 2020

See #852.
The patches in this PR are coming from @dwijnand, @hvr and @phadej. Unfortunately I can't push to the wip/cabal-3.0 branch.

  • We should enable CI again

<*> uniqueField "arch" archL
<*> uniqueField "compiler" compilerL
<*> uniqueField "client" clientL
<*> undefined --monoidalFieldAla "flags" (alaList CommaFSep) flagAssignmentL TODO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this undefined resolved?

Copy link
Member

@hvr hvr Apr 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope; still needs resolving...

UPDATE: resolved

Copy link

@davean davean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need to check the derived instances and run some tests on a snapshot of hackage.

@@ -235,30 +217,13 @@ basicChecks pkgid tarIndex = do
throwError "'cabal-version' must be lower than 2.5"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be bumped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true; but that's gonna happen outside this PR

@@ -27,7 +27,7 @@ import Distribution.Package
import Distribution.Version (nullVersion)

import Control.Arrow (second)
import Data.ByteString.Lazy.Char8 (unpack) -- Build reports are ASCII
import Data.ByteString.Lazy (toStrict)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this subsystem will have issues with multi-library packages but I do not think that is a blocking problem.

Attached here due to limitations in github's review system.

@@ -378,7 +379,7 @@ versionsFeature ServerEnv{ serverVerbosity = verbosity }

formatSinglePreferredVersions :: PackageName -> PreferredInfo -> Maybe String
formatSinglePreferredVersions pkgname pref =
display . Dependency pkgname <$> sumRange pref
display . (\vr -> Dependency pkgname vr Set.empty) <$> sumRange pref -- XXX: ok?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is correct because PreferedVersions should operate at the package level.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually.... see b6fbd24

@@ -117,7 +116,7 @@ instance SafeCopy VersionRange where
6 -> withinVersion <$> safeGet
7 -> unionVersionRanges <$> getVR <*> getVR
8 -> intersectVersionRanges <$> getVR <*> getVR
9 -> VersionRangeParens <$> getVR
9 -> stripParensVersionRange <$> getVR -- XXX: correct?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reading of Cabal says this is correct.

(withComponentName' CLibName <$> libsA)
(withComponentName' CLibName <$> libsB)
(withComponentName' (CLibName LMainLibName) <$> libsA)
(withComponentName' (CLibName LMainLibName) <$> libsB)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can someone else check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was due to haskell/cabal@898b284

(withComponentName CSubLibName <$> sublibsA)
(withComponentName CSubLibName <$> sublibsB)
(withComponentName (CLibName . LSubLibName) <$> sublibsA)
(withComponentName (CLibName . LSubLibName) <$> sublibsB)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can someone else check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was due to haskell/cabal@898b284

where
-- these names are reserved for the time being, as they have
-- special meaning in cabal's UI
reservedPkgNames = map mkPackageName ["all","any","none","setup","lib","exe","test"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does "bench" also?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, because back then when I added those blacklisted names there was already: http://hackage.haskell.org/package/bench-1.0.0

@davean
Copy link

davean commented Mar 12, 2020

I think Distribution/Server/Packages/Render.hs could be improved after the merge.

@phadej
Copy link
Contributor

phadej commented Mar 12, 2020 via email

@bgamari
Copy link
Contributor

bgamari commented Mar 13, 2020

Thank you for doing this, @alexbiehl, @hvr, and @phadej!

hvr added 2 commits April 5, 2020 12:22
NB: This doesn't work as intended as the `cabalSpecFromVersionDigits`
    function is too lax; Cabal 3.4 however will provide a new function
    for helping to converting CabalSpecVersion to a Version
    (`cabalSpecToVersionDigits`)

This reverts commit 0bfe698.
The open TODOs were related to the BuildReport fields for the "time"
and the "flags" field. This change only does the bare minimum and
in the future somebody will have to clean-up the technical debt
incurred here.
@hvr hvr merged commit f8003f3 into master Apr 5, 2020
@hvr
Copy link
Member

hvr commented Apr 5, 2020

I've merged this PR in order to prevent it from bitrotting again as well as to have basic CI enabled again. Some more testing is still required to get better confidence we don't have obvious regressions.

@hvr hvr deleted the wip/cabal-3.0 branch April 5, 2020 14:25
@dwijnand
Copy link
Collaborator

dwijnand commented Apr 5, 2020

Thanks @hvr!

hvr added a commit that referenced this pull request Apr 5, 2020
NB: `Dependency pkgname vr Set.empty` renders as a package
with an empty set of sub-libs, i.e. as

  pkgname:{} >= 1.2.3 && ...

instead of

  pkgname >= 1.2.3 && ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants