commit executable bit change

Any comments on these executable bit fixes? Should I just go ahead? On Wed, Oct 16, 2019 at 02:09:39PM +0200, Stefan Sperling wrote: > 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-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
    - - 2019-10-19 11:05 Stefan Sperling:
        commit executable bit change