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
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
8417751
Implement fs::diff_dirs
Akirathan Feb 3, 2025
b4817dd
Add stdlib_api_check to backend ci-check
Akirathan Feb 3, 2025
09d5e9f
stdlib-api-check is a separate command
Akirathan Feb 3, 2025
a77d7aa
Do API checks only on Standard.Base
Akirathan Feb 3, 2025
2776bc5
[WIP] Merge branch with generated API md files
Akirathan Feb 3, 2025
b440bf9
The stdlib docs/api dir is taken from distribution.
Akirathan Feb 3, 2025
c4eb227
Difference between files is delegated to the git program
Akirathan Feb 3, 2025
ef8aadd
Remove the diff crate dependency
Akirathan Feb 3, 2025
aa0c72e
fs::diff_dirs is async
Akirathan Feb 3, 2025
f0ddc04
fmt
Akirathan Feb 3, 2025
abc2e5a
SPawn git command only once
Akirathan Feb 4, 2025
dba5ba1
Generate new API in built-distribution.
Akirathan Feb 4, 2025
18616b4
Add StandardLibraryAPICheck as new workflow job to Engine Checks
Akirathan Feb 4, 2025
c5c1a97
Revert "[WIP] Merge branch with generated API md files"
Akirathan Feb 4, 2025
c9106ff
Revert MODULE.bazel.lock
Akirathan Feb 4, 2025
e9d5d21
Merge branch 'develop' into wip/akirathan/10507-ci-job-check-api
Akirathan Feb 4, 2025
54e3e4e
Use git --diff on entire dirs, not on single files
Akirathan Feb 4, 2025
0531918
Check API for libraries with non-empty docs/api dirs
Akirathan Feb 4, 2025
e996ecd
Add error message to assertion
Akirathan Feb 5, 2025
13ee891
Hardcode lib dir path inside distribution
Akirathan Feb 5, 2025
3b6bddd
Add a workflow that automatically adds "-libs-API-change" label
Akirathan Feb 5, 2025
e0a8e0d
stdlib-api-changes-label is inside engine-checks workflow.
Akirathan Feb 5, 2025
1c78ec9
Regenerate API files
Akirathan Feb 5, 2025
2bbc038
Fix printing of all changed files in stdlibs-api-changed
Akirathan Feb 5, 2025
0abdea8
Fix pattern for changed stdlib API files
Akirathan Feb 5, 2025
dedf279
Fix pattern for changed stdlib API files
Akirathan Feb 5, 2025
1c3553c
Fix job name
Akirathan Feb 5, 2025
6e7d03e
Fix pattern for changed stdlib API files
Akirathan Feb 5, 2025
6324767
Add all_changed files to the GH action
Akirathan Feb 5, 2025
582a6da
Add id to the GH action
Akirathan Feb 5, 2025
91d20d3
FIx pattern and depdnecy
Akirathan Feb 5, 2025
65f702c
Fix name of add-labels GH action
Akirathan Feb 5, 2025
263ad86
Add github token to stdlib-api-changes-label job
Akirathan Feb 5, 2025
4cf63d2
Add permissions to the engine-pull-request workflow
Akirathan Feb 5, 2025
9c81c1e
[WIP] Do not run Stdlib API check job on revision that is merged with…
Akirathan Feb 6, 2025
02377a9
Allow customization of checkout action
Akirathan Feb 6, 2025
745f798
Revert "Allow customization of checkout action"
Akirathan Feb 6, 2025
0b13c54
Revert "[WIP] Do not run Stdlib API check job on revision that is mer…
Akirathan Feb 6, 2025
96b577d
Merge branch 'develop' into wip/akirathan/10507-ci-job-check-api
Akirathan Feb 6, 2025
7ec7b4c
Regenerate API after merge with develop
Akirathan Feb 6, 2025
471c0b5
Git tests are run sequentially
Akirathan Feb 6, 2025
0afc190
lint
Akirathan Feb 6, 2025
5fa5acf
Revert "Git tests are run sequentially"
Akirathan Feb 6, 2025
45370e2
Ignore git tests.
Akirathan Feb 6, 2025
81901ad
Merge branch 'develop' into wip/akirathan/10507-ci-job-check-api
Akirathan Feb 10, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/engine-changed-files.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ on:
any_changed:
description: "Returns `true` when any of the filenames have changed"
value: ${{ jobs.engine-changed-files.outputs.any_changed }}
stdlib_api_any_changed:
description: "Returns `true` when any of the Standard library API filenames have changed"
value: ${{ jobs.stdlib-api-changed-files.outputs.any_changed }}

jobs:
engine-changed-files:
Expand Down Expand Up @@ -53,3 +56,29 @@ jobs:
for file in ${ALL_CHANGED_FILES}; do
echo "$file"
done
stdlib-api-changed-files:
runs-on: ubuntu-latest
name: Changed Standard libs API docs files
outputs:
all_changed_files: ${{ steps.stdlib-api-changed-files.outputs.all_changed_files }}
any_changed: ${{ steps.stdlib-api-changed-files.outputs.any_changed }}
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 2
- name: Get changed files
id: stdlib-api-changed-files
uses: tj-actions/changed-files@v45
with:
files: |
distribution/lib/Standard/**/docs/api/**.md
- name: List all changed files
env:
ALL_CHANGED_FILES: ${{ steps.stdlib-api-changed-files.outputs.all_changed_files }}
run: |
if [[ "${{ steps.stdlib-api-changed-files.outputs.any_changed }}" == "true" ]]; then
echo "API docs files changed:"
fi
for file in ${ALL_CHANGED_FILES}; do
echo "$file"
done
41 changes: 41 additions & 0 deletions .github/workflows/engine-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,47 @@ jobs:
GRAAL_EDITION: GraalVM CE
permissions:
checks: write
enso-build-ci-gen-job-standard-library-api-check-linux-amd64:
name: Standard Library API check (linux, amd64)
runs-on:
- self-hosted
- Linux
steps:
- if: runner.os == 'Windows'
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
name: Setup required bazel environment
run: "\n\"BAZEL_SH=C:\\Program Files\\Git\\bin\\bash.exe\" >> $env:GITHUB_ENV\n\"BAZEL_VC=C:\\BuildTools\\VC\" >> $env:GITHUB_ENV\n "
shell: pwsh
- name: Setup bazel environment
uses: bazel-contrib/setup-bazel@0.13.0
with:
bazelrc: build --remote_cache=grpcs://${{ vars.ENSO_BAZEL_CACHE_URI }} --remote_cache_header="authorization=Basic ${{ secrets.ENSO_BAZEL_CACHE_TOKEN }}"
output-base: ${{ runner.os == 'Windows' && 'c:/_bazel' || '' }}
- name: Expose Artifact API and context information.
uses: actions/github-script@v7
with:
script: "\n core.exportVariable(\"ACTIONS_RUNTIME_TOKEN\", process.env[\"ACTIONS_RUNTIME_TOKEN\"])\n core.exportVariable(\"ACTIONS_RUNTIME_URL\", process.env[\"ACTIONS_RUNTIME_URL\"])\n core.exportVariable(\"GITHUB_RETENTION_DAYS\", process.env[\"GITHUB_RETENTION_DAYS\"])\n console.log(context)\n "
- name: Checking out the repository
uses: actions/checkout@v4
with:
clean: false
submodules: recursive
- name: Build Script Setup
run: ./run --help || (git clean -ffdx && ./run --help)
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- if: "(contains(github.event.pull_request.labels.*.name, 'CI: Clean build required') || inputs.clean_build_required)"
name: Clean before
run: ./run git-clean
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- run: ./run backend stdlib-api-check
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- if: "(always()) && (contains(github.event.pull_request.labels.*.name, 'CI: Clean build required') || inputs.clean_build_required)"
name: Clean after
run: ./run git-clean
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
enso-build-ci-gen-job-standard-library-tests-graal-vm-ce-linux-amd64:
name: Standard Library Tests (GraalVM CE) (linux, amd64)
runs-on:
Expand Down
15 changes: 14 additions & 1 deletion .github/workflows/engine-pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ concurrency:

permissions:
checks: write
pull-requests: write

jobs:
engine-changed-files:
Expand All @@ -35,10 +36,22 @@ jobs:
if: needs.engine-changed-files.outputs.any_changed == 'true' || github.ref == 'refs/heads/develop'
secrets: inherit

stdlib-api-changes-label:
name: 🏷 Append Standard library API change label
runs-on: ubuntu-latest
needs: [engine-changed-files]
if: needs.engine-changed-files.outputs.stdlib_api_any_changed == 'true'
steps:
- name: Add label to the PR
uses: actions-ecosystem/action-add-labels@v1
with:
labels: "-libs-API-change"
github_token: ${{ secrets.GITHUB_TOKEN }}

required-checks:
name: Engine Required Checks
runs-on: ubuntu-latest
needs: [engine-checks]
needs: [engine-checks, stdlib-api-changes-label]
if: always()
steps:
- name: Checks Summary
Expand Down
1 change: 1 addition & 0 deletions build_tools/build/src/ci_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ pub fn engine_checks() -> Result<Workflow> {
..default()
};
workflow.add(PRIMARY_TARGET, job::VerifyLicensePackages);
workflow.add(PRIMARY_TARGET, job::StandardLibraryApiCheck);
for target in PR_REQUIRED_TARGETS {
add_backend_checks(&mut workflow, target, graalvm::Edition::Community);
}
Expand Down
12 changes: 12 additions & 0 deletions build_tools/build/src/ci_gen/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,18 @@ impl JobArchetype for StandardLibraryTests {
}
}

#[derive(Clone, Copy, Debug)]
pub struct StandardLibraryApiCheck;

impl JobArchetype for StandardLibraryApiCheck {
fn job(&self, target: Target) -> Job {
let job_name = "Standard Library API check";
let run_command = "backend stdlib-api-check";
let job = RunStepsBuilder::new(run_command).build_job(job_name, target);
job
}
}

/** This is a temporary workaround.
*
* The Cloud tests preparation requires `aws` CLI to be installed on the machine.
Expand Down
5 changes: 5 additions & 0 deletions build_tools/build/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ pub struct BuildConfigurationFlags {
pub test_java_generated_from_rust: bool,
/// Verify License Packages in Distributions.
pub verify_packages: bool,
pub stdlib_api_check: bool,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -294,6 +295,9 @@ impl BuildConfigurationResolved {
config.generate_java_from_rust = true;
}

if config.stdlib_api_check {
config.build_engine_package = true;
}
Self(config)
}
}
Expand Down Expand Up @@ -352,6 +356,7 @@ impl Default for BuildConfigurationFlags {
generate_java_from_rust: false,
test_java_generated_from_rust: false,
verify_packages: false,
stdlib_api_check: false,
}
}
}
Expand Down
99 changes: 99 additions & 0 deletions build_tools/build/src/engine/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,12 @@ impl RunContext {

perhaps_test_java_generated_from_rust_job.await.transpose()?;

// === Stdlib API check ===
debug!("Running standard libraries API check.");
if self.config.stdlib_api_check {
self.stdlib_api_check(&enso).await?;
}

// === Run benchmarks ===
let build_benchmark_task = if self.config.build_benchmarks {
let build_benchmark_task_names = [
Expand Down Expand Up @@ -564,6 +570,99 @@ impl RunContext {

Ok(())
}

/// Checks API for all the standard libraries that have non-empty `docs/api` directory.
async fn stdlib_api_check(&self, built_enso: &BuiltEnso) -> Result {
let libs_to_check = self.stdlibs_with_api()?;
debug!("Checking API for standard libraries: {:?}", libs_to_check);
for lib in self.stdlibs_with_api()? {
self.lib_api_check(built_enso, &lib).await?;
}
Ok(())
}

/// Returns list of all the standard libraries with non-empty `docs/api` directory.
fn stdlibs_with_api(&self) -> Result<Vec<StdLib>> {
let mut stdlibs: Vec<StdLib> = vec![];
let lib_root = self.repo_root.distribution.lib.join("Standard");
for dir in std::fs::read_dir(lib_root)? {
// The path to the library is of this form:
// distribution/lib/Standard/<lib_name>/0.0.0-dev
let dir = dir?;
let lib_dir = dir.path().join("0.0.0-dev");
assert!(lib_dir.exists(), "Directory {} does not exist", lib_dir.display());
let api_dir = lib_dir.join("docs").join("api");
if api_dir.exists() {
let libname = dir.file_name().to_string_lossy().to_string();
stdlibs.push(StdLib { name: libname.to_string(), path: lib_dir });
}
}
Ok(stdlibs)
}

/// Checks the API for a single library
/// #parameters
/// - `lib_path` Path to the library inside the `distribution` directory. The `docs/api`
/// directory inside `lib_path` is expected to exist.
async fn lib_api_check(&self, built_enso: &BuiltEnso, lib: &StdLib) -> Result {
let old_api_dir = lib.path.join("docs").join("api");
assert!(old_api_dir.exists());
debug!(
"Checking API for library Standard.{}, its API dir is in {:?}",
lib.name, old_api_dir
);
// `lib_path_in_built_distribution` points to the lib in the `built-distribution`
// directory, which is not under VCS. We will regenerate the API in this directory
// and compare it to the one from `lib_path`.
let lib_path_in_built_distribution = self
.repo_root
.built_distribution
.enso_engine_triple
.engine_package
.lib
.join_iter(["Standard", lib.name.as_str()])
.join(self.paths.version().to_string());
let new_api_dir = lib_path_in_built_distribution.join("docs").join("api");
let mut cmd = built_enso
.cmd()?
.with_arg("--docs")
.with_arg("api")
.with_arg("--in-project")
.with_arg(lib_path_in_built_distribution);
cmd.run_ok().await?;
let diff = ide_ci::fs::diff_dirs(&old_api_dir, &new_api_dir).await;
match diff {
Ok(_) => Ok(()),
Err(err) => {
let suggested_cmd = built_enso
.cmd()?
.with_arg("--docs")
.with_arg("api")
.with_arg("--in-project")
.with_arg(lib.path.clone());
error!("API check failed for library Standard.{}", lib.name);
error!("Current API vs Old API: {}", err);
error!("If you wish to overwrite the current API in the directory {}, run the following command {},
and commit the modified files",
old_api_dir.display(),
suggested_cmd.describe()
);
bail!("API check failed for library Standard.{}", lib.name);
}
}
}
}

#[derive(Debug)]
struct StdLib {
name: String,
path: PathBuf,
}

impl Display for StdLib {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "Standard.{}", self.name)
}
}

/// Upload the directory with Enso-generated test results.
Expand Down
Loading
Loading