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

Build (after clean) with disk cache is missing empty subdirectories #15789

Closed
george-enf opened this issue Jul 1, 2022 · 7 comments · May be fixed by #16015
Closed

Build (after clean) with disk cache is missing empty subdirectories #15789

george-enf opened this issue Jul 1, 2022 · 7 comments · May be fixed by #16015
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug

Comments

@george-enf
Copy link
Contributor

george-enf commented Jul 1, 2022

Description of the bug:

I build a rule with disk cache enabled. I've added a 5 second delay in rule1.sh to prove that it is executing:

$ bazel build //:rule1 --disk_cache=diskcache
Target //:rule1 up-to-date:
  bazel-bin/tree
  bazel-bin/empty_dir
INFO: Elapsed time: 5.092s, Critical Path: 5.02s

The rule generates 2 directory trees. The empty_dir is completely empty. And the tree has an empty subdirectory.

$ find bazel-bin/
bazel-bin/
bazel-bin/empty_dir
bazel-bin/tree
bazel-bin/tree/subdir
bazel-bin/tree/subdir/file
bazel-bin/tree/empty_subdir

I do:

$ bazel clean

Then, I rebuild. rule1.sh is not re-executed.

$ bazel build //:rule1 --disk_cache=diskcache
Target //:rule1 up-to-date:
  bazel-bin/tree
  bazel-bin/empty_dir
INFO: Elapsed time: 0.163s, Critical Path: 0.01s

The empty subdirectory empty_subdir is missing:

$ find bazel-bin/
bazel-bin/
bazel-bin/empty_dir
bazel-bin/tree
bazel-bin/tree/subdir
bazel-bin/tree/subdir/file

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

BUILD

genrule(
    name = "rule1",
    cmd = "./rule1.sh $(RULEDIR)",
    tools = [
        "rule1.sh",
    ],
    outs = [
        "tree",
        "empty_tree",
    ],
)

rule1.sh

#!/bin/bash

set -e

RULEDIR="$1"
mkdir -p "$RULEDIR"
cd "$RULEDIR"

sleep 5

mkdir -p tree/subdir tree/empty_subdir
echo test > tree/subdir/file

mkdir -p empty_tree

Which operating system are you running Bazel on?

Ubuntu 20.04.1 LTS

What is the output of bazel info release?

release 5.2.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

It looks like there is special handing for empty directories here:

for (Entry<Path, DirectoryMetadata> entry : metadata.directories()) {
// For empty output directories, create the empty directory itself
entry.getKey().createDirectoryAndParents();
symlinksInDirectories.addAll(entry.getValue().symlinks());
}

I am not sure how empty subdirectories are/should be handled.

@fmeum
Copy link
Collaborator

fmeum commented Jul 2, 2022

@coeuvre Resolving this would probably require generalizing what I did in #15276 to apply to all empty (leaf) directories, not just the top-level one.

What do you think of the approach to add a new artifact type representing an empty directory? I think that this could even clean up some of the existing logic introduced for #15276.

@sgowroji sgowroji added type: bug untriaged team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Jul 4, 2022
@meisterT meisterT added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jul 14, 2022
@meisterT
Copy link
Member

@tjgq did you look into this recently?

@tjgq
Copy link
Contributor

tjgq commented Jul 15, 2022

I had a chat with @coeuvre. We believe the issue is that we fail to create empty directories here when downloading from the remote cache.

@tjgq
Copy link
Contributor

tjgq commented Jul 18, 2022

I think this issue actually runs deeper: we don't track empty directories inside tree artifacts properly (see #15901). However, it should be possible to fix the disk cache issue separately.

@coeuvre
Copy link
Member

coeuvre commented Oct 20, 2023

Duplicated of #15789.

@coeuvre coeuvre closed this as completed Oct 20, 2023
@brentleyjones
Copy link
Contributor

@coeuvre duplicated what? Did you mean #15901?

@coeuvre
Copy link
Member

coeuvre commented Oct 20, 2023

Argh, yes, that's what I had in my mind.

@brentleyjones brentleyjones closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants