Include package version in --promised-dependency flag #10248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mpickering
commented
Aug 5, 2024
| | Just cid <- Map.lookup (dep_pkgname, CLibName lib) promisedIndex = | ||
| return $ PromisedDependency (PromisedComponent dep_pkgname (AnnotatedId currentCabalId (CLibName lib) cid)) | ||
| | Just pc <- Map.lookup (dep_pkgname, CLibName lib) promisedIndex = | ||
| return $ PromisedDependency (ConfiguredPromisedComponent dep_pkgname (AnnotatedId (promisedComponentPackage pc) (CLibName lib) (promisedComponentId pc))) |
There was a problem hiding this comment.
This is the main relevant change, removing currentCabalId.
geekosaur
approved these changes
Aug 5, 2024
mpickering
force-pushed
the
wip/t10166
branch
2 times, most recently
from
8fc8b9b
to
05b1fa9
Compare
sheaf
approved these changes
Aug 26, 2024
mpickering
force-pushed
the
wip/t10166
branch
3 times, most recently
from
654dd18
to
a64244e
Compare
|
Can I merge this without a delay? I want to get this and #10256 landed before I go on holiday next week but they both update the structured hashes so it will be a minimum of 5ish days to land both. |
Mikolaj
added
the
merge delay passed
In the original implementation of promised dependencies I accidentally left over the hard coded `currentCabalId` in the `configureDependencies` function. This led to several errors happening later when the package name and version would be incorrect if you looked at this field (package arguments are not computed using it), it is used when generating cabal macros and something in the haddock options. The solution is to pass the package version in the `--promised-depenency` flag so the format is now ``` NAME-VER[:COMPONENT_NAME]=CID` ``` rather than ``` NAME[:COMPONENT_NAME]=CID ``` Fixes haskell#10166
mergify
bot
added
the
ready and waiting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the original implementation of promised dependencies I accidentally left over the hard coded
currentCabalIdin theconfigureDependenciesfunction.This led to several errors happening later when the package name and version would be incorrect if you looked at this field (package arguments are not computed using it), it is used when generating cabal macros and something in the haddock options.
The solution is to pass the package version in the
--promised-depenencyflag so the format is nowrather than
Fixes #10166
Please read Github PR Conventions and then fill in one of these two templates.
Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR: