Skip to content
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

CI: Please, check cross-subproject deps consistancy with freeze #2566

Closed
Anton-Latukha opened this issue Jan 5, 2022 · 3 comments
Closed
Labels
CI Continuous integration dependencies Pull requests that update a dependency file type: enhancement New feature or request

Comments

@Anton-Latukha
Copy link
Collaborator

Anton-Latukha commented Jan 5, 2022

Input

As raised by michaelpj in #2563 (comment)

Currently, CI allows merging subprojects (the best example - plugins) when their dependency requirements are mutually exclusive. For HLS as a distribution with enabled by default plugins - that is a bug.

As HLS with enabled by default plugins - probably should have consistent dependency requirements to infer 1 set of deps (to be sure that all parts of the system are built with the same versions).

In practice, building the CI, we met cases where the freeze stage does not work.

Currently freeze is used in CI supplementary to the caching identification, and that process is made fault-tolerant.

Keeping (if contributions are not checked for dep consistency) all moving parts in synchronization may become a difficulty for maintainers.

Solution

freeze in itself is a regular build target dependency set consistency test. It probably should be made & placed so accordingly.

Having CI to check with freeze for merge would seem viable.

Output

It seems a good strategy to check PRs for consistency in update/dependency management. So if someone would want to drop old dependency - all the project parts would be checked & suggested to be adjusted for that agenda. So it also would enforce also to have some backward compatibility of versions between projects, wich is both a good thing and may be a security concern, but security concern is when mainteiners can come into play. So for example, having consistency means restricting from old versions in one place, which would mean it can be at once removed in all other projects. And adding the new support in one part of the project would softly suggest to the person in CI build logs that that new version is still not used, because other project parts should have the support of it also (CI is able to get a report on what deps are inferred across project).

This would have a useful side-effect of dependency tree becoming consistent, which would ease the software packaging in other package managers, for example, Stackage or Nix would have an easier time shipping the HLS in them & HLS would get fewer dep inconsistency reports from them.

@Anton-Latukha Anton-Latukha added type: enhancement New feature or request status: needs triage CI Continuous integration dependencies Pull requests that update a dependency file and removed status: needs triage labels Jan 5, 2022
@jneira
Copy link
Member

jneira commented Jan 5, 2022

Not sure if it helps but i've made the freeze step strict in #2563, so failing to generate the freeze file will make the build fail.
Afaik cabal build in the project root (as done in the test workflow) needs all subprojects to be solvable. That is the reason we have to comment subprojects in cabal-ghc921.project cause even if you are not going tot effectively build a subproject, the solver takes it in account. Same for cabal freeze.

Before having the freeze step the pr already needed to make the project solvable as a whole.

@michaelpj
Copy link
Collaborator

Currently, CI allows merging subprojects (the best example - plugins) when their dependency requirements are mutually exclusive.

Sorry, maybe I'm being confused, but is this true? It looks like we just call cabal build == cabal build all in the build workflow, which will build everything, so will produce a coherent build plan.

@Anton-Latukha
Copy link
Collaborator Author

the reason we have to comment subprojects in cabal-ghc921.project cause even if you are not going tot effectively build a subproject, the solver takes it in account.

Ok, then my perception/memory probably inferred wrong.

Ok, just closing this since > freeze step strict in #2563

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration dependencies Pull requests that update a dependency file type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants