Skip to content

Commit a2c9a39

Browse files
committed
Only chmod files in ActionMetadataHandler if necessary
This ensures that the ctime of the files doesn't change if no permission change is necessary, thereby improving the hit rate of the digest cache.
1 parent 0ba0fad commit a2c9a39

File tree

5 files changed

+38
-2
lines changed

5 files changed

+38
-2
lines changed

src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,8 @@ private static FileArtifactValue fileArtifactValueFromStat(
722722
}
723723

724724
private void setPathPermissionsIfFile(Path path) throws IOException {
725-
if (path.isFile(Symlinks.NOFOLLOW)) {
725+
FileStatus stat = path.stat(Symlinks.NOFOLLOW);
726+
if (stat.isFile() && stat.getPermissions() != outputPermissions.getPermissionsMode()) {
726727
setPathPermissions(path);
727728
}
728729
}

src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ public long getNodeId() {
120120
return status.getInodeNumber();
121121
}
122122

123-
int getPermissions() {
123+
@Override
124+
public int getPermissions() {
124125
return status.getPermissions();
125126
}
126127

src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,16 @@ public interface FileStatus {
8585
* ought to cause the node ID of b to change, but appending / modifying b should not.
8686
*/
8787
long getNodeId() throws IOException;
88+
89+
/**
90+
* Returns the file's permissions in POSIX format (e.g. 0755) if possible without performing
91+
* additional IO, otherwise (or if unsupported by the file system) returns -1.
92+
*
93+
* <p>If accurate group and other permissions aren't available, the returned value should attempt
94+
* to mimic a umask of 022 (i.e. read and execute permissions extend to group and other, write
95+
* does not).
96+
*/
97+
default int getPermissions() {
98+
return -1;
99+
}
88100
}

src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,22 @@ public long getNodeId() {
120120
return System.identityHashCode(this);
121121
}
122122

123+
@Override
124+
public synchronized int getPermissions() {
125+
int permissions = 0;
126+
// Emulate the default umask of 022.
127+
if (isReadable) {
128+
permissions |= 0444;
129+
}
130+
if (isWritable) {
131+
permissions |= 0200;
132+
}
133+
if (isExecutable) {
134+
permissions |= 0111;
135+
}
136+
return permissions;
137+
}
138+
123139
@Override
124140
public final InMemoryContentInfo inode() {
125141
return this;

src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,12 @@ public long getNodeId() {
202202
// TODO(bazel-team): Consider making use of attributes.fileKey().
203203
return -1;
204204
}
205+
206+
@Override
207+
public int getPermissions() {
208+
// Files on Windows are implicitly readable and executable.
209+
return 0555 | (attributes.isReadOnly() ? 0 : 0200);
210+
}
205211
};
206212

207213
return status;

0 commit comments

Comments
 (0)