Skip to content

Update the 9.0.2 release used to 9.0.2a so profiling libs are included #102

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

Merged

Conversation

telser
Copy link
Contributor

@telser telser commented May 19, 2022

No description provided.

@telser telser force-pushed the telser/update-902-with-profiling-fix branch from 5368913 to 88ea4b8 Compare May 19, 2022 21:41
@telser
Copy link
Contributor Author

telser commented May 19, 2022

I missed the shas the first time.

@hasufell
Copy link
Contributor

hasufell commented Jul 9, 2022

This will (potentially) break existing complied artifacts (e.g. cached libraries), because of diverging ABI. Afaik, neither stack nor cabal have a concept of GHC ABI.

@telser
Copy link
Contributor Author

telser commented Jul 11, 2022

This will (potentially) break existing complied artifacts (e.g. cached libraries), because of diverging ABI. Afaik, neither stack nor cabal have a concept of GHC ABI.

@hasufell This is a great point. As I think about this more, it seems to me that we maybe need a 9.0.3 release that is otherwise identical to 9.0.2 to fix this issue, short of getting everyone on board with GHC ABI. The situation as it stands is quite frustrating for end users and the work around isn't exactly a great look. So something more immediate to remedy the issue would be a win, at least to me.

@hasufell
Copy link
Contributor

I don't think packaging issues should require a new release.

Tools like cabal and stack simply need to be ABI aware: in addition to checking ghc version, they also need to check the ABI.

I'm not 100% sure how stack handles this, though. So maybe ask someone who knows more.

@bgamari
Copy link

bgamari commented Jul 11, 2022

Tools like cabal and stack simply need to be ABI aware: in addition to checking ghc version, they also need to check the ABI.

While I agree that this is in principle true, currently GHC itself currently isn't sufficiently careful about ABI to avoid this sort of issue. Until that is fixed, I think all we can do is cut a new release. I have opened GHC #21841 to track the prospect of a 9.0.3.

Furthermore, the issue with 9.0.2 should be avoided by construction in 9.4 and later due to improvements in GHC's packaging infrastructure.

@mpilgrem
Copy link
Member

In Stack:

Stack.Types.SourceMap.SourceMap has this comment:

    -- ^ Need to hash the compiler version _and_ its installation
    -- path.  Ideally there would be some kind of output from GHC
    -- telling us some unique ID for the compiler itself.

and Stack.Build.Source.hashSourceMapData has this comment:

-- | Get a 'SourceMapHash' for a given 'SourceMap'
--
-- Basic rules:
--
-- * If someone modifies a GHC installation in any way after Stack
--   looks at it, they voided the warranty. This includes installing a
--   brand new build to the same directory, or registering new
--   packages to the global database.
--
-- * We should include everything in the hash that would relate to
--   immutable packages and identifying the compiler itself. Mutable
--   packages (both project packages and dependencies) will never make
--   it into the snapshot database, and can be ignored.
--
-- * Target information is only relevant insofar as it effects the
--   dependency map. The actual current targets for this build are
--   irrelevant to the cache mechanism, and can be ignored.
--
-- * Make sure things like profiling and haddocks are included in the hash
--

I think that is how Stack tries to distinguish one GHC from another.

@hasufell
Copy link
Contributor

Need to hash the compiler version and its installation path

That doesn't seem too reliable. The installation path doesn't tell us anything about ABI and can be exactly the same, for all we know (e.g. in this case, the installation path would be ~/.stack/programs/x86_64-linux/ghc-tinfo6-9.0.2 for both bindists?)? And what about --system-ghc?

@mpilgrem
Copy link
Member

Looking at Stack.Build.Source.hashSourceMapData itself, it looks to me that what is hashed includes the output of command ghc --info. However, I do not know if that would distinguish '9.0.2' from '9.0.2a' (I am a Windows user and cannot simply compare the output.)

@hasufell
Copy link
Contributor

--- 9.0.2.info	2022-07-12 00:20:50.700705218 +0200
+++ 9.0.2_prof.info	2022-07-12 00:17:32.176552211 +0200
@@ -44,7 +44,7 @@
  ,("bignum backend","gmp")
  ,("Use interpreter","YES")
  ,("Support SMP","YES")
- ,("RTS ways","l debug thr thr_debug thr_l  dyn debug_dyn thr_dyn thr_debug_dyn l_dyn thr_l_dyn ")
+ ,("RTS ways","l debug thr thr_debug thr_l thr_p dyn debug_dyn thr_dyn thr_debug_dyn l_dyn thr_l_dyn thr_debug_p debug_p")
  ,("Tables next to code","YES")
  ,("Leading underscore","NO")
  ,("Use LibFFI","NO")
@@ -52,7 +52,7 @@
  ,("Use Debugging","NO")
  ,("RTS expects libdw","NO")
  ,("Project version","9.0.2")
- ,("Project Git commit id","6554ff2843d53dddeb875cb145ab892725eac54c")
+ ,("Project Git commit id","251bdb8d0ef09f81a4dd225bc39077ba4669b55b")
  ,("Booter version","8.10.7")
  ,("Stage","2")
  ,("Build platform","x86_64-unknown-linux")

@mpilgrem
Copy link
Member

@hasufell, thanks. Referring back to your #102 (comment), I think that means that the pull request will not be problematic, because Stack will know that what it currently needs is different from what it has cached.

@hasufell
Copy link
Contributor

@hasufell, thanks. Referring back to your #102 (comment), I think that means that the pull request will not be problematic, because Stack will know that what it currently needs is different from what it has cached.

Yep. Although I think ghc --info will not catch all possible ABI breakages. So this needs fixing in stack regardless.

The main problem is that we don't have a clear way to decide what ABI hash really means. In HLS we currently just query abi for all boot libraries and concatenate that. But there's no standard way to get this from GHC itself: https://github.com/haskell/haskell-language-server/blob/e6dfa4e9c437a02452fc52130ba1e4d007e4fd64/bindist/wrapper.in#L76

@mpilgrem
Copy link
Member

If it is not problematic, I'll merge it then.

@mpilgrem mpilgrem merged commit bc9f218 into commercialhaskell:master Jul 25, 2022
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.

4 participants