Fix #9534 Add constructor PPC64LE to type Arch, for powerpc64le

[mpilgrem]

Also makes consequent changes.

• 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. There is no existing documentation.
• Manual QA notes have been included. I don't think QA notes are required.
• Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!) I don't think a test is needed for this change.

[mpilgrem]

I did not know what to do about the failing 'validate' CI. Perhaps naively, I just changed the hash to the one that was observed.

[Mikolaj]

That's probably fine, but we need to understand why the hash changed --- perhaps it's computed using the datatype you added a constructor to?

BTW, does the change affect parsing of the .cabal files? If so, it needs to be guarded behind the 3.12 cabal-version. Some recent PRs did the same, so should be easy to find. But perhaps, by luck, it affects checksums but not the cabal format. I'm still fuzzy on what breaking changes need to be guarded by cabal-version format stamp and what only by a major cabal package version bump.

[mpilgrem]

@Mikolaj, on Cabal files, I think not, based on https://cabal.readthedocs.io/en/stable/cabal-package.html#conditions. That says for arch(name), used in conditionals, name is matched against System.Info.arch (which is unaffected by this). As far as I can see, Cabal's documented 'field syntax reference' (https://cabal.readthedocs.io/en/stable/buildinfo-fields-reference.html) makes no reference to 'architecture'. EDIT: Unlike os(name), there is no suggestion - in the documentation - of any canonicalisation by Cabal. EDIT2: That assumes that the documentation is reliable - I'll see if I can work out what actually happens in the code base.

[mpilgrem]

@Mikolaj, it looks to me like the Cabal User Guide documentation is incomplete, in that arch(name) matches not only System.Info.arch but also constructors of Distribution.System.Arch, based on the following (extracts from code):

module Distribution.Fields.ConfVar ...

parser :: Parser (Condition ConfVar)
parser = condOr
  where
    ...
    osCond = Var . OS <$ string "os" <*> parens fromParsec
    ...
    archCond = Var . Arch <$ string "arch" <*> parens fromParsec

module Distribution.Types.ConfVar ...

-- | A @ConfVar@ represents the variable type used.
data ConfVar
  = OS OS
  | Arch Arch
  | PackageFlag FlagName
  | Impl CompilerFlavor VersionRange
  deriving (Eq, Show, Typeable, Data, Generic)

module Distribution.System
...

archAliases :: ClassificationStrictness -> Arch -> [String]
archAliases Strict _ = []
...

instance Pretty Arch where
  pretty (OtherArch name) = Disp.text name
  pretty other = Disp.text (lowercase (show other))

instance Parsec Arch where
  parsec = classifyArch Strict <$> parsecIdent

classifyArch :: ClassificationStrictness -> String -> Arch
classifyArch strictness s =
  fromMaybe (OtherArch s) $ lookup (lowercase s) archMap
  where
    archMap =
      [ (name, arch)
      | arch <- knownArches
      , name <- prettyShow arch : archAliases strictness arch
      ]

I think the use of strictness Strict in the parser means that the parsing results in OtherArch "powerpc64" or OtherArch "powerpc64le" or PPC64 (if "ppc64" is specified). That is, currently, users on both powerpc64 and powerpc64le architectures can specify "arch(ppc64)" to refer to both architectures - but that is undocumented.

EDIT: Perhaps it is more complex than that. If Cabal 'permissively' maps "powerpc64" and "powerpc64le" to PPC64 for the purposes of buildArch, buildPlatform and 'strictly' maps them to OtherArch s when parsing arch(name) in a Cabal file condition, then perhaps following the User Guide does not work; that is, perhaps arch(powerpc64) and arch(powerpc64le) do not currently work.

[Mikolaj]

Yes, the documentation if quite outdated, despite the continuous effort to keep it in shape. Thanks a lot for looking deeper into this. If you are stuck, feel free to ask on Matrix --- some people understand this better than me.

[Mikolaj]

BTW, let me a add a crosslink to the issue this PR fixes (I think?): #9534

[mpilgrem]

@Mikolaj, thanks. It is odd that GitHub will automatically hyperlink back here at the #9534 end but not vice versa.

[mpilgrem]

Further on cabal-version (the package description format specification) and guarding:

On one view, the specification is unaffected. It was arch(name) before this pull request and it will be arch(name) if it is merged. (What will change, however, is how different versions of Cabal implement the interpretation of name. Before the pull request, ppc64 will be interpreted as referring to either powerpc64 or powerpc64le, depending on whether the actual machine architecture (as reported by System.Info.arch) is one or the other. If it is merged, ppc64 will never be interpreted as meaning powerpc64le.)

On another view, the (implicit) specification is unaffected even it is understood, in principle, to specify the set of valid values of name and their meaning from time to time, and this pull request is fixing a bug in Cabal's implementatation. That is, the specification is such that distinct machine architectures should have distinct names (and the test for machine architectures A and B being distinct is that the GHC project (including all its various components) built for A will not work on a machine B, or vice versa). The intended specification cannot have been that ppc64 could refer to two distinct architectures.

On third view, the (implicit) specification is affected. This view would assume that the set of valid values of name and their meaning from time to time are part of the existing specification, even if that is not documented/documented accurately, and that specification is defined by 'what Cabal has actually been doing, given its code'.

Personally, I prefer the first two views over the third. It seems to me that the third has 'the tail wagging the dog', but I could see that some might say that is too Platonic a perspective, given the circumstances.

[mpilgrem]

Related to this issue/pull request but not fixed by it, Cabal treats aarch64 and arm64 as synonyms and, I think, they are truely synonyms - see, for example, this Stackoverflow discussion Differences between arm64 and aarch64.

However, it appears to me that other Cabal 'synonyms' are not truely synonyms. They include:

• sparc/sparc64/sun4 (ChatGPT tells me that sparc and sun4 may be synonyms, but sparc64 is distinct - 32 bit versus 64 bit)
• mips/mipseb/mipsel (ChatGTP tells me that the 'eb' and 'el' suffixes refer to different endianness; I understand that 'mips' without an endianness is likely to refer to big-endian)
• arm/armeb/armel (similar to the above; but I understand that 'arm' without an endianness is likely to refer to little-endian)

EDIT: So if people follow and agree with my reasoning for data constrctors PPC64 / new PPC64LE, there may be a good case also for Sparc (with synoynm sun4) / new Sparc64; Mips (with synonym mipseb) / new Mipsel; Arm (with synonyn armel) / new Armeb.

[Mikolaj]

@Mikolaj, thanks. It is odd that GitHub will automatically hyperlink back here at the #9534 end but not vice versa.

It crosslinks from commits (where you mentioned the issue), but then the link points to a commit, from which one can't easily reach the PR.

I will ask at today's cabal dev meeting about the "guarding behind cabal-version" policy, because I'm unclear myself. BTW, you are very welcome to attend (as always) and lay out your enquiries yourself.

[mpilgrem]

@Mikolaj, I will try to attend the Cabal dev meeting (my first).

[mpilgrem]

Following the 21 Dec 2023 Cabal dev meeting, I will compare Distribution.System.Arch with GHC's (ghc-platform package) GHC.Platform.ArchOS.Arch: https://gitlab.haskell.org/ghc/ghc/-/blob/master/libraries/ghc-platform/src/GHC/Platform/ArchOS.hs. EDIT: now done at https://gitlab.haskell.org/ghc/ghc/-/issues/24283.

[mpilgrem]

I am returning to this issue/pull request because it appears that the GHC project (based on @mpickering's comment) may well not have a desire to provide a 'canonical' 'architecture' type useful to all of GHC, Cabal and Stack.

[Mikolaj]

Right. @mpickering: is this the right approach then, given https://gitlab.haskell.org/ghc/ghc/-/issues/24283#note_542188? If so, could you review this here PR?

[alt-romes]

I am convinced by Matthew's argument, so this looks good to me too. Unfortunately it seems you'll have to resolve a conflict in the Structured.hs hashes @mpilgrem. Thanks for the patch!

[Mikolaj]

Looks good! @mpilgrem: a kind reminder of how to proceed: https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#github-pull-request-conventions