chore: add a macro for duplicating an e2e for bazel-lib 2.x #1366
Conversation
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
3 times, most recently
from
1973e59
to
89d35f4
Compare
| e2e/bzlmod | ||
| e2e/bzlmod_bazel_lib_2 | ||
| e2e/git_dep_metadata | ||
| e2e/gyp_no_install_script | ||
| e2e/js_image_docker | ||
| e2e/js_image_oci | ||
| e2e/js_run_devserver | ||
| e2e/npm_link_package | ||
| e2e/npm_link_package-esm | ||
| e2e/npm_translate_lock | ||
| e2e/npm_translate_lock_auth | ||
| e2e/npm_translate_lock_empty | ||
| e2e/npm_translate_lock_git+ssh | ||
| e2e/npm_translate_lock_multi | ||
| e2e/npm_translate_lock_partial_clone | ||
| e2e/npm_translate_lock_subdir_patch | ||
| e2e/npm_translate_package_lock | ||
| e2e/npm_translate_yarn_lock | ||
| e2e/package_json_module | ||
| e2e/patch_from_repo | ||
| e2e/pnpm_repo_install | ||
| e2e/pnpm_workspace | ||
| e2e/pnpm_workspace_bazel_lib_2 | ||
| e2e/pnpm_workspace_deps | ||
| e2e/pnpm_workspace_rerooted | ||
| e2e/rules_foo | ||
| e2e/update_pnpm_lock | ||
| e2e/update_pnpm_lock_with_import | ||
| e2e/vendored_node | ||
| e2e/vendored_tarfile | ||
| e2e/verify_patches | ||
| e2e/webpack_devserver | ||
| e2e/webpack_devserver_esm | ||
| e2e/worker | ||
| e2e/workspace |
There was a problem hiding this comment.
We can no longer just ignore e2e because the write_source_files target which writes the duplicate e2e with bazel-lib 2.x will puke on the analysis phase test where it globs to check for existence.
However I added some additional tests to ensure all e2es are added here.
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
from
89d35f4
to
2be23b9
Compare
hidden-files.patch
Outdated
| diff --git a/lib/private/write_source_file.bzl b/lib/private/write_source_file.bzl | ||
| index f177118..db25c72 100644 | ||
| --- a/lib/private/write_source_file.bzl | ||
| +++ b/lib/private/write_source_file.bzl | ||
| @@ -219,8 +219,8 @@ else | ||
| echo "Copying directory $in to $out in $PWD" | ||
| rm -Rf "$out"/* | ||
| mkdir -p "$out" | ||
| - cp -fRL "$in"/* "$out" | ||
| - chmod -R ug+w "$out"/* | ||
| + cp -fRL "$in"/. "$out" | ||
| + chmod -R ug+w "$out"/. | ||
| {executable_dir} | ||
| fi | ||
| """.format( |
There was a problem hiding this comment.
I found a bug in write_source_files while working on this. It doesn't copy over hidden files.
There was a problem hiding this comment.
The patch itself has a bug though where it doesn't preserve executable bits.
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
4 times, most recently
from
19edb29
to
7f37a30
Compare
|
|
||
| aspect_bazel_lib_dependencies() | ||
|
|
||
| register_coreutils_toolchains() |
There was a problem hiding this comment.
I shouldn't have to add this to the bazel-lib 1.x e2e, but it is needed for the bazel-lib 2.x version of the e2e. So flipping the version isn't always the only thing needed to test both. Perhaps this is a good enough reason not to go with this approach..
There was a problem hiding this comment.
I could add an input to the e2e copying macro that takes buildozer commands to apply to the WORKSPACE and the 2.0 version could be modified that way..
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
from
7f37a30
to
8853e46
Compare
|
My overall take is that it's simpler to just make a couple copies of e2es manually and point them to use bazel-lib 2.x This seems like overkill. |
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
2 times, most recently
from
193ff5b
to
a64bed4
Compare
.github/workflows/ci.yaml
Outdated
| @@ -279,7 +283,7 @@ jobs: | |||
| # still bootstraps Aspect CLI from configuration in .bazeliskrc. Aspect CLI will | |||
| # then use .bazelversion to determine which Bazel version to use | |||
| run: | | |||
| echo "${{ matrix.bazelversion }}" > .bazelversion | |||
| find . -name ".bazelversion" -type f -exec echo "${{ matrix.bazelversion }}" > {} | |||
There was a problem hiding this comment.
The .bazelversion symlinks in bazel-lib 2 e2es that are written back to the source tree are not preserved. Find all non-symlink .bazelversion files and replace them.
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
3 times, most recently
from
f921ac0
to
7398231
Compare
kormide
force-pushed
the
auto-bazel-lib2-e2e
branch
from
7398231
to
d29eb67
Compare
|
Yeah this does feel like a bunch of complexity for a case that we don't care that much about, and in 6mo or something we'll expect most users to be on bazel-lib 2.0 simply because there will be some rulesets that take that as a lower-bound (e.g. rules_distroless to get the tar toolchain) I think we should just mix-and-match for now, most e2e on bazel-lib 2 and a couple on bazel-lib 1. We won't have comprehensive test coverage, so there's some risk, but it will be a smoke test to catch some obvious things. WDYT? |
Yeah I agree. I'll just select a few and flip them to use bazel-lib 2.x. |
|
Closing in favour of #1369. |
This adds a macro that will take any e2e, swap bazel-lib 1.x for 2.x, and write it to the source tree. The goal here is to make it easy to run the same e2e against bazel-lib 2.x while reducing the test maintenance burden of keeping both version of the test identical.
It uses the buildozer toolchain to modify version in the WORKSPACE and MODULE files of e2es.
I don't think we need to test every e2e with bazel-lib 2.x since our matrix is already large. This just adds a couple.
Related to #1363.
Type of change
Test plan