commit executable bit change

Hi Stefan, They worked ok for me > On 19 Oct 2019, at 9:18 pm, Stefan Sperling <stsp@stsp.name> wrote: > > 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: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