-
Notifications
You must be signed in to change notification settings - Fork 0
cabal-install: stage-qualified package configuration (package build:*) #378
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
base: stable-haskell/master
Are you sure you want to change the base?
Changes from all commits
1ab1670
480c9c2
a8d97fd
e1b10cc
b2b9f00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,9 +57,6 @@ import qualified Data.Text.Encoding as T | |
| import qualified Data.Text.Encoding.Error as T | ||
| #endif | ||
|
|
||
| -- $setup | ||
| -- >>> import Data.Either (isLeft) | ||
|
|
||
| -- | The 'LexState'' (with a prime) is an instance of parsec's 'Stream' | ||
| -- wrapped around lexer's 'LexState' (without a prime) | ||
| data LexState' = LexState' !LexState (LToken, LexState') | ||
|
|
@@ -135,8 +132,32 @@ tokCloseBrace = getToken $ \t -> case t of CloseBrace -> Just (); _ -> Nothing | |
| tokFieldLine = getTokenWithPos $ \t -> case t of L pos (TokFieldLine s) -> Just (FieldLine pos s); _ -> Nothing | ||
|
|
||
| colon, openBrace, closeBrace :: Parser () | ||
|
|
||
| -- | A single section argument. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is a section argument?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Section arguments are what follows the opening a section, e.g. |
||
| -- | ||
| -- Section arguments may contain colons, e.g. a stage qualifier such as | ||
| -- @build:pkg@ or @build:*@ in a @package@ stanza. At this point we are already | ||
| -- committed to parsing section arguments (the field-versus-section choice is | ||
| -- decided by whether the first token after the section name is a colon), so a | ||
| -- colon here can only join adjacent argument tokens. A colon in this position | ||
| -- was previously a parse error, so this accepts strictly more input. | ||
| sectionArg :: Parser (SectionArg Position) | ||
| sectionArg = tokSym' <|> tokStr <|> tokOther <?> "section parameter" | ||
| sectionArg = do | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is the test for the new syntax?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change in Cabal-syntax is not specific to the host: or build: syntax but it is necessary to allow it. I guess I should add some tests to Cabal-syntax to describe this change anyway.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Cabal-syntax has no in-package tests. Tests for the |
||
| arg0 <- sectionArgAtom | ||
| rest <- many (try (colon *> sectionArgAtom)) | ||
| pure (foldl joinSectionArgColon arg0 rest) | ||
| where | ||
| sectionArgAtom = tokSym' <|> tokStr <|> tokOther <?> "section parameter" | ||
|
|
||
| -- | Join two section arguments with a colon, reconstructing e.g. @build:pkg@ | ||
| -- from the @build@, @:@ and @pkg@ tokens. | ||
| joinSectionArgColon :: SectionArg Position -> SectionArg Position -> SectionArg Position | ||
| joinSectionArgColon a b = | ||
| SecArgOther (sectionArgAnn a) (sectionArgBytes a <> ":" <> sectionArgBytes b) | ||
| where | ||
| sectionArgBytes (SecArgName _ s) = s | ||
| sectionArgBytes (SecArgStr _ s) = s | ||
| sectionArgBytes (SecArgOther _ s) = s | ||
|
|
||
| fieldSecName :: Parser (Name Position) | ||
| fieldSecName = tokSym <?> "field or section name" | ||
|
|
@@ -350,13 +371,14 @@ fieldInlineOrBraces name = | |
| -- | ||
| -- 'readFields' won't (necessarily) fail on invalid UTF8 data, but the reported positions may be off. | ||
| -- | ||
| -- __You may get weird errors on non-UTF8 input__, for example 'readFields' will fail on latin1 encoded non-breaking space: | ||
| -- __You may get weird results on non-UTF8 input__, for example 'readFields' | ||
| -- treats a latin1 encoded non-breaking space as a section name: | ||
| -- | ||
| -- >>> isLeft (readFields "\xa0 foo: bar") | ||
| -- True | ||
| -- >>> readFields "\xa0 foo: bar" | ||
| -- Right [Section (Name (Position 1 1) "\160") [SecArgOther (Position 1 3) "foo:bar"] []] | ||
| -- | ||
| -- That is rejected because parser thinks @\\xa0@ is a section name, | ||
| -- and section arguments may not contain colon. | ||
| -- The parser thinks @\\xa0@ is a section name and @foo: bar@ a single | ||
| -- (colon-joined) section argument. | ||
| -- If there are just latin1 non-breaking spaces, they become part of the name: | ||
| -- | ||
| -- >>> readFields "\xa0\&foo: bar" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ import Distribution.Pretty (prettyShow) | |
|
|
||
| import Distribution.Solver.Modular.Version | ||
| import Distribution.Solver.Types.PackagePath | ||
| import Distribution.Solver.Types.Stage (Stage, showStage) | ||
| import Distribution.Solver.Types.Stage (Stage) | ||
|
|
||
| -- | A package name. | ||
| type PN = PackageName | ||
|
|
@@ -57,9 +57,17 @@ data I = I Stage Ver Loc | |
| deriving (Eq, Ord, Show) | ||
|
|
||
| -- | String representation of an instance. | ||
| -- The stage is not shown here; it is carried by the qualified package name | ||
| -- (see the 'Pretty' instance for 'Distribution.Solver.Types.PackagePath.QPN'), | ||
| -- so an instance renders the same regardless of stage. | ||
| showI :: I -> String | ||
| showI (I s v (InRepo pn)) = intercalate ":" [showStage s, "source", prettyShow (PackageIdentifier pn v)] | ||
| showI (I s _v (Inst uid)) = intercalate ":" [showStage s, "installed", prettyShow uid] | ||
| showI (I _s v (InRepo _pn)) = showVer v | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there some unit tests for this that showcase what the changes are?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| showI (I _s v (Inst uid)) = showVer v ++ "/installed" ++ extractPackageAbiHash uid | ||
| where | ||
| extractPackageAbiHash xs = | ||
| case first reverse $ break (== '-') $ reverse (prettyShow xs) of | ||
| (ys, []) -> ys | ||
| (ys, _) -> '-' : ys | ||
|
|
||
| -- | Package instance. A package name and an instance. | ||
| data PI qpn = PI qpn I | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,10 @@ constraintQualifierMatches (ScopeAnySetupQualifier pn) (QualSetup _) pn' = pn == | |
| constraintQualifierMatches (ScopeAnyExeQualifier pn) (QualExe _ _) pn' = pn == pn' | ||
| constraintQualifierMatches (ScopeAnyExeQualifier _) QualToplevel _ = False | ||
| constraintQualifierMatches (ScopeAnyExeQualifier _) (QualSetup _) _compile = False | ||
| -- A base-qualified dependency is never matched by a toplevel/setup/exe scope. | ||
| constraintQualifierMatches (ScopeTarget _) (QualBase _) _ = False | ||
| constraintQualifierMatches (ScopeAnySetupQualifier _) (QualBase _) _ = False | ||
| constraintQualifierMatches (ScopeAnyExeQualifier _) (QualBase _) _ = False | ||
|
Comment on lines
+97
to
+100
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this part related to the PR? It seems like an unrelated bugfix?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct. It is unrelated. |
||
| constraintQualifierMatches (ScopeAnyQualifier pn) _ pn' = pn == pn' | ||
|
|
||
| instance Pretty ConstraintScope where | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ import Prelude () | |
| import Distribution.Package (PackageName) | ||
| import Distribution.Pretty (pretty, flatStyle, Pretty) | ||
| import qualified Text.PrettyPrint as Disp | ||
| import Distribution.Solver.Types.Stage (Stage) | ||
| import Distribution.Solver.Types.Stage (Stage (..)) | ||
|
|
||
| data PackagePath = PackagePath Stage Qualifier | ||
| deriving (Eq, Ord, Show, Generic) | ||
|
|
@@ -25,7 +25,16 @@ instance Structured PackagePath | |
|
|
||
| instance Pretty PackagePath where | ||
| pretty (PackagePath stage qualifier) = | ||
| pretty stage <<>> Disp.text ":" <<>> pretty qualifier | ||
| dispStage stage <<>> pretty qualifier | ||
|
|
||
| -- | Pretty-prints a build stage as a prefix. The result is either empty (for | ||
| -- the default 'Host' stage) or ends in a colon, so it can be prepended onto a | ||
| -- qualifier or package name. Keeping the 'Host' stage invisible means logs for | ||
| -- ordinary (non-cross) builds, where every package is on the 'Host' stage, | ||
| -- stay free of stage noise; only the 'Build' stage is called out explicitly. | ||
| dispStage :: Stage -> Disp.Doc | ||
| dispStage Host = Disp.empty | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems confusing if the default is build+host? |
||
| dispStage Build = pretty Build <<>> Disp.colon | ||
|
|
||
| -- | Qualifier of a package within a namespace (see 'PackagePath') | ||
| data Qualifier = | ||
|
|
@@ -64,6 +73,7 @@ instance Structured Qualifier | |
|
|
||
| instance Pretty Qualifier where | ||
| pretty QualToplevel = Disp.text "toplevel" | ||
| pretty (QualBase pn) = pretty pn <<>> Disp.text ":base" | ||
| pretty (QualSetup pn) = pretty pn <<>> Disp.text ":setup" | ||
| pretty (QualExe pn pn2) = pretty pn <<>> Disp.text ":" <<>> | ||
| pretty pn2 <<>> Disp.text ":exe" | ||
|
|
@@ -78,6 +88,7 @@ instance Pretty Qualifier where | |
| -- 'Base' qualifier, will always be @base@). | ||
| dispQualifier :: Qualifier -> Disp.Doc | ||
| dispQualifier QualToplevel = mempty | ||
| dispQualifier (QualBase pn) = pretty pn <> Disp.text "." | ||
| dispQualifier (QualSetup pn) = pretty pn <> Disp.text ":setup." | ||
| dispQualifier (QualExe pn pn2) = | ||
| pretty pn | ||
|
|
@@ -97,7 +108,7 @@ type QPN = Qualified PackageName | |
|
|
||
| instance Pretty (Qualified PackageName) where | ||
| pretty (Q (PackagePath stage qual) pn) = | ||
| pretty stage <<>> Disp.colon <<>> dispQualifier qual <<>> pretty pn | ||
| dispStage stage <<>> dispQualifier qual <<>> pretty pn | ||
|
|
||
| -- | Pretty-prints a qualified package name. | ||
| dispQPN :: QPN -> Disp.Doc | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package * | ||
| static: True | ||
|
|
||
| package build:* | ||
| shared: False | ||
| executable-dynamic: False | ||
|
|
||
| package host:* | ||
| shared: True | ||
|
Comment on lines
+4
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we test as well? I would assume that applies to build + host?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't that in assertEqual
"Unqualified 'package *' should apply to all packages, not a single stage"
expectedAll
(projectConfigAllPackages (snd (condTreeData config))) |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is unfortunate but required to be able to parse
host:pkg-namecorrectly, as it stands the parser will stop at:. The commit message describes this well:480c9c2