Re: What's cooking in git.git (Jul 2015, #05; Mon, 20)
On Mon, 2015-07-20 at 22:15 -0700, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: * dt/refs-backend-preamble (2015-07-13) 7 commits - git-stash: use update-ref --create-reflog instead of creating files - update-ref and tag: add --create-reflog arg - refs: add REF_FORCE_CREATE_REFLOG flag - git-reflog: add exists command - refs: new public ref function: safe_create_reflog - refs: Break out check for reflog autocreation - refs.c: add err arguments to reflog functions ... Still under discussion. Will hold. What's left to discuss on this one? I think the latest version addresses all concerns, but I'm happy to do another reroll or discuss further if necessary. I think I found the series more or less ready when I saw v7, but there was a good discussion between Michael Haggerty and you on that round after I thought so, and then you posted the reroll (v8) which is what is queued above. As I was not closely following the earlier exchange, I wanted to hear from Michael if everything is now good with v8, which hasn't happened as far as I recall. Thanks. I've incorporated Michael Haggerty's suggestions into v9. Also in $gmane/273828 [*1*] you hinted (but without showing a firm commitment) that there might be a reroll coming, which was another reason why I wasn't in a hurry to merge v8 iteration down to 'next'. The current version does not use the term pseudoref (it is used in the refs backend series, and added to the glossary in those patches). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 6/7] update-ref and tag: add --create-reflog arg
David Turner dtur...@twopensource.com writes: diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 6763cf1..d9646ef 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static int update_flags; +int create_reflog_flag; No need to reroll only for this, but I'll s/^int/static /; while queuing. I may have more comments later; this was found in the first pass. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 0/7] refs backend preamble
This reroll addresses Michael Haggerty's comments: - Error messages are now in the form error: reason - We no longer unnecessarily set errno in write_ref_to_lockfile - Corrected a spelling error in the new docs and another in the tests - Corrected some copypasta in a test. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 7/7] git-stash: use update-ref --create-reflog instead of creating files
This is in support of alternate ref backends which don't necessarily store reflogs as files. Signed-off-by: David Turner dtur...@twopensource.com --- git-stash.sh | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 8e9e2cd..1d5ba7a 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -183,9 +183,7 @@ store_stash () { stash_msg=Created via \git stash store\. fi - # Make sure the reflog for stash is kept. - : $(git rev-parse --git-path logs/$ref_stash) - git update-ref -m $stash_msg $ref_stash $w_commit + git update-ref --create-reflog -m $stash_msg $ref_stash $w_commit ret=$? test $ret != 0 test -z $quiet die $(eval_gettext Cannot update \$ref_stash with \$w_commit) @@ -262,7 +260,7 @@ save_stash () { say $(gettext No local changes to save) exit 0 fi - test -f $(git rev-parse --git-path logs/$ref_stash) || + git reflog exists $ref_stash || clear_stash || die $(gettext Cannot initialize stash) create_stash $stash_msg $untracked -- 2.4.2.586.g889ef79-twtrsrc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 2/7] refs: Break out check for reflog autocreation
This is just for clarity. Signed-off-by: David Turner dtur...@twopensource.com --- refs.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index f090720..2efa2dc 100644 --- a/refs.c +++ b/refs.c @@ -3118,6 +3118,16 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } +static int should_autocreate_reflog(const char *refname) +{ + if (!log_all_ref_updates) + return 0; + return starts_with(refname, refs/heads/) || + starts_with(refname, refs/remotes/) || + starts_with(refname, refs/notes/) || + !strcmp(refname, HEAD); +} + /* This function will fill in *err and return -1 on failure */ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { @@ -3128,11 +3138,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile-buf; /* make sure the rest of the function can't change logfile */ sb_logfile = NULL; - if (log_all_ref_updates - (starts_with(refname, refs/heads/) || -starts_with(refname, refs/remotes/) || -starts_with(refname, refs/notes/) || -!strcmp(refname, HEAD))) { + if (should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) 0) { strbuf_addf(err, unable to create directory for %s: %s, logfile, strerror(errno)); -- 2.4.2.586.g889ef79-twtrsrc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 4/7] git-reflog: add exists command
Theis are necessary because alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-reflog.txt | 4 builtin/reflog.c | 33 - t/t1411-reflog-show.sh | 5 + 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 5e7908e..44c736f 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,6 +23,7 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | refs...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... +'git reflog exists' ref Reference logs, or reflogs, record when the tips of branches and other references were updated in the local repository. Reflogs are @@ -52,6 +53,9 @@ argument must be an _exact_ entry (e.g. `git reflog delete master@{2}`). This subcommand is also typically not used directly by end users. +The exists subcommand checks whether a ref has a reflog. It exits +with zero status if the reflog exists, and non-zero status if it does +not. OPTIONS --- diff --git a/builtin/reflog.c b/builtin/reflog.c index c2eb8ff..7ed0e85 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -13,6 +13,8 @@ static const char reflog_expire_usage[] = git reflog expire [--expire=time] [--expire-unreachable=time] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] refs...; static const char reflog_delete_usage[] = git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] refs...; +static const char reflog_exists_usage[] = +git reflog exists ref; static unsigned long default_reflog_expire; static unsigned long default_reflog_expire_unreachable; @@ -699,12 +701,38 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) return status; } +static int cmd_reflog_exists(int argc, const char **argv, const char *prefix) +{ + int i, start = 0; + + for (i = 1; i argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, --)) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_exists_usage); + else + break; + } + + start = i; + + if (argc - start != 1) + usage(reflog_exists_usage); + + if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL)) + die(invalid ref format: %s, argv[start]); + return !reflog_exists(argv[start]); +} + /* * main reflog */ static const char reflog_usage[] = -git reflog [ show | expire | delete ]; +git reflog [ show | expire | delete | exists ]; int cmd_reflog(int argc, const char **argv, const char *prefix) { @@ -724,5 +752,8 @@ int cmd_reflog(int argc, const char **argv, const char *prefix) if (!strcmp(argv[1], delete)) return cmd_reflog_delete(argc - 1, argv + 1, prefix); + if (!strcmp(argv[1], exists)) + return cmd_reflog_exists(argc - 1, argv + 1, prefix); + return cmd_log_reflog(argc, argv, prefix); } diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh index 6f47c0d..3eb4f10 100755 --- a/t/t1411-reflog-show.sh +++ b/t/t1411-reflog-show.sh @@ -166,4 +166,9 @@ test_expect_success 'git log -g -p shows diffs vs. parents' ' test_cmp expect actual ' +test_expect_success 'reflog exists works' ' + git reflog exists refs/heads/master + ! git reflog exists refs/heads/nonexistent +' + test_done -- 2.4.2.586.g889ef79-twtrsrc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 3/7] refs: new public ref function: safe_create_reflog
The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 14 +- refs.c | 24 refs.h | 2 +- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index beea1eb..3165553 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -620,24 +620,20 @@ static void update_refs_for_switch(const struct checkout_opts *opts, if (opts-new_branch) { if (opts-new_orphan_branch) { if (opts-new_branch_log !log_all_ref_updates) { - int temp; - struct strbuf log_file = STRBUF_INIT; int ret; - const char *ref_name; + char *refname; struct strbuf err = STRBUF_INIT; - ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); - temp = log_all_ref_updates; - log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, log_file, err); - log_all_ref_updates = temp; - strbuf_release(log_file); + refname = mkpathdup(refs/heads/%s, opts-new_orphan_branch); + ret = safe_create_reflog(refname, err, 1); + free(refname); if (ret) { fprintf(stderr, _(Can not do reflog for '%s': %s\n), opts-new_orphan_branch, err.buf); strbuf_release(err); return; } + strbuf_release(err); } } else diff --git a/refs.c b/refs.c index 2efa2dc..3277768 100644 --- a/refs.c +++ b/refs.c @@ -3128,8 +3128,13 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, HEAD); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * Create a reflog for a ref. If force_create = 0, the reflog will + * only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, create it + * regardless of the ref name. Fill in *err and return -1 on failure. + */ +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3138,7 +3143,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile-buf; /* make sure the rest of the function can't change logfile */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) 0) { strbuf_addf(err, unable to create directory for %s: %s, logfile, strerror(errno)); @@ -3173,6 +3178,17 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf return 0; } + +int safe_create_reflog(const char *refname, struct strbuf *err, int force_create) +{ + int ret; + struct strbuf sb = STRBUF_INIT; + + ret = log_ref_setup(refname, sb, err, force_create); + strbuf_release(sb); + return ret; +} + static int log_ref_write_fd(int fd, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *committer, const char *msg) @@ -3209,7 +3225,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, sb_log_file, err); + result = log_ref_setup(refname, sb_log_file, err, 0); if (result) return result; diff --git a/refs.h b/refs.h index debdefc..3b90e16 100644 --- a/refs.h +++ b/refs.h @@ -228,7 +228,7 @@ int pack_refs(unsigned int flags); /* *
[PATCH v9 1/7] refs.c: add err arguments to reflog functions
Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Some error messages are slightly reordered. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 8 ++-- refs.c | 129 ++--- refs.h | 4 +- 3 files changed, 81 insertions(+), 60 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c018ab3..beea1eb 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -624,16 +624,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts, struct strbuf log_file = STRBUF_INIT; int ret; const char *ref_name; + struct strbuf err = STRBUF_INIT; ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); temp = log_all_ref_updates; log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, log_file); + ret = log_ref_setup(ref_name, log_file, err); log_all_ref_updates = temp; strbuf_release(log_file); if (ret) { - fprintf(stderr, _(Can not do reflog for '%s'\n), - opts-new_orphan_branch); + fprintf(stderr, _(Can not do reflog for '%s': %s\n), + opts-new_orphan_branch, err.buf); + strbuf_release(err); return; } } diff --git a/refs.c b/refs.c index fb568d7..f090720 100644 --- a/refs.c +++ b/refs.c @@ -2975,9 +2975,11 @@ static int rename_ref_available(const char *oldname, const char *newname) return ret; } -static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1); +static int write_ref_to_lockfile(struct ref_lock *lock, +const unsigned char *sha1, struct strbuf *err); static int commit_ref_update(struct ref_lock *lock, -const unsigned char *sha1, const char *logmsg); +const unsigned char *sha1, const char *logmsg, +struct strbuf *err); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -3038,9 +3040,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms } hashcpy(lock-old_oid.hash, orig_sha1); - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, logmsg)) { - error(unable to write current sha1 into %s, newrefname); + if (write_ref_to_lockfile(lock, orig_sha1, err) || + commit_ref_update(lock, orig_sha1, logmsg, err)) { + error(unable to write current sha1 into %s: %s, newrefname, err.buf); + strbuf_release(err); goto rollback; } @@ -3056,9 +3059,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms flag = log_all_ref_updates; log_all_ref_updates = 0; - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, NULL)) - error(unable to write current sha1 into %s, oldrefname); + if (write_ref_to_lockfile(lock, orig_sha1, err) || + commit_ref_update(lock, orig_sha1, NULL, err)) { + error(unable to write current sha1 into %s: %s, oldrefname, err.buf); + strbuf_release(err); + } log_all_ref_updates = flag; rollbacklog: @@ -3113,8 +3118,8 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } -/* This function must set a meaningful errno on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile) +/* This function will fill in *err and return -1 on failure */ +int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3129,9 +3134,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
[PATCH v9 6/7] update-ref and tag: add --create-reflog arg
Allow the creation of a ref (e.g. stash) with a reflog already in place. For most refs (e.g. those under refs/heads), this happens automatically, but for others, we need this option. Currently, git does this by pre-creating the reflog, but alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead use git plumbing commands. I also added --create-reflog to git tag, just for completeness. In a moment, we will use this argument to make git stash work with alternate ref backends. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-tag.txt| 5 - Documentation/git-update-ref.txt | 5 - builtin/tag.c| 5 - builtin/update-ref.c | 14 +++--- t/t1400-update-ref.sh| 38 ++ t/t7004-tag.sh | 14 +- 6 files changed, 74 insertions(+), 7 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 034d10d..2312980 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -13,7 +13,7 @@ SYNOPSIS tagname [commit | object] 'git tag' -d tagname... 'git tag' [-n[num]] -l [--contains commit] [--points-at object] - [--column[=options] | --no-column] [pattern...] + [--column[=options] | --no-column] [--create-reflog] [pattern...] [pattern...] 'git tag' -v tagname... @@ -143,6 +143,9 @@ This option is only applicable when listing tags without annotation lines. all, 'whitespace' removes just leading/trailing whitespace lines and 'strip' removes both whitespace and commentary. +--create-reflog:: + Create a reflog for the tag. + tagname:: The name of the tag to create, delete, or describe. The new tag name must pass all checks defined by diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index c8f5ae5..969bfab 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely SYNOPSIS [verse] -'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] ref newvalue [oldvalue] | --stdin [-z]) +'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] [--create-reflog] ref newvalue [oldvalue] | --stdin [-z]) DESCRIPTION --- @@ -67,6 +67,9 @@ performs all modifications together. Specify commands of the form: verify SP ref [SP oldvalue] LF option SP opt LF +With `--create-reflog`, update-ref will create a reflog for each ref +even if one would not ordinarily be created. + Quote fields containing whitespace as if they were strings in C source code; i.e., surrounded by double-quotes and with backslash escapes. Use 40 0 characters or the empty string to specify a zero value. To diff --git a/builtin/tag.c b/builtin/tag.c index 5f6cdc5..a99 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -579,6 +579,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) struct create_tag_options opt; char *cleanup_arg = NULL; int annotate = 0, force = 0, lines = -1; + int create_reflog = 0; int cmdmode = 0; const char *msgfile = NULL, *keyid = NULL; struct msg_arg msg = { 0, STRBUF_INIT }; @@ -605,6 +606,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_STRING('u', local-user, keyid, N_(key-id), N_(use another key to sign the tag)), OPT__FORCE(force, N_(replace the tag if exists)), + OPT_BOOL(0, create-reflog, create_reflog, N_(create_reflog)), OPT_GROUP(N_(Tag listing options)), OPT_COLUMN(0, column, colopts, N_(show tag list in columns)), @@ -733,7 +735,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) transaction = ref_transaction_begin(err); if (!transaction || ref_transaction_update(transaction, ref.buf, object, prev, - 0, NULL, err) || + create_reflog ? REF_FORCE_CREATE_REFLOG : 0, + NULL, err) || ref_transaction_commit(transaction, err)) die(%s, err.buf); ref_transaction_free(transaction); diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 6763cf1..d9646ef 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static int update_flags; +int create_reflog_flag; static const char *msg; /* @@ -200,7 +201,8 @@ static const char *parse_cmd_update(struct ref_transaction *transaction, if (ref_transaction_update(transaction, refname, new_sha1, have_old ?
[PATCH v9 5/7] refs: add REF_FORCE_CREATE_REFLOG flag
Add a flag to allow forcing the creation of a reflog even if the ref name and core.logAllRefUpdates setting would not ordinarily cause ref creation. In a moment, we will use this to add options to git tag and git update-ref to force reflog creation. Signed-off-by: David Turner dtur...@twopensource.com --- refs.c | 34 +- refs.h | 1 + 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/refs.c b/refs.c index 3277768..bdeae8b 100644 --- a/refs.c +++ b/refs.c @@ -63,6 +63,11 @@ static unsigned char refname_disposition[256] = { #define REF_NEEDS_COMMIT 0x20 /* + * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a + * value to ref_update::flags + */ + +/* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is * not legal. It is legal if it is something reasonable to have under @@ -2979,7 +2984,7 @@ static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1, struct strbuf *err); static int commit_ref_update(struct ref_lock *lock, const unsigned char *sha1, const char *logmsg, -struct strbuf *err); +int flags, struct strbuf *err); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -3041,7 +3046,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms hashcpy(lock-old_oid.hash, orig_sha1); if (write_ref_to_lockfile(lock, orig_sha1, err) || - commit_ref_update(lock, orig_sha1, logmsg, err)) { + commit_ref_update(lock, orig_sha1, logmsg, 0, err)) { error(unable to write current sha1 into %s: %s, newrefname, err.buf); strbuf_release(err); goto rollback; @@ -3060,7 +3065,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms flag = log_all_ref_updates; log_all_ref_updates = 0; if (write_ref_to_lockfile(lock, orig_sha1, err) || - commit_ref_update(lock, orig_sha1, NULL, err)) { + commit_ref_update(lock, orig_sha1, NULL, 0, err)) { error(unable to write current sha1 into %s: %s, oldrefname, err.buf); strbuf_release(err); } @@ -3217,7 +3222,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1, static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg, - struct strbuf *sb_log_file, struct strbuf *err) + struct strbuf *sb_log_file, int flags, + struct strbuf *err) { int logfd, result, oflags = O_APPEND | O_WRONLY; char *log_file; @@ -3225,7 +3231,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, sb_log_file, err, 0); + result = log_ref_setup(refname, sb_log_file, err, flags REF_FORCE_CREATE_REFLOG); if (result) return result; @@ -3254,10 +3260,11 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, static int log_ref_write(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg, -struct strbuf *err) +int flags, struct strbuf *err) { struct strbuf sb = STRBUF_INIT; - int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, sb, err); + int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, sb, flags, + err); strbuf_release(sb); return ret; } @@ -3311,12 +3318,12 @@ static int write_ref_to_lockfile(struct ref_lock *lock, */ static int commit_ref_update(struct ref_lock *lock, const unsigned char *sha1, const char *logmsg, -struct strbuf *err) +int flags, struct strbuf *err) { clear_loose_ref_cache(ref_cache); - if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg, err) 0 || + if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg, flags, err) 0 || (strcmp(lock-ref_name, lock-orig_ref_name) -log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg, err) 0)) { +log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg, flags, err) 0)) { char *old_msg = strbuf_detach(err, NULL); strbuf_addf(err, Cannot update the ref '%s': '%s', lock-ref_name, old_msg); @@ -3346,7 +3353,7 @@ static int
Re: [PATCH v3 1/9] ref-filter: add option to align atoms to the left
On Mon, Jul 20, 2015 at 10:59 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Your caller is iterating over the elements in a format string, e.g. 'A %(align:20)%(foo) B %(bar) C', and its caller is iterating over a list of refs, e.g. 'maint', 'master' branches. With that format string, as long as %(foo) does not expand to something that exceeds 20 display places or so, I'd expect literal 'B' for all refs to align, but I do not think this code gives me that; what happens if '%(foo)' happens to be an empty string for 'maint' but is a string, say 'x', for 'master'? Having looked at the caller once again, I have to say that the interface to this function is poorly designed. 'info' might have been a convenient place to keep the formatting state during this loop (e.g. was the previous atom tell us to format this atom in a special way and if so how?), but that state does not belong to the 'info' thing we are getting from our caller. It is something we'd want to clear before we come into the for() loop, and mutate and utilize while in the loop. For example, if the caller ever wants to show the same ref twice by calling this function with the same ref twice, and if the format string ended with %(align:N), you do not want that leftover state to right-pad the first atom in the second invocation. You do have a point, my current implementation won't work with the scenario you just mentioned. Imagine that in the future you might want to affect how things are formatted based on how much we have already output for the ref so far (e.g. limiting the total line length). Where would you implement such a feature and hook it in this loop? I'd imagine that a sensible way to organize and structure the codeflow to support this align and related enhancement we may want to have in the future cleanly would be to teach print_value about the formatting state and share it with this loop. Roughly... void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style) { const char *cp, *sp, *ep; Insert something like this here: struct ref_formatting_state state; memset(state, 0, sizeof(state)); state.quote_style = quote_style; for (cp = format; *cp (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + int parsed_atom; ep = strchr(sp, ')'); if (cp sp) emit(cp, sp); - get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), atomv); + parsed_atom = parse_ref_filter_atom(sp + 2, ep); + get_ref_atom_value(info, parsed_atom, atomv); + assign_formating(info, parsed_atom, atomv); print_value(atomv, quote_style); and replace all of the above with something like this (a separate variable parsed_atom may not be necessary): get_ref_atom_value(state, info, parse_ref_filter_atom(sp + 2, ep), atomv); print_value(state, atomv); Things like %(align:20) are not really atoms in the sense that they are not used as placeholders for attributes that refs being printed have, but they are there solely in order to affect the formating state. Introduce a new field struct atom_value.pseudo_atom to tell print_value() that fact from get_ref_atom_value(), e.g. static void print_value(struct ref_formatting_state *state, struct atom_value *v) { struct strbuf value = STRBUF_INIT; struct strbuf formatted = STRBUF_INIT; if (v-pseudo_atom) return; if (state-pad_to_right) { strbuf_addf(value, %.*s, state-pad_to_right, v-s); state-pad_to_right = 0; } switch (state-quote_style) { case QUOTE_SHELL: sq_quote_buf(formatted, value.buf); break; ... } fputs(formatted.buf, stdout); strbuf_release(value); strbuf_release(formatted); } or something like that. As this print_value() knows everything that happens to a single output line during that loop and is allowed to keep track of what it sees in 'state', this would give a natural and codeflow to add 'limit the total line length' and things like that if desired. This implementation looks good. Will include this, thanks a bunch. We may want to further clean up to update %(color) thing to clarify that it is a pseudo atom. I suspect %(align:20)%(color:blue) would do a wrong thing with the current code, and it would be a reasonable thing to allow both of these interchangeably: %(align:20)%(color:blue)%(refname:short)%(color:reset)
Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
On 2015-07-02 at 00:37, Junio C Hamano wrote: What's cooking in git.git (Jul 2015, #01; Wed, 1) -- * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter - gitweb: fix typo in man page Update gitweb to make it more pleasant to deal with a hierarchical forest of repositories. Any comments from those who use or have their own code in Gitweb? The first one is a simple typo fix (plural - singular), so it can be accepted without problems. Second one, gitweb: if the PATH_INFO is incomplete, use it as a project_filter looks interesting and quite useful. Though it doesn't do much: it allows for handcrafted URL, and provides mechanism to create breadcrumbs. It doesn't use this feature in its output... Well, I think it doesn't: I cannot check it at this moment. What is missing is a support for query parameters path, and not only path info. Though that *might* be postponed for later patch; the path info API is obvious, query params API for this feature isn't. Thought some thought is needed for generating (or not) breadcrumbs if path_info is turned off. The third, gitweb: add a link under the search box to clear a project filter notices a problem... then solves it in strange way. IMVHO a better solution would be to add List all projects URL together with / (or other separator) conditionally, if $project_filter is set. Or have List all projects and add List projects$limit if $project_filter is set. The last two, which form the crux of this patch series, looks like a good idea, though not without a few caveats. I am talking here only about conceptual level, not about how it is coded (which has few issues as well): - I think that non-bare repositories repo/.git should be treated as one directory entry, i.e. gitweb should not create a separate category for repo/. This is admittedly a corner case, but useful for git-instaweb - I think that people would want to be able to configure how many levels of directory hierarchy gets turned into categories. Perhaps only top level should be turned into category? Deep hierarchies means deep categories (usually with very few repositories) with current implementation. - New global configuration variable, or new %features entry? * tr/remerge-diff (2014-11-10) 9 commits - t4213: avoid | in sed regexp - log --remerge-diff: show what the conflict resolution changed - name-hash: allow dir hashing even when !ignore_case - merge-recursive: allow storing conflict hunks in index - merge_diff_mode: fold all merge diff variants into an enum - combine-diff: do not pass revs-dense_combined_merges redundantly - merge-recursive: -Xindex-only to leave worktree unchanged - merge-recursive: internal flag to avoid touching the worktree - merge-recursive: remove dead conditional in update_stages() log -p output learns a new way to let users inspect a merge commit by showing the differences between the automerged result with conflicts the person who recorded the merge would have seen and the final conflict resolution that was recorded in the merge. Waiting for a reroll. ($gmane/256591). Is it something that Atlassian uses as a differentiatior (instead of sending patch upstream): https://developer.atlassian.com/blog/2015/01/a-better-pull-request/ -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] receive-pack: crash when checking with non-exist HEAD
If HEAD of a repository points to a conflict reference, such as: * There exist a reference named 'refs/heads/jx/feature1', but HEAD points to 'refs/heads/jx', or * There exist a reference named 'refs/heads/feature', but HEAD points to 'refs/heads/feature/bad'. When we push to delete a reference for this repo, such as: git push /path/to/bad-head-repo.git :some/good/reference The git-receive-pack process will crash. This is because if HEAD points to a conflict reference, the function `resolve_refdup(HEAD, ...)` does not return a valid reference name, but a null buffer. Later matching the delete reference against the null buffer will cause git-receive-pack crash. Signed-off-by: Jiang Xin worldhello@gmail.com --- I'm not sure this email is well-formed for git-am. Because gmail changed it's auth policy, I can not use git send-email command line to send mail. You may know, in our China, we can not visit google/gmail directly, I must access the outside world use VPN! builtin/receive-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 94d0571..04cb5a1 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -911,7 +911,7 @@ static const char *update(struct command *cmd, struct shallow_info *si) return deletion prohibited; } - if (!strcmp(namespaced_name, head_name)) { + if (head_name !strcmp(namespaced_name, head_name)) { switch (deny_delete_current) { case DENY_IGNORE: break; -- 2.5.0.rc2.34.gfbdeabf.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again
From: Junio C Hamano gits...@pobox.com Philip Oakley philipoak...@iee.org writes: This updates two patches in the series based on Eric Sunshine's comments. Patch 8b updates the commit message to make clear what was going wrong. Patch 10b improves the perl code. Is v2b like saying v3 or something else? Does 8b replaces 8 or updates it (i.e. comes between 8 and 9)? Sorry for the confusion. Yes these are quick replacements, rather than re-rolling the whole series immediately. Junio: would a full re-roll be appropriate at a suitable point? Probably, but I'd like to see people try it out and give positive feedback first. This part of the tree I can give no input or pre- pushout testing at all. It has been tried out on the Msysgit list, and also against the github Pull Request, with responses noted there. I'll do another branch / rebase and PR onto the G4W SDK as well, for an extra chance at getting more replies. Ideally, if part of this mainstream Git, it would get picked up automatically by them (rather than being local 'fixes' endlessly carried forward). Who are the people involved in this part of the system in the past? Does shortlog -n --no-merges contrib/buildsystems compat/vcbuild tell us anything? There has been no activity here on the 'create a visual studio project' aspects in the last few years. Any changes listed in the logs relate to ensuring that the MSVC compiler will run as part of a regular Makefile run (IIUC). The last significant commit was 74cf9bd (engine.pl: Fix a recent breakage of the buildsystem generator, 2010-01-22) Ramsay Jones, so that's five and a half years. Mind you, it's taken me a while to find all the bit rots. Philip Oakley (2): -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2015, #05; Mon, 20)
On 2015-07-21, Junio C Hamano wrote: -- [Stalled] * sg/config-name-only (2015-05-28) 3 commits - completion: use new 'git config' options to reliably list variable names - SQUASH - config: add options to list only variable names git config --list output was hard to parse when values consist of multiple lines. Introduce a way to show only the keys. Errr... isn't it what we have -z / --null for? Adding a single --name-only option may be a better way to go than adding two new options. But that is a good idea anyway. * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter - gitweb: fix typo in man page Update gitweb to make it more pleasant to deal with a hierarchical forest of repositories. Any comments from those who use or have their own code in Gitweb? Sent a reply to wrong (old) thread, sorry. tl;dr - first can be applied unconditionally, others have slight issues. Cgit implements something like this, though with limit (only first directory path), and different UI - it looks useful on https://git.kernel.org/cgit/ -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again
Philip Oakley philipoak...@iee.org writes: ... Ideally, if part of this mainstream Git, it would get picked up automatically by them (rather than being local 'fixes' endlessly carried forward). Actually, that is not ideal, but what I want to avoid. As I do not do Windows, it simply is wrong for me to apply changes that are very likely to affect Windows folks without seeing their support first, which they may end up having to revert on their end and endlessly carry that revert forward. That is why I very much prefer to see changes get there first and then forwarded in my direction once they are happy with them. There has been no activity here on the 'create a visual studio project' aspects in the last few years. Any changes listed in the logs relate to ensuring that the MSVC compiler will run as part of a regular Makefile run (IIUC). The last significant commit was 74cf9bd (engine.pl: Fix a recent breakage of the buildsystem generator, 2010-01-22) Ramsay Jones, so that's five and a half years. I think Ramsay is still around on the list; I do not know if he still does Windows, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
On Thu, Jul 16, 2015 at 3:08 PM, Dave Borowitz dborow...@google.com wrote: On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano gits...@pobox.com wrote: Perhaps something like this? Seems like it should work. Jonathan had suggested there might be some principled reason why send-pack does not respect config options, and suggested passing it in as a flag. But that would be more work, certainly, as it would also have to get passed through git-remote-http somehow. I actually was wondering about exactly the same thing as Jonathan, and that is where my Perhaps came from. I will say, though, as the maintainer of a handful of custom remote helpers, I would prefer a solution that does not involve changing the implementation of those just to pass this configuration through. That is not a controversial part ;-) So my vote would be for send-pack to respect the normal config options. The thing is what should be included in the normal config options. The something like this? patch was deliberately narrow, including only the GPG thing and nothing else. But anticipating that the ref backend would be per repo configuration, and send-pack would want to read from refs (and possibly write back tracking?), we may want to prepare ourselves by reading a bit wider than GPG thing and nothing else, e.g. git_default_config() or something like that. Ah, now I understand the question. I have no opinion other than that we shouldn't let discussion about future features prevent us from fixing this obvious signed push bug :) Should I formally send a patch with your configuration one-liner? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
Dave Borowitz dborow...@google.com writes: Should I formally send a patch with your configuration one-liner? Yeah, the log message, that explains the motivation of the change and the decision to read which part of the configuration, is much more important than the actual patch, so please do so ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2015, #05; Mon, 20)
Jakub Narębski jna...@gmail.com writes: On 2015-07-21, Junio C Hamano wrote: -- [Stalled] * sg/config-name-only (2015-05-28) 3 commits - completion: use new 'git config' options to reliably list variable names - SQUASH - config: add options to list only variable names git config --list output was hard to parse when values consist of multiple lines. Introduce a way to show only the keys. Errr... isn't it what we have -z / --null for? Yes, but try to read that from a bash completion script ;-) Adding a single --name-only option may be a better way to go than adding two new options. But that is a good idea anyway. Yup, I think that is where the discussion ended, IIRC. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 9/9] tag.c: implement '--merged' and '--no-merged' options
On Mon, Jul 20, 2015 at 12:50 AM, Christian Couder christian.cou...@gmail.com wrote: On Sun, Jul 19, 2015 at 12:00 AM, Karthik Nayak karthik@gmail.com wrote: From: Karthik Nayak karthik@gmail.com Using 'ref-filter' APIs implement the '--merged' and '--no-merged' options into 'tag.c'. The '--merged' option lets the user to only list tags merged into the named commit. The '--no-merged' option lets the user to only list tags not merged into the named commit. If no object is provided it assumes HEAD as the object. Add documentation and tests for the same. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- Documentation/git-tag.txt | 10 +- builtin/tag.c | 6 +- t/t7004-tag.sh| 27 +++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 16e396c..74ed157 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -14,7 +14,7 @@ SYNOPSIS 'git tag' -d tagname... 'git tag' [-n[num]] -l [--contains commit] [--points-at object] [--column[=options] | --no-column] [--sort=key] [--format=format] - [pattern...] + [(--merged | --no-merged) [commit]] [pattern...] Maybe [--[no-]merged [commit]] instead of [(--merged | --no-merged) [commit]]. Looks better. will use. 'git tag' -v tagname... DESCRIPTION @@ -169,6 +169,14 @@ This option is only applicable when listing tags without annotation lines. `%09` to `\t` (TAB) and `%0a` to `\n` (LF). The fields are same as those in `git for-each-ref`. +--merged [commit]:: + Only list tags whose tips are reachable from the + specified commit (HEAD if not specified). + +--no-merged [commit]:: + Only list tags whose tips are not reachable from the + specified commit (HEAD if not specified). Here also you could write something like: +--[no-]merged [commit]:: + Only list tags whose tips are reachable, or not reachable + if --no-merged is used, from the specified commit + (HEAD if not specified). CONFIGURATION - diff --git a/builtin/tag.c b/builtin/tag.c index cae113b..0fa1d31 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = { N_(git tag [-a | -s | -u key-id] [-f] [-m msg | -F file] tagname [head]), N_(git tag -d tagname...), N_(git tag -l [-n[num]] [--contains commit] [--points-at object] - \n\t\t[pattern...]), + \n\t\t[--merged [commit]] [--no-merged [commit]] [pattern...]), [--[no-]merged [commit]] here too. Thanks, Christian. Here too, thanks :) -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] userdiff: add support for Fountain documents
Zoë Blade z...@bytenoise.co.uk writes: H, the pattern has too many question marks to make a simple panda brain spin. ^((\\.|((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ).+)$ it can start with a dot, or match something at the beginning, followed by a SP (is a tab allowed there instead of SP, I have to wonder). One problem I noticed immediately: this allows a line that begins with ..., which I learned in http://fountain.io/syntax when I wrote $gmane/274127 is explicitly excluded. A line that begin with a dot followed by a non-dot is a forced scene heading. Now disecting that something (i.e. not a forced scene heading), which is this part ... ((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ... that gives us largely two choices: - It can begin with zero or one int/est/ext and can optionally be followed by a dot, or - It can be one of (i, int), optionally followed by a dot, followed by slash, followed by one of (e, ext), optionally followed by a dot. Second problem. Doesn't the early part of this something pattern allow an empty string to match by having zero int and zero .? With this edit to the test vector (add either one of these two, removing the other, before you run this test twice), you can see that these over-eager matches in action. t/t4018/fountain-scene | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene index 6b3257d..94f0513 100644 --- a/t/t4018/fountain-scene +++ b/t/t4018/fountain-scene @@ -1,4 +1,7 @@ EXT. STREET RIGHT OUTSIDE - DAY + An indented line is WRONG. +...we do not want ellipses. + CHARACTER You didn't say the magic phrase, ChangeMe. Perhaps the pattern is trying to be too clever for its own good. I'd prefer to see us doing simple, stupid and obviously correct. That syntax page says: You can force a Scene Heading by starting the line with a single period. ... Note that only a single leading period followed by an alphanumeric character will force a Scene Heading. This allows the writer to begin Action and Dialogue elements with ellipses ... which would give us the first part, i.e. the line may start with a dot and then an alnum \\.[a-z0-9] And then A line beginning with any of the following, followed by either a dot or a space, is considered a Scene Heading (unless the line is preceded by an exclamation point !). Case insensitive. INT EXT EST INT./EXT INT/EXT I/E which translates to (int|ext|est|int\\.?/ext|i/e)[. ] So taking these all together, something like this? ^((\\.[a-z0-9]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$ I personally prefer to make it slightly lenient to exclude dot instead of forcing US-ASCII view of alnum, i.e. ^((\\.[^.]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$ I'll queue a SQUASH??? on top while waiting for a response. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Question: .idx without .pack causes performance issues?
Hi all, I just wanted to relay an issue we've seen before at my day job (and it just recently cropped up again). When moving users from Git for Windows 1.8.3 to 1.9.5, we found a few users started having operations take an excruciatingly long amount of time. At some point, we traced the issue to a number of .pack files had been deleted (possibly garbage collected?) -- but their associated .idx files were still present. Upon removing the orphaned idx files, we found performance returned to normal. Otherwise, git fsck reported no issues with the repositories. Other users have noted that using git gc would sometimes correct the issue for them, but not always. Anyway, has anyone else experienced this performance degradation? I have some feeling that it's an issue that may be exclusive to Windows (or at least, only slow enough to matter on Windows), but I have no proof, and I've never heard of an issue like this outside work. (One idea that came to mind was even the .idx files were locked, and thus not deleted.) Something tells me deleting the orphaned .idx files isn't the nicest solution, either. Thanks! --Doug P.S. In addition to running the Git for Windows/msysgit builds, we have a handful of users running Git Extensions as well, and also have been seeing an increase in use of Visual Studio 2013 -- which of course has libgit2 integrated. So, I think the chance that any one of these might be using the repo or holding files open is very high. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 4/7] git-reflog: add exists command
On 07/21/2015 08:45 AM, David Turner wrote: Theis are necessary because alternate ref backends might store reflogs s/Theis are/This is/ somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. Signed-off-by: David Turner dtur...@twopensource.com [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/7] refs: new public ref function: safe_create_reflog
On 07/21/2015 08:45 AM, David Turner wrote: The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 14 +- refs.c | 24 refs.h | 2 +- 3 files changed, 26 insertions(+), 14 deletions(-) [...] diff --git a/refs.c b/refs.c index 2efa2dc..3277768 100644 --- a/refs.c +++ b/refs.c @@ -3128,8 +3128,13 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, HEAD); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * Create a reflog for a ref. If force_create = 0, the reflog will + * only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, create it + * regardless of the ref name. Fill in *err and return -1 on failure. + */ +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3138,7 +3143,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile-buf; /* make sure the rest of the function can't change logfile */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) 0) { strbuf_addf(err, unable to create directory for %s: %s, logfile, strerror(errno)); I think usually the strbuf *err parameter is the last one in our APIs. I noticed this when reviewing patch 5/7, where there is a similar flags argument that comes *before* err. Consistency would suggest that force_create come before err. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 6/7] update-ref and tag: add --create-reflog arg
On 07/21/2015 09:42 AM, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 6763cf1..d9646ef 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static int update_flags; +int create_reflog_flag; No need to reroll only for this, but I'll s/^int/static /; while queuing. I may have more comments later; this was found in the first pass. In fact, this variable is used as the flags argument to ref_transaction_create(), so it should be s/^int/static unsigned / Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again
Junio C Hamano gits...@pobox.com writes: Philip Oakley philipoak...@iee.org writes: ... Ideally, if part of this mainstream Git, it would get picked up automatically by them (rather than being local 'fixes' endlessly carried forward). Actually, that is not ideal, but what I want to avoid. As I do not do Windows, it simply is wrong for me to apply changes that are very likely to affect Windows folks without seeing their support first,... Just to clarify this part. I do not do RedHat, Solaris or OSX, either. Also MSVC may not be what GfW folks primarily target. But the thing is that (1) Windows is so much different, and (2) GfW folks are much more qualified to judge platform-specific issues on Windows than I am. Even though I may still need to have a say in the overall structure of the changes to the upstream tree coming from them (e.g. Don't sprinkle #ifdef all over the generic code; instead add a wrapper or two in compat/ to keep the generic code generic is something I may tell them when rejecting a change forwarded to me), I trust them a lot more than I trust myself when it comes to what the changes do in the platform-specific part and how they do it. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] --count feature for git shortlog
Lawrence Siebert wrote: On Fri, Jul 3, 2015 at 10:31 AM, Junio C Hamano gits...@pobox.com wrote: John Keeping j...@keeping.me.uk writes: Or even `git rev-list --count HEAD -- $FILENAME`. Ahh, OK. I didn't know we already had rev-list --count. Then please disregard the suggestion to add the option to log; it still holds true that the option does not belong to shortlog, but I do think how many changes were made to this path statistics driven by a script should use rev-list plumbing, and if it already has --count option, that is perfect ;-) Junio, I think, respectfully, there is still a benefit to adding it as a feature to log, in that more Git users know of and use log than rev-list. I hadn't heard of rev-list before joining this mailing list. That means log --count will get used more. That also means that more eyeballs will hit --count with bug reports and better tests; I've already seen 2-3 suggestions for log --count tests that rev-list --count also doesn't have tests for. I would like to keep working on implementing log --count, sharing code with rev-list where possible so they both are improved, unless you are saying you won't merge. Lawrence, As git-rev-list is (mainly) plumbing for git-log porcelain, I think what you would need to do to add --count support to git log is just parse option, exclude nonsense combinations, and pass down to the revision parsing machinery. HTH -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2015, #05; Mon, 20)
Jakub Narębski jna...@gmail.com writes: * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter - gitweb: fix typo in man page Update gitweb to make it more pleasant to deal with a hierarchical forest of repositories. Any comments from those who use or have their own code in Gitweb? Sent a reply to wrong (old) thread, sorry. tl;dr - first can be applied unconditionally, others have slight issues. Cgit implements something like this, though with limit (only first directory path), and different UI - it looks useful on https://git.kernel.org/cgit/ I'll merge the first one down, and discard the rest without prejudice, then. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question: .idx without .pack causes performance issues?
Doug Kelly dougk@gmail.com writes: I just wanted to relay an issue we've seen before at my day job (and it just recently cropped up again). When moving users from Git for Windows 1.8.3 to 1.9.5, we found a few users started having operations take an excruciatingly long amount of time. At some point, we traced the issue to a number of .pack files had been deleted (possibly garbage collected?) -- but their associated .idx files were still present. Upon removing the orphaned idx files, we found performance returned to normal. Otherwise, git fsck reported no issues with the repositories. Other users have noted that using git gc would sometimes correct the issue for them, but not always. Anyway, has anyone else experienced this performance degradation? I wouldn't be surprised if such a configuration to have leftover .idx files that lack .pack affected performance, but I think you really have to work on getting into such a situation (unless your operating system is very cooperative and tries hard to corrupt your repository, that is ;-), so I wouldn't be surprised if you were the first one to report this. We open the .idx file and try to keep as many of them in-core, without opening corresponding .pack until the data is needed. When we need an object, we learn from an .idx file that a particular pack ought to have a copy of it, and then attempt to open the corresponding .pack file. If this fails, we do protect ourselves from strange repositories with only .idx files by not using that .idx and try to see if the sought-after object exists elsewhere (and if there isn't we say no such object, which is also a correct thing to do). I however do not think that we mark the in-core structure that corresponds to an open .idx file in any way when such a failure happens. If we really cared enough, we could do so, saying we know there is .idx file, but do not bother looking at it again, as we know the corresponding .pack is missing, and that would speed things up a bit, essentially bringing us back to a sane situation without any .idx without corresponding .pack. I do not think it is worth the effort, though. It would be more fruitful to find out how you end up with .idx exists but not corresponding .pack and if that is some systemic failure, see if there is a way to prevent that from happening in the first place. Also, I think it may not be a bad idea to teach gc to remove stale .idx files that do not have corresponding .pack as garbage. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question: .idx without .pack causes performance issues?
Junio C Hamano gits...@pobox.com writes: I however do not think that we mark the in-core structure that corresponds to an open .idx file in any way when such a failure happens. If we really cared enough, we could do so, saying we know there is .idx file, but do not bother looking at it again, as we know the corresponding .pack is missing, and that would speed things up a bit, essentially bringing us back to a sane situation without any .idx without corresponding .pack. I do not think it is worth the effort, though. It would be more fruitful to find out how you end up with .idx exists but not corresponding .pack and if that is some systemic failure, see if there is a way to prevent that from happening in the first place. While I still think that it is more important to prevent such a situation from occurring in the first place, ignoring .idx that lack corresponding .pack should be fairly simple, perhaps like this. Note that if we wanted to do this for real, I think such an .idx file should also be added to the garbage list in the loop in which the second hunk of the following patch appears. sha1_file.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/sha1_file.c b/sha1_file.c index 1cee438..b69298e 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1240,6 +1240,19 @@ static void report_pack_garbage(struct string_list *list) report_helper(list, seen_bits, first, list-nr); } +static int packfile_exists(const char *base, size_t base_len) +{ + struct strbuf path = STRBUF_INIT; + int status; + + strbuf_add(path, base, base_len); + strbuf_addstr(path, .pack); + status = file_exists(path.buf); + + strbuf_release(path); + return status; +} + static void prepare_packed_git_one(char *objdir, int local) { struct strbuf path = STRBUF_INIT; @@ -1281,6 +1294,7 @@ static void prepare_packed_git_one(char *objdir, int local) break; } if (p == NULL + packfile_exists(path.buf, base_len) /* * See if it really is a valid .idx file with * corresponding .pack file that we can map. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/9] ref-filter: add option to match literal pattern
On Mon, Jul 20, 2015 at 11:42 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Jul 20, 2015 at 4:01 AM, Christian Couder christian.cou...@gmail.com wrote: On Mon, Jul 20, 2015 at 8:24 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Sat, Jul 18, 2015 at 3:12 PM, Karthik Nayak karthik@gmail.com wrote: +static int filter_pattern_match(struct ref_filter *filter, const char *refname) +{ + if (!*filter-name_patterns) + return 1; + if (filter-match_as_path) + return match_name_as_path(filter-name_patterns, refname); + return match_pattern(filter-name_patterns, refname); +} @@ -1034,7 +1057,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, - if (*filter-name_patterns !match_name_as_path(filter-name_patterns, refname)) + if (!filter_pattern_match(filter, refname)) return 0; I find it much more difficult to grok the new logic due to '*filter-name_patterns' having moved into the called function and its negation inside the function returning 1 which is then negated (again) upon return here. This sort of twisty logic places a higher cognitive load on the reader. Retaining the original logic makes the code far simpler to understand: if (*filter-name_patterns !filter_pattern_match(filter, refname)) return 0; although it's a bit less nicely encapsulated, so I dunno... I think a comment before filter_pattern_match() and perhaps also one inside it might help. For example something like: /* Return 1 if the refname matches one of the patterns, otherwise 0. */ static int filter_pattern_match(struct ref_filter *filter, const char *refname) { if (!*filter-name_patterns) return 1; /* No pattern always matches */ if (filter-match_as_path) return match_name_as_path(filter-name_patterns, refname); return match_pattern(filter-name_patterns, refname); } Yes, the comments do improve the situation and may be a reasonable compromise... Yes, these comments would help, thanks :D -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question: .idx without .pack causes performance issues?
On Tue, Jul 21, 2015 at 1:57 PM, Junio C Hamano gits...@pobox.com wrote: I wouldn't be surprised if such a configuration to have leftover .idx files that lack .pack affected performance, but I think you really have to work on getting into such a situation (unless your operating system is very cooperative and tries hard to corrupt your repository, that is ;-), so I wouldn't be surprised if you were the first one to report this. I'm inclined to believe Windows isn't helping this situation: seems like something it might do, especially because of how it behaves if one process has a file open. Since I haven't caught a case where these files show up, maybe adding some tweaks to look for it occurring (such as on our Jenkins workers, if it's happening there now) would give us a better indication of the why question. It could even be that it has occurred long ago, and the performance issue is just now observed: our environment has run 1.7.4, 1.8.3, and now 1.9.5 -- so even an unknown bug in a previous version could impact us now. We open the .idx file and try to keep as many of them in-core, without opening corresponding .pack until the data is needed. When we need an object, we learn from an .idx file that a particular pack ought to have a copy of it, and then attempt to open the corresponding .pack file. If this fails, we do protect ourselves from strange repositories with only .idx files by not using that .idx and try to see if the sought-after object exists elsewhere (and if there isn't we say no such object, which is also a correct thing to do). I however do not think that we mark the in-core structure that corresponds to an open .idx file in any way when such a failure happens. If we really cared enough, we could do so, saying we know there is .idx file, but do not bother looking at it again, as we know the corresponding .pack is missing, and that would speed things up a bit, essentially bringing us back to a sane situation without any .idx without corresponding .pack. I think this is where the performance hit occurs on Windows: file stat operations in general are pretty slow, and I know msysgit did some things to emulate as much of the POSIX API as possible -- which isn't always easy on Windows. But, some of the developers that know compat/win32/ better would know more (I recall the dirent stuff being pretty complicated, but open/fopen seem rather straightforward). And yes -- retrying the operation each time and failing only compounds the issue. I do not think it is worth the effort, though. It would be more fruitful to find out how you end up with .idx exists but not corresponding .pack and if that is some systemic failure, see if there is a way to prevent that from happening in the first place. Agreed. It feels like a workaround for a case where you're already in a bad state... Also, I think it may not be a bad idea to teach gc to remove stale .idx files that do not have corresponding .pack as garbage. I agree. This seems like a more correct solution -- if gc understands to clean up these bad .idx files, it would then be a non-issue when searching the packs. The solution you posted to check if an associated packfile exists -- while perhaps not belonging there -- could still be useful to delete orphanend .idx files. I think you're correct, though -- if you did propose the solution to sha1_file.c, it would be necessary to prevent scanning that .idx again, or else any potential gains would be lost continually stat()'ing the file. Now, msysgit does have core.fscache to try caching the stat()/lstat() results to lessen the impact, but this isn't on by default, I believe. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/7] refs.c: add err arguments to reflog functions
There's one last error formatting niggle below. On 07/21/2015 08:44 AM, David Turner wrote: Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Some error messages are slightly reordered. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 8 ++-- refs.c | 129 ++--- refs.h | 4 +- 3 files changed, 81 insertions(+), 60 deletions(-) [...] diff --git a/refs.c b/refs.c index fb568d7..f090720 100644 --- a/refs.c +++ b/refs.c [...] @@ -3288,12 +3288,17 @@ static int write_ref_to_lockfile(struct ref_lock *lock, * necessary, using the specified lockmsg (which can be NULL). */ static int commit_ref_update(struct ref_lock *lock, - const unsigned char *sha1, const char *logmsg) + const unsigned char *sha1, const char *logmsg, + struct strbuf *err) { clear_loose_ref_cache(ref_cache); - if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg) 0 || + if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg, err) 0 || (strcmp(lock-ref_name, lock-orig_ref_name) - log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg) 0)) { + log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg, err) 0)) { + char *old_msg = strbuf_detach(err, NULL); + strbuf_addf(err, Cannot update the ref '%s': '%s', + lock-ref_name, old_msg); The above error message has unnecessary quotes around old_msg. + free(old_msg); unlock_ref(lock); return -1; } [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v2.5.0-rc3
A release candidate Git v2.5.0-rc3 is now available for testing at the usual places. It is comprised of 579 non-merge commits since v2.4.0, contributed by 70 people, 21 of which are new faces. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The following public repositories all have a copy of the 'v2.5.0-rc3' tag and the 'master' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git New contributors whose contributions weren't in v2.4.0 are as follows. Welcome to the Git development community! Allen Hubbe, Ariel Faigon, Blair Holloway, Christian Neukirchen, Danny Lin, Enrique Tobis, Frans Klaver, Fredrik Medley, Joe Cridge, Lars Kellogg-Stedman, Lawrence Siebert, Lex Spoon, Luke Mewburn, Miguel Torroja, Mike Edgar, Ossi Herrala, Panagiotis Astithas, Quentin Neill, Remi Lespinet, Sébastien Guimmara, and Thomas Schneider. Returning contributors who helped this release are as follows. Thanks for your continued support. Alexander Shopov, Alex Henrie, brian m. carlson, Carlos Martín Nieto, Charles Bailey, Clemens Buchacher, David Aguilar, David Turner, Dennis Kaarsemaker, Dimitriy Ryazantcev, Elia Pinto, Eric Sunshine, Fredrik Gustafsson, Jean-Noel Avila, Jeff King, Jiang Xin, Jim Hill, Johannes Schindelin, Johannes Sixt, Jonathan Nieder, Junio C Hamano, Karsten Blees, Karthik Nayak, Luke Diamand, Matthieu Moy, Max Kirillov, Michael Coleman, Michael Haggerty, Michael J Gruber, Mike Hommey, Nguyễn Thái Ngọc Duy, Patrick Steinhardt, Paul Tan, Peter Krefting, Phil Hord, Phillip Sz, Ralf Thielow, Ramsay Allan Jones, René Scharfe, Richard Hansen, Sebastian Schuberth, Stefan Beller, SZEDER Gábor, Thomas Braun, Thomas Gummerer, Tony Finch, Torsten Bögershausen, Trần Ngọc Quân, and Vitor Antunes. Git 2.5 Release Notes (draft) = Updates since v2.4 -- UI, Workflows Features * The bash completion script (in contrib/) learned a few options that git revert takes. * Whitespace breakages in deleted and context lines can also be painted in the output of git diff and friends with the new --ws-error-highlight option. * List of commands shown by git help are grouped along the workflow elements to help early learners. * git p4 now detects the filetype (e.g. binary) correctly even when the files are opened exclusively. * git p4 attempts to better handle branches in Perforce. * git p4 learned --changes-block-size n to read the changes in chunks from Perforce, instead of making one call to p4 changes that may trigger too many rows scanned error from Perforce. * More workaround for Perforce's row number limit in git p4. * Unlike $EDITOR and $GIT_EDITOR that can hold the path to the command and initial options (e.g. /path/to/emacs -nw), 'git p4' did not let the shell interpolate the contents of the environment variable that name the editor $P4EDITOR (and $EDITOR, too). This release makes it in line with the rest of Git, as well as with Perforce. * A new short-hand branch@{push} denotes the remote-tracking branch that tracks the branch at the remote the branch would be pushed to. * git show-branch --topics HEAD (with no other arguments) did not do anything interesting. Instead, contrast the given revision against all the local branches by default. * A replacement for contrib/workdir/git-new-workdir that does not rely on symbolic links and make sharing of objects and refs safer by making the borrowee and borrowers aware of each other. Consider this as still an experimental feature; its UI is still likely to change. * Tweak the sample store backend of the credential helper to honor XDG configuration file locations when specified. * A heuristic we use to catch mistyped paths on the command line git cmd revs pathspec is to make sure that all the non-rev parameters in the later part of the command line are names of the files in the working tree, but that means git grep $str -- \*.c must always be disambiguated with --, because nobody sane will create a file whose name literally is asterisk-dot-see. Loosen the heuristic to declare that with a wildcard string the user likely meant to give us a pathspec. * git merge FETCH_HEAD learned that the previous git fetch could be to create an Octopus merge, i.e. recording multiple branches that are not marked as not-for-merge; this allows us to lose an old style invocation git merge msg HEAD $commits... in the implementation of git pull script; the old style syntax can now be deprecated (but not removed
What's cooking in git.git (Jul 2015, #06; Tue, 21)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Tagged v2.5-rc3 today; hopefully we can have an uneventful 2.5 next week and then start the next cycle. I'll eject/drop the stalled topics and ask people to reroll/rebase if their topics are still viable. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Stalled] * jc/clone-bundle (2015-04-30) 1 commit - repack: optionally create a clone.bundle Waiting for further work. Still an early WIP. * jh/strbuf-read-use-read-in-full (2015-06-01) 1 commit - strbuf_read(): skip unnecessary strbuf_grow() at eof Avoid one extra iteration and strbuf_grow() of 8kB in strbuf_read(). Looked reasonable; perhaps a log message clarification is needed. Expecting a reroll. * mg/index-read-error-messages (2015-06-01) 2 commits - messages: uniform error messages for index write - show-index: uniform error messages for index read The tip was RFC. Expecting a reroll. * hv/submodule-config (2015-06-15) 4 commits - do not die on error of parsing fetchrecursesubmodules option - use new config API for worktree configurations of submodules - extract functions for submodule config set and lookup - implement submodule config API for lookup of .gitmodules values The gitmodules API accessed from the C code learned to cache stuff lazily. Needs another reroll? ($gmane/273743). * jk/log-missing-default-HEAD (2015-06-03) 1 commit - log: diagnose empty HEAD more clearly git init empty git -C empty log said bad default revision 'HEAD', which was found to be a bit confusing to new users. What's the status of this one? * bc/object-id (2015-06-17) 10 commits . remote.c: use struct object_id in many functions . object-id: use struct object_id in struct object . remote.c: use struct object_id in ref_newer() . transport-helper.c: use struct object_id in push_refs_with_export() . connect.c: use struct object_id in get_remote_heads() . remote-curl: use struct object_id in parse_fetch() . fetch-pack: use struct object_id in add_sought_entry_mem() . object_id: convert struct ref to use object_id. . sha1_file: introduce has_object_file() helper . refs: convert some internal functions to use object_id More transition from unsigned char[40] to struct object_id. While GSoC and other topics are actively moving existing code around, this cannot go in; ejected from 'pu'. * wp/sha1-name-negative-match (2015-06-08) 2 commits - sha1_name.c: introduce '^{/!-negative pattern}' notation - test for '!' handling in rev-parse's named commits Introduce branch^{/!-pattern} notation to name a commit reachable from branch that does not match the given pattern. Expecting a reroll. * mk/utf8-no-iconv-warn (2015-06-08) 1 commit - utf8.c: print warning about disabled iconv Warn when a reencoding is requested in a build without iconv support, as the end user is likely to get an unexpected result. I think the same level of safety should be added to a build with iconv support when the specified encoding is not available, but the patch does not go there. Expecting a reroll. * ak/format-patch-odir-config (2015-06-19) 1 commit - format-patch: introduce format.outputDirectory configuration Reroll exists but didn't pick it up as it seemed to be still collecting review comments. Expecting a reroll. ($gmane/272180). * mh/tempfile (2015-06-10) 14 commits - credential-cache--daemon: use tempfile module - credential-cache--daemon: delete socket from main() - gc: use tempfile module to handle gc.pid file - lock_repo_for_gc(): compute the path to gc.pid only once - diff: use tempfile module - setup_temporary_shallow(): use tempfile module - write_shared_index(): use tempfile module - register_tempfile(): new function to handle an existing temporary file - tempfile: add several functions for creating temporary files - register_tempfile_object(): new function, extracted from create_tempfile() - commit_lock_file(): use get_locked_file_path() - lockfile: remove some redundant functions - tempfile: a new module for handling temporary files - Move lockfile API documentation to lockfile.h Rebuild lockfile API on top of a new tempfile API. This needs rerolling, to include tempfile.h in lockfile.h, at least. Expecting a reroll. ($gmane/271353) * ad/bisect-terms (2015-06-29) 4 commits - bisect: allow setting any user-specified in 'git bisect start' - bisect: add 'git bisect terms' to view the current terms - bisect: add the terms old/new - bisect: sanity check on terms (this branch uses ad/bisect-cleanup.) The use of 'good/bad' in git bisect made it confusing to use when hunting for a state change that is not a regression (e.g. bugfix). The command learned 'old/new'
Re: [PATCH v9 6/7] update-ref and tag: add --create-reflog arg
On 07/21/2015 01:02 PM, Michael Haggerty wrote: On 07/21/2015 09:42 AM, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 6763cf1..d9646ef 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static int update_flags; +int create_reflog_flag; No need to reroll only for this, but I'll s/^int/static /; while queuing. I may have more comments later; this was found in the first pass. In fact, this variable is used as the flags argument to ref_transaction_create(), so it should be s/^int/static unsigned / Actually, the same comment applies to static int update_flags, which I know was already wrong. Obviously neither of these are terribly critical. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 0/7] refs backend preamble
On 07/21/2015 08:44 AM, David Turner wrote: This reroll addresses Michael Haggerty's comments: - Error messages are now in the form error: reason - We no longer unnecessarily set errno in write_ref_to_lockfile - Corrected a spelling error in the new docs and another in the tests - Corrected some copypasta in a test. Isn't it sobering that, every time one looks at source code, one finds things that could be improved? (Not just yours but all source code!) Happily it seems like the items are getting ever more trivial, and either with or without the last round of suggestions I think the code is correct and is a nice step forward. Reviewed-by: Michael Haggerty mhag...@alum.mit.edu Off topic: I wonder whether sooner or later we will need a git reflog remove ref to delete an existing reference's reflog entirely without deleting the reference and without assuming a filesystem backend. (Note that this is different than `git reflog expire`, which leaves the empty reflog file around and thus doesn't undo a --create-reflog.) But perhaps there is no call for that operation. In a pinch a user could delete the reference and recreate it to get rid of its reflog. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 5/7] refs: add REF_FORCE_CREATE_REFLOG flag
Add a flag to allow forcing the creation of a reflog even if the ref name and core.logAllRefUpdates setting would not ordinarily cause ref creation. In a moment, we will use this to add options to git tag and git update-ref to force reflog creation. Signed-off-by: David Turner dtur...@twopensource.com --- refs.c | 34 +- refs.h | 1 + 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/refs.c b/refs.c index 1abfe64..cd207c2 100644 --- a/refs.c +++ b/refs.c @@ -63,6 +63,11 @@ static unsigned char refname_disposition[256] = { #define REF_NEEDS_COMMIT 0x20 /* + * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a + * value to ref_update::flags + */ + +/* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is * not legal. It is legal if it is something reasonable to have under @@ -2979,7 +2984,7 @@ static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1, struct strbuf *err); static int commit_ref_update(struct ref_lock *lock, const unsigned char *sha1, const char *logmsg, -struct strbuf *err); +int flags, struct strbuf *err); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -3041,7 +3046,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms hashcpy(lock-old_oid.hash, orig_sha1); if (write_ref_to_lockfile(lock, orig_sha1, err) || - commit_ref_update(lock, orig_sha1, logmsg, err)) { + commit_ref_update(lock, orig_sha1, logmsg, 0, err)) { error(unable to write current sha1 into %s: %s, newrefname, err.buf); strbuf_release(err); goto rollback; @@ -3060,7 +3065,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms flag = log_all_ref_updates; log_all_ref_updates = 0; if (write_ref_to_lockfile(lock, orig_sha1, err) || - commit_ref_update(lock, orig_sha1, NULL, err)) { + commit_ref_update(lock, orig_sha1, NULL, 0, err)) { error(unable to write current sha1 into %s: %s, oldrefname, err.buf); strbuf_release(err); } @@ -3217,7 +3222,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1, static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg, - struct strbuf *sb_log_file, struct strbuf *err) + struct strbuf *sb_log_file, int flags, + struct strbuf *err) { int logfd, result, oflags = O_APPEND | O_WRONLY; char *log_file; @@ -3225,7 +3231,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, sb_log_file, err, 0); + result = log_ref_setup(refname, sb_log_file, err, flags REF_FORCE_CREATE_REFLOG); if (result) return result; @@ -3254,10 +3260,11 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, static int log_ref_write(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg, -struct strbuf *err) +int flags, struct strbuf *err) { struct strbuf sb = STRBUF_INIT; - int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, sb, err); + int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, sb, flags, + err); strbuf_release(sb); return ret; } @@ -3311,12 +3318,12 @@ static int write_ref_to_lockfile(struct ref_lock *lock, */ static int commit_ref_update(struct ref_lock *lock, const unsigned char *sha1, const char *logmsg, -struct strbuf *err) +int flags, struct strbuf *err) { clear_loose_ref_cache(ref_cache); - if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg, err) 0 || + if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg, flags, err) 0 || (strcmp(lock-ref_name, lock-orig_ref_name) -log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg, err) 0)) { +log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg, flags, err) 0)) { char *old_msg = strbuf_detach(err, NULL); strbuf_addf(err, Cannot update the ref '%s': %s, lock-ref_name, old_msg); @@ -3346,7 +3353,7 @@ static int
[PATCH v10 2/7] refs: Break out check for reflog autocreation
This is just for clarity. Signed-off-by: David Turner dtur...@twopensource.com --- refs.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index ca68509..cf1abeb 100644 --- a/refs.c +++ b/refs.c @@ -3118,6 +3118,16 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } +static int should_autocreate_reflog(const char *refname) +{ + if (!log_all_ref_updates) + return 0; + return starts_with(refname, refs/heads/) || + starts_with(refname, refs/remotes/) || + starts_with(refname, refs/notes/) || + !strcmp(refname, HEAD); +} + /* This function will fill in *err and return -1 on failure */ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { @@ -3128,11 +3138,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile-buf; /* make sure the rest of the function can't change logfile */ sb_logfile = NULL; - if (log_all_ref_updates - (starts_with(refname, refs/heads/) || -starts_with(refname, refs/remotes/) || -starts_with(refname, refs/notes/) || -!strcmp(refname, HEAD))) { + if (should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) 0) { strbuf_addf(err, unable to create directory for %s: %s, logfile, strerror(errno)); -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 3/7] refs: new public ref function: safe_create_reflog
The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 14 +- refs.c | 24 refs.h | 2 +- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index beea1eb..b3057ec 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -620,24 +620,20 @@ static void update_refs_for_switch(const struct checkout_opts *opts, if (opts-new_branch) { if (opts-new_orphan_branch) { if (opts-new_branch_log !log_all_ref_updates) { - int temp; - struct strbuf log_file = STRBUF_INIT; int ret; - const char *ref_name; + char *refname; struct strbuf err = STRBUF_INIT; - ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); - temp = log_all_ref_updates; - log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, log_file, err); - log_all_ref_updates = temp; - strbuf_release(log_file); + refname = mkpathdup(refs/heads/%s, opts-new_orphan_branch); + ret = safe_create_reflog(refname, 1, err); + free(refname); if (ret) { fprintf(stderr, _(Can not do reflog for '%s': %s\n), opts-new_orphan_branch, err.buf); strbuf_release(err); return; } + strbuf_release(err); } } else diff --git a/refs.c b/refs.c index cf1abeb..1abfe64 100644 --- a/refs.c +++ b/refs.c @@ -3128,8 +3128,13 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, HEAD); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * Create a reflog for a ref. If force_create = 0, the reflog will + * only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, create it + * regardless of the ref name. Fill in *err and return -1 on failure. + */ +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3138,7 +3143,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile-buf; /* make sure the rest of the function can't change logfile */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) 0) { strbuf_addf(err, unable to create directory for %s: %s, logfile, strerror(errno)); @@ -3173,6 +3178,17 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf return 0; } + +int safe_create_reflog(const char *refname, int force_create, struct strbuf *err) +{ + int ret; + struct strbuf sb = STRBUF_INIT; + + ret = log_ref_setup(refname, sb, err, force_create); + strbuf_release(sb); + return ret; +} + static int log_ref_write_fd(int fd, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *committer, const char *msg) @@ -3209,7 +3225,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, sb_log_file, err); + result = log_ref_setup(refname, sb_log_file, err, 0); if (result) return result; diff --git a/refs.h b/refs.h index debdefc..f82fc51 100644 --- a/refs.h +++ b/refs.h @@ -228,7 +228,7 @@ int pack_refs(unsigned int flags); /* *
[PATCH v10 6/7] update-ref and tag: add --create-reflog arg
Allow the creation of a ref (e.g. stash) with a reflog already in place. For most refs (e.g. those under refs/heads), this happens automatically, but for others, we need this option. Currently, git does this by pre-creating the reflog, but alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead use git plumbing commands. I also added --create-reflog to git tag, just for completeness. In a moment, we will use this argument to make git stash work with alternate ref backends. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-tag.txt| 5 - Documentation/git-update-ref.txt | 5 - builtin/tag.c| 5 - builtin/update-ref.c | 14 +++--- t/t1400-update-ref.sh| 38 ++ t/t7004-tag.sh | 14 +- 6 files changed, 74 insertions(+), 7 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 034d10d..2312980 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -13,7 +13,7 @@ SYNOPSIS tagname [commit | object] 'git tag' -d tagname... 'git tag' [-n[num]] -l [--contains commit] [--points-at object] - [--column[=options] | --no-column] [pattern...] + [--column[=options] | --no-column] [--create-reflog] [pattern...] [pattern...] 'git tag' -v tagname... @@ -143,6 +143,9 @@ This option is only applicable when listing tags without annotation lines. all, 'whitespace' removes just leading/trailing whitespace lines and 'strip' removes both whitespace and commentary. +--create-reflog:: + Create a reflog for the tag. + tagname:: The name of the tag to create, delete, or describe. The new tag name must pass all checks defined by diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index c8f5ae5..969bfab 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely SYNOPSIS [verse] -'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] ref newvalue [oldvalue] | --stdin [-z]) +'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] [--create-reflog] ref newvalue [oldvalue] | --stdin [-z]) DESCRIPTION --- @@ -67,6 +67,9 @@ performs all modifications together. Specify commands of the form: verify SP ref [SP oldvalue] LF option SP opt LF +With `--create-reflog`, update-ref will create a reflog for each ref +even if one would not ordinarily be created. + Quote fields containing whitespace as if they were strings in C source code; i.e., surrounded by double-quotes and with backslash escapes. Use 40 0 characters or the empty string to specify a zero value. To diff --git a/builtin/tag.c b/builtin/tag.c index 5f6cdc5..a99 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -579,6 +579,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) struct create_tag_options opt; char *cleanup_arg = NULL; int annotate = 0, force = 0, lines = -1; + int create_reflog = 0; int cmdmode = 0; const char *msgfile = NULL, *keyid = NULL; struct msg_arg msg = { 0, STRBUF_INIT }; @@ -605,6 +606,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_STRING('u', local-user, keyid, N_(key-id), N_(use another key to sign the tag)), OPT__FORCE(force, N_(replace the tag if exists)), + OPT_BOOL(0, create-reflog, create_reflog, N_(create_reflog)), OPT_GROUP(N_(Tag listing options)), OPT_COLUMN(0, column, colopts, N_(show tag list in columns)), @@ -733,7 +735,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) transaction = ref_transaction_begin(err); if (!transaction || ref_transaction_update(transaction, ref.buf, object, prev, - 0, NULL, err) || + create_reflog ? REF_FORCE_CREATE_REFLOG : 0, + NULL, err) || ref_transaction_commit(transaction, err)) die(%s, err.buf); ref_transaction_free(transaction); diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 6763cf1..04dd00f 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static int update_flags; +static unsigned create_reflog_flag; static const char *msg; /* @@ -200,7 +201,8 @@ static const char *parse_cmd_update(struct ref_transaction *transaction, if (ref_transaction_update(transaction, refname, new_sha1,
[PATCH v10 7/7] git-stash: use update-ref --create-reflog instead of creating files
This is in support of alternate ref backends which don't necessarily store reflogs as files. Signed-off-by: David Turner dtur...@twopensource.com --- git-stash.sh | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 8e9e2cd..1d5ba7a 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -183,9 +183,7 @@ store_stash () { stash_msg=Created via \git stash store\. fi - # Make sure the reflog for stash is kept. - : $(git rev-parse --git-path logs/$ref_stash) - git update-ref -m $stash_msg $ref_stash $w_commit + git update-ref --create-reflog -m $stash_msg $ref_stash $w_commit ret=$? test $ret != 0 test -z $quiet die $(eval_gettext Cannot update \$ref_stash with \$w_commit) @@ -262,7 +260,7 @@ save_stash () { say $(gettext No local changes to save) exit 0 fi - test -f $(git rev-parse --git-path logs/$ref_stash) || + git reflog exists $ref_stash || clear_stash || die $(gettext Cannot initialize stash) create_stash $stash_msg $untracked -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question: .idx without .pack causes performance issues?
On Tue, Jul 21, 2015 at 3:48 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: While I still think that it is more important to prevent such a situation from occurring in the first place, ignoring .idx that lack corresponding .pack should be fairly simple, perhaps like this. ... Sorry for the noise, but this patch is worthless. We already have an equivalent test in add_packed_git() that is called from this same place. And a few extra updates from me: we found that this appears to occur even after update to 1.9.5, and setting core.fscache on 2.4.6 has no appreciable impact on the time it takes to run git fetch, either. Our thought was antivirus (or something else?) might have the file open when git attempts to unlink the .idx, but perhaps it's something else, too? In one case, we had ~560 orphaned .idx files, but 150 seems sufficient to slow a fetch operation for a few minutes until it actually begins transferring objects. The git gc approach to cleaning up the mess is certainly looking more and more attractive... :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] builtin/send-pack.c: Respect http.signingkey
From: Junio C Hamano gits...@pobox.com Prior to this patch, when git-send-pack was exec'ed, as is done by git-remote-http, it did not reread the config, so it did not respect the configured value of http.signingkey. Thus it was impossible to specify a signing key over HTTP, other than the default key in the keyring having a User ID matching the Name email format. This patch at least partially fixes the problem by reading in the GPG config from within send-pack. It does not address the related problem of plumbing a value for this configuration option using `git -c http.signingkey push ...`. Signed-off-by: Dave Borowitz dborow...@google.com --- builtin/send-pack.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/send-pack.c b/builtin/send-pack.c index b961e5a..23b2962 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -11,6 +11,7 @@ #include transport.h #include version.h #include sha1-array.h +#include gpg-interface.h static const char send_pack_usage[] = git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] [host:]directory [ref...]\n @@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) int from_stdin = 0; struct push_cas_option cas = {0}; + git_config(git_gpg_config, NULL); + argv++; for (i = 1; i argc; i++, argv++) { const char *arg = *argv; -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 1/7] refs.c: add err arguments to reflog functions
Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Some error messages are slightly reordered. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 8 ++-- refs.c | 129 ++--- refs.h | 4 +- 3 files changed, 81 insertions(+), 60 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c018ab3..beea1eb 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -624,16 +624,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts, struct strbuf log_file = STRBUF_INIT; int ret; const char *ref_name; + struct strbuf err = STRBUF_INIT; ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); temp = log_all_ref_updates; log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, log_file); + ret = log_ref_setup(ref_name, log_file, err); log_all_ref_updates = temp; strbuf_release(log_file); if (ret) { - fprintf(stderr, _(Can not do reflog for '%s'\n), - opts-new_orphan_branch); + fprintf(stderr, _(Can not do reflog for '%s': %s\n), + opts-new_orphan_branch, err.buf); + strbuf_release(err); return; } } diff --git a/refs.c b/refs.c index fb568d7..ca68509 100644 --- a/refs.c +++ b/refs.c @@ -2975,9 +2975,11 @@ static int rename_ref_available(const char *oldname, const char *newname) return ret; } -static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1); +static int write_ref_to_lockfile(struct ref_lock *lock, +const unsigned char *sha1, struct strbuf *err); static int commit_ref_update(struct ref_lock *lock, -const unsigned char *sha1, const char *logmsg); +const unsigned char *sha1, const char *logmsg, +struct strbuf *err); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -3038,9 +3040,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms } hashcpy(lock-old_oid.hash, orig_sha1); - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, logmsg)) { - error(unable to write current sha1 into %s, newrefname); + if (write_ref_to_lockfile(lock, orig_sha1, err) || + commit_ref_update(lock, orig_sha1, logmsg, err)) { + error(unable to write current sha1 into %s: %s, newrefname, err.buf); + strbuf_release(err); goto rollback; } @@ -3056,9 +3059,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms flag = log_all_ref_updates; log_all_ref_updates = 0; - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, NULL)) - error(unable to write current sha1 into %s, oldrefname); + if (write_ref_to_lockfile(lock, orig_sha1, err) || + commit_ref_update(lock, orig_sha1, NULL, err)) { + error(unable to write current sha1 into %s: %s, oldrefname, err.buf); + strbuf_release(err); + } log_all_ref_updates = flag; rollbacklog: @@ -3113,8 +3118,8 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } -/* This function must set a meaningful errno on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile) +/* This function will fill in *err and return -1 on failure */ +int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3129,9 +3134,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
[PATCH v10 4/7] git-reflog: add exists command
This is necessary because alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-reflog.txt | 4 builtin/reflog.c | 33 - t/t1411-reflog-show.sh | 5 + 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 5e7908e..44c736f 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,6 +23,7 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | refs...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... +'git reflog exists' ref Reference logs, or reflogs, record when the tips of branches and other references were updated in the local repository. Reflogs are @@ -52,6 +53,9 @@ argument must be an _exact_ entry (e.g. `git reflog delete master@{2}`). This subcommand is also typically not used directly by end users. +The exists subcommand checks whether a ref has a reflog. It exits +with zero status if the reflog exists, and non-zero status if it does +not. OPTIONS --- diff --git a/builtin/reflog.c b/builtin/reflog.c index c2eb8ff..7ed0e85 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -13,6 +13,8 @@ static const char reflog_expire_usage[] = git reflog expire [--expire=time] [--expire-unreachable=time] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] refs...; static const char reflog_delete_usage[] = git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] refs...; +static const char reflog_exists_usage[] = +git reflog exists ref; static unsigned long default_reflog_expire; static unsigned long default_reflog_expire_unreachable; @@ -699,12 +701,38 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) return status; } +static int cmd_reflog_exists(int argc, const char **argv, const char *prefix) +{ + int i, start = 0; + + for (i = 1; i argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, --)) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_exists_usage); + else + break; + } + + start = i; + + if (argc - start != 1) + usage(reflog_exists_usage); + + if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL)) + die(invalid ref format: %s, argv[start]); + return !reflog_exists(argv[start]); +} + /* * main reflog */ static const char reflog_usage[] = -git reflog [ show | expire | delete ]; +git reflog [ show | expire | delete | exists ]; int cmd_reflog(int argc, const char **argv, const char *prefix) { @@ -724,5 +752,8 @@ int cmd_reflog(int argc, const char **argv, const char *prefix) if (!strcmp(argv[1], delete)) return cmd_reflog_delete(argc - 1, argv + 1, prefix); + if (!strcmp(argv[1], exists)) + return cmd_reflog_exists(argc - 1, argv + 1, prefix); + return cmd_log_reflog(argc, argv, prefix); } diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh index 6f47c0d..3eb4f10 100755 --- a/t/t1411-reflog-show.sh +++ b/t/t1411-reflog-show.sh @@ -166,4 +166,9 @@ test_expect_success 'git log -g -p shows diffs vs. parents' ' test_cmp expect actual ' +test_expect_success 'reflog exists works' ' + git reflog exists refs/heads/master + ! git reflog exists refs/heads/nonexistent +' + test_done -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 0/7] refs backend preamble
On Tue, 2015-07-21 at 13:49 -0700, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: Junio, now that Michael has marked this as reviewed, do you want to pull from github, or do you want me to send a re-roll to the mailing list? Let's see the final round; that would give me a chance to properly sign-off your patch, and also give others a chance to eyeball them for one last time. Thanks. Sent, thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again
From: Junio C Hamano gits...@pobox.com Junio C Hamano gits...@pobox.com writes: Philip Oakley philipoak...@iee.org writes: ... Ideally, if part of this mainstream Git, it would get picked up automatically by them (rather than being local 'fixes' endlessly carried forward). Actually, that is not ideal, but what I want to avoid. As I do not do Windows, it simply is wrong for me to apply changes that are very likely to affect Windows folks without seeing their support first,... Just to clarify this part. I do not do RedHat, Solaris or OSX, either. Also MSVC may not be what GfW folks primarily target. But the thing is that (1) Windows is so much different, and (2) GfW folks are much more qualified to judge platform-specific issues on Windows than I am. Even though I may still need to have a say in the overall structure of the changes to the upstream tree coming from them (e.g. Don't sprinkle #ifdef all over the generic code; instead add a wrapper or two in compat/ to keep the generic code generic is something I may tell them when rejecting a change forwarded to me), I trust them a lot more than I trust myself when it comes to what the changes do in the platform-specific part and how they do it. Thanks. -- Thanks for the extra clarification. I'll ask if they can confirm this update on GfW. I've also asked Ramsay what his current situation is re- you previous email and the msvc-build. regards Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 0/7] refs backend preamble
David Turner dtur...@twopensource.com writes: Junio, now that Michael has marked this as reviewed, do you want to pull from github, or do you want me to send a re-roll to the mailing list? Let's see the final round; that would give me a chance to properly sign-off your patch, and also give others a chance to eyeball them for one last time. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] unpack-trees: don't update files with CE_WT_REMOVE set
Junio C Hamano gits...@pobox.com writes: Duy Nguyen pclo...@gmail.com writes: Thank you both for catching this. Just a small suggestion. Perhaps we should do this instead. apply_sparse_checkout() is the function where all action manipulation (add, delete, update files..) for sparse checkout occurs and it should not ask to delete and update both at the same time. Sounds good. The first hunk may merely be a noise, but the second one is the true bugfix for the issue observed, I think. I've replaced as/sparse-checkout-removal with the following. Thanks, all. -- 8 -- From: David Turner dtur...@twopensource.com Date: Fri, 17 Jul 2015 17:19:27 -0400 Subject: [PATCH] unpack-trees: don't update files with CE_WT_REMOVE set Don't update files in the worktree from cache entries which are flagged with CE_WT_REMOVE. When a user does a sparse checkout, git removes files that are marked with CE_WT_REMOVE (because they are out-of-scope for the sparse checkout). If those files are also marked CE_UPDATE (for instance, because they differ in the branch that is being checked out and the outgoing branch), git would previously recreate them. This patch prevents them from being recreated. These erroneously-created files would also interfere with merges, causing pre-merge revisions of out-of-scope files to appear in the worktree. apply_sparse_checkout() is the function where all action manipulation (add, delete, update files..) for sparse checkout occurs; it should not ask to delete and update both at the same time. Signed-off-by: Anatole Shaw git-de...@omni.poc.net Signed-off-by: David Turner dtur...@twopensource.com Helped-by: Duy Nguyen pclo...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t1090-sparse-checkout-scope.sh | 52 unpack-trees.c | 4 2 files changed, 56 insertions(+) create mode 100755 t/t1090-sparse-checkout-scope.sh diff --git a/t/t1090-sparse-checkout-scope.sh b/t/t1090-sparse-checkout-scope.sh new file mode 100755 index 000..1f61eb3 --- /dev/null +++ b/t/t1090-sparse-checkout-scope.sh @@ -0,0 +1,52 @@ +#!/bin/sh + +test_description='sparse checkout scope tests' + +. ./test-lib.sh + +test_expect_success 'setup' ' + echo initial a + echo initial b + echo initial c + git add a b c + git commit -m initial commit +' + +test_expect_success 'create feature branch' ' + git checkout -b feature + echo modified b + echo modified c + git add b c + git commit -m modification +' + +test_expect_success 'perform sparse checkout of master' ' + git config --local --bool core.sparsecheckout true + echo !/* .git/info/sparse-checkout + echo /a .git/info/sparse-checkout + echo /c .git/info/sparse-checkout + git checkout master + test_path_is_file a + test_path_is_missing b + test_path_is_file c +' + +test_expect_success 'merge feature branch into sparse checkout of master' ' + git merge feature + test_path_is_file a + test_path_is_missing b + test_path_is_file c + test $(cat c) = modified +' + +test_expect_success 'return to full checkout of master' ' + git checkout feature + echo /* .git/info/sparse-checkout + git checkout master + test_path_is_file a + test_path_is_file b + test_path_is_file c + test $(cat b) = modified +' + +test_done diff --git a/unpack-trees.c b/unpack-trees.c index 02f69ae..f177c0e 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -227,6 +227,9 @@ static int check_updates(struct unpack_trees_options *o) struct cache_entry *ce = index-cache[i]; if (ce-ce_flags CE_UPDATE) { + if (ce-ce_flags CE_WT_REMOVE) + die(BUG: both update and delete flags are set on %s, + ce-name); display_progress(progress, ++cnt); ce-ce_flags = ~CE_UPDATE; if (o-update !o-dry_run) { @@ -290,6 +293,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt if (!(ce-ce_flags CE_UPDATE) verify_uptodate_sparse(ce, o)) return -1; ce-ce_flags |= CE_WT_REMOVE; + ce-ce_flags = ~CE_UPDATE; } if (was_skip_worktree !ce_skip_worktree(ce)) { if (verify_absent_sparse(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) -- 2.5.0-rc2-392-g7dbe568 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 7/7] git-stash: use update-ref --create-reflog instead of creating files
Compared them with v9, signed them off, and merged to 'next'. Haven't pushed the result out yet. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 0/7] refs backend preamble
On Tue, 2015-07-21 at 13:20 -0700, Michael Haggerty wrote: On 07/21/2015 08:44 AM, David Turner wrote: This reroll addresses Michael Haggerty's comments: - Error messages are now in the form error: reason - We no longer unnecessarily set errno in write_ref_to_lockfile - Corrected a spelling error in the new docs and another in the tests - Corrected some copypasta in a test. Isn't it sobering that, every time one looks at source code, one finds things that could be improved? (Not just yours but all source code!) Indeed. Happily it seems like the items are getting ever more trivial, and either with or without the last round of suggestions I think the code is correct and is a nice step forward. Reviewed-by: Michael Haggerty mhag...@alum.mit.edu Off topic: I wonder whether sooner or later we will need a git reflog remove ref to delete an existing reference's reflog entirely without deleting the reference and without assuming a filesystem backend. (Note that this is different than `git reflog expire`, which leaves the empty reflog file around and thus doesn't undo a --create-reflog.) But perhaps there is no call for that operation. In a pinch a user could delete the reference and recreate it to get rid of its reflog. Some of the tests do delete reflogs (manually), and the refs backend code has a test driver that performs that operation (along with some others). I could move git reflog remove out of that driver and into git reflog, but I'm reluctant to do it just for the tests. If there's a non-test reason for it, I'll be happy to. I've pushed a new version of this series to https://github.com/dturner-tw/git/ on the pluggable-refs-preamble branch. The new version addresses Michael's comments from this around, and makes a variable static as you suggested (and said you would squash in). Junio, now that Michael has marked this as reviewed, do you want to pull from github, or do you want me to send a re-roll to the mailing list? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question: .idx without .pack causes performance issues?
Junio C Hamano gits...@pobox.com writes: While I still think that it is more important to prevent such a situation from occurring in the first place, ignoring .idx that lack corresponding .pack should be fairly simple, perhaps like this. ... Sorry for the noise, but this patch is worthless. We already have an equivalent test in add_packed_git() that is called from this same place. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] doc: give examples for send-email cc-cmd operation
From: Junio C Hamano gits...@pobox.com Junio C Hamano gits...@pobox.com writes: I was trying to use, essentially, 'cat list.txt' as the command,... One thing that needs to be made clear is that I do not think we want to encourage `cat list.txt #` abuse in the first place. OK [1] It is an unacceptable hack for us to encourage in the longer term. It may happen to work with the current implementation, but it does so merely by depending on the implementation too much. If it is so common to want to spray all your patches to exactly the same list of recipients that is unconditionally determined, having It wasn't 'unconditional spraying' ;-), rather I'd carefully select who to send to for each series, previously with multiple cc=.. on the command line. multiple sendemail.cc configuration variables, which are cumulative, is already one way to do so, and you do not have to type such a long option --cc-cmd='cat $filename' every time. And if you do not want configuration for some reason, and having a list of addresses in a flat file is so common, we could have a new option --cc-list=$filename to support that use case. I however doubt anything that starts with First you make a list of addresses in a flat file, and then do this is a good solution. For a longer series, a list will still need collating by the OP somehow, but I don't think that alone would justify a new option. I would think that it would probably be the best way to address I often want to cc these recipients, but not always is to keep a list of aliases, each entry of which expands to the recipients, and say --cc=group from the command line to have it expanded to the set of recipients. [1] You mentioned in the previous email the script command argv[] array, which I hadn't heard of (for a script), having been used the $1, $2, method. Given that new understanding, IIUC the proposal is that the garantee is the --cc-cmd should be a single command/script name, with the patchfilename passed as $1, and that a 'string string' for interpretation would now be deprecated. I'll see about a shorter doc patch that restricts itself to just the base aspects, if that would be acceptable? -- Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] builtin/send-pack.c: Respect http.signingkey
Dave Borowitz dborow...@google.com writes: From: Junio C Hamano gits...@pobox.com Prior to this patch, when git-send-pack was exec'ed, as is done by git-remote-http, it did not reread the config, so it did not respect the configured value of http.signingkey. Thus it was impossible to specify a signing key over HTTP, other than the default key in the keyring having a User ID matching the Name email format. This patch at least partially fixes the problem by reading in the GPG config from within send-pack. It does not address the related problem of plumbing a value for this configuration option using `git -c http.signingkey push ...`. I am hoping that git -c user.signingkey=frotz push ... would solve that problem, but I probably am being too optimistic ;-) Thanks. Signed-off-by: Dave Borowitz dborow...@google.com --- builtin/send-pack.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/send-pack.c b/builtin/send-pack.c index b961e5a..23b2962 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -11,6 +11,7 @@ #include transport.h #include version.h #include sha1-array.h +#include gpg-interface.h static const char send_pack_usage[] = git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] [host:]directory [ref...]\n @@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) int from_stdin = 0; struct push_cas_option cas = {0}; + git_config(git_gpg_config, NULL); + argv++; for (i = 1; i argc; i++, argv++) { const char *arg = *argv; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] contrib/subtree: ignore log.date configuration
git-subtree's log format string uses %ad and %cd, which respect the user's configured log.date value. This is problematic for git-subtree because it needs to use real dates so that copied commits come through unchanged. Add a test and tweak the format strings to use %aD and %cD so that the default date format is used instead. Reported-by: Bryan Jacobs b...@q3q.us Signed-off-by: David Aguilar dav...@gmail.com --- contrib/subtree/git-subtree.sh | 2 +- contrib/subtree/t/t7900-subtree.sh | 5 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 07bd77c..9f06571 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -305,7 +305,7 @@ copy_commit() # We're going to set some environment vars here, so # do it in a subshell to get rid of them safely later debug copy_commit {$1} {$2} {$3} - git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' $1 | + git log -1 --pretty=format:'%an%n%ae%n%aD%n%cn%n%ce%n%cD%n%B' $1 | ( read GIT_AUTHOR_NAME read GIT_AUTHOR_EMAIL diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index bd3df97..cd2dfe7 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -94,6 +94,11 @@ test_expect_success 'add sub3' ' # Back to mainline cd .. +# Enable log.date=relative to catch errors +test_expect_success 'enable log.date=relative to catch error' ' + git config log.date relative +' + test_expect_success 'add main4' ' create main4 git commit -m main4 -- 2.5.0.rc3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] contrib/subtree: ignore log.date configuration
git-subtree's log format string uses %ad and %cd, which respect the user's configured log.date value. This is problematic for git-subtree because it needs to use real dates so that copied commits come through unchanged. Add a test and tweak the format strings to use %aD and %cD so that the default date format is used instead. Reported-by: Bryan Jacobs b...@q3q.us Signed-off-by: David Aguilar dav...@gmail.com --- Please ignore the previous patch -- it had a stray comment in the test code that is not present in this version. contrib/subtree/git-subtree.sh | 2 +- contrib/subtree/t/t7900-subtree.sh | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 07bd77c..9f06571 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -305,7 +305,7 @@ copy_commit() # We're going to set some environment vars here, so # do it in a subshell to get rid of them safely later debug copy_commit {$1} {$2} {$3} - git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' $1 | + git log -1 --pretty=format:'%an%n%ae%n%aD%n%cn%n%ce%n%cD%n%B' $1 | ( read GIT_AUTHOR_NAME read GIT_AUTHOR_EMAIL diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index bd3df97..9051982 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -94,6 +94,10 @@ test_expect_success 'add sub3' ' # Back to mainline cd .. +test_expect_success 'enable log.date=relative to catch errors' ' + git config log.date relative +' + test_expect_success 'add main4' ' create main4 git commit -m main4 -- 2.5.0.rc3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] gitk: Add a Copy commit summary command
On 21.07.15 12:28, Paul Mackerras wrote: On Tue, Jul 21, 2015 at 12:19:23PM +0200, Beat Bolli wrote: Guys, can I get a Yea or Nay for this patch? Does it go in via Paul's gitk repo or directly through Junio? I'll put it in. It goes into my repo and from there into Junio's. I'm on vacation and travelling this week, so please be patient. No problem, enjoy your vacation! Beat -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] gitk: Add a Copy commit summary command
Guys, can I get a Yea or Nay for this patch? Does it go in via Paul's gitk repo or directly through Junio? Thanks, Beat On 18.07.15 13:15, Beat Bolli wrote: When referring to earlier commits in commit messages or other text, one of the established formats is abbrev-sha (summary, author-date) Add a Copy commit summary command to the context menu that puts this text for the currently selected commit on the clipboard. This makes it easy for our users to create well-formatted commit references. The abbrev-sha is produced with the %h format specifier to make it unique. Its length can be controlled with the gitk preference Auto-select SHA1 (length), or, if this preference is set to its default value (40), with the Git config setting core.abbrev. Signed-off-by: Beat Bolli dev+...@drbeat.li Cc: Paul Mackerras pau...@samba.org --- Changes since v3: - consider $autosellen for the --abbrev value Changes since v2: - call git show to produce a unique abbrev-sha - use the short date format Changes since v1: - drop the commit literal in front of the abbrev-sha Signed-off-by: Beat Bolli dev+...@drbeat.li --- gitk-git/gitk | 15 +++ 1 file changed, 15 insertions(+) diff --git a/gitk-git/gitk b/gitk-git/gitk index 9a2daf3..d05169a 100755 --- a/gitk-git/gitk +++ b/gitk-git/gitk @@ -2617,6 +2617,7 @@ proc makewindow {} { {mc Diff selected - this command {diffvssel 1}} {mc Make patch command mkpatch} {mc Create tag command mktag} + {mc Copy commit summary command copysummary} {mc Write commit to file command writecommit} {mc Create new branch command mkbranch} {mc Cherry-pick this commit command cherrypick} @@ -9341,6 +9342,20 @@ proc mktaggo {} { mktagcan } +proc copysummary {} { +global rowmenuid autosellen + +set format %h (\%s\, %ad) +set cmd [list git show -s --pretty=format:$format --date=short] +if {$autosellen 40} { +lappend cmd --abbrev=$autosellen +} +set summary [eval exec $cmd $rowmenuid] + +clipboard clear +clipboard append $summary +} + proc writecommit {} { global rowmenuid wrcomtop commitinfo wrcomcmd NS -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] gitk: Add a Copy commit summary command
On Tue, Jul 21, 2015 at 12:19:23PM +0200, Beat Bolli wrote: Guys, can I get a Yea or Nay for this patch? Does it go in via Paul's gitk repo or directly through Junio? I'll put it in. It goes into my repo and from there into Junio's. I'm on vacation and travelling this week, so please be patient. Regards, Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] gitk deals badly with --not
When gitk is run with some refs --not some other refs args, things work fine until one tries to modify the view. There it considers --not to be an extra git-log argument on its own, and groups the negative refs together with the positive refs, which naturally gives completely wrong results. Not sure what the best course of action would be to fix that: * add negative refs the extra args could be an option, since those are added last to the commandline, but then the edit dialog becomes a bit strange, with refs listed at both ends of it * add a dedicated field for negative refs * convert them to the ^ref syntax Either of those last 2 solution raise the concern of convertion between ^ and --not: in the case where we add a dedicated field, would there be any reason not to move ^refs to this field instead ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 6/7] update-ref and tag: add --create-reflog arg
On 07/09/2015 03:50 PM, David Turner wrote: Allow the creation of a ref (e.g. stash) with a reflog already in place. For most refs (e.g. those under refs/heads), this happens automatically, but for others, we need this option. Currently, git does this by pre-creating the reflog, but alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead use git plumbing commands. I also added --create-reflog to git tag, just for completeness. In a moment, we will use this argument to make git stash work with alternate ref backends. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-tag.txt| 5 - Documentation/git-update-ref.txt | 5 - builtin/tag.c| 5 - builtin/update-ref.c | 14 +++--- t/t1400-update-ref.sh| 38 ++ t/t7004-tag.sh | 14 +- 6 files changed, 74 insertions(+), 7 deletions(-) [...] diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index d1ff5c9..75423ab 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -51,7 +51,19 @@ test_expect_success 'creating a tag using default HEAD should succeed' ' echo foo foo git add foo git commit -m Foo - git tag mytag + git tag mytag + test_must_fail git reflog exists refs/tags/mytag +' + +test_expect_success 'creating a tag with --create-reflog should create reflog' ' + test_when_finished git tag -d tag_with_reflog + git tag --create-reflog tag_with_reflog + git reflog exists refs/tags/tag_with_reflog +' + +test_expect_success '--create-reflog does not creates reflog on failure' ' s/creates/create/ + test_must_fail git tag --create-reflog mytag + test_must_fail git reflog exists refs/tags/tag_with_reflog Shouldn't this be test_must_fail git reflog exists refs/tags/mytag ? [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/7] ref backend preamble
On 07/09/2015 03:50 PM, David Turner wrote: The current state of the discussion on alternate ref backends is that we're going to continue to store pseudorefs (e.g. CHERRY_PICK_HEAD) as files in $GIT_DIR. So this re-roll of the refs backend preamble doesn't do anything to pseudorefs. It just does reflog stuff. In addition, this version removes the over-aggressive die() on reflog update failure from v7. It adds the REF_FORCE_CREATE_REFLOG flag, as Michael Haggerty suggested. And it fixes commit message or two, as suggested. I believe this addresses all comments I've seen on v7. This addresses Johannes Sixt's concerns too, by removing the offending code. I just reviewed the whole patch series. Aside from the minor points that I commented on, it all looks good to me. Reviewed-by: Michael Haggerty mhag...@alum.mit.edu Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] userdiff: add support for Fountain documents
Add support for Fountain, a plain text screenplay format. Git facilitates not just programming specifically, but creative writing in general, so it makes sense to also support other plain text documents besides source code. In the structure of a screenplay specifically, scenes are roughly analogous to functions, in the sense that it makes your job easier if you can see which ones were changed in a given range of patches. More information about the Fountain format can be found on its official website, at http://fountain.io . Signed-off-by: Zoë Blade z...@bytenoise.co.uk --- Documentation/gitattributes.txt | 2 ++ t/t4018-diff-funcname.sh| 1 + t/t4018/fountain-scene | 4 userdiff.c | 2 ++ 4 files changed, 9 insertions(+) create mode 100644 t/t4018/fountain-scene diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 81fe586..e3b1de8 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -527,6 +527,8 @@ patterns are available: - `fortran` suitable for source code in the Fortran language. +- `fountain` suitable for Fountain documents. + - `html` suitable for HTML/XHTML documents. - `java` suitable for source code in the Java language. diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 1dbaa38..67373dc 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -31,6 +31,7 @@ diffpatterns= cpp csharp fortran + fountain html java matlab diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene new file mode 100644 index 000..6b3257d --- /dev/null +++ b/t/t4018/fountain-scene @@ -0,0 +1,4 @@ +EXT. STREET RIGHT OUTSIDE - DAY + +CHARACTER +You didn't say the magic phrase, ChangeMe. diff --git a/userdiff.c b/userdiff.c index 2ccbee5..eec939a 100644 --- a/userdiff.c +++ b/userdiff.c @@ -35,6 +35,8 @@ IPATTERN(fortran, * they would have been matched above as a variable anyway. */ |[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)? |//|\\*\\*|::|[/=]=), +IPATTERN(fountain, ^((\\.|((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ).+)$, +[^ \t-]+), PATTERNS(html, ^[ \t]*([Hh][1-6][ \t].*.*)$, [^= \t]+), PATTERNS(java, -- 1.9.5 (Apple Git-50.3) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] userdiff: add support for Fountain documents
On 20 Jul 2015, at 22:17, Junio C Hamano gits...@pobox.com wrote: +PATTERNS(fountain, ^((\\.|(([Ii][Nn][Tt]|[Ee][Ss][Tt]|[Ee][Xx][Tt])?\\.?|[Ii]([Nn][Tt])?\\.?/[Ee]([Xx][Tt])?\\.?) ).+)$, + [^ \t-]+), Wouldn't IPATTERN() be a better choice here? Good point, thank you! Much better: IPATTERN(fountain, ^((\\.|((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ).+)$, [^ \t-]+), It looks like some of the others might benefit from being case insensitive too, but I'm not sure, and at any rate it would warrant a separate patch. I'll send another revision next... :) Thanks, Zoë.-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 4/7] git-reflog: add exists command
On 07/09/2015 03:50 PM, David Turner wrote: Theis are necessary because alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-reflog.txt | 4 builtin/reflog.c | 33 - t/t1411-reflog-show.sh | 5 + 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 5e7908e..4b08fc7 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,6 +23,7 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | refs...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... +'git reflog exists' ref Reference logs, or reflogs, record when the tips of branches and other references were updated in the local repository. Reflogs are @@ -52,6 +53,9 @@ argument must be an _exact_ entry (e.g. `git reflog delete master@{2}`). This subcommand is also typically not used directly by end users. +The exists subcommand checks whether a ref has a reflog. It exists The second exists should be exits. +with zero status if the reflog exists, and non-zero status if it does +not. [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/7] refs.c: add err arguments to reflog functions
On 07/09/2015 03:50 PM, David Turner wrote: Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Some error messages are slightly reordered. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 8 ++-- refs.c | 127 +++-- refs.h | 4 +- 3 files changed, 81 insertions(+), 58 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c018ab3..93f63d3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -624,16 +624,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts, struct strbuf log_file = STRBUF_INIT; int ret; const char *ref_name; + struct strbuf err = STRBUF_INIT; ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); temp = log_all_ref_updates; log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, log_file); + ret = log_ref_setup(ref_name, log_file, err); log_all_ref_updates = temp; strbuf_release(log_file); if (ret) { - fprintf(stderr, _(Can not do reflog for '%s'\n), - opts-new_orphan_branch); + fprintf(stderr, _(Can not do reflog for '%s'. %s\n), + opts-new_orphan_branch, err.buf); Our usual pattern for chaining error messages is $problem: $reason In this patch (and maybe later patches too? I haven't checked yet) I see $problem. $reason and also $problem. '$reason' I think it would be good to use the first pattern consistently. + strbuf_release(err); return; } } diff --git a/refs.c b/refs.c index fb568d7..03e7505 100644 --- a/refs.c +++ b/refs.c [...] @@ -3247,25 +3247,28 @@ int is_branch(const char *refname) /* * Write sha1 into the open lockfile, then close the lockfile. On - * errors, rollback the lockfile and set errno to reflect the problem. + * errors, rollback the lockfile, fill in *err and + * return -1. */ static int write_ref_to_lockfile(struct ref_lock *lock, - const unsigned char *sha1) + const unsigned char *sha1, struct strbuf *err) { static char term = '\n'; struct object *o; o = parse_object(sha1); if (!o) { - error(Trying to write ref %s with nonexistent object %s, - lock-ref_name, sha1_to_hex(sha1)); + strbuf_addf(err, + Trying to write ref %s with nonexistent object %s, + lock-ref_name, sha1_to_hex(sha1)); unlock_ref(lock); errno = EINVAL; Is it intentional that this function is still setting errno (here and a few lines farther down)? I'm guessing that this is no longer needed, though I haven't audited the callers. return -1; } if (o-type != OBJ_COMMIT is_branch(lock-ref_name)) { - error(Trying to write non-commit object %s to branch %s, - sha1_to_hex(sha1), lock-ref_name); + strbuf_addf(err, + Trying to write non-commit object %s to branch %s, + sha1_to_hex(sha1), lock-ref_name); unlock_ref(lock); errno = EINVAL; return -1; [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html