From: Stefan Sperling Subject: commit executable bit change To: gameoftrees@openbsd.org Date: Tue, 15 Oct 2019 17:06:08 +0200 I have accidentally committed regress/cmdline/integrate.sh without the executable bit because of a bug in 'got revert' where the reverted file's permissions were overwritten with those of a temporary file. This diff fixes that bug, makes xbit changes show up in 'got status', and allows 'got commit' to create new commits which contain only xbit changes. 'got diff' will currently produce an empty diff for such commits, but that is a separate issue. It is possible to create such commits with Git, too. No new regression test is added for the 'revert' bug because showing the bogus mode change in 'status' already made test_revert_patch_one_change fail. Also, Got was only setting the owner's x bit, but not group/other as Git would do. This gets fixed here as well. ok? diff refs/heads/master refs/heads/xbit blob - 07c1a76b07c5e373591b4238f041c215199048a2 blob + 703e42f8dad320122e89dcebabfda4f26a60a497 --- got/got.1 +++ got/got.1 @@ -252,6 +252,7 @@ using the following status codes: .It \(a~ Ta versioned file is obstructed by a non-regular file .It ? Ta unversioned item not tracked by .Nm +.It m Ta modified file modes (executable bit only) .It N Ta non-existent .Ar path specified on the command line @@ -706,6 +707,7 @@ Show the status of each affected file, using the follo .It M Ta modified file .It D Ta file was deleted .It A Ta new file was added +.It m Ta modified file modes (executable bit only) .El .Pp Files which are not part of the new commit will retain their previously blob - add6c8aa88f249f82d6451270fc7c7c6952abc9b blob + 336553780903efcf81551b9db8c4a76a0ab2f148 --- include/got_worktree.h +++ include/got_worktree.h @@ -26,6 +26,7 @@ struct got_fileindex; #define GOT_STATUS_UPDATE 'U' #define GOT_STATUS_DELETE 'D' #define GOT_STATUS_MODIFY 'M' +#define GOT_STATUS_MODE_CHANGE 'm' #define GOT_STATUS_CONFLICT 'C' #define GOT_STATUS_MERGE 'G' #define GOT_STATUS_MISSING '!' blob - 35d6705944c9ba45ca47f53a0109f5cc466e5957 blob + 62fe20f7905b2378140690fcaec185c0abae22d3 --- lib/worktree.c +++ lib/worktree.c @@ -911,6 +911,23 @@ update_blob_fileindex_entry(struct got_worktree *workt return err; } +static mode_t +get_ondisk_perms(int executable, mode_t st_mode) +{ + mode_t xbits = S_IXUSR; + + if (executable) { + /* Map read bits to execute bits. */ + if (st_mode & S_IRGRP) + xbits |= S_IXGRP; + if (st_mode & S_IROTH) + xbits |= S_IXOTH; + return st_mode | xbits; + } + + return (st_mode & ~(S_IXUSR | S_IXGRP | S_IXOTH)); +} + static const struct got_error * install_blob(struct got_worktree *worktree, const char *ondisk_path, const char *path, uint16_t te_mode, uint16_t st_mode, @@ -1000,16 +1017,10 @@ install_blob(struct got_worktree *worktree, const char } } - if (te_mode & S_IXUSR) { - if (chmod(ondisk_path, st_mode | S_IXUSR) == -1) { - err = got_error_from_errno2("chmod", ondisk_path); - goto done; - } - } else { - if (chmod(ondisk_path, st_mode & ~S_IXUSR) == -1) { - err = got_error_from_errno2("chmod", ondisk_path); - goto done; - } + if (chmod(ondisk_path, + get_ondisk_perms(te_mode & S_IXUSR, st_mode)) == -1) { + err = got_error_from_errno2("chmod", ondisk_path); + goto done; } done: @@ -1056,13 +1067,21 @@ get_modified_file_content_status(unsigned char *status } static int +xbit_differs(struct got_fileindex_entry *ie, uint16_t st_mode) +{ + mode_t ie_mode = got_fileindex_perms_to_st(ie); + return ((ie_mode & S_IXUSR) != (st_mode & S_IXUSR)); +} + +static int stat_info_differs(struct got_fileindex_entry *ie, struct stat *sb) { return !(ie->ctime_sec == sb->st_ctime && ie->ctime_nsec == sb->st_ctimensec && ie->mtime_sec == sb->st_mtime && ie->mtime_nsec == sb->st_mtimensec && - ie->size == (sb->st_size & 0xffffffff)); + ie->size == (sb->st_size & 0xffffffff) && + !xbit_differs(ie, sb->st_mode)); } static unsigned char @@ -1175,7 +1194,8 @@ get_file_status(unsigned char *status, struct stat *sb if (*status == GOT_STATUS_MODIFY) { rewind(f); err = get_modified_file_content_status(status, f); - } + } else if (xbit_differs(ie, sb->st_mode)) + *status = GOT_STATUS_MODE_CHANGE; done: if (blob) got_object_blob_close(blob); @@ -1282,6 +1302,9 @@ update_blob(struct got_worktree *worktree, err = got_fileindex_entry_update(ie, ondisk_path, blob->id.sha1, worktree->base_commit_id->sha1, update_timestamps); + } else if (status == GOT_STATUS_MODE_CHANGE) { + err = got_fileindex_entry_update(ie, ondisk_path, + blob->id.sha1, worktree->base_commit_id->sha1, 0); } else if (status == GOT_STATUS_DELETE) { err = (*progress_cb)(progress_arg, GOT_STATUS_MERGE, path); if (err) @@ -3078,6 +3101,7 @@ create_patched_content(char **path_outfile, int revers const struct got_error *err; struct got_blob_object *blob = NULL; FILE *f1 = NULL, *f2 = NULL, *outfile = NULL; + int fd2 = -1; char *path1 = NULL, *id_str = NULL; struct stat sb1, sb2; struct got_diff_changes *changes = NULL; @@ -3093,11 +3117,22 @@ create_patched_content(char **path_outfile, int revers if (err) return err; - f2 = fopen(path2, "r"); + fd2 = open(path2, O_RDONLY | O_NOFOLLOW); + if (fd2 == -1) { + err = got_error_from_errno2("open", path2); + goto done; + } + if (fstat(fd2, &sb2) == -1) { + err = got_error_from_errno2("fstat", path2); + goto done; + } + + f2 = fdopen(fd2, "r"); if (f2 == NULL) { err = got_error_from_errno2("fopen", path2); goto done; } + fd2 = -1; err = got_object_open_as_blob(&blob, repo, blob_id, 8192); if (err) @@ -3115,10 +3150,6 @@ create_patched_content(char **path_outfile, int revers err = got_error_from_errno2("stat", path1); goto done; } - if (stat(path2, &sb2) == -1) { - err = got_error_from_errno2("stat", path2); - goto done; - } err = got_diff_files(&changes, &ds, &args, &diff_flags, f1, sb1.st_size, id_str, f2, sb2.st_size, path2, 3, NULL); @@ -3148,10 +3179,18 @@ create_patched_content(char **path_outfile, int revers else if (choice == GOT_PATCH_CHOICE_QUIT) break; } - if (have_content) + if (have_content) { err = copy_remaining_content(f1, f2, &line_cur1, &line_cur2, reverse_patch ? NULL : outfile, reverse_patch ? outfile : NULL); + if (err) + goto done; + + if (chmod(*path_outfile, sb2.st_mode) == -1) { + err = got_error_from_errno2("chmod", path2); + goto done; + } + } done: free(id_str); if (blob) @@ -3160,6 +3199,8 @@ done: err = got_error_from_errno2("fclose", path1); if (f2 && fclose(f2) == EOF && err == NULL) err = got_error_from_errno2("fclose", path2); + if (fd2 != -1 && close(fd2) == -1 && err == NULL) + err = got_error_from_errno2("close", path2); if (outfile && fclose(outfile) == EOF && err == NULL) err = got_error_from_errno2("fclose", *path_outfile); if (path1 && unlink(path1) == -1 && err == NULL) @@ -3298,6 +3339,7 @@ revert_file(void *arg, unsigned char status, unsigned } /* fall through */ case GOT_STATUS_MODIFY: + case GOT_STATUS_MODE_CHANGE: case GOT_STATUS_CONFLICT: case GOT_STATUS_MISSING: { struct got_object_id id; @@ -3337,7 +3379,8 @@ revert_file(void *arg, unsigned char status, unsigned a->repo, a->progress_cb, a->progress_arg); if (err) goto done; - if (status == GOT_STATUS_DELETE) { + if (status == GOT_STATUS_DELETE || + status == GOT_STATUS_MODE_CHANGE) { err = update_blob_fileindex_entry(a->worktree, a->fileindex, ie, ondisk_path, ie->path, blob, 1); @@ -3454,6 +3497,7 @@ collect_commitables(void *arg, unsigned char status, return got_error(GOT_ERR_COMMIT_CONFLICT); if (status != GOT_STATUS_MODIFY && + status != GOT_STATUS_MODE_CHANGE && status != GOT_STATUS_ADD && status != GOT_STATUS_DELETE) return NULL; @@ -3748,6 +3792,7 @@ match_deleted_or_modified_ct(struct got_commitable **c if (ct->staged_status == GOT_STATUS_NO_CHANGE) { if (ct->status != GOT_STATUS_MODIFY && + ct->status != GOT_STATUS_MODE_CHANGE && ct->status != GOT_STATUS_DELETE) continue; } else { @@ -3935,6 +3980,7 @@ write_tree(struct got_object_id **new_tree_id, if (ct) { /* NB: Deleted entries get dropped here. */ if (ct->status == GOT_STATUS_MODIFY || + ct->status == GOT_STATUS_MODE_CHANGE || ct->staged_status == GOT_STATUS_MODIFY) { err = alloc_modified_blob_tree_entry( &new_te, te, ct); @@ -4113,7 +4159,8 @@ commit_worktree(struct got_object_id **new_commit_id, continue; if (ct->status != GOT_STATUS_ADD && - ct->status != GOT_STATUS_MODIFY) + ct->status != GOT_STATUS_MODIFY && + ct->status != GOT_STATUS_MODE_CHANGE) continue; if (asprintf(&ondisk_path, "%s/%s", blob - 55340c74b234a5573505c25f1137cc947e5d43d5 blob + fbf48ee5570790ff88f8e64e0c4f1a1aab82289f --- regress/cmdline/commit.sh +++ regress/cmdline/commit.sh @@ -645,6 +645,104 @@ function test_commit_gitconfig_author { test_done "$testroot" "$ret" } +function test_commit_xbit_change { + local testroot=`test_init commit_xbit_change` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret="$?" + if [ "$ret" != "0" ]; then + test_done "$testroot" "$ret" + return 1 + fi + + chmod +x $testroot/wt/alpha + + echo 'm alpha' > $testroot/stdout.expected + (cd $testroot/wt && got status > $testroot/stdout) + + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got commit -mx > $testroot/stdout) + ret="$?" + if [ "$ret" != "0" ]; then + echo "got commit failed unexpectedly" + test_done "$testroot" "$ret" + return 1 + fi + + local commit_id=`git_show_head $testroot/repo` + echo 'm alpha' > $testroot/stdout.expected + echo "Created commit $commit_id" >> $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got status > $testroot/stdout) + + echo -n > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + chmod -x $testroot/wt/alpha + + echo 'm alpha' > $testroot/stdout.expected + (cd $testroot/wt && got status > $testroot/stdout) + + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got commit -mx > $testroot/stdout) + ret="$?" + if [ "$ret" != "0" ]; then + echo "got commit failed unexpectedly" + test_done "$testroot" "$ret" + return 1 + fi + + local commit_id=`git_show_head $testroot/repo` + echo 'm alpha' > $testroot/stdout.expected + echo "Created commit $commit_id" >> $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + chmod +x $testroot/wt/alpha + + echo 'm alpha' > $testroot/stdout.expected + (cd $testroot/wt && got status > $testroot/stdout) + + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi + test_done "$testroot" "$ret" +} + run_test test_commit_basic run_test test_commit_new_subdir run_test test_commit_subdir @@ -661,3 +759,4 @@ run_test test_commit_outside_refs_heads run_test test_commit_no_email run_test test_commit_tree_entry_sorting run_test test_commit_gitconfig_author +run_test test_commit_xbit_change