Skip to content

Conversation

@dzbarsky
Copy link
Contributor

No description provided.

@dzbarsky dzbarsky changed the title Enable some path-mapping support in common rules feat: Enable some path-mapping support in common rules Dec 10, 2025
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dzbarsky dzbarsky force-pushed the zbarsky/path-mapping branch from f254157 to b18b770 Compare December 10, 2025 21:51
@aspect-workflows
Copy link

aspect-workflows bot commented Dec 10, 2025

Bazel 8 (Test)

2 test targets passed

Targets
//lib/tests/copy_to_directory:case_22_test [k8-fastbuild]                    155ms
//lib/tests/copy_to_directory_bin_action:test [k8-fastbuild]                 193ms

Total test execution time was 348ms. 128 tests (98.5%) were fully cached saving 41s.


Bazel 9 (Test)

2 test targets passed

Targets
//lib/tests/copy_to_directory:case_22_test [k8-fastbuild]                    93ms
//lib/tests/copy_to_directory_bin_action:test [k8-fastbuild]                 155ms

Total test execution time was 248ms. 128 tests (98.5%) were fully cached saving 23s.


Bazel 7 (Test)

e2e/api_entries

All tests were cache hits

1 test (100.0%) was fully cached saving 23ms.


Bazel 8 (Test)

e2e/api_entries

All tests were cache hits

1 test (100.0%) was fully cached saving 23ms.


Bazel 9 (Test)

e2e/api_entries

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.


Bazel 7 (Test)

e2e/copy_action

All tests were cache hits

1 test (100.0%) was fully cached saving 20ms.


Bazel 8 (Test)

e2e/copy_action

All tests were cache hits

1 test (100.0%) was fully cached saving 21ms.


Bazel 9 (Test)

e2e/copy_action

All tests were cache hits

1 test (100.0%) was fully cached saving 33ms.


Bazel 7 (Test)

e2e/copy_to_directory

All tests were cache hits

6 tests (100.0%) were fully cached saving 309ms.


Bazel 8 (Test)

e2e/copy_to_directory

All tests were cache hits

6 tests (100.0%) were fully cached saving 295ms.


Bazel 9 (Test)

e2e/copy_to_directory

All tests were cache hits

6 tests (100.0%) were fully cached saving 263ms.


Bazel 7 (Test)

e2e/coreutils

All tests were cache hits

4 tests (100.0%) were fully cached saving 204ms.


Bazel 8 (Test)

e2e/coreutils

All tests were cache hits

4 tests (100.0%) were fully cached saving 260ms.


Bazel 9 (Test)

e2e/coreutils

All tests were cache hits

4 tests (100.0%) were fully cached saving 271ms.


Bazel 7 (Test)

e2e/external_copy_to_directory

All tests were cache hits

1 test (100.0%) was fully cached saving 20ms.


Bazel 8 (Test)

e2e/external_copy_to_directory

All tests were cache hits

1 test (100.0%) was fully cached saving 21ms.


Bazel 9 (Test)

e2e/external_copy_to_directory

All tests were cache hits

1 test (100.0%) was fully cached saving 33ms.


Bazel 7 (Test)

e2e/smoke

All tests were cache hits

4 tests (100.0%) were fully cached saving 415ms.


Bazel 8 (Test)

e2e/smoke

All tests were cache hits

4 tests (100.0%) were fully cached saving 636ms.


Bazel 9 (Test)

e2e/smoke

All tests were cache hits

4 tests (100.0%) were fully cached saving 732ms.


Bazel 7 (Test)

e2e/write_source_files

All tests were cache hits

1 test (100.0%) was fully cached saving 30ms.


Bazel 8 (Test)

e2e/write_source_files

All tests were cache hits

1 test (100.0%) was fully cached saving 28ms.


Bazel 9 (Test)

e2e/write_source_files

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.

@jbedard jbedard requested review from alexeagle and jbedard December 10, 2025 22:20
@jbedard
Copy link
Collaborator

jbedard commented Dec 10, 2025

Ref aspect-build/rules_js#2574

@dzbarsky dzbarsky force-pushed the zbarsky/path-mapping branch from b18b770 to 5b5ba57 Compare December 11, 2025 04:18
@jbedard jbedard enabled auto-merge (squash) December 11, 2025 04:22
@jbedard jbedard merged commit 34b7564 into bazel-contrib:main Dec 11, 2025
19 of 20 checks passed
# Sandboxing for this action is wasteful since there is a 1:1 mapping of input file/directory to
# output file/directory so little room for non-hermetic inputs to sneak in to the execution.
"no-sandbox": "1",
"supports-path-mapping": "1",
Copy link
Member

Choose a reason for hiding this comment

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

With sandboxing disabled path mapping will only work with remote execution. Is skipping the sandbox for these actions really worth it? With so few inputs there should be very little overhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. That's a good point. I don't think disabling sandbox is better than avoiding rerunning these actions when changing config...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants