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

Symlink loops cause repinning to fail #3166

Open
bradzacher opened this issue Jan 8, 2025 · 4 comments
Open

Symlink loops cause repinning to fail #3166

bradzacher opened this issue Jan 8, 2025 · 4 comments

Comments

@bradzacher
Copy link

This is similar to #3056

In our repo we have a few symlink loops created by pnpm which causes repinning to fail:

Error: Failed to splice workspace

Caused by:
    0: Failed to walk filesystem finding workspace Cargo.toml files
    1: File system loop found: <omitted> points to an ancestor <omitted>

Stack backtrace:
   0: std::backtrace::Backtrace::create
   1: cargo_bazel::metadata::workspace_discoverer::discover_workspaces_with_cache
   2: cargo_bazel::splicing::splicer::Splicer::splice_workspace
   3: cargo_bazel::cli::splice::splice
   4: cargo_bazel::main
   5: std::sys::backtrace::__rust_begin_short_backtrace
   6: std::rt::lang_start::{{closure}}
   7: std::rt::lang_start_internal
   8: _main

The ruleset should handle this gracefully by ignoring the loop.

Alternately the ruleset should respect .bazelignore -- which we currently use to exclude all of the node_modules folders in the repo from bazel. Perhaps this should be a separate request?

@UebelAndre
Copy link
Collaborator

I think it would be a good improvement to allow crate_universe rules to consider the .bazelignore file. I've run into these kinds of issues recently as well. I'd be happy to review a this change if someone wanted to put it together. Maybe explicitly providing it would be good but if there's a consistent auto-magic method that could work too! Perhaps @illicitonion might have some suggestions there 😄

@illicitonion
Copy link
Collaborator

Yeah, either following .bazelignore or allowing explicit excludes in the crate_universe seems reasonable.

@bradzacher
Copy link
Author

what's the best way to make it follow .bazelignore?
are there existing utilities we can leverage to get the contents, or would we need to build our own into the ruleset?
should we incorporate the .bazelignore into the lock file?

I'd be interested in looking into this because this is blocking my repo's upgrade.

@illicitonion
Copy link
Collaborator

illicitonion commented Jan 9, 2025

The quick answer is: //crate_universe/src/cli/splice.rs is the CLI that drives metadata analysis. It already has a workspace_dir flag. So we can join .bazelignore to it, see if it exists, and pass an Option of its contents down the call chain to the workspace discoverer.

If we want to be "correct" about this (i.e. changes to the .bazelignore file mean we re-evaluate things):

  • There's this other CLI at //crate_universe/src/cli/generate.rs which has a paths_to_track flag - this is a path to a file where it will write a list of paths that it read and it would like Bazel to re-run the repository rule if any of those paths change. We basically want to copy this mechanism into the splice binary, the 0 or 1 files depending on whether the .bazelignore file existed.

So roughly:

  • Add a --paths_to_track flag to splice.rs, make it required, track down all the starlark places where we need to pass this new flag basically by grepping for "splice" (see
    paths_to_track_file = repository_ctx.path("paths-to-track")
    and
    paths_to_track_file = paths_to_track_file,
    for what to pass to it). Have the splicer write a JSON file there just like the generator does. Have the starlark read those files (see
    paths_to_track = json.decode(repository_ctx.read(paths_to_track_file))
    for path in paths_to_track:
    # This read triggers watching the file at this path and invalidates the repository_rule which will get re-run.
    # Ideally we'd use repository_ctx.watch, but it doesn't support files outside of the workspace, and we need to support that.
    repository_ctx.read(path)
    )
  • Pipe the optional .bazelignore contents through to wherever it's needed in workspace_discoverer by joining .bazelignore to the workspace_dir.
  • If the .bazelignore file existed, write it to the paths_to_track file, otherwise don't.
  • Be a little sad that I don't think Bazel offers a way of expressing "please re-run this repository rule if the .bazelignore file was created since we last ran".

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

No branches or pull requests

3 participants