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

New gate for stdlib API check #12223

Merged
merged 45 commits into from
Feb 10, 2025
Merged

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Feb 4, 2025

Closes #10507

Standard Library API Check job

Introduces a new command ./run backend stdlib-api-check that compares the current API of Standard.Base to the old one. The old API will be inside the directory distribution/lib/Standard/Base/0.0.0-dev/docs/api, after #12203 is merged. The new API is generated inside the directory built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/lib/Standard/Base/0.0.0-dev/docs that is not under VCS. The command compares these two directories. It uses git-diff under the hood.

Introduces a new workflow job to Engine checks - 'Standard Library API check'. Defined inside

enso-build-ci-gen-job-standard-library-api-check-linux-amd64:

What happens if the API is changed?

Let's modify Vector.enso with:

diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso
index a05719073b..c34efa1341 100644
--- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso
+++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso
@@ -114,6 +114,8 @@ type Vector a
     from_array : Array -> Vector
     from_array array = @Builtin_Method "Vector.from_array"
 
+    my_super_method self = 42
+
     ## PRIVATE
        A helper method that takes a vector or an array or a single element and
        returns a vector.

and run ./run --skip-version-check backend stdlib-api-check. The command fails with:

ERROR main_internal: enso_build::engine::context: API check failed for library Standard.Base
ERROR main_internal: enso_build::engine::context: Current API vs Old API: diff --git a/home/pavel/dev/enso/distribution/lib/Standard/Base/0.0.0-dev/docs/api/Data/Vector.md b/home/pavel/dev/enso/built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/lib/Standard/Base/0.0.0-dev/docs/api/Data/Vector.md
index 1a57150a53..9149be339b 100644
--- a/home/pavel/dev/enso/distribution/lib/Standard/Base/0.0.0-dev/docs/api/Data/Vector.md
+++ b/home/pavel/dev/enso/built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/lib/Standard/Base/0.0.0-dev/docs/api/Data/Vector.md
@@ -54,6 +54,7 @@
     - length self -> Standard.Base.Any.Any
     - map self function:Standard.Base.Any.Any on_problems:(Standard.Base.Errors.Problem_Behavior.Problem_Behavior|Standard.Base.Data.Vector.No_Wrap)= -> Standard.Base.Any.Any
     - map_with_index self function:Standard.Base.Any.Any on_problems:(Standard.Base.Errors.Problem_Behavior.Problem_Behavior|Standard.Base.Data.Vector.No_Wrap)= -> Standard.Base.Any.Any
+    - my_super_method self -> Standard.Base.Any.Any
     - new length:Standard.Base.Any.Any constructor:Standard.Base.Any.Any -> Standard.Base.Any.Any
     - not_empty self -> Standard.Base.Any.Any
     - pad self n:Standard.Base.Any.Any elem:Standard.Base.Any.Any -> Standard.Base.Any.Any

ERROR main_internal: enso_build::engine::context: If you wish to overwrite the current API in the directory /home/pavel/dev/enso/distribution/lib/Standard/Base/0.0.0-dev/docs/api, run the following command Command:
	"/usr/bin/bash" "/home/pavel/dev/enso/built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso" "--docs" "api" "--in-project" "/home/pavel/dev/enso/distribution/lib/Standard/Base/0.0.0-dev",
                       and commit the modified files
ERROR main_internal: enso_build_cli: error=API check failed for library Standard.Base
 INFO main_internal: enso_build_cli: close
Error: API check failed for library Standard.Base

After running the suggested command, the api-stdlib-check succeeds.

Example failure of the job: https://github.com/enso-org/enso/actions/runs/13157161601/job/36716899929?pr=12223

Standard Library API changed label

  • Added -libs-API-changed label
  • Added new job that checks if any of distribution/lib/Standard/**/docs/api/*.md was changed, and if so, automatically appends the -libs-API-changed label to the PR.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • After Generate API docs for Standard.Base #12203 is merged, revert 2776bc5
  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@Akirathan Akirathan added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Feb 4, 2025
@Akirathan Akirathan self-assigned this Feb 4, 2025
@Akirathan
Copy link
Member Author

Blocked by #12203. After #12203 is merged, revert 2776bc5

@Akirathan Akirathan marked this pull request as ready for review February 4, 2025 10:00
Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

Great:

  • I am glad the description of the PR contains information about what to do when the CI fails
  • Why don't you invoke git diff --no-index on the whole directories?

@Akirathan
Copy link
Member Author

Engine / Files Changed / Changed Standard libs API docs files correctly recognizes that distribution/lib/Standard/Base/0.0.0-dev/docs/api/Widget_Helpers.md was modified. Which enables Engine / Append Standard library API change label and that has successfully appended -libs-API-changed label to this PR, in event #12223 (comment).

- make_regex_text_widget -> Standard.Base.Any.Any
- make_regex_text_widget display:Standard.Base.Metadata.Display= -> Standard.Base.Any.Any
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could indicate the presence of PRIVATE tag?

From GUI perspective an API change of a fully visible component VS of a component that is exported but is not displayed in Component Browser, have slightly different 'severity' - so it could be easier to judge the changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is theoretically possible. However, I would like to integrate this PR as is, as soon as I fix all the tests. Let's keep your suggestions for some follow-ups.

Copy link
Member

Choose a reason for hiding this comment

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

Sure!

Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

Very cool!

@Akirathan Akirathan merged commit 082b09a into develop Feb 10, 2025
49 of 50 checks passed
@Akirathan Akirathan deleted the wip/akirathan/10507-ci-job-check-api branch February 10, 2025 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI check for Standard.Base API changes via snapshotting
5 participants