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

Path mapping #59

Closed
wants to merge 15 commits into from
Closed

Path mapping #59

wants to merge 15 commits into from

Conversation

jjudd
Copy link

@jjudd jjudd commented Oct 17, 2024

Adds support for path mapping and improves a variety of other small things.

@jjudd jjudd changed the base branch from lucid-master to implement-a-toolchain October 17, 2024 16:33
@jjudd jjudd changed the title [WIP] Path mapping (stack on the toolchains PR) [WIP] Path mapping (stacked on the toolchains PR) Oct 17, 2024
@jadenPete jadenPete force-pushed the implement-a-toolchain branch 4 times, most recently from 1fd0fb1 to 80af0ac Compare November 14, 2024 16:14
…apping

Path mapping requires a custom Java toolchain where
javac_supports_worker_multiplex_sandboxing is set to True.

There was an issue where rules_jvm_external came before rules_java in
the WORKSPACE and it called the default rules_java toolchain setup
functions, preventing our custom toolchain from being used correctly.

This in turn broke path mapping.
…ferent configurations

Currently we compare the absolute path in the deps checker. This causes
us problems when there are configuration specific parts of one input and
not the other. This happens when there we've written a configuration
specific part of a path to disk and are comparing it to a path mapped
argument.

I imagine this is not the only place we'll run into issues with this.
@jjudd jjudd force-pushed the path-mapping-stacked-toolchain branch 3 times, most recently from 2abc7d0 to fc1fd79 Compare November 15, 2024 07:23
@jjudd jjudd changed the title [WIP] Path mapping (stacked on the toolchains PR) Path mapping Nov 15, 2024
@jjudd jjudd changed the base branch from implement-a-toolchain to lucid-master November 15, 2024 07:44
@jjudd jjudd marked this pull request as ready for review November 15, 2024 07:44
@jjudd jjudd requested a review from jadenPete November 15, 2024 07:45
We replace the string ${workDir} with the workDir path. Problem is,
sometimes the workDir path is " " (the current directory). If that's the
case, then we replace ${workDir} with a blank string. Using an absolute
path prevents that blank string from happening.
Because we're using strings, names like zinc_3 would likely conflict in
other repos.
For some reason multiplex sandboxing seems to have issues with files
going missing. It's been happening for a while, but intermittently. It
got much worse recently.

Not sure if this is a rule set bug or a Bazel bug. Regardless, disabling
multiplex sandboxing fixes it.
@jjudd jjudd force-pushed the path-mapping-stacked-toolchain branch from fc1fd79 to c33607b Compare November 15, 2024 07:52
@jjudd jjudd closed this Nov 15, 2024
@jjudd jjudd removed the request for review from jadenPete November 15, 2024 12:42
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.

None yet

1 participant