[PATCH 05/23] expire_reflog(): rename "ref" parameter to "refname"
This is our usual convention. Signed-off-by: Michael Haggerty --- builtin/reflog.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index 3e11bee..df302f4 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -349,7 +349,7 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int return 0; } -static int expire_reflog(const char *ref, const unsigned char *sha1, void *cb_data) +static int expire_reflog(const char *refname, const unsigned char *sha1, void *cb_data) { struct cmd_reflog_expire_cb *cmd = cb_data; struct expire_reflog_cb cb; @@ -365,20 +365,20 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, void *cb_da * we take the lock for the ref itself to prevent it from * getting updated. */ - lock = lock_any_ref_for_update(ref, sha1, 0, NULL); + lock = lock_any_ref_for_update(refname, sha1, 0, NULL); if (!lock) - return error("cannot lock ref '%s'", ref); - log_file = git_pathdup("logs/%s", ref); - if (!reflog_exists(ref)) + return error("cannot lock ref '%s'", refname); + log_file = git_pathdup("logs/%s", refname); + if (!reflog_exists(refname)) goto finish; if (!cmd->dry_run) { - newlog_path = git_pathdup("logs/%s.lock", ref); + newlog_path = git_pathdup("logs/%s.lock", refname); cb.newlog = fopen(newlog_path, "w"); } cb.cmd = cmd; - if (!cmd->expire_unreachable || !strcmp(ref, "HEAD")) { + if (!cmd->expire_unreachable || !strcmp(refname, "HEAD")) { tip_commit = NULL; cb.unreachable_expire_kind = UE_HEAD; } else { @@ -407,7 +407,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, void *cb_da mark_reachable(&cb); } - for_each_reflog_ent(ref, expire_reflog_ent, &cb); + for_each_reflog_ent(refname, expire_reflog_ent, &cb); if (cb.unreachable_expire_kind != UE_ALWAYS) { if (cb.unreachable_expire_kind == UE_HEAD) { -- 2.1.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
[PATCH 11/23] expire_reflog(): move dry_run to flags argument
The policy objects don't care about "--dry-run". So move it to expire_reflog()'s flags parameter. Signed-off-by: Michael Haggerty --- builtin/reflog.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index 319f0d2..a490193 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -22,7 +22,6 @@ static unsigned long default_reflog_expire_unreachable; struct cmd_reflog_expire_cb { struct rev_info revs; - int dry_run; int stalefix; int rewrite; int updateref; @@ -415,6 +414,10 @@ static void reflog_expiry_cleanup(struct expire_reflog_cb *cb) static struct lock_file reflog_lock; +enum expire_reflog_flags { + EXPIRE_REFLOGS_DRY_RUN = 1 << 0 +}; + static int expire_reflog(const char *refname, const unsigned char *sha1, unsigned int flags, void *cb_data) { @@ -439,7 +442,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, } log_file = git_pathdup("logs/%s", refname); - if (!cmd->dry_run) { + if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) { if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) goto failure; cb.newlog = fdopen_lock_file(&reflog_lock, "w"); @@ -453,7 +456,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, for_each_reflog_ent(refname, expire_reflog_ent, &cb); reflog_expiry_cleanup(&cb); - if (cb.newlog) { + if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) { if (close_lock_file(&reflog_lock)) { status |= error("Couldn't write %s: %s", log_file, strerror(errno)); @@ -644,7 +647,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) for (i = 1; i < argc; i++) { const char *arg = argv[i]; if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n")) - cb.dry_run = 1; + flags |= EXPIRE_REFLOGS_DRY_RUN; else if (starts_with(arg, "--expire=")) { if (parse_expiry_date(arg + 9, &cb.expire_total)) die(_("'%s' is not a valid timestamp"), arg); @@ -738,7 +741,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) for (i = 1; i < argc; i++) { const char *arg = argv[i]; if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n")) - cb.dry_run = 1; + flags |= EXPIRE_REFLOGS_DRY_RUN; else if (!strcmp(arg, "--rewrite")) cb.rewrite = 1; else if (!strcmp(arg, "--updateref")) -- 2.1.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
[PATCH 21/23] refs.c: remove unlock_ref/close_ref/commit_ref from the refs api
From: Ronnie Sahlberg unlock|close|commit_ref can be made static since there are no more external callers. Signed-off-by: Ronnie Sahlberg Signed-off-by: Stefan Beller Signed-off-by: Michael Haggerty --- refs.c | 24 refs.h | 9 - 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index 5a12d37..c6ee775 100644 --- a/refs.c +++ b/refs.c @@ -2090,6 +2090,16 @@ int refname_match(const char *abbrev_name, const char *full_name) return 0; } +static void unlock_ref(struct ref_lock *lock) +{ + /* Do not free lock->lk -- atexit() still looks at them */ + if (lock->lk) + rollback_lock_file(lock->lk); + free(lock->ref_name); + free(lock->orig_ref_name); + free(lock); +} + /* This function should make sure errno is meaningful on error */ static struct ref_lock *verify_lock(struct ref_lock *lock, const unsigned char *old_sha1, int mustexist) @@ -2888,7 +2898,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms return 1; } -int close_ref(struct ref_lock *lock) +static int close_ref(struct ref_lock *lock) { if (close_lock_file(lock->lk)) return -1; @@ -2896,7 +2906,7 @@ int close_ref(struct ref_lock *lock) return 0; } -int commit_ref(struct ref_lock *lock) +static int commit_ref(struct ref_lock *lock) { if (commit_lock_file(lock->lk)) return -1; @@ -2904,16 +2914,6 @@ int commit_ref(struct ref_lock *lock) return 0; } -void unlock_ref(struct ref_lock *lock) -{ - /* Do not free lock->lk -- atexit() still looks at them */ - if (lock->lk) - rollback_lock_file(lock->lk); - free(lock->ref_name); - free(lock->orig_ref_name); - free(lock); -} - /* * copy the reflog message msg to buf, which has been allocated sufficiently * large, while cleaning up the whitespaces. Especially, convert LF to space, diff --git a/refs.h b/refs.h index d777f76..82611b5 100644 --- a/refs.h +++ b/refs.h @@ -198,15 +198,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname, const unsigned char *old_sha1, int flags, int *type_p); -/** Close the file descriptor owned by a lock and return the status */ -extern int close_ref(struct ref_lock *lock); - -/** Close and commit the ref locked by the lock */ -extern int commit_ref(struct ref_lock *lock); - -/** Release any lock taken but not written. **/ -extern void unlock_ref(struct ref_lock *lock); - /* * Setup reflog before using. Set errno to something meaningful on failure. */ -- 2.1.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 04/23] expire_reflog(): remove unused parameter
Michael Haggerty wrote: > It was called "unused", so at least it was self-consistent. The missing context is that this was a callback function that had to match the each_ref_fn signature (where that parameter is 'flags') until v1.5.4~14 (reflog-expire: avoid creating new files in a directory inside readdir(3) loop, 2008-01-25). v1.5.4~14 forgot to clean up. With or without a note in the commit message explaining that, Reviewed-by: Jonathan Nieder -- 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/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
Jonathan Nieder writes: > Signed-off-by: Jonathan Nieder > --- > Junio C Hamano wrote: >> Jonathan Nieder writes: > >>> -extern int copy_fd(int ifd, int ofd); >>> +extern int copy_fd(int ifd, int ofd, struct strbuf *err); >> >> It is not limited to this single function, but what contract do we >> envision this "error messages are given back to the caller via >> strbuf" convention should give between the callers and the callee? > > Here's a draft for documentation on that. Thanks; looks reasonable; even if the discussion between you and Peff took us to a slightly different direction than what you described here, the earlier description of long established practice is a welcome addition. > Documentation/technical/api-error-handling.txt | 75 > ++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/technical/api-error-handling.txt > > diff --git a/Documentation/technical/api-error-handling.txt > b/Documentation/technical/api-error-handling.txt > new file mode 100644 > index 000..fc68db1 > --- /dev/null > +++ b/Documentation/technical/api-error-handling.txt > @@ -0,0 +1,75 @@ > +Error reporting in git > +== > + > +`die`, `usage`, `error`, and `warning` report errors of various > +kinds. > + > +- `die` is for fatal application errors. It prints a message to > + the user and exits with status 128. > + > +- `usage` is for errors in command line usage. After printing its > + message, it exits with status 129. (See also `usage_with_options` > + in the link:api-parse-options.html[parse-options API].) > + > +- `error` is for non-fatal library errors. It prints a message > + to the user and returns -1 for convenience in signaling the error > + to the caller. > + > +- `warning` is for reporting situations that probably should not > + occur but which the user (and Git) can continue to work around > + without running into too many problems. Like `error`, it > + returns -1 after reporting the situation to the caller. > + > +Customizable error handlers > +--- > + > +The default behavior of `die` and `error` is to write a message to > +stderr and then exit or return as appropriate. This behavior can be > +overridden using `set_die_routine` and `set_error_routine`. For > +example, "git daemon" uses set_die_routine to write the reason `die` > +was called to syslog before exiting. > + > +Library errors > +-- > + > +Functions return a negative integer on error. Details beyond that > +vary from function to function: > + > +- Some functions return -1 for all errors. Others return a more > + specific value depending on how the caller might want to react > + to the error. > + > +- Some functions report the error to stderr with `error`, > + while others leave that for the caller to do. > + > +- errno is not meaningful on return from most functions (except > + for thin wrappers for system calls). > + > +Check the function's API documentation to be sure. > + > +Caller-handled errors > +- > + > +An increasing number of functions take a parameter 'struct strbuf *err'. > +On error, such functions append a message about what went wrong to the > +'err' strbuf. The message is meant to be complete enough to be passed > +to `die` or `error` as-is. For example: > + > + if (ref_transaction_commit(transaction, &err)) > + die("%s", err.buf); > + > +The 'err' parameter will be untouched if no error occured, so multiple > +function calls can be chained: > + > + t = ref_transaction_begin(&err); > + if (!t || > + ref_transaction_update(t, "HEAD", ..., &err) || > + ret_transaction_commit(t, &err)) > + die("%s", err.buf); > + > +The 'err' parameter must be a pointer to a valid strbuf. To silence > +a message, pass a strbuf that is explicitly ignored: > + > + if (thing_that_can_fail_in_an_ignorable_way(..., &err)) > + /* This failure is okay. */ > + strbuf_reset(&err); -- 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 04/23] expire_reflog(): remove unused parameter
Michael Haggerty wrote: > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -349,7 +349,7 @@ static int push_tip_to_list(const char *refname, const > unsigned char *sha1, int > return 0; > } > > -static int expire_reflog(const char *ref, const unsigned char *sha1, int > unused, void *cb_data) > +static int expire_reflog(const char *ref, const unsigned char *sha1, void > *cb_data) > { > struct cmd_reflog_expire_cb *cmd = cb_data; On second thought: why not update the last parameter to be a 'struct cmd_reflog_expire_cb *' instead of 'void *' while at it, like this? builtin/reflog.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git i/builtin/reflog.c w/builtin/reflog.c index 3e11bee..d860624 100644 --- i/builtin/reflog.c +++ w/builtin/reflog.c @@ -349,9 +349,8 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int return 0; } -static int expire_reflog(const char *ref, const unsigned char *sha1, void *cb_data) +static int expire_reflog(const char *ref, const unsigned char *sha1, struct cmd_reflog_expire_cb *cmd) { - struct cmd_reflog_expire_cb *cmd = cb_data; struct expire_reflog_cb cb; struct ref_lock *lock; char *log_file, *newlog_path = NULL; -- 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
GIT: ignoring changes in tracked files and bug report
Hi, I'm trying find a solution where I can change file in a devel environment , and not commit it into git . git update-index --assume-unchanged is one solution , not the best solution but one solution. I add 2 files that I want ignore on commits git update-index --assume-unchanged configurations/local.defs git update-index --assume-unchanged processor/default.defs git diff -a is clean git diff . is clean git commit -a nothing added to commit but git commit . # Changes to be committed: # modified: configurations/local.defs # modified: processor/default.defs this is a bug ? Anyway what is best way to deal with some files where we want change locally and not commit in git . The solution of have one environment variable that says if we are in devel or in production , and system read the variable and choose the files to read can't be applied in my case, and is a no solution , is save a variable outside of source code which I can't. Thanks, -- Sérgio M. B. -- 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/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
Hi, Junio C Hamano wrote: > Jonathan Nieder writes: >> Here's a draft for documentation on that. > > Thanks; looks reasonable; even if the discussion between you and > Peff took us to a slightly different direction than what you > described here, the earlier description of long established practice > is a welcome addition. Thanks. Can you say more? I didn't think the discussion had reached a conclusion yet. Unless there's a strong reason to do otherwise, I prefer to stick with the strbufs in this series for now. I prefer strbuf for this purpose over char[1024]. And switching later doesn't seem hard. But if others prefer char[1024], I can update the existing code that uses strbuf from the transaction API to use that and reroll this series with that convention. A part of me dislikes that but I'll go along. Jonathan -- 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/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
Junio C Hamano wrote: > Jonathan Nieder writes: >> Here's a draft for documentation on that. > > Thanks; looks reasonable; even if the discussion between you and > Peff took us to a slightly different direction than what you > described here, the earlier description of long established practice > is a welcome addition. I think I see what I misunderstood. Do you mean "even if we settle on a different API, this documentation of what you started with should be easy to adapt and will make life easier"? In other words, did you mean to say that things are still up in the air (which I agree with) or that the project has already settled on a different direction (which I do not)? Sorry for the confusion, Jonathan -- 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 05/23] expire_reflog(): rename "ref" parameter to "refname"
Michael Haggerty wrote: > Signed-off-by: Michael Haggerty > --- > builtin/reflog.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Jonathan Nieder -- 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/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
On Thu, Dec 04, 2014 at 03:41:47PM -0800, Jonathan Nieder wrote: > Junio C Hamano wrote: > > Jonathan Nieder writes: > > >> Here's a draft for documentation on that. > > > > Thanks; looks reasonable; even if the discussion between you and > > Peff took us to a slightly different direction than what you > > described here, the earlier description of long established practice > > is a welcome addition. > > I think I see what I misunderstood. Do you mean "even if we settle on > a different API, this documentation of what you started with should be > easy to adapt and will make life easier"? > > In other words, did you mean to say that things are still up in the > air (which I agree with) or that the project has already settled on a > different direction (which I do not)? FWIW, that is how I read it (up in the air), and where I thought our discussion was. -Peff -- 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 00/23] Add reflog_expire() to the references API
Michael Haggerty writes: > I propose this patch series as an alternative to Ronnie's "reflog > transactions" series. After coasting my eyes over individual patches, it was delightful to look at the endgame state of the expiry codepath in builtin/reflog.c that is left ;-) -- 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 06/23] expire_reflog(): exit early if the reference has no reflog
Michael Haggerty wrote: > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -368,9 +368,11 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c > lock = lock_any_ref_for_update(refname, sha1, 0, NULL); > if (!lock) > return error("cannot lock ref '%s'", refname); > + if (!reflog_exists(refname)) { > + unlock_ref(lock); > + return 0; > + } > log_file = git_pathdup("logs/%s", refname); > - if (!reflog_exists(refname)) > - goto finish; Okay. Reviewed-by: Jonathan Nieder -- 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/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
Yeah, that is what I meant. The earlier part will not go to waste no matter what happens to the discussion. I am not a fan of char[1024], if only because our error message may have to mention things whose length is not under our control, e.g. a filename in the working tree, but I do share your concern that "strbuf"-approach calls for more boilerplate. I offhand do not have a magic silver bullet for it, though. On Thu, Dec 4, 2014 at 3:44 PM, Jeff King wrote: > On Thu, Dec 04, 2014 at 03:41:47PM -0800, Jonathan Nieder wrote: > >> Junio C Hamano wrote: >> > Jonathan Nieder writes: >> >> >> Here's a draft for documentation on that. >> > >> > Thanks; looks reasonable; even if the discussion between you and >> > Peff took us to a slightly different direction than what you >> > described here, the earlier description of long established practice >> > is a welcome addition. >> >> I think I see what I misunderstood. Do you mean "even if we settle on >> a different API, this documentation of what you started with should be >> easy to adapt and will make life easier"? >> >> In other words, did you mean to say that things are still up in the >> air (which I agree with) or that the project has already settled on a >> different direction (which I do not)? > > FWIW, that is how I read it (up in the air), and where I thought our > discussion was. > > -Peff -- 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 06/23] expire_reflog(): exit early if the reference has no reflog
Michael Haggerty wrote: > [Subject: expire_reflog(): exit early if the reference has no reflog] The caller moves on to expire other reflogs, so it's not exiting. "return early", maybe? Except the function already returned early. I think the purpose of this patch is to simplify the no-reflog case by handling it separately. Anyway, that's just nitpicking about the subject line. With s/exit/return/ it should be clear that this is a refactoring change, which for someone looking at the shortlog is the important thing. Thanks, Jonathan -- 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
Git's configure script --mandir doesn't work
I'm installing Git 2.2.0 from source distribution on NetBSD 6.1.5 (amd64) and when I specify --mandir=/usr/local/man, it still installs man pages in the default /usr/local/share/man directory. Is there a fix available for this? -- 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/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
On Thu, Dec 04, 2014 at 03:52:45PM -0800, Junio C Hamano wrote: > Yeah, that is what I meant. The earlier part will not go to waste no matter > what happens to the discussion. > > I am not a fan of char[1024], if only because our error message may have > to mention things whose length is not under our control, e.g. a filename > in the working tree, but I do share your concern that "strbuf"-approach > calls for more boilerplate. I offhand do not have a magic silver bullet for > it, though. The only downside I can think of is that we may truncate the message in exceptional circumstances. But is it really any less helpful to say: error: unable to open file: some-incredibly-long-filename-aa... than printing out an extra 100 lines of "a"? And I mean the "..." literally. I think mkerror() should indicate the truncation with a "...", just so that it is clear to the user. It should almost never happen, but when it does, it can be helpful to show the user that yes, we know we are truncating the message, and it is not that git truncated your filename during the operation. Is this truncation really a concern, and/or is there some other downside I'm not thinking of? -Peff -- 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 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
Michael Haggerty wrote: > We don't actually need the locking functionality, because we already > hold the lock on the reference itself, which is how the reflog file is > locked. But the lock_file code still does some of the bookkeeping for > us and is more careful than the old code here was. As you say, the ref lock takes care of mutual exclusion, so we do not have to be too careful about compatibility with other tools that might not know to lock the reflog. And this is not tying our hands for a future when I might want to lock logs/refs/heads/topic/1 while logs/refs/heads/topic still exists as part of the implementation of "git mv topic/1 topic". Stefan and I had forgotten about that guarantee when looking at that kind of operation --- thanks for the reminder. Should updates to the HEAD reflog acquire HEAD.lock? (They don't currently.) [...] > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -349,12 +349,14 @@ static int push_tip_to_list(const char *refname, const > unsigned char *sha1, int > return 0; > } > > +static struct lock_file reflog_lock; If this lockfile is only used in that one function, it can be declared inside the function. If it is meant to be used throughout the 'git reflog' command, then it can go near the top of the file. > + > static int expire_reflog(const char *refname, const unsigned char *sha1, > void *cb_data) > { > struct cmd_reflog_expire_cb *cmd = cb_data; > struct expire_reflog_cb cb; > struct ref_lock *lock; > - char *log_file, *newlog_path = NULL; > + char *log_file; > struct commit *tip_commit; > struct commit_list *tips; > int status = 0; > @@ -372,10 +374,14 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c > unlock_ref(lock); > return 0; > } > + > log_file = git_pathdup("logs/%s", refname); > if (!cmd->dry_run) { > - newlog_path = git_pathdup("logs/%s.lock", refname); > - cb.newlog = fopen(newlog_path, "w"); > + if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) > + goto failure; hold_lock_file_for_update doesn't print a message. Code to print one looks like if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) { unable_to_lock_message(log_file, errno, &err); error("%s", err.buf); goto failure; } (A patch in flight changes that to if (hold_lock_file_for_update(&reflog_lock, log_file, 0, &err) < 0) { error("%s", err.buf); goto failure; } ) > + cb.newlog = fdopen_lock_file(&reflog_lock, "w"); > + if (!cb.newlog) > + goto failure; Hm. lockfile.c::fdopen_lock_file ought to use xfdopen to make this case impossible. And xfdopen should use try_to_free_routine() and try again on failure. [...] > @@ -423,10 +429,9 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c > } > > if (cb.newlog) { > - if (fclose(cb.newlog)) { > - status |= error("%s: %s", strerror(errno), > - newlog_path); > - unlink(newlog_path); > + if (close_lock_file(&reflog_lock)) { > + status |= error("Couldn't write %s: %s", log_file, > + strerror(errno)); Style nit: error messages usually start with a lowercase letter (though I realize nearby examples are already inconsistent). commit_lock_file() can take care of the close_lock_file automatically. [...] > @@ -434,21 +439,23 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c >close_ref(lock) < 0)) { > status |= error("Couldn't write %s", > lock->lk->filename.buf); > - unlink(newlog_path); > - } else if (rename(newlog_path, log_file)) { > - status |= error("cannot rename %s to %s", > - newlog_path, log_file); > - unlink(newlog_path); > + rollback_lock_file(&reflog_lock); > + } else if (commit_lock_file(&reflog_lock)) { > + status |= error("cannot rename %s.lock to %s", > + log_file, log_file); Most callers say "unable to commit reflog '%s'", log_file to hedge their bets in case the close failed (which may be what you were avoiding above. errno is meaningful when commit_lock_file fails, making a more detailed diagnosis from strerror(errno) possible. Thanks, Jonathan -- 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] for_each_reflog_ent_reverse: fix newlines on block boundaries
On Thu, Dec 04, 2014 at 02:14:50PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I think the bug is in the reverse-reflog reader in > > for_each_reflog_ent_reverse. It reads BUFSIZ chunks of the file in > > reverse order, and then parses them individually. If the trailing > > newline for a line falls directly on the block boundary, we may not have > > it in our current block, and pass the line to show_one_reflog_ent > > without a trailing newline. > > Ahh, thanks for helping spot it. A code that uses BUFSIZ explains > why a single reproduction recipe is platform dependent. It also makes writing portable tests annoying, but I think I managed it in the patch below. :) > > So this is a long-standing bug in for_each_reflog_ent_reverse. It just > > showed up recently because we started using that function for > > read_ref_at_ent. > > ... > > The above is a workaround. I think the right solution is probably to > > teach for_each_reflog_ent_reverse to makes sure the trailing newline is > > included (either by tweaking the reverse code, or conditionally adding > > it to the parsed buffer). > > Sounds correct. Unfortunately I no longer remember how I decided to > deal with a line that spans the block boundary in that piece of code > X-<. Here's my proposed fix. -- >8 -- When we read a reflog file in reverse, we read whole chunks of BUFSIZ bytes, then loop over the buffer, parsing any lines we find. We find the beginning of each line by looking for the newline from the previous line. If we don't find one, we know that we are either at the beginning of the file, or that we have to read another block. In the latter case, we stuff away what we have into a strbuf, read another block, and continue our parse. But we missed one case here. If we did find a newline, and it is at the beginning of the block, we must also stuff that newline into the strbuf, as it belongs to the block we are about to read. The minimal fix here would be to add this special case to the conditional that checks whether we found a newline. But we can make the flow a little clearer by rearranging a bit: we first handle lines that we are going to show, and then at the end of each loop, stuff away any leftovers if necessary. That lets us fold this special-case in with the more common "we ended in the middle of a line" case. Signed-off-by: Jeff King --- I really needed this rearranging to figure out what the fix _should_ be. Now that I did that, I was able to write the above paragraph explaining what the minimal fix would be. And I can do that if we prefer. But I think the fact that I had to go through the untangling step first is an indication that maybe the end result is better. Of course that's all subjective. :) I waffled on the test script between generating it on the fly (as below), and just including a complete 8K example that provokes the failure. I don't care about the size that much, but rather on which is more readable. My goal with showing the generation steps was to make it clear how we arrived there, but I fear it may have ended up too convoluted to do anyone any good. Opinions welcome. refs.c| 47 --- t/t1410-reflog.sh | 30 ++ 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/refs.c b/refs.c index 5ff457e..ccb8834 100644 --- a/refs.c +++ b/refs.c @@ -3404,24 +3404,49 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void bp = find_beginning_of_line(buf, scanp); - if (*bp != '\n') { - strbuf_splice(&sb, 0, 0, buf, endp - buf); - if (pos) - break; /* need to fill another block */ - scanp = buf - 1; /* leave loop */ - } else { + if (*bp == '\n') { /* -* (bp + 1) thru endp is the beginning of the -* current line we have in sb +* The newline is the end of the previous line, +* so we know we have complete line starting +* at (bp + 1). Prefix it onto any prior data +* we collected for the line and process it. */ strbuf_splice(&sb, 0, 0, bp + 1, endp - (bp + 1)); scanp = bp; endp = bp + 1; + ret = show_one_reflog_ent(&sb, fn, cb_data); + strbuf_reset(&sb); + if (ret) + break; + } else if (!pos) { + /* +* We are at the st
[PATCH] for_each_reflog_ent_reverse: turn leftover check into assertion
On Thu, Dec 04, 2014 at 08:28:54PM -0500, Jeff King wrote: > The minimal fix here would be to add this special case to > the conditional that checks whether we found a newline. > But we can make the flow a little clearer by rearranging a > bit: we first handle lines that we are going to show, and > then at the end of each loop, stuff away any leftovers if > necessary. That lets us fold this special-case in with the > more common "we ended in the middle of a line" case. > > Signed-off-by: Jeff King > --- > I really needed this rearranging to figure out what the fix > _should_ be. Now that I did that, I was able to write the above > paragraph explaining what the minimal fix would be. And I can do that if > we prefer. But I think the fact that I had to go through the untangling > step first is an indication that maybe the end result is better. Of > course that's all subjective. :) I _think_ the patch below is also applicable to the code before my boundary-fixing patch. But the rearranging also made me more confident in it. -- >8 -- Subject: for_each_reflog_ent_reverse: turn leftover check into assertion Our loop should always process all lines, even if we hit the beginning of the file. We have a conditional after the loop ends to double-check that there is nothing left and to process it. But this should never happen, and is a sign of a logic bug in the loop. Let's turn it into a BUG assertion. Signed-off-by: Jeff King --- Of course I cannot say something like "this can never happen; the old code was wrong to handle this case" without a nagging feeling that I am missing something, so extra careful eyes are appreciated (and are why I would rather have an assert here than removing the code and silently dropping lines if I am wrong). refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index ccb8834..1f77fa6 100644 --- a/refs.c +++ b/refs.c @@ -3451,7 +3451,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void } if (!ret && sb.len) - ret = show_one_reflog_ent(&sb, fn, cb_data); + die("BUG: reverse reflog parser had leftover data"); fclose(logfp); strbuf_release(&sb); -- 2.2.0.390.gf60752d -- 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/RFC v2] Squashed changes for multiple worktrees vs. submodules
On Fri, Dec 5, 2014 at 3:06 AM, Jens Lehmann wrote: > Wow, so the .git/config is shared between all worktrees? I > suspect you have very good reasons for that, most of config vars are at repo-level, not worktree-level, except maybe submodule.* and something else. Technically we could use "include.path" to point to a non-shared file, where we store worktree-specific config. > but I believe > that'll make multiple work trees surprise the user from time > to time when used with submodules. Because initializing a > submodule in one worktree initializes it in all other > worktrees too (I suspect other users regard "git submodule > init" being a worktree local command too). And setting > "submodule..update" to "none" will also affect all > other worktrees. But I'd need to have separate settings for > our CI server, e.g. to checkout the sources without the > largish documentation submodule in one test job (=worktree) > while checking out the whole documentation for another job > building the setup in another worktree. > > And if I understand the "checkout: reject if the branch is > already checked out elsewhere" thread correctly, I won't be > able to build "master" in two jobs at the same time? If you do "git checkout --to ... master^{}", it should run fine. I'm still considering doing something better with the read-only refs, but haven't found time to really think it through yet. > Thanks. But I changed my mind about the details (now that I > know about .git/config and multiple worktrees). I think you'd > have to connect a .git directory in the submodule to the > common git dir directly, as you cannot use the core.worktree > setting (which could be different between commits due to > renames) when putting it into /.git/modules. And > then you couldn't remove or rename a submodule anymore, > because that fails when it contains a .git directory. > > Seems like we should put a "Warning: may do unexpected things > when used with submodules" (with some examples about what might > happen) in the multiple worktrees documentation. And I don't > believe anymore that teaching submodules to use the common git > dir makes that much sense after I know about the restrictions > it imposes. I'm ok with such a warning fwiw. -- Duy -- 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
[GIT PULL] l10n update on maint branch
Hi Junio, Please pull l10n update to maint branch. The following changes since commit b260d265e189728b26e50506ac6ffab6a7d588da: Git 2.2 (2014-11-26 13:18:34 -0800) are available in the git repository at: git://github.com/git-l10n/git-po maint for you to fetch changes up to ff51f5619d52410e4343c7cd75889e7d07f5659e: Merge branch 'master' of https://github.com/ralfth/git-po-de (2014-11-29 10:44:48 +0800) Hartmut Henkel (1): l10n: de.po: fix typos Jiang Xin (1): Merge branch 'master' of https://github.com/ralfth/git-po-de po/de.po | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) -- 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 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
On Thu, Dec 04, 2014 at 04:23:31PM -0800, Jonathan Nieder wrote: > Michael Haggerty wrote: > > > We don't actually need the locking functionality, because we already > > hold the lock on the reference itself, which is how the reflog file is > > locked. But the lock_file code still does some of the bookkeeping for > > us and is more careful than the old code here was. > > As you say, the ref lock takes care of mutual exclusion, so we do not > have to be too careful about compatibility with other tools that might > not know to lock the reflog. And this is not tying our hands for a > future when I might want to lock logs/refs/heads/topic/1 while > logs/refs/heads/topic still exists as part of the implementation of > "git mv topic/1 topic". > > Stefan and I had forgotten about that guarantee when looking at that > kind of operation --- thanks for the reminder. I did not forget about it, I did not know about that in the first hand. We don't seem to have documentation on it? So sorry for heading in a direction, which would have been avoidable. Thanks, Stefan -- 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] introduce git root
Jeff King wrote: > Some of the discussion has involved mixing config options into this > kitchen sink, but that does not make any sense to me (and is why I > find "git var -l" so odd). Config options are fundamentally different, in > that they are set and retrieved, not computed (from other config > variables, or from hard-coded values). And we already have a nice > tool for working with them (well...nice-ish, let's say). Yeah, but "git config" cannot say which config option applies in some context and why. For example, to chose the editor all the following could apply: GIT_SEQUENCE_EDITOR env variable sequence.editor config variable GIT_EDITOR env variable core.editor config variable VISUAL env variable EDITOR env variable editor configured at compile time and the user or our own scripts right now cannot easily know which editor should be used when editing the sequence list. The best they can do is: - first check if GIT_SEQUENCE_EDITOR is set, and if yes, use it - then check if sequence.editor config variable is set, and if yes, use it - then use "git var GIT_EDITOR" that will check the other options I don't think it is very nice. Jeff King also wrote: > My issue is only that "git --foo" has other options besides computables. > So you need to name each option in a way that makes it clear it is > reporting a computable and not doing something else. > > Take "git --pager" for instance. That would be a natural choice to > replace "git var GIT_PAGER". But shouldn't "--pager" be the opposite of > the existing "--no-pager"? > > So instead we probably need some namespace to indicate that it is a > "showing" option. Like "--show-pager". And then for consistency, we > would probably want to move "--exec-path" to "--show-exec-path", > creating a new "--show-" namespace. Or we could call that namespace > "git var". :) I agree with that, but I think it could be better if there was also a notion of context, > I do not think "git var --exec-path" is a good idea, nor GIT_EXEC_PATH > for the environment-variable confusion you mentioned. I was thinking of > just creating a new namespace, like: > > git var exec-path > git var author-ident I agree that this is nice, but I wonder what we would do for the sequence editor and the default editor. Maybe: git var sequence-editor git var editor That would already be nicer than what we have now, but maybe we should consider the following instead: git var sequence.editor git var core.editor (and maybe also some aliases to core.editor, like: git var default.editor git var editor) I think "sequence.editor" and "core.editor" are better because: - they use the same syntax as the config variables, so they are easier to remember and to discover, and - they provide a notion of context. The notion of context is interesting because suppose that we later introduce the "commit.editor" config variable. If we decide now that "foo.editor" means just "core.editor" if we don't know about any "editor" variable related to the "foo" context, then the scripts that might later be written using "git var commit.editor" will not have to worry about the fact that previous versions of Git didn't know about "commit.editor". People could even start using "git var commit.editor" now, because it would work even if "commit.editor" is unused by git commit. Of course when the user asks for "git var foo.editor" and we don't know about any "editor" variable related to the "foo" context, we first should check if "foo.editor" exists in the config file and we should use that if it exists, before we default to "git var core.editor". Best, Christian. -- 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 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
On Thu, Dec 4, 2014 at 3:08 PM, Michael Haggerty wrote: > We don't actually need the locking functionality, because we already > hold the lock on the reference itself, No. You do need the lock. The ref is locked only during transaction_commit() If you don't want to lock the reflog file and instead rely on the lock on the ref itself you will need to rework your patches so that the lock on the ref is taken already during, for example, transaction_update_ref() instead. But without doing those changes and moving the ref locking from _commit() to _update_ref() you will risk reflog corruption/surprises if two operations collide and both rewrite the reflog without any lock held. > which is how the reflog file is > locked. But the lock_file code still does some of the bookkeeping for > us and is more careful than the old code here was. > > For example: > > * Correctly handle the case that the reflog lock file already exists > for some reason or cannot be opened. > > * Correctly clean up the lockfile if the program dies. > > Signed-off-by: Michael Haggerty > --- > builtin/reflog.c | 37 ++--- > 1 file changed, 22 insertions(+), 15 deletions(-) > > diff --git a/builtin/reflog.c b/builtin/reflog.c > index a282e60..d344d45 100644 > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -349,12 +349,14 @@ static int push_tip_to_list(const char *refname, const > unsigned char *sha1, int > return 0; > } > > +static struct lock_file reflog_lock; > + > static int expire_reflog(const char *refname, const unsigned char *sha1, > void *cb_data) > { > struct cmd_reflog_expire_cb *cmd = cb_data; > struct expire_reflog_cb cb; > struct ref_lock *lock; > - char *log_file, *newlog_path = NULL; > + char *log_file; > struct commit *tip_commit; > struct commit_list *tips; > int status = 0; > @@ -372,10 +374,14 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c > unlock_ref(lock); > return 0; > } > + > log_file = git_pathdup("logs/%s", refname); > if (!cmd->dry_run) { > - newlog_path = git_pathdup("logs/%s.lock", refname); > - cb.newlog = fopen(newlog_path, "w"); > + if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) > + goto failure; > + cb.newlog = fdopen_lock_file(&reflog_lock, "w"); > + if (!cb.newlog) > + goto failure; > } > > cb.cmd = cmd; > @@ -423,10 +429,9 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c > } > > if (cb.newlog) { > - if (fclose(cb.newlog)) { > - status |= error("%s: %s", strerror(errno), > - newlog_path); > - unlink(newlog_path); > + if (close_lock_file(&reflog_lock)) { > + status |= error("Couldn't write %s: %s", log_file, > + strerror(errno)); > } else if (cmd->updateref && > (write_in_full(lock->lock_fd, > sha1_to_hex(cb.last_kept_sha1), 40) != 40 || > @@ -434,21 +439,23 @@ static int expire_reflog(const char *refname, const > unsigned char *sha1, void *c > close_ref(lock) < 0)) { > status |= error("Couldn't write %s", > lock->lk->filename.buf); > - unlink(newlog_path); > - } else if (rename(newlog_path, log_file)) { > - status |= error("cannot rename %s to %s", > - newlog_path, log_file); > - unlink(newlog_path); > + rollback_lock_file(&reflog_lock); > + } else if (commit_lock_file(&reflog_lock)) { > + status |= error("cannot rename %s.lock to %s", > + log_file, log_file); > } else if (cmd->updateref && commit_ref(lock)) { > status |= error("Couldn't set %s", lock->ref_name); > - } else { > - adjust_shared_perm(log_file); > } > } > - free(newlog_path); > free(log_file); > unlock_ref(lock); > return status; > + > + failure: > + rollback_lock_file(&reflog_lock); > + free(log_file); > + unlock_ref(lock); > + return -1; > } > > static int collect_reflog(const char *ref, const unsigned char *sha1, int > unused, void *cb_data) > -- > 2.1.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
error: core.autocrlf=input conflicts with core.eol=crlf
Hi, There seems to be problems with the checks in the git code for conflicts between config values of core.autocrlf and core.eol. Because the various config files are read in separate passes, and the conflict check happens on the fly, it creates a situation where the order of the config file entries matters. This seems like a bug or at least a POLA violation -- ordering of lines within a section of a config file is not usually significant. Example: User has core.autocrlf=input in his ~/.gitconfig. In his project-level .git/config he wants to set core.autocrlf=false and core.eol=crlf. If the core.autocrlf=false comes first, then all is well and no conflict is reported. If the core.eol=crlf line comes first, then at the time this line is getting parsed, core.autocrlf is still set at "input" from ~/.gitconfig, and execution aborts: error: core.autocrlf=input conflicts with core.eol=crlf It seems like the conflict check should be made at the end of the config file parsing, not on the fly. I was tempted to create a patch, however I am unfamilar with the codebase, and didn't understand all the places where the config file parsing is called, so I'm not sure of the ramifications of any proposed change. A benefit of the current approach is that it reports the line number where it aborted in the config file. Trying to retain this and at the same time defer the check until the end could get complicated. Besides interaction between multiple levels of config files, the same sort of ordering issue can arise in conjunction with command-line config overrides. Example: User has core.autocrlf=input in his project-level .git/config. This command works fine: $ git -c core.autocrlf=false -c core.eol=crlf status This command blows up with conflict error: $ git -c core.eol=crlf -c core.autocrlf=false status I tested with git versions 1.9.3 and 2.1.0. Alex -- 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 report on update-index --assume-unchanged
Hi, I add 2 files that I want ignore on commits git update-index --assume-unchanged configurations/local.defs git update-index --assume-unchanged processor/default.defs git diff -a is clean git diff . is clean git commit -a nothing added to commit but git commit . # Changes to be committed: # modified: configurations/local.defs # modified: processor/default.defs this is a bug . thanks -- Sérgio M. B. -- 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/RFC v2] Squashed changes for multiple worktrees vs. submodules
On Thu, Dec 4, 2014 at 10:06 PM, Jens Lehmann wrote: > But I'd need to have separate settings for > our CI server, e.g. to checkout the sources without the > largish documentation submodule in one test job (=worktree) > while checking out the whole documentation for another job > building the setup in another worktree. Currently I'm estimating approach when submodules which have .git file or directory inside are updated, and those which do not have it are not. I have added a config variable submodule.updateIgnoringConfigUrl (because usually the submodule..url is what turns on the update). It looks working, maybe I even add setting the variable when chackout --to is used. > And if I understand the "checkout: reject if the branch is > already checked out elsewhere" thread correctly, I won't be > able to build "master" in two jobs at the same time? You are alerady second person complaining about it, but I don't really see how this can be a problem. Make a branch 'master2', it's another 40 bytes. > So two reasons against using multiple worktrees on our CI > server to save quite some disk space :-( My use is not to save space (working tree files often takes more than the repository itself), but for development, I have like 3-5 checkouts usually, which used to be local clones, but not having to keep synching them is really life changing. > Thanks. But I changed my mind about the details (now that I > know about .git/config and multiple worktrees). I think you'd > have to connect a .git directory in the submodule to the > common git dir directly, as you cannot use the core.worktree > setting (which could be different between commits due to > renames) when putting it into /.git/modules. And > then you couldn't remove or rename a submodule anymore, > because that fails when it contains a .git directory. I need to think more about it. > Seems like we should put a "Warning: may do unexpected things > when used with submodules" (with some examples about what might > happen) in the multiple worktrees documentation. And I don't > believe anymore that teaching submodules to use the common git > dir makes that much sense after I know about the restrictions > it imposes. btw, I thought even about making it an error to add/remove/(de)initialize submodule not in the main working tree. But I'm afraid it would not be considered appropriate for merging. -- Max -- 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 report on update-index --assume-unchanged
Am 05.12.2014 07:12, schrieb Sérgio Basto: Hi, I add 2 files that I want ignore on commits git update-index --assume-unchanged configurations/local.defs git update-index --assume-unchanged processor/default.defs git diff -a is clean git diff . is clean git commit -a nothing added to commit but git commit . # Changes to be committed: # modified: configurations/local.defs # modified: processor/default.defs this is a bug . Actually, it's a user error. When you set --assume-unchanged, then you give a promise to git that you do not change the files, and git does not have to check itself whether there is a change. But since you did not keep your promise, you get what you deserve. ;-) So, to follow-up on your nearby post: --assume-unchanged is *not* a tool to avoid accidentally committing changes to files that are tracked. -- Hannes -- 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