Add s/cmm-sources + extra-bundled-libraries #4857

angerman · 2017-11-01T14:30:26Z

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.

Please also shortly describe how you tested your change. Bonus points for added tests!

So this is primarily, so that I can give the rts package rts.cabal file.
But I believe these have uses outside of the rts as well.

# Conflicts: # Cabal/Distribution/PackageDescription/Check.hs # Cabal/Distribution/PackageDescription/Parsec/FieldDescr.hs # Cabal/Distribution/Parsec/Types/FieldDescr.hs # Cabal/doc/developing-packages.rst

# Conflicts: # Cabal/Distribution/PackageDescription/Parsec/FieldDescr.hs

23Skidoo

Looks okay to me, but please add changelog notes.

23Skidoo · 2017-11-01T14:42:40Z

Cabal/Distribution/PackageDescription/Check.hs

@@ -1031,6 +1031,8 @@ checkPaths pkg =
      ++ concat
         [    [ (path, "c-sources")        | path <- cSources        bi ]
           ++ [ (path, "cxx-sources")      | path <- cxxSources      bi ]
+           ++ [ (path, "s-sources")        | path <- sSources        bi ]


IMO asm-sources would be a better name. One question is whether we should have asm-x86-sources, asm-x86_64-sources, etc. instead of a single catch-all field... probably not.

I'd argue for using if arch(...): in those cases.

I'm fine with asm-soures as well.

Yep, I also think that if(arch) is the right approach.

especially given @phadej's elif using if arch(...) is quite ok I think

23Skidoo · 2017-11-01T14:43:36Z

Cabal/Distribution/Simple/BuildPaths.hs

+mkProfLibName lib =  mkGenericStaticLibName (getHSLibraryName lib ++ "_p")
+
+
+mkGenericSharedLibName :: CompilerId -> String -> String


Please add a Haddock comment.

23Skidoo · 2017-11-01T14:45:23Z

Cabal/Distribution/PackageDescription/Parse.hs

@@ -493,6 +493,9 @@ binfoFieldDescrs =
 , listFieldWithSep vcat "extra-ghci-libraries"
           showToken          parseTokenQ
           extraGHCiLibs      (\xs    binfo -> binfo{extraGHCiLibs=xs})
+ , listFieldWithSep vcat "extra-bundled-libraries"


Needs docs.

Will see to this and the haddocks tomorrow. Thanks for the review!

hvr · 2017-11-01T21:23:56Z

This is not critical/urgent, but in general it'd be sensible to have things like {c,cmm,asm}-sources imply a constraint on the compiler implementation. E.g. specifying c-sources would imply a compiler that supports linking C FFI functions; currently afaik, GHCJS doesn't support C FFI as an example.

angerman · 2017-11-02T08:27:13Z

Building test suite 'check-tests' for Cabal-2.1.0.0..
[1 of 1] Compiling Main             ( tests\CheckTests.hs, dist\build\check-tests\check-tests-tmp\Main.o )
Linking dist\build\check-tests\check-tests.exe ...
C:/ProgramData/chocolatey/lib/ghc/tools/ghc-8.0.2/mingw/bin/ld.exe: cannot find -lHSCabal-2.1.0.0-FQui7antlcKGtClAj2Mabp
collect2.exe: error: ld returned 1 exit status
`gcc.exe' failed in phase `Linker'. (Exit code: 1)

Hmm... so how did I cause that. let's see.

Not absolutely happy with this solution. I fail to come up with a better one though. The issue is that for some libraries, you might want to have _debug, _p, _l, as well as _thr_debug, _thr_p, _thr_l flavours.

angerman · 2017-11-03T01:56:36Z

I'm pretty annoyed that travis isn't checking this :(

23Skidoo · 2017-11-03T15:17:57Z

Merged, thanks!

Summary: This is in prerpation for cabalification of the `rts`. To be actually able to parse this file, a rather recent Cabal is required. One after commit `357d49d` of haskell/cabal. The relevant PR to support the new `asm-srouces` and `cmm-sources` is haskell/cabal/pull/4857. Not that this does *not* allow cabal to build the RTS. It does however provide enough information such that cabal can `copy` and `register` the package properly in the package database, if all the build artifacts have been build properly. As such it does not require any custom handling of the `rts` package. As the rts as well as all the other packages built by the GHC built system are built outside of cabal anyway. Reviewers: bgamari, hvr, erikd, simonmar Subscribers: rwbarton, thomie, erikd Differential Revision: https://phabricator.haskell.org/D4174

Summary: With the introduction of `asm-srouces` and `cmm-sources` in haskell/cabal/pull/4857. Which was merged into `haskell/cabal` HEAD in`357d49d`. We can now declare the `cmm-files` properly, and as such they can be read out by the cabal library. Reviewers: bgamari, hvr Subscribers: rwbarton, thomie Differential Revision: https://phabricator.haskell.org/D4176

@hvr

…res, extra-library-flavours, and virtual-modules. As pointed out by @hvr, haskell#4857, haskell#4875 did not contain the necessary "check" logic. This PR tries to address this shortcoming.

This is in preparation for cabalification of the `rts`. To be actually able to parse this file, a rather recent Cabal is required. One after commit 357d49d of haskell/cabal. The relevant PR to support the new `asm-sources` and `cmm-sources` is haskell/cabal/pull/4857. Not that this does *not* allow cabal to build the RTS. It does however provide enough information such that cabal can `copy` and `register` the package properly in the package database, if all the build artifacts have been build properly. As such it does not require any custom handling of the `rts` package. As the rts as well as all the other packages built by the GHC built system are built outside of cabal anyway. Reviewers: bgamari, hvr, erikd, simonmar Reviewed By: bgamari Subscribers: rwbarton, thomie, erikd Differential Revision: https://phabricator.haskell.org/D4174

angerman added 3 commits November 1, 2017 22:26

Adds sSources and cmmSources.

4a28765

# Conflicts: # Cabal/Distribution/PackageDescription/Check.hs # Cabal/Distribution/PackageDescription/Parsec/FieldDescr.hs # Cabal/Distribution/Parsec/Types/FieldDescr.hs # Cabal/doc/developing-packages.rst

generic lib names

2d5ccc6

Adds suppor for extra-bundled-libraries

f6b2394

# Conflicts: # Cabal/Distribution/PackageDescription/Parsec/FieldDescr.hs

angerman mentioned this pull request Nov 1, 2017

WIP: Relocation based build (inplace free) snowleopard/hadrian#445

Closed

7 tasks

23Skidoo approved these changes Nov 1, 2017

View reviewed changes

angerman added 4 commits November 2, 2017 10:23

Adds options; renames sSoruces to asmSources

57d7f28

Adds haddocks.

3cf44df

Adds bundled lirbaries documentation.

8f0d257

No name shaddowing.

3e37a5c

angerman added 2 commits November 2, 2017 16:30

Revert back to always use Archive extension. Even on Windows.

d0e14f5

Adds extra-library-flavours

7577105

Not absolutely happy with this solution. I fail to come up with a better one though. The issue is that for some libraries, you might want to have _debug, _p, _l, as well as _thr_debug, _thr_p, _thr_l flavours.

angerman force-pushed the feature/extra-fields branch from 8eb3bd1 to 7577105 Compare November 3, 2017 01:56

23Skidoo merged commit 357d49d into haskell:master Nov 3, 2017

angerman mentioned this pull request Nov 15, 2017

Add checkVersion for added fields #4893

Merged

4 tasks

angerman mentioned this pull request Jun 27, 2018

Make Setup.hs configure more CWD-independent. #5397

Merged

4 tasks

bgamari mentioned this pull request Jul 20, 2019

Release for GHC 8.8 #5819

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add s/cmm-sources + extra-bundled-libraries #4857

Add s/cmm-sources + extra-bundled-libraries #4857

angerman commented Nov 1, 2017 •

edited

Loading

23Skidoo left a comment

23Skidoo Nov 1, 2017

angerman Nov 1, 2017

23Skidoo Nov 1, 2017

hvr Nov 1, 2017

23Skidoo Nov 1, 2017

23Skidoo Nov 1, 2017

angerman Nov 1, 2017

hvr commented Nov 1, 2017 •

edited

Loading

angerman commented Nov 2, 2017

angerman commented Nov 3, 2017

23Skidoo commented Nov 3, 2017

		mkProfLibName lib = mkGenericStaticLibName (getHSLibraryName lib ++ "_p")


		mkGenericSharedLibName :: CompilerId -> String -> String

Add s/cmm-sources + extra-bundled-libraries #4857

Add s/cmm-sources + extra-bundled-libraries #4857

Conversation

angerman commented Nov 1, 2017 • edited Loading

23Skidoo left a comment

Choose a reason for hiding this comment

23Skidoo Nov 1, 2017

Choose a reason for hiding this comment

angerman Nov 1, 2017

Choose a reason for hiding this comment

23Skidoo Nov 1, 2017

Choose a reason for hiding this comment

hvr Nov 1, 2017

Choose a reason for hiding this comment

23Skidoo Nov 1, 2017

Choose a reason for hiding this comment

23Skidoo Nov 1, 2017

Choose a reason for hiding this comment

angerman Nov 1, 2017

Choose a reason for hiding this comment

hvr commented Nov 1, 2017 • edited Loading

angerman commented Nov 2, 2017

angerman commented Nov 3, 2017

23Skidoo commented Nov 3, 2017

angerman commented Nov 1, 2017 •

edited

Loading

hvr commented Nov 1, 2017 •

edited

Loading