Re: [msysGit] [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()

2015-04-24 Thread Johannes Schindelin
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

2015-04-24 Thread Johannes Schindelin
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

2015-04-24 Thread Johannes Schindelin
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

2015-04-24 Thread Kevin Daudt
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

2015-04-24 Thread Michael Haggerty
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

2015-04-24 Thread Michael Haggerty
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()

2015-04-24 Thread Michael Haggerty
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()

2015-04-24 Thread Michael Haggerty
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()

2015-04-24 Thread Michael Haggerty
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

2015-04-24 Thread Michael Haggerty
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

2015-04-24 Thread Luke Diamand

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

2015-04-24 Thread Luke Diamand

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

2015-04-24 Thread SZEDER Gábor

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.

2015-04-24 Thread Matthieu Moy
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.

2015-04-24 Thread Scott Meyer
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.

2015-04-24 Thread Matthieu Moy
[ 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.

2015-04-24 Thread Scott Meyer
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

2015-04-24 Thread Stefan Beller
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

2015-04-24 Thread Jonathan Nieder
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

2015-04-24 Thread brian m. carlson
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

2015-04-24 Thread Michael Haggerty
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

2015-04-24 Thread brian m. carlson
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

2015-04-24 Thread brian m. carlson
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

2015-04-24 Thread brian m. carlson
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)

2015-04-24 Thread Eloy Espinaco
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

2015-04-24 Thread Pete Harlan
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

2015-04-24 Thread brian m. carlson
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

2015-04-24 Thread FusionX86
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

2015-04-24 Thread Marc Khouzam
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

2015-04-24 Thread Jeff King
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

2015-04-24 Thread Jeff King
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

2015-04-24 Thread Chris

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

2015-04-24 Thread Fredrik Gustafsson
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

2015-04-24 Thread Junio C Hamano
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()

2015-04-24 Thread Junio C Hamano
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

2015-04-24 Thread Junio C Hamano
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)

2015-04-24 Thread Junio C Hamano
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

2015-04-24 Thread Michael Haggerty
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()

2015-04-24 Thread Jeff King
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

2015-04-24 Thread Jeff King
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

2015-04-24 Thread Stefan Beller
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

2015-04-24 Thread Jeff King
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)

2015-04-24 Thread Eloy Espinaco
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()

2015-04-24 Thread Stefan Beller
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

2015-04-24 Thread Torsten Bögershausen
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

2015-04-24 Thread Fredrik Gustafsson
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

2015-04-24 Thread Eric Sunshine
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

2015-04-24 Thread Jeff King
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)

2015-04-24 Thread Matthieu Moy
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

2015-04-24 Thread Michael Haggerty
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

2015-04-24 Thread Jeff King
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

2015-04-24 Thread Eric Sunshine
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