Skip to content

Treat source directories as directories#29340

Open
cameron-martin wants to merge 1 commit intobazelbuild:masterfrom
cameron-martin:fix-26705-12954-source-directories
Open

Treat source directories as directories#29340
cameron-martin wants to merge 1 commit intobazelbuild:masterfrom
cameron-martin:fix-26705-12954-source-directories

Conversation

@cameron-martin
Copy link
Copy Markdown
Contributor

Description

Treat source directories as directories in analysis by carrying a directory bit on SourceArtifact instead of classifying all source artifacts as files.

This fixes two related bugs:

  • source directories now report File.is_directory == True
  • ctx.actions.symlink(target_file=...) now accepts source directories with declare_directory() outputs

Motivation

Fixes #12954
Fixes #26705

Build API Changes

No

Checklist

  • I have added tests for the new use cases (if any).
  • I have updated the documentation (if applicable).

Release Notes

RELNOTES: None

Treat source directories as directories in analysis by carrying a directory bit on SourceArtifact instead of classifying all source artifacts as files.

This fixes two related bugs:
- source directories now report File.is_directory == True
- ctx.actions.symlink(target_file=...) now accepts source directories with declare_directory() outputs

Fixes bazelbuild#12954
Fixes bazelbuild#26705
@cameron-martin cameron-martin requested a review from a team as a code owner April 19, 2026 14:13
@cameron-martin cameron-martin requested review from dabanki and removed request for a team April 19, 2026 14:13
@github-actions github-actions Bot added team-Performance Issues for Performance teams team-Configurability platforms, toolchains, cquery, select(), config transitions awaiting-review PR is awaiting review from an assigned reviewer labels Apr 19, 2026
@cameron-martin cameron-martin changed the title Fix source directory handling for Starlark File artifacts Treat source directories as directories Apr 19, 2026
expect_symlink bazel-bin/a/a.link
}

function test_source_directory_is_directory_from_glob() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This likely isn't the best location for this test because it doesn't have anything to do with symlinks as the test below does. Are there any better places for this?

@tjgq
Copy link
Copy Markdown
Contributor

tjgq commented Apr 23, 2026

I'm supportive of this PR, but as currently written, this incurs in an extra stat() for every source artifact, which I suspect is going to cause a performance regression at Google (and possibly also for other large Bazel projects). I wonder if we're already doing a stat() elsewhere and could piggyback on it. Another alternative is to delay the stat() until SourceArtifact.isDirectory() is called, but that doesn't help if it ends up getting called for most artifacts anyway; I'm also unsure about the consequences for exception handling.

Nevertheless, it would be worthwhile (for someone at Google) to run a benchmark on this PR as written, just to get a sense of the performance hit. I can't volunteer to do so at this time, though.

@susinmotion susinmotion removed the request for review from dabanki April 23, 2026 15:54
@susinmotion susinmotion removed the team-Configurability platforms, toolchains, cquery, select(), config transitions label Apr 23, 2026
@cameron-martin
Copy link
Copy Markdown
Contributor Author

cameron-martin commented Apr 30, 2026

@tjgq Presumably we have to do a stat when globbing, so it should be possible to feed this info through from that when the source of the input is a glob. Would just covering glob inputs be sufficient? How many source inputs come from globs vs not?

Do you know of anyone who can do this internal benchmarking?

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

Labels

awaiting-review PR is awaiting review from an assigned reviewer team-Performance Issues for Performance teams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot symlink source directories with declare_directory Directories from glob are not marked as is_directory

3 participants