Skip to content

GHA: add a job that test opam libs dependencies compilation#6394

Merged
kit-ty-kate merged 4 commits intoocaml:masterfrom
rjbou:gha-plugins
Aug 26, 2025
Merged

GHA: add a job that test opam libs dependencies compilation#6394
kit-ty-kate merged 4 commits intoocaml:masterfrom
rjbou:gha-plugins

Conversation

@rjbou
Copy link
Copy Markdown
Collaborator

@rjbou rjbou commented Feb 21, 2025 •
edited
Loading

In it based on the same mechanism than opam-rt, it permits to keep them synchronised with the current API
TODO

  • add a release step to check CI log for failing projects
  • add a mechanism to retrieve automatically opam lib rev deps
  • add a mechanism to eliminate pinned packages that rely on old versions of opam
  • condition job launch to changes of mli files

This PR also contains an enhancement of changed files step (and it use/conditionning).

@rjbou rjbou requested a review from kit-ty-kate February 21, 2025 12:37
kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
.github/scripts/main/main.sh Outdated
Comment on lines +197 to +222
test_project "ocaml-opam" "opam-publish" 0 "make"
test_project "AltGr" "opam-bundle" 0 "make"
test_project "ocamlpro" "opam-custom-install" 1 "dune build"

Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also suggest adding dune-release, odep, odig, odoc-driver, opam-check-npm-deps, opam-build, opam-test, opam-dune-lint, opam-graph, opam-package-upgrade, orb, opam-0install and topkg-care

Copy link
Copy Markdown
Collaborator Author

@rjbou rjbou Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was package to test (some are failing on purpose as they have a dependency on opam < 2.2, see commits message). We'll need to select what package to add here : they are blocking for the PR merge.
Ideally we would have another more light mechanism, one that sends the information as a comment in the PR for ex., like that we are aware without blocking the PR (for packages that we don't maintain).

@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented Feb 25, 2025

Discussion on dev meeting:
We don't want to enforce failling jobs on package that we don't maintain, but the information can be used to know which maintainer/project to ping. Ideally, it would be a job that opens an issue on the good repository, or adds a comment in the PR itself.
In the meanwhile, we would add 2 sets of dependencies to test:

  • the one that we maintain as opam team or as maintainer (opam-publish, opam-build, etc.) that would hard fail if there in an API breakage
  • the one that we don't maintain that would not make the job fail. It's up to us to go and look at the jobs, especially on releases
    It would also be nice to have an automatic selection of opam library rev deps from opam repo, but for that we need to add a step that eliminates package that are just incompatible (for ex ocaml-format < 2.3 soent' make sense to test).

main comment updated with TODOs

@kit-ty-kate kit-ty-kate added this to the 2.4.0~alpha1 milestone Apr 7, 2025
@kit-ty-kate kit-ty-kate mentioned this pull request Apr 7, 2025
Merged
@rjbou rjbou changed the title GHA: add a job that test plugins compilation GHA: add a job that test opam libs dependencies compilation Apr 14, 2025
@kit-ty-kate kit-ty-kate modified the milestones: 2.4.0~alpha2, 2.4.0~alpha3 May 5, 2025
@kit-ty-kate kit-ty-kate modified the milestones: 2.4.0~alpha3, 2.4.0~beta2 Jun 16, 2025
@coderabbitai coderabbitai bot mentioned this pull request Jun 24, 2025
Open
@arozovyk
Copy link
Copy Markdown
Collaborator

arozovyk commented Jun 30, 2025 •
edited
Loading

arozovyk#3

Added

  • Auto reverse dependencies testing
  • Error tracking (hard fail on packages we maintain, continue and report for others)
  • Summary comment to the PR at the end of the run through github API, citing failing packages, with links to the failing job and the commit

@kit-ty-kate kit-ty-kate modified the milestones: 2.4.0~rc1, 2.4.0 Jul 3, 2025
@rjbou rjbou modified the milestones: 2.4.0, 2.5.0~alpha1 Jul 15, 2025
@arozovyk arozovyk force-pushed the gha-plugins branch 2 times, most recently from 3b16c68 to b936860 Compare July 28, 2025 14:52
arozovyk
arozovyk reviewed Jul 28, 2025
View reviewed changes
.github/workflows/main.yml Outdated
env:
OPAM_DEPENDS: 1
permissions:
pull-requests: write
Copy link
Copy Markdown
Collaborator

@arozovyk arozovyk Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to fix gha lacking permissions to comment on the PR, probably due to the way this repo is configured
https://github.com/ocaml/opam/actions/runs/16571896348/job/46866212682?pr=6394#step:12:120
(My fork doesn't have this issue)

Copy link
Copy Markdown
Collaborator

@arozovyk arozovyk Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, PR from forks seem to have limited permissions by default actions/first-interaction#10 (comment)

@arozovyk arozovyk force-pushed the gha-plugins branch 2 times, most recently from 3a35a2e to 73b818d Compare July 29, 2025 08:35
@rjbou rjbou mentioned this pull request Aug 22, 2025
Draft
@rjbou rjbou requested a review from kit-ty-kate August 22, 2025 16:02
@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented Aug 22, 2025

I've extracted commenting in another PR (#6646) to unblock this one.

kit-ty-kate
kit-ty-kate requested changes Aug 22, 2025
View reviewed changes
@kit-ty-kate kit-ty-kate mentioned this pull request Aug 22, 2025
Merged
kit-ty-kate
kit-ty-kate reviewed Aug 26, 2025
View reviewed changes
@arozovyk arozovyk force-pushed the gha-plugins branch 2 times, most recently from b86c512 to 9a91757 Compare August 26, 2025 14:16
rjbou
rjbou commented Aug 26, 2025
View reviewed changes
kit-ty-kate
kit-ty-kate approved these changes Aug 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor suggestions but overall it looks alright.
There are definitely some parts that can be improved (e.g. every project reinstall their dependencies, even if they have been installed before because a new local switch was created), but it's more than i'm personally willing to invest in that so since it's blocking other PRs i'm fine with it.

@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented Aug 26, 2025

There are definitely some parts that can be improved (e.g. every project reinstall their dependencies, even if they have been installed before because a new local switch was created), but it's more than i'm personally willing to invest in that so since it's blocking other PRs i'm fine with it.

Yes, there is some caching that can be added, definitively.

kit-ty-kate
kit-ty-kate reviewed Aug 26, 2025
View reviewed changes
rjbou
rjbou commented Aug 26, 2025
View reviewed changes
.github/workflows/ci.ml
++ checkout ()
++ cache Archives
++ uses "Get changed files" ~id:"files" ~cond:(Predicate(true, Compare("github.event_name", "pull_request"))) (* ~continue_on_error:true see https://github.com/jitterbit/get-changed-files/issues/19 *) "Ana06/get-changed-files@v2.3.0" (* see https://github.com/jitterbit/get-changed-files/issues/55 ; Ana06'fork contains #19 and #55 fixes *)
++ run "Changed files list" [
Copy link
Copy Markdown
Collaborator Author

@rjbou rjbou Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the changed files printing as now it is present in the step output itself

@kit-ty-kate
Copy link
Copy Markdown
Member

kit-ty-kate commented Aug 26, 2025

Thanks!

@kit-ty-kate kit-ty-kate merged commit 32841ae into ocaml:master Aug 26, 2025
46 checks passed
@arozovyk arozovyk mentioned this pull request Aug 28, 2025
Merged
@kit-ty-kate kit-ty-kate mentioned this pull request Sep 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kit-ty-kate kit-ty-kate kit-ty-kate approved these changes

@arozovyk arozovyk arozovyk left review comments

Assignees

No one assigned

Labels

AREA: TEST

Projects

None yet

Milestone

2.5.0~alpha1

Development

Successfully merging this pull request may close these issues.

3 participants

@rjbou @arozovyk @kit-ty-kate