commit executable bit change

On Wed, Oct 16, 2019 at 02:05:16PM +0200, Stefan Sperling wrote: > On Tue, Oct 15, 2019 at 05:06:08PM +0200, Stefan Sperling wrote: > > 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. > > Here is a follow-up diff which applies on top of the previous diff. > > With this, chmod +x/-x changes are properly handled during 'got update'. And here is a patch to make 'got diff' and 'tog diff' show file mode changes. I think the basics are complete with this. There are still parts that could be improved. E.g. unlike 'got status', 'got diff' will not show chmod +x/-x for files in the work tree. And it is impossible to stage file mode changes with 'got stage' but that seems like an acceptable restriction. Implementing support for this would add more code than it is worth, I think. I am still looking for reviews of all these 3 xbit-related diffs ;-) diff f57d86d87012b20b13d9ea2269f28a0afd072dcd ae3c4c0c3c15546e64d83ff0dfe53da4bcc83a63 blob - 5423c33764d81bfc3fd2c71dc7f9ca32f22bad4a blob + 15002586c02db1d645ee8138c70c4fa191c0db37 --- got/got.c +++ got/got.c @@ -5024,7 +5024,7 @@ static const struct got_error * check_path_prefix_in_diff(void *arg, struct got_blob_object *blob1, struct got_blob_object *blob2, struct got_object_id *id1, struct got_object_id *id2, const char *path1, const char *path2, - struct got_repository *repo) + mode_t mode1, mode_t mode2, struct got_repository *repo) { struct check_path_prefix_arg *a = arg; blob - 2cbdb362bbc46817c1d3e664247b978b73e7f02c blob + d7f3479403b5d8286740e682f101b99f5f2782b4 --- include/got_diff.h +++ include/got_diff.h @@ -44,11 +44,13 @@ const struct got_error *got_diff_blob_file(struct got_ * the second blob contains content on the new side of the diff. * The set of arguments relating to either blob may be NULL to indicate * that no content is present on its respective side of the diff. + * File modes from relevant tree objects which contain the blobs may + * also be passed. These will be zero if not available. */ typedef const struct got_error *(*got_diff_blob_cb)(void *, struct got_blob_object *, struct got_blob_object *, struct got_object_id *, struct got_object_id *, - const char *, const char *, struct got_repository *); + const char *, const char *, mode_t, mode_t, struct got_repository *); /* * A pre-defined implementation of got_diff_blob_cb() which appends unidiff @@ -63,7 +65,7 @@ struct got_diff_blob_output_unidiff_arg { const struct got_error *got_diff_blob_output_unidiff(void *, struct got_blob_object *, struct got_blob_object *, struct got_object_id *, struct got_object_id *, - const char *, const char *, struct got_repository *); + const char *, const char *, mode_t, mode_t, struct got_repository *); /* * Compute the differences between two trees and invoke the provided blob - c100c664ee6266b7d278ba0176da2648da0f1053 blob + c98502c82d7d8b1ed38dfe6ab2fbf0b1fbb5d095 --- lib/diff.c +++ lib/diff.c @@ -38,8 +38,9 @@ static const struct got_error * diff_blobs(struct got_blob_object *blob1, struct got_blob_object *blob2, - const char *label1, const char *label2, int diff_context, - int ignore_whitespace, FILE *outfile, struct got_diff_changes *changes) + const char *label1, const char *label2, mode_t mode1, mode_t mode2, + int diff_context, int ignore_whitespace, FILE *outfile, + struct got_diff_changes *changes) { struct got_diff_state ds; struct got_diff_args args; @@ -110,8 +111,27 @@ diff_blobs(struct got_blob_object *blob1, struct got_b flags |= D_IGNOREBLANKS; if (outfile) { - fprintf(outfile, "blob - %s\n", idstr1); - fprintf(outfile, "blob + %s\n", idstr2); + char *modestr1 = NULL, *modestr2 = NULL; + if (mode1 && mode1 != mode2) { + if (asprintf(&modestr1, " (mode %o)", + mode1 & (S_IRWXU | S_IRWXG | S_IRWXO)) == -1) { + err = got_error_from_errno("asprintf"); + goto done; + } + } + if (mode2 && mode1 != mode2) { + if (asprintf(&modestr2, " (mode %o)", + mode2 & (S_IRWXU | S_IRWXG | S_IRWXO)) == -1) { + err = got_error_from_errno("asprintf"); + goto done; + } + } + fprintf(outfile, "blob - %s%s\n", idstr1, + modestr1 ? modestr1 : ""); + fprintf(outfile, "blob + %s%s\n", idstr2, + modestr2 ? modestr2 : ""); + free(modestr1); + free(modestr2); } err = got_diffreg(&res, f1, f2, flags, &args, &ds, outfile, changes); got_diff_state_free(&ds); @@ -127,12 +147,12 @@ const struct got_error * got_diff_blob_output_unidiff(void *arg, struct got_blob_object *blob1, struct got_blob_object *blob2, struct got_object_id *id1, struct got_object_id *id2, const char *label1, const char *label2, - struct got_repository *repo) + mode_t mode1, mode_t mode2, struct got_repository *repo) { struct got_diff_blob_output_unidiff_arg *a = arg; - return diff_blobs(blob1, blob2, label1, label2, a->diff_context, - a->ignore_whitespace, a->outfile, NULL); + return diff_blobs(blob1, blob2, label1, label2, mode1, mode2, + a->diff_context, a->ignore_whitespace, a->outfile, NULL); } const struct got_error * @@ -140,7 +160,7 @@ got_diff_blob(struct got_blob_object *blob1, struct go const char *label1, const char *label2, int diff_context, int ignore_whitespace, FILE *outfile) { - return diff_blobs(blob1, blob2, label1, label2, diff_context, + return diff_blobs(blob1, blob2, label1, label2, 0, 0, diff_context, ignore_whitespace, outfile, NULL); } @@ -255,7 +275,7 @@ got_diff_blob_lines_changed(struct got_diff_changes ** if (err) return err; - err = diff_blobs(blob1, blob2, NULL, NULL, 3, 0, NULL, *changes); + err = diff_blobs(blob1, blob2, NULL, NULL, 0, 0, 3, 0, NULL, *changes); if (err) { got_diff_free_changes(*changes); *changes = NULL; @@ -276,7 +296,7 @@ got_diff_free_changes(struct got_diff_changes *changes } static const struct got_error * -diff_added_blob(struct got_object_id *id, const char *label, +diff_added_blob(struct got_object_id *id, const char *label, mode_t mode, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg) { const struct got_error *err; @@ -290,7 +310,7 @@ diff_added_blob(struct got_object_id *id, const char * err = got_object_blob_open(&blob, repo, obj, 8192); if (err) goto done; - err = cb(cb_arg, NULL, blob, NULL, id, NULL, label, repo); + err = cb(cb_arg, NULL, blob, NULL, id, NULL, label, 0, mode, repo); done: got_object_close(obj); if (blob) @@ -300,8 +320,8 @@ done: static const struct got_error * diff_modified_blob(struct got_object_id *id1, struct got_object_id *id2, - const char *label1, const char *label2, struct got_repository *repo, - got_diff_blob_cb cb, void *cb_arg) + const char *label1, const char *label2, mode_t mode1, mode_t mode2, + struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg) { const struct got_error *err; struct got_object *obj1 = NULL; @@ -333,7 +353,8 @@ diff_modified_blob(struct got_object_id *id1, struct g if (err) goto done; - err = cb(cb_arg, blob1, blob2, id1, id2, label1, label2, repo); + err = cb(cb_arg, blob1, blob2, id1, id2, label1, label2, mode1, mode2, + repo); done: if (obj1) got_object_close(obj1); @@ -347,7 +368,7 @@ done: } static const struct got_error * -diff_deleted_blob(struct got_object_id *id, const char *label, +diff_deleted_blob(struct got_object_id *id, const char *label, mode_t mode, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg) { const struct got_error *err; @@ -361,7 +382,7 @@ diff_deleted_blob(struct got_object_id *id, const char err = got_object_blob_open(&blob, repo, obj, 8192); if (err) goto done; - err = cb(cb_arg, blob, NULL, id, NULL, label, NULL, repo); + err = cb(cb_arg, blob, NULL, id, NULL, label, NULL, mode, 0, repo); done: got_object_close(obj); if (blob) @@ -512,11 +533,11 @@ diff_entry_old_new(const struct got_tree_entry *te1, cb, cb_arg, diff_content); else { if (diff_content) - err = diff_deleted_blob(te1->id, label1, repo, - cb, cb_arg); + err = diff_deleted_blob(te1->id, label1, + te1->mode, repo, cb, cb_arg); else err = cb(cb_arg, NULL, NULL, te1->id, NULL, - label1, NULL, repo); + label1, NULL, te1->mode, 0, repo); } return err; } else if (got_object_tree_entry_is_submodule(te2)) @@ -528,13 +549,16 @@ diff_entry_old_new(const struct got_tree_entry *te1, return diff_modified_tree(te1->id, te2->id, label1, label2, repo, cb, cb_arg, diff_content); } else if (S_ISREG(te1->mode) && S_ISREG(te2->mode)) { - if (!id_match) { + if (!id_match || + (te1->mode & S_IXUSR) != (te2->mode & S_IXUSR)) { if (diff_content) return diff_modified_blob(te1->id, te2->id, - label1, label2, repo, cb, cb_arg); + label1, label2, te1->mode, te2->mode, + repo, cb, cb_arg); else return cb(cb_arg, NULL, NULL, te1->id, - te2->id, label1, label2, repo); + te2->id, label1, label2, te1->mode, + te2->mode, repo); } } @@ -562,9 +586,11 @@ diff_entry_new_old(const struct got_tree_entry *te2, diff_content); if (diff_content) - return diff_added_blob(te2->id, label2, repo, cb, cb_arg); + return diff_added_blob(te2->id, label2, te2->mode, repo, cb, + cb_arg); - return cb(cb_arg, NULL, NULL, NULL, te2->id, NULL, label2, repo); + return cb(cb_arg, NULL, NULL, NULL, te2->id, NULL, label2, 0, + te2->mode, repo); } const struct got_error * blob - edb81fa00de43c1b50eba8f7ba30d5959593e344 blob + e289542324a5a24dba52bcccb0d657378875d631 --- lib/worktree.c +++ lib/worktree.c @@ -1986,7 +1986,7 @@ static const struct got_error * merge_file_cb(void *arg, struct got_blob_object *blob1, struct got_blob_object *blob2, struct got_object_id *id1, struct got_object_id *id2, const char *path1, const char *path2, - struct got_repository *repo) + mode_t mode1, mode_t mode2, struct got_repository *repo) { static const struct got_error *err = NULL; struct merge_file_cb_arg *a = arg; blob - 0454b55efa3ced59507448a4ccee518a3a29f74e blob + 31587c10a62466ca183a27d0985a8816a43d7053 --- regress/cmdline/diff.sh +++ regress/cmdline/diff.sh @@ -217,7 +217,8 @@ function test_diff_tag { echo "blob - /dev/null" >> $testroot/stdout.expected echo -n 'blob + ' >> $testroot/stdout.expected got tree -r $testroot/repo -i -c $commit_id2 | grep 'new$' | \ - cut -d' ' -f 1 >> $testroot/stdout.expected + cut -d' ' -f 1 | tr -d '\n' >> $testroot/stdout.expected + echo " (mode 644)" >> $testroot/stdout.expected echo '--- /dev/null' >> $testroot/stdout.expected echo '+++ new' >> $testroot/stdout.expected echo '@@ -0,0 +1 @@' >> $testroot/stdout.expected blob - 42ff36721b7a4349961e76289fca5eca64b2a559 (mode 755) blob + a7b1c1d2ee078473b35a60b0ba9068fdac25b9d7 (mode 744) --- regress/cmdline/import.sh +++ regress/cmdline/import.sh @@ -67,25 +67,25 @@ function test_import_basic { echo " " >> $testroot/stdout.expected echo "diff /dev/null $tree_id" >> $testroot/stdout.expected echo "blob - /dev/null" >> $testroot/stdout.expected - echo "blob + $id_alpha" >> $testroot/stdout.expected + echo "blob + $id_alpha (mode 644)" >> $testroot/stdout.expected echo "--- /dev/null" >> $testroot/stdout.expected echo "+++ alpha" >> $testroot/stdout.expected echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected echo "+alpha" >> $testroot/stdout.expected echo "blob - /dev/null" >> $testroot/stdout.expected - echo "blob + $id_beta" >> $testroot/stdout.expected + echo "blob + $id_beta (mode 644)" >> $testroot/stdout.expected echo "--- /dev/null" >> $testroot/stdout.expected echo "+++ beta" >> $testroot/stdout.expected echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected echo "+beta" >> $testroot/stdout.expected echo "blob - /dev/null" >> $testroot/stdout.expected - echo "blob + $id_zeta" >> $testroot/stdout.expected + echo "blob + $id_zeta (mode 644)" >> $testroot/stdout.expected echo "--- /dev/null" >> $testroot/stdout.expected echo "+++ epsilon/zeta" >> $testroot/stdout.expected echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected echo "+zeta" >> $testroot/stdout.expected echo "blob - /dev/null" >> $testroot/stdout.expected - echo "blob + $id_delta" >> $testroot/stdout.expected + echo "blob + $id_delta (mode 644)" >> $testroot/stdout.expected echo "--- /dev/null" >> $testroot/stdout.expected echo "+++ gamma/delta" >> $testroot/stdout.expected echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected blob - c847136cca4bae60bd72b654eb15593e7739fc09 (mode 755) blob + ba1b5eaaf04bac104befb9a8defef1522ad1de37 (mode 744) --- regress/cmdline/stage.sh +++ regress/cmdline/stage.sh @@ -1359,8 +1359,9 @@ function test_stage_commit { echo '+modified file' >> $testroot/stdout.expected echo -n 'blob - ' >> $testroot/stdout.expected got tree -r $testroot/repo -i -c $first_commit \ - | grep 'beta$' | cut -d' ' -f 1 \ + | grep 'beta$' | cut -d' ' -f 1 | tr -d '\n' \ >> $testroot/stdout.expected + echo " (mode 644)" >> $testroot/stdout.expected echo 'blob + /dev/null' >> $testroot/stdout.expected echo '--- beta' >> $testroot/stdout.expected echo '+++ /dev/null' >> $testroot/stdout.expected @@ -1368,7 +1369,8 @@ function test_stage_commit { echo '-beta' >> $testroot/stdout.expected echo 'blob - /dev/null' >> $testroot/stdout.expected echo -n 'blob + ' >> $testroot/stdout.expected - cat $testroot/blob_id_foo >> $testroot/stdout.expected + cat $testroot/blob_id_foo | tr -d '\n' >> $testroot/stdout.expected + echo " (mode 644)" >> $testroot/stdout.expected echo '--- /dev/null' >> $testroot/stdout.expected echo '+++ foo' >> $testroot/stdout.expected echo '@@ -0,0 +1 @@' >> $testroot/stdout.expected

2019-10-15 15:06 Stefan Sperling:
commit executable bit change
- 2019-10-16 12:05 Stefan Sperling:
  commit executable bit change
- - 2019-10-16 12:09 Stefan Sperling:
    commit executable bit change
  - - 2019-10-19 10:18 Stefan Sperling:
      commit executable bit change
    - - 2019-10-19 10:21 Mark Jamsek:
        commit executable bit change