Re: [msysGit] [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Hi Rupert, On 2015-04-23 21:25, rupert thurner wrote: On Thursday, April 16, 2015 at 2:45:11 PM UTC+2, Johannes Schindelin wrote: However, using this code for `getppid()` would be serious overkill (not to mention an unbearable performance hit because you have to enumerate *all* processes to get that information). is the windows JobObject similar to processgroup? at least killing the parent process in a jobobject will kill all childs as well: https://msdn.microsoft.com/en-us/library/windows/desktop/ms681949%28v=vs.85%29.aspx Reading this page carefully reveals that you have to construct JobObjects explicitly. So you cannot get from a process ID to a JobObject; there is probably none. Or there is one. Or many. Ciao, Johannes -- 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 2/2] connect: improve check for plink to reduce false positives
Hi Peff, On 2015-04-23 17:53, Jeff King wrote: What about plink-0.83 that was mentioned earlier in the thread? I was working a lot with Java projects where the base name is often considered to be the part before a version number that is encoded into the file name, so I think it would be a good idea here, too. Essentially, it would be the regex ^(.*[/\\])?plink(-.*|\.exe)$ ... and maybe we should actually do exactly this, use a regex? I know that many developers try to stay away from them, but if you can read them, they are an efficient encoding of expectations. Ciao, Dscho -- 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 2/2] connect: improve check for plink to reduce false positives
Hi Brian, On 2015-04-24 01:14, brian m. carlson wrote: On Thu, Apr 23, 2015 at 11:53:04AM -0400, Jeff King wrote: If I were writing from scratch, I would probably keep things as tight as possible, like: const char *base = basename(ssh); plink = !strcasecmp(base, plink) || !strcasecmp(base, plink.exe); tplink = !strcasecmp(base, tortoiseplink) || !strcasecmp(base, tortoiseplink.exe)); but maybe that is too tight at this point in time; we don't really know what's out there and working (or maybe _we_ do, but _I_ do not :) ). At any rate, brian's patch only looks for a dir-separator anywhere, not the actual basename. So: /path/to/plink/ssh would match, and I'm not sure if that's a good thing or not. This is true. In hindsight, I think it's probably better to be a bit stricter, so I'll reroll to use the stricter format. Thank you so much! Dscho -- 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: About git log
On Mon, Apr 06, 2015 at 10:21:37PM +0800, niu2x wrote: I'm a beginner. Please tell me the log of git commit is exist forever or 90 days As long as a commit is (indirectly) referenced by a branch or tag, it will be kept forever. Only if you rewrite history causing commits to be unreferenced (for example, by using git reset --hard), the commits will be eventually removed. The 90 days you might have heard of, refers to the time the reflog keeps reference to commits so that you can find them back in case you accidentally caused the commit to be unreferenced. Entries in the reflog will expire after 90 days, after which no reference to the commits remain, and the garbage collector will remove those commits. -- 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 3/5] write_ref_sha1(): inline function at callers
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 38 +++--- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/refs.c b/refs.c index 9b68aec..a55d541 100644 --- a/refs.c +++ b/refs.c @@ -2770,8 +2770,10 @@ static int rename_ref_available(const char *oldname, const char *newname) return ret; } -static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, - const char *logmsg); +static int write_ref_to_lockfile(struct ref_lock *lock, +const unsigned char *sha1); +static int commit_ref_update(struct ref_lock *lock, +const unsigned char *sha1, const char *logmsg); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -2829,7 +2831,8 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms goto rollback; } hashcpy(lock-old_sha1, orig_sha1); - if (write_ref_sha1(lock, orig_sha1, logmsg)) { + if (write_ref_to_lockfile(lock, orig_sha1) || + commit_ref_update(lock, orig_sha1, logmsg)) { error(unable to write current sha1 into %s, newrefname); goto rollback; } @@ -2845,7 +2848,8 @@ 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_sha1(lock, orig_sha1, NULL)) + if (write_ref_to_lockfile(lock, orig_sha1) || + commit_ref_update(lock, orig_sha1, NULL)) error(unable to write current sha1 into %s, oldrefname); log_all_ref_updates = flag; @@ -3093,21 +3097,6 @@ static int commit_ref_update(struct ref_lock *lock, return 0; } -/* - * Write sha1 as the new value of the reference specified by the - * (open) lock. On error, roll back the lockfile and set errno - * appropriately. - */ -static int write_ref_sha1(struct ref_lock *lock, - const unsigned char *sha1, const char *logmsg) -{ - if (write_ref_to_lockfile(lock, sha1) || - commit_ref_update(lock, sha1, logmsg)) - return -1; - - return 0; -} - int create_symref(const char *ref_target, const char *refs_heads_master, const char *logmsg) { @@ -3801,15 +3790,18 @@ int ref_transaction_commit(struct ref_transaction *transaction, */ unlock_ref(update-lock); update-lock = NULL; - } else if (write_ref_sha1(update-lock, update-new_sha1, - update-msg)) { - update-lock = NULL; /* freed by write_ref_sha1 */ + } else if (write_ref_to_lockfile(update-lock, +update-new_sha1) || + commit_ref_update(update-lock, +update-new_sha1, +update-msg)) { + update-lock = NULL; /* freed by the above calls */ strbuf_addf(err, Cannot update the ref '%s'., update-refname); ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } else { - /* freed by write_ref_sha1(): */ + /* freed by the above calls: */ update-lock = NULL; } } -- 2.1.4 -- 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 5/5] ref_transaction_commit(): only keep one lockfile open at a time
The old code locked all of the references in the first loop, leaving all of the lockfiles open until later loops. Since we might be updating a lot of references, this could cause file descriptor exhaustion. But there is no reason to keep the lockfiles open after the first loop: * For references being deleted or verified, we don't have to write anything into the lockfile, so we can close it immediately. * For references being updated, we can write the new SHA-1 into the lockfile immediately and close the lockfile without renaming it into place. In the second loop, the already-written contents can be renamed into place using commit_ref_update(). To simplify the bookkeeping across loops, add a new REF_NEEDS_COMMIT bit to update-flags, which keeps track of whether the corresponding lockfile needs to be committed, as opposed to just unlocked. This change fixes two tests in t1400. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c| 68 +-- t/t1400-update-ref.sh | 4 +-- 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index 782e777..2bdd93c 100644 --- a/refs.c +++ b/refs.c @@ -36,25 +36,31 @@ static unsigned char refname_disposition[256] = { * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken * refs (i.e., because the reference is about to be deleted anyway). */ -#define REF_DELETING 0x02 +#define REF_DELETING 0x02 /* * Used as a flag in ref_update::flags when a loose ref is being * pruned. */ -#define REF_ISPRUNING 0x04 +#define REF_ISPRUNING 0x04 /* * Used as a flag in ref_update::flags when the reference should be * updated to new_sha1. */ -#define REF_HAVE_NEW 0x08 +#define REF_HAVE_NEW 0x08 /* * Used as a flag in ref_update::flags when old_sha1 should be * checked. */ -#define REF_HAVE_OLD 0x10 +#define REF_HAVE_OLD 0x10 + +/* + * Used as a flag in ref_update::flags when the lockfile needs to be + * committed. + */ +#define REF_NEEDS_COMMIT 0x20 /* * Try to read one refname component from the front of refname. @@ -3749,7 +3755,12 @@ int ref_transaction_commit(struct ref_transaction *transaction, goto cleanup; } - /* Acquire all locks while verifying old values */ + /* +* Acquire all locks, verify old values if provided, check +* that new values are valid, and write new values to the +* lockfiles, ready to be activated. Only keep one lockfile +* open at a time to avoid running out of file descriptors. +*/ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; @@ -3770,13 +3781,7 @@ int ref_transaction_commit(struct ref_transaction *transaction, update-refname); goto cleanup; } - } - - /* Perform updates first so live commits remain referenced */ - for (i = 0; i n; i++) { - struct ref_update *update = updates[i]; - - if ((update-flags REF_HAVE_NEW) !is_null_sha1(update-new_sha1)) { + if ((update-flags REF_HAVE_NEW) !(update-flags REF_DELETING)) { int overwriting_symref = ((update-type REF_ISSYMREF) (update-flags REF_NODEREF)); @@ -3786,14 +3791,39 @@ int ref_transaction_commit(struct ref_transaction *transaction, * The reference already has the desired * value, so we don't need to write it. */ - unlock_ref(update-lock); - update-lock = NULL; } else if (write_ref_to_lockfile(update-lock, -update-new_sha1) || - commit_ref_update(update-lock, -update-new_sha1, -update-msg)) { - update-lock = NULL; /* freed by the above calls */ +update-new_sha1)) { + update-lock = NULL; /* freed by the above call */ + strbuf_addf(err, Cannot update the ref '%s'., + update-refname); + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } else { + update-flags |= REF_NEEDS_COMMIT; + } + } + if (!(update-flags REF_NEEDS_COMMIT)) { + /* +* We didn't have to write anything to the lockfile. +* Close it to free up the file
[PATCH 0/5] Avoid file descriptor exhaustion in ref_transaction_commit()
In ref_transaction_commit(), close the reference lockfiles immediately to avoid keeping too many file descriptors open at a time. This is pretty easy, because in the first loop (where we create the locks) we already know what, if anything, has to be written into the lockfile. So write it and close the lockfile immediately. In the second loop, rename the lockfiles for reference updates into place, and in the cleanup loop, unlock any references that are still locked (i.e., those that were only being verified or deleted). I think this is a cleaner solution than Stefan's approach [1] of closing and reopening fds based on an estimate of how many fds we can afford to waste--we only need a single file descriptor at a time, and we never have to close then reopen a lockfile. But it is a bit more intrusive, so it might still be preferable to use Stefan's approach for release 2.4.0, if indeed any fix for this problem is still being considered for that release. This patch series applies on top of Stefan's c1f0ca9 refs.c: remove lock_fd from struct ref_lock (2015-04-16) and it fixes two tests that Stefan introduced earlier in that series. It is also available from my GitHub account: https://github.com/mhagger/git branch close-ref-locks-promptly Michael [1] http://article.gmane.org/gmane.comp.version-control.git/267548 Michael Haggerty (5): write_ref_to_lockfile(): new function, extracted from write_ref_sha1() commit_ref_update(): new function, extracted from write_ref_sha1() write_ref_sha1(): inline function at callers ref_transaction_commit(): remove the local flags variables ref_transaction_commit(): only keep one lockfile open at a time refs.c| 107 ++ t/t1400-update-ref.sh | 4 +- 2 files changed, 75 insertions(+), 36 deletions(-) -- 2.1.4 -- 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 1/5] write_ref_to_lockfile(): new function, extracted from write_ref_sha1()
This is the first step towards separating the checking and writing of the new reference value to committing the change. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 4f495bd..72dae21 100644 --- a/refs.c +++ b/refs.c @@ -3017,11 +3017,10 @@ int is_branch(const char *refname) } /* - * Write sha1 into the ref specified by the lock. Make sure that errno - * is sane on error. + * Write sha1 into the open lockfile, then close the lockfile. On + * errors, rollback the lockfile and set errno to reflect the problem. */ -static int write_ref_sha1(struct ref_lock *lock, - const unsigned char *sha1, const char *logmsg) +static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1) { static char term = '\n'; struct object *o; @@ -3050,6 +3049,19 @@ static int write_ref_sha1(struct ref_lock *lock, errno = save_errno; return -1; } + return 0; +} + +/* + * Write sha1 into the ref specified by the lock. Make sure that errno + * is sane on error. + */ +static int write_ref_sha1(struct ref_lock *lock, + const unsigned char *sha1, const char *logmsg) +{ + if (write_ref_to_lockfile(lock, sha1)) + return -1; + clear_loose_ref_cache(ref_cache); if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 || (strcmp(lock-ref_name, lock-orig_ref_name) -- 2.1.4 -- 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 2/5] commit_ref_update(): new function, extracted from write_ref_sha1()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 26 +- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index 72dae21..9b68aec 100644 --- a/refs.c +++ b/refs.c @@ -3052,16 +3052,9 @@ static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha return 0; } -/* - * Write sha1 into the ref specified by the lock. Make sure that errno - * is sane on error. - */ -static int write_ref_sha1(struct ref_lock *lock, - const unsigned char *sha1, const char *logmsg) +static int commit_ref_update(struct ref_lock *lock, +const unsigned char *sha1, const char *logmsg) { - if (write_ref_to_lockfile(lock, sha1)) - return -1; - clear_loose_ref_cache(ref_cache); if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 || (strcmp(lock-ref_name, lock-orig_ref_name) @@ -3100,6 +3093,21 @@ static int write_ref_sha1(struct ref_lock *lock, return 0; } +/* + * Write sha1 as the new value of the reference specified by the + * (open) lock. On error, roll back the lockfile and set errno + * appropriately. + */ +static int write_ref_sha1(struct ref_lock *lock, + const unsigned char *sha1, const char *logmsg) +{ + if (write_ref_to_lockfile(lock, sha1) || + commit_ref_update(lock, sha1, logmsg)) + return -1; + + return 0; +} + int create_symref(const char *ref_target, const char *refs_heads_master, const char *logmsg) { -- 2.1.4 -- 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 4/5] ref_transaction_commit(): remove the local flags variables
Instead, work directly with update-flags. This has the advantage that the REF_DELETING bit, set in the first loop, can be read in the third loop instead of having to compute the same expression again. Plus, it was kindof confusing having both update-flags and flags, which sometimes had different values. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index a55d541..782e777 100644 --- a/refs.c +++ b/refs.c @@ -3752,16 +3752,15 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Acquire all locks while verifying old values */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; - unsigned int flags = update-flags; - if ((flags REF_HAVE_NEW) is_null_sha1(update-new_sha1)) - flags |= REF_DELETING; + if ((update-flags REF_HAVE_NEW) is_null_sha1(update-new_sha1)) + update-flags |= REF_DELETING; update-lock = lock_ref_sha1_basic( update-refname, ((update-flags REF_HAVE_OLD) ? update-old_sha1 : NULL), NULL, - flags, + update-flags, update-type); if (!update-lock) { ret = (errno == ENOTDIR) @@ -3776,9 +3775,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Perform updates first so live commits remain referenced */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; - int flags = update-flags; - if ((flags REF_HAVE_NEW) !is_null_sha1(update-new_sha1)) { + if ((update-flags REF_HAVE_NEW) !is_null_sha1(update-new_sha1)) { int overwriting_symref = ((update-type REF_ISSYMREF) (update-flags REF_NODEREF)); @@ -3810,15 +3808,14 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Perform deletes now that updates are safely completed */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; - int flags = update-flags; - if ((flags REF_HAVE_NEW) is_null_sha1(update-new_sha1)) { + if (update-flags REF_DELETING) { if (delete_ref_loose(update-lock, update-type, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } - if (!(flags REF_ISPRUNING)) + if (!(update-flags REF_ISPRUNING)) string_list_append(refs_to_delete, update-lock-ref_name); } -- 2.1.4 -- 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: git-p4 Question
On 23/04/15 14:42, FusionX86 wrote: Hi Luke, I found a silly mistake I was making in the command I've been using. The folder under the depot should have been capitalized, but it wasn't. Also, I expected that if there was a problem with the command, it would fail with some message instead of creating an empty local git repo. I would expect that as well - it will usually create the empty git repo, but it should then fail with an error message, like this: $ git p4 clone //depot/main/nosuchpath Importing from //depot/main/nosuchpath into nosuchpath Initialized empty Git repository in /home/lgd/p4-hacking/git/nosuchpath/.git/ Doing initial import of //depot/main/nosuchpath/ from revision #head into refs/remotes/p4/master p4 returned an error: //depot/main/nosuchpath/...#head - no such file(s). $ echo $? 1 If you get a moment can you send your command output; if it's not doing something like the above, then it's a bug. Thanks! Luke -- 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] fast-import: add options to enable/disable case folding
On 18/04/15 08:36, Mike Hommey wrote: On Fri, Apr 17, 2015 at 11:44:00AM -0700, Junio C Hamano wrote: So perhaps we should rip the case folding out altogether instead? The entry for the change in the Release Notes may say: * git fast-import incorrectly case-folded the paths recorded in the history when core.ignorease is set (i.e. the repository's working tree is incapable of expressing paths that differ only in their cases); this old bug was reported in 2012 and was finally corrected. or something like that? Is anything else then git-p4 known to rely on case folding? If not, I guess that's a reasonable plan. We could even add an option to fast-import that would allow to turn case folding back on, and make git-p4 use it, so that its expectations are fulfilled. Although at some point, it could (should?) do case folding itself(?) git-p4 has a single line of code that checks if core.ignorecase is turned on, and uses this to decide whether to skip files that are outside the depot being tracked and I *think* is not really related to fast-import. I don't know to what extent though git-p4 relies on the current behaviour of git fast-import to fold case for it. There's a 'p4 info' command which tells you what the server thinks it's doing: $ p4 info | grep Case Case Handling: sensitive I don't know how long that support has been present (it might not work on older servers that some people are still using). It's also possible to force the server to be case-insensitive on the Linux version. That's useful, as it we could construct some test cases to see what we're likely to break without having to force people to install a case-insensitive OS in order to run the git regression tests. Luke Mike -- 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 -- 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: git-completion.tcsh
Hi, Quoting Marc Khouzam marc.khou...@gmail.com: Hi, I did notice the problem a while ago and had traced it back to the fact that the bash completion scripts no longer adds the trailing '/' at the end of directories. Tcsh needs that '/' to know not to add that annoying extra space. Bash 3 needed to put it that trailing '/' but bash 4 did not. Two years ago (!) changes were made in commit 3ffa4df4b2a26768938fc6bf1ed0640885b2bdf1 to allow bash 3 to work without the trailing '/'. That caused the problem in the tcsh script. The thing is that with master of today, I don't see the problem any more. I can't tell you when it started working again. What is interesting is that the reason it now works is that the git-completion.bash script no longer returns anything for the case you mention: git add ftab Instead, it seems to rely on file completion only. I can't reproduce it with git-completion.bash from current master on its own on with bash 3.1.20(4) from MSysGit, it seems to work as intended here wrt tracked-file-aware file completion. Set up test repo with these commands: git init tracked git add tracked non-tracked mkdir -p foo/bar foo/bar/somefile.c Now let's see what happens with 'git add': $ git add TAB foo/ non-tracked Note, that the file 'tracked' is not offered, so this is clearly not standard bash file completion, but our completion script. Also note the trailing '/' in 'foo/'. $ git add fTAB Just completes to 'git add foo/', no space after '/'. Add the file: $ git add foo/bar/somefile.c Now let's see 'git rm': $ git rm TAB foo/ tracked Note, that the file 'non-tracked' is not offered, so again this comes from our bash completion script. Did you test the bash completion script on its own, or only through the tcsh wrapper? I'm on MSysGit now, so no tcsh or bash v4 at hand, and no time either, so can't dig further at the moment. Gábor -- 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: URL not displaying change made with git.
Scott Meyer dutch...@gmail.com writes: When I use my browser to bring up the site the change is not visible. Which site are you talking about? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
URL not displaying change made with git.
I used git in the following steps. This is a local directory on a Mac, with Yosemite, using the latest git version. The directory name is development. using eclipse I created a branch WO_1 I made a change to the file eclipse indicates the change I use the plus to add it to the Index I commit the change I go to the development directory using the terminal and checkout branch WO_1 using git status shows I am in branch WO_1 I used nano on the file to verify the change is there. When I use my browser to bring up the site the change is not visible. I have cleared the cache in the browser and have tried other browsers. What I am I missing? -- 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: URL not displaying change made with git.
[ Please, don't top-post on this list ] Scott Meyer dutch...@gmail.com writes: The site is local to my laptop. I am attempting to establish a development environment where I can view and test changes before moving to production. OK, so you're doing web development, right? Then, the problem is unrelated from Git: your webserver looks at files on disk, not at the Git repository (i.e. whether your changes are staged in the index, commited, or totally outside Git does not change the result). My guess would be that you have two clones of the same project. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: URL not displaying change made with git.
The site is local to my laptop. I am attempting to establish a development environment where I can view and test changes before moving to production. On Fri, Apr 24, 2015 at 8:50 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Scott Meyer dutch...@gmail.com writes: When I use my browser to bring up the site the change is not visible. Which site are you talking about? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [PATCHv3] refs.c: enable large transactions
On Fri, Apr 24, 2015 at 11:12 AM, Jonathan Nieder jrnie...@gmail.com wrote: Stefan Beller wrote: I understood that you were talking about Could being capitalized. Though it's distributed 30/70 which hints to me we do not care in enough to explain the capitalized letters as slip-throughs on review or such, but it looks like the authors choice to me. Lowercase, brief, with no period at the end is the convention. This is user-facing UI, so consistency matters. It's not author's choice. Existing examples of capitalized messages fall into a few categories: - shell scripts used to tend to use the capitalized form, and when they got rewritten as builtins the messages weren't cleaned up at the same time - some patch authors have different habits and it wasn't worth going back and forth to fix it (but the convention still stands, and the result of a program that can't decide how to talk to the user is still harmful) - once there are a few examples, they get copy/pasted and imitated I think it's a mistake to s/Could/could/g for all errors in the code base as it reduces the information provided in the error messages. This seems like an after-the-fact justification for a mistake. Often there is a choice about whether the caller or the callee to a function prints an error. If the caller does, it can say more about the context. If the callee does, the message is in one place and can be tweaked more easily to be more useful. To get the benefits of both, we could print a backtrace with every error. That way, the callee can describe the error in one place but the context is all there. But I'm really glad we don't do that! The main purpose of most error messages is instructional: they give information to the user in a way that is abstracted from the implementation (the user doesn't care what function it happened in!) that tells them what happened and what they can do next. Gratuitous inconsistency in formatting doesn't help with that. I agree we should fix the formatting, but I was pointing out the side effects and how to avoid the side effects. So what I am proposing is a cleanup of the warning messages as well as a GIT_TRACE_ERRORS variable as Jeff proposed, because then we have all the benefits. If we were to just cleanup the error messages we improve on certain aspects (UI), while making others worse. Actually, I wouldn't mind an environment variable that tells error() to include a backtrace if someone wants it. (See backtrace(3) for implementation hints if interested in doing that.) CONFORMING TO These functions are GNU extensions. I guess I can live with backtrace, but the Git for Windows people may hate me for it. Just 3 days ago (Regular Rebase Failure). I used different capitalization to get a better understanding where the error may be. Wouldn't it be better if there weren't so many similar messages produced in different contexts in the first place? (I.e., this suggests to me that we should move more in the direction of callee-produces-error.) Sorry, that was a long way to say very little. I just wanted to emphasize that when a UI inconsistency has a useful side effect, while that can be useful to understand and learn from, we should not be afraid to fix it. And to clear up any ambiguity about git's error message convention. :) Thanks and hope that helps, I really appreciate the background information provided! Thanks, Stefan 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: [PATCHv3] refs.c: enable large transactions
Stefan Beller wrote: I understood that you were talking about Could being capitalized. Though it's distributed 30/70 which hints to me we do not care in enough to explain the capitalized letters as slip-throughs on review or such, but it looks like the authors choice to me. Lowercase, brief, with no period at the end is the convention. This is user-facing UI, so consistency matters. It's not author's choice. Existing examples of capitalized messages fall into a few categories: - shell scripts used to tend to use the capitalized form, and when they got rewritten as builtins the messages weren't cleaned up at the same time - some patch authors have different habits and it wasn't worth going back and forth to fix it (but the convention still stands, and the result of a program that can't decide how to talk to the user is still harmful) - once there are a few examples, they get copy/pasted and imitated I think it's a mistake to s/Could/could/g for all errors in the code base as it reduces the information provided in the error messages. This seems like an after-the-fact justification for a mistake. Often there is a choice about whether the caller or the callee to a function prints an error. If the caller does, it can say more about the context. If the callee does, the message is in one place and can be tweaked more easily to be more useful. To get the benefits of both, we could print a backtrace with every error. That way, the callee can describe the error in one place but the context is all there. But I'm really glad we don't do that! The main purpose of most error messages is instructional: they give information to the user in a way that is abstracted from the implementation (the user doesn't care what function it happened in!) that tells them what happened and what they can do next. Gratuitous inconsistency in formatting doesn't help with that. Actually, I wouldn't mind an environment variable that tells error() to include a backtrace if someone wants it. (See backtrace(3) for implementation hints if interested in doing that.) Just 3 days ago (Regular Rebase Failure). I used different capitalization to get a better understanding where the error may be. Wouldn't it be better if there weren't so many similar messages produced in different contexts in the first place? (I.e., this suggests to me that we should move more in the direction of callee-produces-error.) Sorry, that was a long way to say very little. I just wanted to emphasize that when a UI inconsistency has a useful side effect, while that can be useful to understand and learn from, we should not be afraid to fix it. And to clear up any ambiguity about git's error message convention. :) Thanks and hope that helps, 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 v2 01/16] refs: convert struct ref_entry to use struct object_id
On Wed, Apr 22, 2015 at 05:52:09PM -0700, Stefan Beller wrote: So you're switching the code for a possible future In an earlier series/cover letter you wrote The goal of this series to improve type-checking in the codebase and to make it easier to move to a different hash function if the project decides to do that. This series does not convert all of the codebase, but only parts. I've dropped some of the patches from earlier (which no longer apply) and added others. Which yields the question if you also want to take care of the error message (It may not be a SHA1 any more but some $HASHFUNCTION)? This is true. However, I'll clean those up with a future patch series when we get to that point. I'll need to pass through the documentation as well to make it accurate and consistent, and I'll want to discuss the words we want to use before I make those changes. That said I'll focus on the type checking part in this review and not annotate the SHA1s I find any more. ;) Please do comment on any hardcoded 20s or 40s (or quantities based off those), as I do want to fix those up. I want to fix up any hardcoded assumptions we may have about the hash that don't involve text or documentation at this point, if only for maintainability reasons. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 4/5] ref_transaction_commit(): remove the local flags variables
On 04/24/2015 11:51 PM, Eric Sunshine wrote: On Fri, Apr 24, 2015 at 7:35 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Instead, work directly with update-flags. This has the advantage that the REF_DELETING bit, set in the first loop, can be read in the third loop instead of having to compute the same expression again. Plus, it was kindof confusing having both update-flags and flags, which s/kindof/kind of/ sometimes had different values. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Hehe thanks for looking over my scribblings. In this case, neither kindof nor kind of is in fact correct English. I used the slang word kindof (sometimes spelled kinda) to mean somewhat, I guess because somewhat sounded too formal for my slapdash opinion. But I suppose it's kindof thoughtless to use slang in commit messages :-), especially given that they are also meant for people for whom English is a second language (let alone sloppy American English). I suggest s/kindof/potentially/, at least until I have time to submit a patch to the English language to make kindof a reputable word :-) 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 v2 1/2] connect: simplify SSH connection code path
The code path used in git_connect pushed the majority of the SSH connection code into an else block, even though the if block returns. Simplify the code by eliminating the else block, as it is unneeded. Signed-off-by: Jeff King p...@peff.net Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- connect.c | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/connect.c b/connect.c index 391d211..749a07b 100644 --- a/connect.c +++ b/connect.c @@ -743,28 +743,28 @@ struct child_process *git_connect(int fd[2], const char *url, free(path); free(conn); return NULL; - } else { - ssh = getenv(GIT_SSH_COMMAND); - if (ssh) { - conn-use_shell = 1; - putty = 0; - } else { - ssh = getenv(GIT_SSH); - if (!ssh) - ssh = ssh; - putty = !!strcasestr(ssh, plink); - } - - argv_array_push(conn-args, ssh); - if (putty !strcasestr(ssh, tortoiseplink)) - argv_array_push(conn-args, -batch); - if (port) { - /* P is for PuTTY, p is for OpenSSH */ - argv_array_push(conn-args, putty ? -P : -p); - argv_array_push(conn-args, port); - } - argv_array_push(conn-args, ssh_host); } + + ssh = getenv(GIT_SSH_COMMAND); + if (ssh) { + conn-use_shell = 1; + putty = 0; + } else { + ssh = getenv(GIT_SSH); + if (!ssh) + ssh = ssh; + putty = !!strcasestr(ssh, plink); + } + + argv_array_push(conn-args, ssh); + if (putty !strcasestr(ssh, tortoiseplink)) + argv_array_push(conn-args, -batch); + if (port) { + /* P is for PuTTY, p is for OpenSSH */ + argv_array_push(conn-args, putty ? -P : -p); + argv_array_push(conn-args, port); + } + argv_array_push(conn-args, ssh_host); } else { /* remove repo-local variables from the environment */ conn-env = local_repo_env; -- 2.3.5 -- 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 2/2] connect: improve check for plink to reduce false positives
The git_connect function has code to handle plink and tortoiseplink specially, as they require different command line arguments from OpenSSH. However, the match was done by checking for plink case-insensitively in the string, which led to false positives when GIT_SSH contained uplink. Improve the check by looking for plink or tortoiseplink (or those names suffixed with .exe) in the final component of the path. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- connect.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/connect.c b/connect.c index 749a07b..c0144d8 100644 --- a/connect.c +++ b/connect.c @@ -724,7 +724,7 @@ struct child_process *git_connect(int fd[2], const char *url, conn-in = conn-out = -1; if (protocol == PROTO_SSH) { const char *ssh; - int putty; + int putty, tortoiseplink = 0; char *ssh_host = hostandport; const char *port = NULL; get_host_and_port(ssh_host, port); @@ -750,14 +750,26 @@ struct child_process *git_connect(int fd[2], const char *url, conn-use_shell = 1; putty = 0; } else { + const char *base; + char *ssh_dup; + ssh = getenv(GIT_SSH); if (!ssh) ssh = ssh; - putty = !!strcasestr(ssh, plink); + + ssh_dup = xstrdup(ssh); + base = basename(ssh_dup); + + tortoiseplink = !strcasecmp(base, tortoiseplink) || + !strcasecmp(base, tortoiseplink.exe); + putty = !strcasecmp(base, plink) || + !strcasecmp(base, plink.exe) || tortoiseplink; + + free(ssh_dup); } argv_array_push(conn-args, ssh); - if (putty !strcasestr(ssh, tortoiseplink)) + if (tortoiseplink) argv_array_push(conn-args, -batch); if (port) { /* P is for PuTTY, p is for OpenSSH */ -- 2.3.5 -- 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 2/2] connect: improve check for plink to reduce false positives
On Fri, Apr 24, 2015 at 03:46:30PM -0700, Pete Harlan wrote: On Fri, Apr 24, 2015 at 3:28 PM, brian m. carlson sand...@crustytoothpaste.net wrote: The git_connect function has code to handle plink and tortoiseplink specially, as they require different command line arguments from OpenSSH. However, the match was done by checking for plink case-insensitively in the string, which led to false positives when Perhaps s/case-insensitively/anywhere/? It's not important that it ignored case (your change ignores it too), it's that it was too lenient about its context. Yes, I don't know what I was thinking. I'll wait a bit to see if there are any more comments and then reroll. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: Verbose as default for commit (optional)
Ok, now I found [this thread](http://thread.gmane.org/gmane.comp.version-control.git/251376) that seems abandoned, but implements this config, a --no-verbose that disable it for one-time and the tests, but was not merged (don't know why) This was the patch I've intended to attach: -8 Subject: [PATCH] Add commit.verbose config to set default. --- builtin/commit.c | 4 1 file changed, 4 insertions(+) diff --git a/builtin/commit.c b/builtin/commit.c index da79ac4..ad588ff 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1506,6 +1506,10 @@ static int git_commit_config(const char *k, const char *v, void *cb) sign_commit = git_config_bool(k, v) ? : NULL; return 0; } + if (!strcmp(k, commit.verbose)) { + verbose = git_config_bool(k, v); + return 0; + } status = git_gpg_config(k, v, NULL); if (status) -- 2.1.4 En Fri, Apr 24, 2015 at 10:03:14PM +0200, Matthieu Moy escribió: Eloy Espinaco eloy...@gmail.com writes: Hi, It is my first mail to the list, so hello world. Hi, and welcome to the list. I wanted to make a feature-request about a config setting to make the commit always verbose. I'm not the only one asking for that, there is an old question in [Stack Overflow][1]. This seems a reasonable addition. In general, we commonly have config options for commonly used CLI options. So I was thinking if it was possible to make a pull request for that, so I attach the patch. (I'm proud of it :) ). Nice try, but the attached file is empty ;-). Actually, as much as possible, avoid sending attachments but prefer inline patches. You'll need a bit of reading to send a proper patch: https://github.com/git/git/blob/master/Documentation/SubmittingPatches -- Matthieu Moy http://www-verimag.imag.fr/~moy/ --- Eloy Espinaco -- 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 2/2] connect: improve check for plink to reduce false positives
On Fri, Apr 24, 2015 at 3:28 PM, brian m. carlson sand...@crustytoothpaste.net wrote: The git_connect function has code to handle plink and tortoiseplink specially, as they require different command line arguments from OpenSSH. However, the match was done by checking for plink case-insensitively in the string, which led to false positives when Perhaps s/case-insensitively/anywhere/? It's not important that it ignored case (your change ignores it too), it's that it was too lenient about its context. --Pete GIT_SSH contained uplink. Improve the check by looking for plink or tortoiseplink (or those names suffixed with .exe) in the final component of the path. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- connect.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/connect.c b/connect.c index 749a07b..c0144d8 100644 --- a/connect.c +++ b/connect.c @@ -724,7 +724,7 @@ struct child_process *git_connect(int fd[2], const char *url, conn-in = conn-out = -1; if (protocol == PROTO_SSH) { const char *ssh; - int putty; + int putty, tortoiseplink = 0; char *ssh_host = hostandport; const char *port = NULL; get_host_and_port(ssh_host, port); @@ -750,14 +750,26 @@ struct child_process *git_connect(int fd[2], const char *url, conn-use_shell = 1; putty = 0; } else { + const char *base; + char *ssh_dup; + ssh = getenv(GIT_SSH); if (!ssh) ssh = ssh; - putty = !!strcasestr(ssh, plink); + + ssh_dup = xstrdup(ssh); + base = basename(ssh_dup); + + tortoiseplink = !strcasecmp(base, tortoiseplink) || + !strcasecmp(base, tortoiseplink.exe); + putty = !strcasecmp(base, plink) || + !strcasecmp(base, plink.exe) || tortoiseplink; + + free(ssh_dup); } argv_array_push(conn-args, ssh); - if (putty !strcasestr(ssh, tortoiseplink)) + if (tortoiseplink) argv_array_push(conn-args, -batch); if (port) { /* P is for PuTTY, p is for OpenSSH */ -- 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 02/16] refs: convert for_each_tag_ref to struct object_id
On Thu, Apr 23, 2015 at 03:27:23PM -0400, Jeff King wrote: +static int do_for_each_ref(struct ref_cache *refs, const char *base, +each_ref_fn fn, int trim, int flags, void *cb_data) +{ + return do_for_each_ref_generic(refs, base, fn, NULL, trim, flags, cb_data); +} + +static int do_for_each_ref_oid(struct ref_cache *refs, const char *base, +each_ref_fn_oid fn, int trim, int flags, void *cb_data) +{ + return do_for_each_ref_generic(refs, base, NULL, fn, trim, flags, cb_data); +} + static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data) { unsigned char sha1[20]; You can even dispense with the _oid variant wrapper, and just call into the generic version directly from the new callsites. Sounds like a good idea. I'll reroll with that change probably sometime this weekend. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: git-p4 Question
I get an error if I misspell part of the path. For example, if I type //depot/maain instead of //depot/main I will get the no such files message you indicated. BUT using incorrect case like //depot/main instead of //depot/Main doesn't return any error, but still completes and creates an empty repo. If it does require correct case, then it should throw an error for //depot/main as well. Let me know if you need any additional information. On Fri, Apr 24, 2015 at 3:20 AM, Luke Diamand l...@diamand.org wrote: On 23/04/15 14:42, FusionX86 wrote: Hi Luke, I found a silly mistake I was making in the command I've been using. The folder under the depot should have been capitalized, but it wasn't. Also, I expected that if there was a problem with the command, it would fail with some message instead of creating an empty local git repo. I would expect that as well - it will usually create the empty git repo, but it should then fail with an error message, like this: $ git p4 clone //depot/main/nosuchpath Importing from //depot/main/nosuchpath into nosuchpath Initialized empty Git repository in /home/lgd/p4-hacking/git/nosuchpath/.git/ Doing initial import of //depot/main/nosuchpath/ from revision #head into refs/remotes/p4/master p4 returned an error: //depot/main/nosuchpath/...#head - no such file(s). $ echo $? 1 If you get a moment can you send your command output; if it's not doing something like the above, then it's a bug. Thanks! Luke -- 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: git-completion.tcsh
On Fri, Apr 24, 2015 at 7:30 AM, SZEDER Gábor sze...@ira.uka.de wrote: Hi, Quoting Marc Khouzam marc.khou...@gmail.com: Hi, I did notice the problem a while ago and had traced it back to the fact that the bash completion scripts no longer adds the trailing '/' at the end of directories. Tcsh needs that '/' to know not to add that annoying extra space. Bash 3 needed to put it that trailing '/' but bash 4 did not. Two years ago (!) changes were made in commit 3ffa4df4b2a26768938fc6bf1ed0640885b2bdf1 to allow bash 3 to work without the trailing '/'. That caused the problem in the tcsh script. The thing is that with master of today, I don't see the problem any more. I can't tell you when it started working again. What is interesting is that the reason it now works is that the git-completion.bash script no longer returns anything for the case you mention: git add ftab Instead, it seems to rely on file completion only. I can't reproduce it with git-completion.bash from current master on its own on with bash 3.1.20(4) from MSysGit, it seems to work as intended here wrt tracked-file-aware file completion. Set up test repo with these commands: git init tracked git add tracked non-tracked mkdir -p foo/bar foo/bar/somefile.c Now let's see what happens with 'git add': $ git add TAB foo/ non-tracked Note, that the file 'tracked' is not offered, so this is clearly not standard bash file completion, but our completion script. Also note the trailing '/' in 'foo/'. $ git add fTAB Just completes to 'git add foo/', no space after '/'. Add the file: $ git add foo/bar/somefile.c Now let's see 'git rm': $ git rm TAB foo/ tracked Note, that the file 'non-tracked' is not offered, so again this comes from our bash completion script. Did you test the bash completion script on its own, or only through the tcsh wrapper? I'm on MSysGit now, so no tcsh or bash v4 at hand, and no time either, so can't dig further at the moment. Thanks Gábor, I can see the behaviour you describe now. I was running the new bash completion with an old git and that had an impact. With a recent git installation I can see the problem, which includes that with tcsh, the / at the end of foo is missing. I had a patch for that a while ago that I can revive and post soon. Thanks again! Marc -- 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: [PATCHv3] refs.c: enable large transactions
On Fri, Apr 24, 2015 at 09:23:16PM -0700, Junio C Hamano wrote: The proposals so far, including this one, assume that the bug reporter will first fail the operation with normal invocation of Git (e.g. without GIT_DIE_ABORT exported) and then retry the same operation in a different way (e.g. with GIT_DIE_ABORT) to give us something that would help diagnosis. Such a user could just rerun Git under gdb with breakpoint set to die_builtin, no? Good point. I was trying to automate the gathering of the backtrace so that even bug-reporters who have not used gdb could easily get us more information. But of course if a coredump only gets us halfway there and we have to script gdb to convert the core into a backtrace anyway, it is not buying us much over just scripting gdb in the first place. A better solution to what I proposed earlier is perhaps: git config alias.debug '!gdb --quiet \ -ex break exit \ -ex run \ -ex bt full \ -ex continue \ -ex quit \ --args git \ ' git debug rev-parse foobar It has the minor irritation that gdb will control the process stdio (slurping from stdin and polluting stdout, whereas we would prefer no input and output to stderr). There might be a way to convince gdb to do otherwise, or you could probably go pretty far with some shell fd redirects and using set args inside gdb. Or maybe something with gdbserver. But the point is that yeah, we shouldn't try to build really good introspection inside git. Debuggers already do a way better job of this. If they're hard for people to use to obtain simple information like a backtrace, we should work on wrapping that difficulty up in a script. It might still be useful to provide a much lesser form of introspection, if it would be available in a lot more places than gdb would. E.g., __FILE__ and __LINE__ markers on error messages might be useful. A mediocre backtrace() that is only available on glibc systems is probably not. -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: [PATCHv3] refs.c: enable large transactions
On Sat, Apr 25, 2015 at 01:00:58AM -0400, Jeff King wrote: A better solution to what I proposed earlier is perhaps: git config alias.debug '!gdb --quiet \ -ex break exit \ -ex run \ -ex bt full \ -ex continue \ -ex quit \ --args git \ ' git debug rev-parse foobar It has the minor irritation that gdb will control the process stdio (slurping from stdin and polluting stdout, whereas we would prefer no input and output to stderr). There might be a way to convince gdb to do otherwise, or you could probably go pretty far with some shell fd redirects and using set args inside gdb. Or maybe something with gdbserver. Just to extend the thought exercise, here is something marginally less horrible. Save as git-debug in your PATH and chmod +x. -- 8 -- #!/bin/sh if ! type gdb /dev/null 21; then echo 2 Sorry, you don't seem to have gdb installed. exit 1 fi args= for i in $@; do args=$args '$(printf '%s' $i | sed s/'/'''/)' done gdb --quiet \ -ex set args $args 7 8 29 70 81 92 /dev/null 2 \ -ex 'break exit' \ -ex 'run' \ -ex 'bt full' \ -ex 'continue' \ -ex 'quit' \ git -- 8 -- It's still rather hard to use with sub-programs started by git (e.g., the upload-pack spawned by a fetch), but I think it would work for many simple cases. I'm not sure if I would use it myself. As somebody confident in using gdb, my next step after seeing the backtrace would be to start poking around interactively. Besides the stdio magic, this is not really buying me much beyond running gdb myself. -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
bug report : 2.3.5 ssh repo not found
Hello, Using git version 2.3.5 with kernel 3.19.3-3-ARCH #1 SMP PREEMPT x86_64 I see the following error message when pulling or cloning a repo over ssh: git clone ssh://user@mydomain:/home/user/path/to/project.git Cloning into 'project'... ssh: Could not resolve hostname mydomain:: Name or service not known fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. Obviously I changed the url to hide credential info After ensuring DNS was OK and being able to ssh to that instance directly I tried downgrading git to my distro's last installed version of git version 2.2.2 and now I can clone / pull / push to/from that repo without issue. Is this a bug? Best, Chris -- 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] Clarify documentation on commit message strip
When using `-v` to add a unified diff to the commit message, this is stripped and not a part of the commit message. This is not mentioned. Add a note about this with the `-v` description and slightly modify the description for the default `--cleanup` mode. Signed-off-by: Fredrik Gustafsson iv...@iveqy.com --- I'd prefer the description not to be _too_ explicit e.g. by mentioning unified diff, etc. Personally I think it is sufficient to do s/#comment/comment/ to the existing text, without doing anything else. What is commentary to be removed is fairly clear in the contents given to the user in the editor. I agree that it is very clear once you do edit the commit message. My main point with this patch was to clarify -v, since it's not obvious from the documentation that it will be removed. I've no objections about your suggestions about the `strip` part. I hope I've understood you correctly and that this patch is correct. Documentation/git-commit.txt | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 617e29b..1db4c7f 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -180,8 +180,8 @@ OPTIONS + -- strip:: - Strip leading and trailing empty lines, trailing whitespace, and - #commentary and collapse consecutive empty lines. + Strip leading and trailing empty lines, trailing whitespace, + commentary and collapse consecutive empty lines. whitespace:: Same as `strip` except #commentary is not removed. verbatim:: @@ -283,7 +283,8 @@ configuration variable documented in linkgit:git-config[1]. Show unified diff between the HEAD commit and what would be committed at the bottom of the commit message template. Note that this diff output doesn't have its - lines prefixed with '#'. + lines prefixed with '#'. This diff will not be a part + of the commit message. + If specified twice, show in addition the unified diff between what would be committed and the worktree files, i.e. the unstaged -- 2.1.4 -- 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: [PATCHv3] refs.c: enable large transactions
Jeff King p...@peff.net writes: So if anything, I think my inclination would be to make it easier to help people (and ourselves) get a backtrace from gdb. One can get a core for a running process with gcore, or trigger a coredump by killing with SIGABRT. But catching it at the exact moment of a die() is a bit hard without support from the program. What about something like this: diff --git a/usage.c b/usage.c index ed14645..fa92190 100644 --- a/usage.c +++ b/usage.c @@ -34,6 +34,8 @@ static NORETURN void usage_builtin(const char *err, va_list params) static NORETURN void die_builtin(const char *err, va_list params) { vreportf(fatal: , err, params); + if (git_env_bool(GIT_DIE_ABORT, 0)) + abort(); exit(128); } Two comments. There probably are more than a few places that calls exit(128) without using die(), upon seeing that some helper function returned an error code, because the helper already gave an error message. The proposals so far, including this one, assume that the bug reporter will first fail the operation with normal invocation of Git (e.g. without GIT_DIE_ABORT exported) and then retry the same operation in a different way (e.g. with GIT_DIE_ABORT) to give us something that would help diagnosis. Such a user could just rerun Git under gdb with breakpoint set to die_builtin, no? -- 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 0/5] Avoid file descriptor exhaustion in ref_transaction_commit()
Michael Haggerty mhag...@alum.mit.edu writes: In ref_transaction_commit(), close the reference lockfiles immediately to avoid keeping too many file descriptors open at a time. This is pretty easy, because in the first loop (where we create the locks) we already know what, if anything, has to be written into the lockfile. Nicely analysed and beautifully executed. I like it. This patch series applies on top of Stefan's c1f0ca9 refs.c: remove lock_fd from struct ref_lock (2015-04-16) and it fixes two tests that Stefan introduced earlier in that series. 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] Clarify documentation on commit message strip
Fredrik Gustafsson iv...@iveqy.com writes: diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 617e29b..e31d828 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -180,8 +180,9 @@ OPTIONS + -- strip:: - Strip leading and trailing empty lines, trailing whitespace, and - #commentary and collapse consecutive empty lines. + Strip leading and trailing empty lines, trailing whitespace, + #commentary, unified diff added with `-v` and collapse + consecutive empty lines. I'd prefer the description not to be _too_ explicit e.g. by mentioning unified diff, etc. Personally I think it is sufficient to do s/#comment/comment/ to the existing text, without doing anything else. What is commentary to be removed is fairly clear in the contents given to the user in the editor. -- 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: Verbose as default for commit (optional)
Eric Sunshine sunsh...@sunshineco.com writes: On Fri, Apr 24, 2015 at 7:51 PM, Eloy Espinaco eloy...@gmail.com wrote: Ok, now I found [this thread](http://thread.gmane.org/gmane.comp.version-control.git/251376) that seems abandoned, but implements this config, a --no-verbose that disable it for one-time and the tests, but was not merged (don't know why) I recall reviewing Caleb's patch series and making a number of suggestions for improvement. v6 was the last version he posted[1], and it seems that he intended to post v7 but never got around to it. Apparently, Torstein Hegge asked in February 2015 about picking up where Caleb left off, but nothing has materialized. You are welcome to revive the series by taking reviewer comments into account and submitting v7 (and beyond if necessary). Be sure to keep Caleb's authorship and sign-off intact, and add your own sign-off following his. If you make changes to his patches, briefly describe your changes in a bracketed comment in the commit message, starting with your initials, like this: [ee: changed blah to bleh]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/251943/focus=264608 Also, the world order has changed recently, if I am not mistaken. Back when Caleb's series was done, there were only two choices (i.e. are we verbose, or not verbose?) Now commit and status can take three choices, so commit.verbose boolean would not cut it. Should the configuration variable be commit.verbose and only affect commit and not status, or should both of these commands pay attention to the single variable and behave the same way? I offhand do not have a strong opinion on these questions, but whoever is doing a proposal must think about it and justify the decision. 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 1/5] status: document the -v/--verbose option
On 04/24/2015 02:27 AM, Pete Harlan wrote: Junio writes: Michael Haggerty mhag...@alum.mit.edu writes: Document `git status -v`, including its new doubled `-vv` form. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Will queue on mg/status-v-v series, which did add description for commit -v, but status -v did not have the description to begin with and we missed it. [...] +-v:: +--verbose:: + In addition to the names of files that have been changed, also + show the textual changes that are staged to be committed + (i.e., like the output of `git diff`). If `-v` is specified Should this be `git diff --cached`? + twice, then also show the changes in the working tree that + have not yet been staged (i.e., like the output of `git diff + --cached`). ...and should this just be `git diff`? Yes, you're right. Thanks for catching this, Pete. The shorter the patch, the higher the error density :-( 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 0/5] Avoid file descriptor exhaustion in ref_transaction_commit()
On Fri, Apr 24, 2015 at 01:35:44PM +0200, Michael Haggerty wrote: In ref_transaction_commit(), close the reference lockfiles immediately to avoid keeping too many file descriptors open at a time. This is pretty easy, because in the first loop (where we create the locks) we already know what, if anything, has to be written into the lockfile. So write it and close the lockfile immediately. In the second loop, rename the lockfiles for reference updates into place, and in the cleanup loop, unlock any references that are still locked (i.e., those that were only being verified or deleted). I think this is a cleaner solution than Stefan's approach [1] of closing and reopening fds based on an estimate of how many fds we can afford to waste--we only need a single file descriptor at a time, and we never have to close then reopen a lockfile. But it is a bit more intrusive, so it might still be preferable to use Stefan's approach for release 2.4.0, if indeed any fix for this problem is still being considered for that release. I like this approach much better. It seems like the best of all worlds (same performance, and we don't have to worry about whether and when to close lockfiles). Stefan's patch is just in pu at this point, right? I do not think there is any rushing/release concern. It is too late for either to be in v2.4.0, so the only decision is whether to aim for master or maint. To me, they both seem to be in the same ballpark as far as risking a regression. -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 4/5] ref_transaction_commit(): remove the local flags variables
On Fri, Apr 24, 2015 at 01:35:48PM +0200, Michael Haggerty wrote: Instead, work directly with update-flags. This has the advantage that the REF_DELETING bit, set in the first loop, can be read in the third loop instead of having to compute the same expression again. Plus, it was kindof confusing having both update-flags and flags, which sometimes had different values. Hmm. I think this is losing the distinction of flags the caller has passed in to us versus flags we are using locally only during the transaction_commit routine. If callers look at the flags in the REF_TRANSACTION_CLOSED state, do they care about seeing these new flags? My guess is probably not in practice, and leaking these flags is an acceptable tradeoff for keeping the transaction_commit function simpler. But I haven't looked that closely. -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: [PATCHv3] refs.c: enable large transactions
On Thu, Apr 23, 2015 at 6:37 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: On Thu, Apr 23, 2015 at 10:56 AM, Junio C Hamano gits...@pobox.com wrote: + int save_errno = errno; + error(Couldn't reopen %s, lock-lk-filename.buf); No need to change this line, but I noticed that we might want to do something about the first one of the following two: I personally like to have each error(...) to have a unique string, such that when you run into trouble (most likely reported by a user), you can easily pinpoint where the exact error is. I was hoping that the grep patterns were strong enough hint, but let me be explicit. I was commenting on Could not being spelled as could. $ git grep -e '[]error(_*[A-Z]' | wc -l 146 $ git grep -e '[]error(_*[a-z]' | wc -l 390 I understood that you were talking about Could being capitalized. Though it's distributed 30/70 which hints to me we do not care in enough to explain the capitalized letters as slip-throughs on review or such, but it looks like the authors choice to me. I think it's a mistake to s/Could/could/g for all errors in the code base as it reduces the information provided in the error messages. Just 3 days ago (Regular Rebase Failure). I used different capitalization to get a better understanding where the error may be. So if we throw away that information, we should add new information to make the spot of the error easily findable in the source. That's why I proposed the idea of the version,filename,linenumber as that is one of the strongest signals (most information in a short amount of text) I can imagine. -- 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: [PATCHv3] refs.c: enable large transactions
On Fri, Apr 24, 2015 at 09:16:28AM -0700, Stefan Beller wrote: I think it's a mistake to s/Could/could/g for all errors in the code base as it reduces the information provided in the error messages. Just 3 days ago (Regular Rebase Failure). I used different capitalization to get a better understanding where the error may be. So if we throw away that information, we should add new information to make the spot of the error easily findable in the source. That's why I proposed the idea of the version,filename,linenumber as that is one of the strongest signals (most information in a short amount of text) I can imagine. I do like that idea, and I think you could base it on the trace_printf implementation. Note that it requires variadic macros, but I think that's OK. Just like trace_printf, we can do the macro implementation when we support that feature, and people on older systems just won't get the extra file/line data. I also assume we would not show this information by default, but only with GIT_TRACE_ERRORS or something like that. I would love it if we could also get a stack trace for warnings and errors. Very often the line number of the error() call is not nearly as interesting as the line number of the _caller_. But doing that portably is rather hard. Maybe a better option would be to make it easier to convince git to dump core at the right moments (e.g., dump core when we hit die() rather than calling exit). And then you can run gdb on the core file, which gives you a backtrace and much more. -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
Verbose as default for commit (optional)
Hi, It is my first mail to the list, so hello world. I wanted to make a feature-request about a config setting to make the commit always verbose. I'm not the only one asking for that, there is an old question in [Stack Overflow][1]. So I was thinking if it was possible to make a pull request for that, so I attach the patch. (I'm proud of it :) ). I wasn't able to make the test for it, but I wanted to ask (before I try) if it makes sense to add this feature (or if it is considered feature bloat). Thanks. --- Eloy Espinaco [1]: http://stackoverflow.com/questions/5875275/git-commit-v-by-default
Re: [PATCH 0/5] Avoid file descriptor exhaustion in ref_transaction_commit()
On Fri, Apr 24, 2015 at 10:26 AM, Jeff King p...@peff.net wrote: On Fri, Apr 24, 2015 at 01:35:44PM +0200, Michael Haggerty wrote: In ref_transaction_commit(), close the reference lockfiles immediately to avoid keeping too many file descriptors open at a time. This is pretty easy, because in the first loop (where we create the locks) we already know what, if anything, has to be written into the lockfile. So write it and close the lockfile immediately. In the second loop, rename the lockfiles for reference updates into place, and in the cleanup loop, unlock any references that are still locked (i.e., those that were only being verified or deleted). I think this is a cleaner solution than Stefan's approach [1] of closing and reopening fds based on an estimate of how many fds we can afford to waste--we only need a single file descriptor at a time, and we never have to close then reopen a lockfile. But it is a bit more intrusive, so it might still be preferable to use Stefan's approach for release 2.4.0, if indeed any fix for this problem is still being considered for that release. I like this approach much better. It seems like the best of all worlds (same performance, and we don't have to worry about whether and when to close lockfiles). I would have guessed this approach to take more work to do it right. Thanks Michael for tackling the problem in an elegant way! Stefan's patch is just in pu at this point, right? I do not think there is any rushing/release concern. Yeah it's in pu, so it's easy to remove. It is too late for either to be in v2.4.0, so the only decision is whether to aim for master or maint. To me, they both seem to be in the same ballpark as far as risking a regression. -Peff 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
[PATCH] t0027: Add repoMIX and LF_nul
new safer autocrlf handling: Check if eols in a file are converted at commit, when the file has CR (or CLLF) in the repo (technically speaking in the index). Add a test-file repoMIX with mixed line-endings. When converting LF-CRLF or CRLF-LF: check the warnings checkout_files(): Checking out CRLF_nul and checking for eol coversion does not make much sense (CRLF will stay CRLF). Use the file LF_nul instead: It is handled a binary in auto modes, and when declared as text the LF may be replaced with CRLF, depending on the configuration Signed-off-by: Torsten Bögershausen tbo...@web.de --- t/t0027-auto-crlf.sh | 157 --- 1 file changed, 85 insertions(+), 72 deletions(-) diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index 810934b..2482b2c 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -71,12 +71,21 @@ commit_check_warn () { attr=$2 lfname=$3 crlfname=$4 - lfmixcrlf=$5 - lfmixcr=$6 - crlfnul=$7 - create_gitattributes $attr + repoMIX=$5 + lfmixcrlf=$6 + lfmixcr=$7 + crlfnul=$8 pfx=crlf_${crlf}_attr_${attr} - for f in LF CRLF LF_mix_CR CRLF_mix_LF CRLF_nul + # Special handling for repoMIX: It should already be in the repo + # with CRLF + f=repoMIX + fname=${pfx}_$f.txt + echo .gitattributes + cp $f $fname + git -c core.autocrlf=false add $fname 2${pfx}_$f.err + git commit -m repoMIX + create_gitattributes $attr + for f in LF CRLF repoMIX LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul do fname=${pfx}_$f.txt cp $f $fname @@ -120,7 +129,7 @@ checkout_files () { git config core.autocrlf $crlf pfx=eol_${eol}_crlf_${crlf}_attr_${attr}_ src=crlf_false_attr__ - for f in LF CRLF LF_mix_CR CRLF_mix_LF CRLF_nul + for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul do rm $src$f.txt if test -z $eol; then @@ -142,8 +151,8 @@ checkout_files () { test_expect_success checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF_mix_CR compare_ws_file $pfx $lfmixcr ${src}LF_mix_CR.txt - test_expect_success checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=CRLF_nul - compare_ws_file $pfx $crlfnul ${src}CRLF_nul.txt + test_expect_success checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF_nul + compare_ws_file $pfx $crlfnul ${src}LF_nul.txt } @@ -155,6 +164,7 @@ test_expect_success 'setup master' ' git commit -m add .gitattributes printf line1\nline2\nline3 LF printf line1\r\nline2\r\nline3 CRLF + printf line1\r\nline2\nline3 repoMIX printf line1\r\nline2\nline3 CRLF_mix_LF printf line1\nline2\rline3 LF_mix_CR printf line1\r\nline2\rline3 CRLF_mix_CR @@ -181,40 +191,41 @@ else WAMIX=CRLF_LF fi +# attr LFCRLF repoMIX CRLFmixLF LFmixCR CRLFNUL test_expect_success 'commit files empty attr' ' - commit_check_warn false - commit_check_warn true LF_CRLF LF_CRLF - commit_check_warn input CRLF_LF CRLF_LF + commit_check_warn false + commit_check_warn true LF_CRLF LF_CRLF LF_CRLF + commit_check_warn input CRLF_LF CRLF_LF CRLF_LF ' test_expect_success 'commit files attr=auto' ' - commit_check_warn false auto $WILC $WICL$WAMIX - commit_check_warn true auto LF_CRLF LF_CRLF - commit_check_warn input auto CRLF_LF CRLF_LF + commit_check_warn false auto $WILC $WICL $WAMIX $WAMIX + commit_check_warn true auto LF_CRLF LF_CRLF LF_CRLF + commit_check_warn input auto CRLF_LF CRLF_LF CRLF_LF ' test_expect_success 'commit files attr=text' ' - commit_check_warn false text $WILC $WICL$WAMIX $WILC $WICL - commit_check_warn true text LF_CRLF LF_CRLF LF_CRLF - commit_check_warn input text CRLF_LF CRLF_LF CRLF_LF + commit_check_warn false text $WILC $WICL $WAMIX $WAMIX $WILC $WICL + commit_check_warn true text LF_CRLF LF_CRLF LF_CRLF LF_CRLF + commit_check_warn input text CRLF_LF CRLF_LF CRLF_LF CRLF_LF ' test_expect_success 'commit files attr=-text' ' - commit_check_warn false -text - commit_check_warn true -text - commit_check_warn input -text
[PATCH] Clarify documentation on commit message strip
When using `-v` to add a unified diff to the commit message, this is stripped and not a part of the commit message. This is not mentioned. Add a note about this with the `-v` description as well as description for the default `--cleanup` mode. Signed-off-by: Fredrik Gustafsson iv...@iveqy.com --- Documentation/git-commit.txt | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 617e29b..e31d828 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -180,8 +180,9 @@ OPTIONS + -- strip:: - Strip leading and trailing empty lines, trailing whitespace, and - #commentary and collapse consecutive empty lines. + Strip leading and trailing empty lines, trailing whitespace, + #commentary, unified diff added with `-v` and collapse + consecutive empty lines. whitespace:: Same as `strip` except #commentary is not removed. verbatim:: @@ -283,7 +284,8 @@ configuration variable documented in linkgit:git-config[1]. Show unified diff between the HEAD commit and what would be committed at the bottom of the commit message template. Note that this diff output doesn't have its - lines prefixed with '#'. + lines prefixed with '#'. This diff will not be a part + of the commit message. + If specified twice, show in addition the unified diff between what would be committed and the worktree files, i.e. the unstaged -- 2.1.4 -- 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] t0027: Add repoMIX and LF_nul
On Fri, Apr 24, 2015 at 3:46 PM, Torsten Bögershausen tbo...@web.de wrote: new safer autocrlf handling: Check if eols in a file are converted at commit, when the file has CR (or CLLF) in the repo (technically speaking in the index). Add a test-file repoMIX with mixed line-endings. When converting LF-CRLF or CRLF-LF: check the warnings checkout_files(): Checking out CRLF_nul and checking for eol coversion does not make much sense (CRLF will stay CRLF). Use the file LF_nul instead: It is handled a binary in auto modes, and when declared as text the LF may be replaced with CRLF, depending on the configuration Signed-off-by: Torsten Bögershausen tbo...@web.de --- diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index 810934b..2482b2c 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -71,12 +71,21 @@ commit_check_warn () { attr=$2 lfname=$3 crlfname=$4 - lfmixcrlf=$5 - lfmixcr=$6 - crlfnul=$7 - create_gitattributes $attr + repoMIX=$5 Unusual indentation. + lfmixcrlf=$6 + lfmixcr=$7 + crlfnul=$8 pfx=crlf_${crlf}_attr_${attr} - for f in LF CRLF LF_mix_CR CRLF_mix_LF CRLF_nul + # Special handling for repoMIX: It should already be in the repo + # with CRLF + f=repoMIX + fname=${pfx}_$f.txt + echo .gitattributes + cp $f $fname + git -c core.autocrlf=false add $fname 2${pfx}_$f.err + git commit -m repoMIX + create_gitattributes $attr + for f in LF CRLF repoMIX LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul do fname=${pfx}_$f.txt cp $f $fname -- 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: [PATCHv3] refs.c: enable large transactions
On Fri, Apr 24, 2015 at 11:12:36AM -0700, Jonathan Nieder wrote: Actually, I wouldn't mind an environment variable that tells error() to include a backtrace if someone wants it. (See backtrace(3) for implementation hints if interested in doing that.) Thanks, I didn't know about backtrace(3), and figured we'd be stuck with an ELF library or similar to get at the symbols. That being said, the resulting backtrace is not nearly as nice as what is produced by gdb (which includes pretty-printed variables, or even local variables with bt full). Not everybody will have gdb, of course, but nor will everybody have backtrace(). So if anything, I think my inclination would be to make it easier to help people (and ourselves) get a backtrace from gdb. One can get a core for a running process with gcore, or trigger a coredump by killing with SIGABRT. But catching it at the exact moment of a die() is a bit hard without support from the program. What about something like this: diff --git a/usage.c b/usage.c index ed14645..fa92190 100644 --- a/usage.c +++ b/usage.c @@ -34,6 +34,8 @@ static NORETURN void usage_builtin(const char *err, va_list params) static NORETURN void die_builtin(const char *err, va_list params) { vreportf(fatal: , err, params); + if (git_env_bool(GIT_DIE_ABORT, 0)) + abort(); exit(128); } Usage would be something like: ulimit -c unlimited ;# optional, but many distros seem to ship with ;# cores disabled these days GIT_DIE_ABORT=1 git some-command-that-fails gdb --quiet -ex 'bt full' -ex quit git core We could even wrap that up in a git-debug script. I suspect there may be some complications with finding the core file, though, as the git process may chdir before dumping. I'm not sure if there's a good way around that (obviously setting /proc/sys/kernel/core_pattern is not exactly friendly). I dunno. Certainly there is room for a less-full-featured system that would run more seamlessly, or on more people's systems (i.e., so that we can easily say set GIT_FOO and tell us what it outputs in response to bug reports). Maybe that system is backtrace(), or maybe it is just __FILE__ and __LINE__ markers for each printed error. -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: Verbose as default for commit (optional)
Eloy Espinaco eloy...@gmail.com writes: Hi, It is my first mail to the list, so hello world. Hi, and welcome to the list. I wanted to make a feature-request about a config setting to make the commit always verbose. I'm not the only one asking for that, there is an old question in [Stack Overflow][1]. This seems a reasonable addition. In general, we commonly have config options for commonly used CLI options. So I was thinking if it was possible to make a pull request for that, so I attach the patch. (I'm proud of it :) ). Nice try, but the attached file is empty ;-). Actually, as much as possible, avoid sending attachments but prefer inline patches. You'll need a bit of reading to send a proper patch: https://github.com/git/git/blob/master/Documentation/SubmittingPatches -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 4/5] ref_transaction_commit(): remove the local flags variables
On 04/24/2015 07:30 PM, Jeff King wrote: On Fri, Apr 24, 2015 at 01:35:48PM +0200, Michael Haggerty wrote: Instead, work directly with update-flags. This has the advantage that the REF_DELETING bit, set in the first loop, can be read in the third loop instead of having to compute the same expression again. Plus, it was kindof confusing having both update-flags and flags, which sometimes had different values. Hmm. I think this is losing the distinction of flags the caller has passed in to us versus flags we are using locally only during the transaction_commit routine. If callers look at the flags in the REF_TRANSACTION_CLOSED state, do they care about seeing these new flags? My guess is probably not in practice, and leaking these flags is an acceptable tradeoff for keeping the transaction_commit function simpler. But I haven't looked that closely. struct ref_update is opaque to callers outside of the refs module, and ref_update::flags is not read anywhere outside of ref_transaction_commit() (and its value is passed to lock_ref_sha1_basic()). So I don't think we have to be shy about storing our own internal information there. In fact, REF_DELETING, REF_ISPRUNING, REF_HAVE_NEW, and REF_HAVE_OLD are also private to the refs module. I suppose we could mask out all the private bits in the flags parameter passed by the caller, to make sure that the caller hasn't accidentally set other bits. I think that would be more defensive than our usual practice, but I don't mind doing it if people think it would be prudent. 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 4/5] ref_transaction_commit(): remove the local flags variables
On Fri, Apr 24, 2015 at 11:15:09PM +0200, Michael Haggerty wrote: Hmm. I think this is losing the distinction of flags the caller has passed in to us versus flags we are using locally only during the transaction_commit routine. If callers look at the flags in the REF_TRANSACTION_CLOSED state, do they care about seeing these new flags? My guess is probably not in practice, and leaking these flags is an acceptable tradeoff for keeping the transaction_commit function simpler. But I haven't looked that closely. struct ref_update is opaque to callers outside of the refs module, and ref_update::flags is not read anywhere outside of ref_transaction_commit() (and its value is passed to lock_ref_sha1_basic()). So I don't think we have to be shy about storing our own internal information there. In fact, REF_DELETING, REF_ISPRUNING, REF_HAVE_NEW, and REF_HAVE_OLD are also private to the refs module. Thanks for checking. If nobody is affected (and is not likely to be), I agree it's not worth worrying about. I suppose we could mask out all the private bits in the flags parameter passed by the caller, to make sure that the caller hasn't accidentally set other bits. I think that would be more defensive than our usual practice, but I don't mind doing it if people think it would be prudent. I don't think it's necessary. -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 4/5] ref_transaction_commit(): remove the local flags variables
On Fri, Apr 24, 2015 at 7:35 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Instead, work directly with update-flags. This has the advantage that the REF_DELETING bit, set in the first loop, can be read in the third loop instead of having to compute the same expression again. Plus, it was kindof confusing having both update-flags and flags, which s/kindof/kind of/ sometimes had different values. Signed-off-by: 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