Skip to content

Commit 5436ee2

Browse files
committed
Simplify permission preservation to follow Git's approach
Following the reviewer's suggestion, this commit simplifies Unix permission handling to only preserve the owner executable bit, matching Git's approach. Git stores file permissions as either 100644 (non-executable) or 100755 (executable), focusing on the functional aspect (executability) while ignoring platform-specific permission details that are generally irrelevant for cross-platform builds. Changes: - permissionsToMode(): Only checks OWNER_EXECUTE permission, returns 0100755 if executable, 0100644 otherwise - modeToPermissions(): Restores either rwxr-xr-x (0755) or rw-r--r-- (0644) based on owner executable bit Benefits: - Simpler, more portable implementation - Matches proven Git model for cross-platform permission handling - Solves issue #214 (executable shell scripts) without over-engineering - Avoids cache invalidation from irrelevant permission differences All tests pass, including CacheUtilsPermissionsTest which verifies: - Permission changes affect cache hash when preservePermissions=true - Permissions are not preserved when preservePermissions=false
1 parent 63cf5a8 commit 5436ee2

File tree

1 file changed

+34
-53
lines changed

1 file changed

+34
-53
lines changed

src/main/java/org/apache/maven/buildcache/CacheUtils.java

Lines changed: 34 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -255,80 +255,61 @@ public static <T> void debugPrintCollection(
255255
}
256256

257257
/**
258-
* Convert POSIX file permissions to Unix mode integer.
258+
* Convert POSIX file permissions to Unix mode integer, following Git's approach of only
259+
* preserving the owner executable bit.
260+
*
261+
* <p>Git stores file permissions as either {@code 100644} (non-executable) or {@code 100755}
262+
* (executable). This simplified approach focuses on the functional aspect (executability)
263+
* while ignoring platform-specific permission details that are generally irrelevant for
264+
* cross-platform builds.</p>
259265
*
260266
* @param permissions POSIX file permissions
261-
* @return Unix mode as integer (e.g., {@code 0100755} for regular file with {@code rwxr-xr-x})
267+
* @return Unix mode: {@code 0100755} if owner-executable, {@code 0100644} otherwise
262268
*/
263269
private static int permissionsToMode(Set<PosixFilePermission> permissions) {
264-
// Start with regular file type (0100000 in octal)
265-
int mode = 0100000;
266-
267-
if (permissions.contains(PosixFilePermission.OWNER_READ)) {
268-
mode |= 0400;
269-
}
270-
if (permissions.contains(PosixFilePermission.OWNER_WRITE)) {
271-
mode |= 0200;
272-
}
270+
// Following Git's approach: preserve only the owner executable bit
271+
// Git uses 100644 (rw-r--r--) for regular files and 100755 (rwxr-xr-x) for executables
273272
if (permissions.contains(PosixFilePermission.OWNER_EXECUTE)) {
274-
mode |= 0100;
275-
}
276-
if (permissions.contains(PosixFilePermission.GROUP_READ)) {
277-
mode |= 0040;
278-
}
279-
if (permissions.contains(PosixFilePermission.GROUP_WRITE)) {
280-
mode |= 0020;
281-
}
282-
if (permissions.contains(PosixFilePermission.GROUP_EXECUTE)) {
283-
mode |= 0010;
273+
return 0100755; // Regular file, executable
274+
} else {
275+
return 0100644; // Regular file, non-executable
284276
}
285-
if (permissions.contains(PosixFilePermission.OTHERS_READ)) {
286-
mode |= 0004;
287-
}
288-
if (permissions.contains(PosixFilePermission.OTHERS_WRITE)) {
289-
mode |= 0002;
290-
}
291-
if (permissions.contains(PosixFilePermission.OTHERS_EXECUTE)) {
292-
mode |= 0001;
293-
}
294-
return mode;
295277
}
296278

297279
/**
298-
* Convert Unix mode integer to POSIX file permissions.
280+
* Convert Unix mode integer to POSIX file permissions, following Git's simplified approach.
281+
*
282+
* <p>This method interprets the two Git-standard modes:</p>
283+
* <ul>
284+
* <li>{@code 0100755} - Executable file: sets owner+group+others read/execute, owner write</li>
285+
* <li>{@code 0100644} - Regular file: sets owner+group+others read, owner write</li>
286+
* </ul>
299287
*
300-
* @param mode Unix mode (e.g., {@code 0100755} for regular file with {@code rwxr-xr-x})
288+
* <p>The key distinction is the presence of the execute bit. Other permission variations
289+
* are normalized to these two standard patterns for portability.</p>
290+
*
291+
* @param mode Unix mode (should be either {@code 0100755} or {@code 0100644})
301292
* @return Set of POSIX file permissions
302293
*/
303294
private static Set<PosixFilePermission> modeToPermissions(int mode) {
304295
Set<PosixFilePermission> permissions = new HashSet<>();
305-
// Extract permission bits (lower 9 bits), ignoring file type bits
306-
if ((mode & 0400) != 0) {
296+
297+
// Check owner executable bit (following Git's approach)
298+
if ((mode & 0100) != 0) {
299+
// Mode 100755: rwxr-xr-x (executable file)
307300
permissions.add(PosixFilePermission.OWNER_READ);
308-
}
309-
if ((mode & 0200) != 0) {
310301
permissions.add(PosixFilePermission.OWNER_WRITE);
311-
}
312-
if ((mode & 0100) != 0) {
313302
permissions.add(PosixFilePermission.OWNER_EXECUTE);
314-
}
315-
if ((mode & 0040) != 0) {
316303
permissions.add(PosixFilePermission.GROUP_READ);
317-
}
318-
if ((mode & 0020) != 0) {
319-
permissions.add(PosixFilePermission.GROUP_WRITE);
320-
}
321-
if ((mode & 0010) != 0) {
322304
permissions.add(PosixFilePermission.GROUP_EXECUTE);
323-
}
324-
if ((mode & 0004) != 0) {
325305
permissions.add(PosixFilePermission.OTHERS_READ);
326-
}
327-
if ((mode & 0002) != 0) {
328-
permissions.add(PosixFilePermission.OTHERS_WRITE);
329-
}
330-
if ((mode & 0001) != 0) {
331306
permissions.add(PosixFilePermission.OTHERS_EXECUTE);
307+
} else {
308+
// Mode 100644: rw-r--r-- (regular file)
309+
permissions.add(PosixFilePermission.OWNER_READ);
310+
permissions.add(PosixFilePermission.OWNER_WRITE);
311+
permissions.add(PosixFilePermission.GROUP_READ);
312+
permissions.add(PosixFilePermission.OTHERS_READ);
332313
}
333314
return permissions;
334315
}

0 commit comments

Comments
 (0)