date:20140530

Hi,

Ronnie Sahlberg wrote:

> For these cases, save the errno value and abort and make sure that the caller
> can see errno==ENOTDIR.
>
> Also start defining specific return codes for _commit, assign -1 as a generic
> error and -2 as the error that refers to a name conflict. Callers can (and
> should) use that return value inspecting errno directly.

Heh.  Here's a patch on top to hopefully finish that part of the job.

Unfortunately, the value of errno after calling lock_ref_sha1_basic is
not reliable.
http://thread.gmane.org/gmane.comp.version-control.git/249159/focus=249407
lists the error paths that are broken (marked with "[!]" in that
message).

diff --git i/builtin/fetch.c w/builtin/fetch.c
index b13e8f9..0a01cda 100644
--- i/builtin/fetch.c
+++ w/builtin/fetch.c
@@ -377,6 +377,7 @@ static int s_update_ref(const char *action,
char *rla = getenv("GIT_REFLOG_ACTION");
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
+   int ret, df_conflict = 0;
 
if (dry_run)
return 0;
@@ -387,16 +388,22 @@ static int s_update_ref(const char *action,
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref->name, ref->new_sha1,
-  ref->old_sha1, 0, check_old, msg, &err) ||
-   ref_transaction_commit(transaction, &err)) {
-   ref_transaction_free(transaction);
-   error("%s", err.buf);
-   strbuf_release(&err);
-   return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
-   }
+  ref->old_sha1, 0, check_old, msg, &err))
+   goto fail;
+
+   ret = ref_transaction_commit(transaction, &err);
+   if (ret == UPDATE_REFS_NAME_CONFLICT)
+   df_conflict = 1;
+   if (ret)
+   goto fail;
ref_transaction_free(transaction);
return 0;
+fail:
+   ref_transaction_free(transaction);
+   error("%s", err.buf);
+   strbuf_release(&err);
+   return df_conflict ? STORE_REF_ERROR_DF_CONFLICT
+  : STORE_REF_ERROR_OTHER;
 }
 
 #define REFCOL_WIDTH  10
diff --git i/refs.c w/refs.c
index dbaabba..b22b99b 100644
--- i/refs.c
+++ w/refs.c
@@ -3499,7 +3499,7 @@ static int ref_update_reject_duplicates(struct ref_update 
**updates, int n,
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
-   int ret = 0, delnum = 0, i, save_errno = 0;
+   int ret = 0, delnum = 0, i, df_conflict = 0;
const char **delnames;
int n = transaction->nr;
struct ref_update **updates = transaction->updates;
@@ -3535,7 +3535,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
   delnames, delnum);
if (!update->lock) {
if (errno == ENOTDIR)
-   save_errno = errno;
+   df_conflict = 1;
if (err)
strbuf_addf(err, "Cannot lock the ref '%s'.",
update->refname);
@@ -3590,8 +3590,7 @@ cleanup:
if (updates[i]->lock)
unlock_ref(updates[i]->lock);
free(delnames);
-   errno = save_errno;
-   if (save_errno == ENOTDIR)
+   if (df_conflict)
ret = -2;
return ret;
 }
diff --git i/refs.h w/refs.h
index 88732a1..1583097 100644
--- i/refs.h
+++ w/refs.h
@@ -325,9 +325,11 @@ int ref_transaction_delete(struct ref_transaction 
*transaction,
  * problem.
  * If the transaction is already in failed state this function will return
  * an error.
- * Function returns 0 on success, -1 for generic failures and -2 if the
- * failure was due to a name collision (ENOTDIR).
+ * Function returns 0 on success, -1 for generic failures and
+ * UPDATE_REFS_NAME_CONFLICT (-2) if the failure was due to a name
+ * collision (ENOTDIR).
  */
+#define UPDATE_REFS_NAME_CONFLICT -2
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err);
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell

2014-05-30 Thread Naumov, Michael (North Sydney)

Some git tools such as GitExtensions for Windows use environment
variable TERM=msys which causes the weird ANSI sequence shown for the
messages returned from server-side hooks
We add those ANSI sequences to help format sideband data on the user's
terminal. However, GitExtensions is not using a terminal, and the ANSI
sequences just confuses it. We can recognize this use by checking
isatty().
See https://github.com/gitextensions/gitextensions/issues/1313 for
more details

NOTE: I considered to cover the case that a pager has already been
started. But decided that is probably not worth worrying about here,
though, as we shouldn't be using a pager for commands that do network
communications (and if we do, omitting the magic line-clearing signal
is probably a sane thing to do).

Signed-off-by: Michael Naumov 
Thanks-to: Erik Faye-Lund 
Thanks-to: Jeff King 
---
 sideband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sideband.c b/sideband.c
index d1125f5..7f9dc22 100644
--- a/sideband.c
+++ b/sideband.c
@@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out)
 
memcpy(buf, PREFIX, pf);
term = getenv("TERM");
-   if (term && strcmp(term, "dumb"))
+   if (isatty(2) && term && strcmp(term, "dumb"))
suffix = ANSI_SUFFIX;
else
suffix = DUMB_SUFFIX;
-- 
1.8.3.msysgit.0

P.S. I gave up trying to send this letter from gmail, it eats my tab
character
P.S 2. Sorry, my tab character has been eaten again!
P.S 3. Wrapped the letter lines to fit on the terminal

Regards,
Michael
--
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] refs.c: change read_ref_at to use the reflog iterators

2014-05-30 Thread Eric Sunshine

On Fri, May 30, 2014 at 3:51 PM, Ronnie Sahlberg  wrote:
> read_ref_at has its own parsing of the reflog file for no really good reason
> so lets change this to use the existing reflog iterators. This removes one
> instance where we manually unmarshall the reflog file format.
>
> Log messages for errors are changed slightly. We no longer print the file
> name for the reflog, instead we refer to it as 'Log for ref '.
> This might be a minor useability regression, but I don't really think so, 
> since
> experienced users would know where the log is anyway and inexperienced users
> would not know what to do about/how to repair 'Log ... has gap ...' anyway.
>
> Adapt the t1400 test to handle the cahnge in log messages.

s/cahnge/change/

More below.

> Signed-off-by: Ronnie Sahlberg 
> ---
> diff --git a/refs.c b/refs.c
> index 6898263..99d4832 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char 
> *endp)
> return xmemdupz(line, ep - line);
>  }
>
> +static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
> +   const char *email, unsigned long timestamp, int tz,
> +   const char *message, void *cb_data)
> +{
> +   struct read_ref_at_cb *cb = cb_data;
> +
> +   cb->reccnt++;
> +   cb->tz = tz;
> +   cb->date = timestamp;
> +
> +   if (timestamp <= cb->at_time || cb->cnt == 0) {
> +   if (cb->msg)
> +   *cb->msg = xstrdup(message);
> +   if (cb->cutoff_time)
> +   *cb->cutoff_time = timestamp;
> +   if (cb->cutoff_tz)
> +   *cb->cutoff_tz = tz;
> +   if (cb->cutoff_cnt)
> +   *cb->cutoff_cnt = cb->reccnt - 1;
> +
> +   /*
> +* we have not yet updated cb->[n|o]sha1 so they still
> +* hold the values for the previous record.
> +*/
> +   if (!is_null_sha1(cb->osha1)) {
> +   hashcpy(cb->sha1, nsha1);
> +   if (hashcmp(cb->osha1, nsha1))
> +   warning("Log for ref %s has gap after %s.",
> +   cb->refname, show_date(cb->date, 
> cb->tz, DATE_RFC2822));
> +   }
> +   else if (cb->date == cb->at_time)
> +   hashcpy(cb->sha1, nsha1);
> +   else
> +   if (hashcmp(nsha1, cb->sha1))

This could be an 'else if', allowing you to drop one level of indentation.

> +   warning("Log for ref %s unexpectedly ended on 
> %s.",
> +   cb->refname, show_date(cb->date, 
> cb->tz,
> +  DATE_RFC2822));
> +
> +   /*
> +* return 1. Not to signal an error but to break the loop
> +* since we have found the entry we want.
> +*/
> +   hashcpy(cb->osha1, osha1);
> +   hashcpy(cb->nsha1, nsha1);
> +   cb->found_it = 1;
> +   return 1;
> +   }
> +
> +   hashcpy(cb->osha1, osha1);
> +   hashcpy(cb->nsha1, nsha1);
> +   if (cb->cnt > 0)
> +   cb->cnt--;
> +
> +   return 0;
> +}
--
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] refs.c: change read_ref_at to use the reflog iterators

Ronnie Sahlberg  writes:

> read_ref_at has its own parsing of the reflog file for no really good reason
> so lets change this to use the existing reflog iterators. This removes one
> instance where we manually unmarshall the reflog file format.
>
> Log messages for errors are changed slightly. We no longer print the file
> name for the reflog, instead we refer to it as 'Log for ref '.
> This might be a minor useability regression, but I don't really think so, 
> since
> experienced users would know where the log is anyway and inexperienced users
> would not know what to do about/how to repair 'Log ... has gap ...' anyway.
>
> Adapt the t1400 test to handle the cahnge in log messages.
>
> Signed-off-by: Ronnie Sahlberg 
> ---
>  refs.c| 217 
> --
>  t/t1400-update-ref.sh |   4 +-
>  2 files changed, 122 insertions(+), 99 deletions(-)

After reading the log message "we are removing one redundant
implementation", I would have expected that such a change would
remove a lot more lines than it would add.  Seeing the diffstat, I
have to wonder if the fact that we need more lines to reuse the API
is an indication that the reflog iterator API is overly cumbersome
to use, perhaps requiring too much boilerplate or something.

The update in the error message may be a side-effect, but I think it
is a change in the good direction.

Thanks.

> diff --git a/refs.c b/refs.c
> index 6898263..99d4832 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char 
> *endp)
>   return xmemdupz(line, ep - line);
>  }
>  
> +struct read_ref_at_cb {
> + const char *refname;
> + unsigned long at_time;
> + int cnt;
> + int reccnt;
> + unsigned char *sha1;
> + int found_it;
> +
> + char osha1[20];
> + char nsha1[20];
> + int tz;
> + unsigned long date;
> + char **msg;
> + unsigned long *cutoff_time;
> + int *cutoff_tz;
> + int *cutoff_cnt;
> +};
> +
> +static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
> + const char *email, unsigned long timestamp, int tz,
> + const char *message, void *cb_data)
> +{
> + struct read_ref_at_cb *cb = cb_data;
> +
> + cb->reccnt++;
> + cb->tz = tz;
> + cb->date = timestamp;
> +
> + if (timestamp <= cb->at_time || cb->cnt == 0) {
> + if (cb->msg)
> + *cb->msg = xstrdup(message);
> + if (cb->cutoff_time)
> + *cb->cutoff_time = timestamp;
> + if (cb->cutoff_tz)
> + *cb->cutoff_tz = tz;
> + if (cb->cutoff_cnt)
> + *cb->cutoff_cnt = cb->reccnt - 1;
> +
> + /*
> +  * we have not yet updated cb->[n|o]sha1 so they still
> +  * hold the values for the previous record.
> +  */
> + if (!is_null_sha1(cb->osha1)) {
> + hashcpy(cb->sha1, nsha1);
> + if (hashcmp(cb->osha1, nsha1))
> + warning("Log for ref %s has gap after %s.",
> + cb->refname, show_date(cb->date, 
> cb->tz, DATE_RFC2822));
> + }
> + else if (cb->date == cb->at_time)
> + hashcpy(cb->sha1, nsha1);
> + else
> + if (hashcmp(nsha1, cb->sha1))
> + warning("Log for ref %s unexpectedly ended on 
> %s.",
> + cb->refname, show_date(cb->date, cb->tz,
> +DATE_RFC2822));
> +
> + /* 
> +  * return 1. Not to signal an error but to break the loop
> +  * since we have found the entry we want.
> +  */
> + hashcpy(cb->osha1, osha1);
> + hashcpy(cb->nsha1, nsha1);
> + cb->found_it = 1;
> + return 1;
> + }
> +
> + hashcpy(cb->osha1, osha1);
> + hashcpy(cb->nsha1, nsha1);
> + if (cb->cnt > 0)
> + cb->cnt--;
> +
> + return 0;
> +}
> +
> +static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
> +   const char *email, unsigned long timestamp,
> +   int tz, const char *message, void *cb_data)
> +{
> + struct read_ref_at_cb *cb = cb_data;
> +
> + if (cb->msg)
> + *cb->msg = xstrdup(message);
> + if (cb->cutoff_time)
> + *cb->cutoff_time = timestamp;
> + if (cb->cutoff_tz)
> + *cb->cutoff_tz = tz;
> + if (cb->cutoff_cnt)
> + *cb->cutoff_cnt = cb->reccnt - 1;
> +
> + hashcpy(cb->sha1, osha1);
> + if (is_null_sha1(cb->sha1))
> + hashcpy(cb->sha1, nsha1);
> +
> + /* 
> +  * return 1. Not to signal an error but to break the loop
> +  * since we have found the entr

[ANNOUNCE] Git v1.9.4

A maintenance release Git v1.9.4 is now available at the usual
places.  This is expected to be the final maintenance release for
the 1.9 series, merging the remaining fixes that are relevant and
are already in 2.0.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v1.9.4'
tag and the 'maint-1.9' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.9.4 Release Notes


Fixes since v1.9.3
--

 * Commands that take pathspecs on the command line misbehaved when
   the pathspec is given as an absolute pathname (which is a
   practice not particularly encouraged) that points at a symbolic
   link in the working tree.

 * An earlier fix to the shell prompt script (in contrib/) for using
   the PROMPT_COMMAND interface did not correctly check if the extra
   code path needs to trigger, causing the branch name not to appear
   when 'promptvars' option is disabled in bash or PROMPT_SUBST is
   unset in zsh.
--
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.c: replace signal() with sigaction()

2014-05-30 Thread Andreas Schwab

Jeremiah Mahler  writes:

> From signal(2) man page:
>
>   The behavior of signal() varies across UNIX versions, and has also var‐
>   ied historically across different versions of Linux.   Avoid  its  use:
>   use sigaction(2) instead.

I don't think this matters for SIG_DFL or SIG_IGN.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
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: [ANNOUNCE] Git v2.0.0

2014-05-30 Thread Felipe Contreras

Jeff King wrote:
> On Wed, May 28, 2014 at 06:17:25PM -0500, Felipe Contreras wrote:
> 
> > This is the last mail I sent to you, because you ignore them anyway, and
> > remove them from the mailing list.
> > [...]
> > [2], a mail you conveniently removed from the tracked record.
> > [...]
> > You also conveniently removed this mail from the archives.
> 
> I see you already noticed the changes in v2.0, but I wanted to address
> these points, because I consider silent censorship to be a serious
> accusation.

Yes, I also think silent censorship is a very seriours matter, and I was
very dissapointed that this mailing list would engage in that.

> I've reported the bug to gmane.discuss (no link yet, as I'm waiting
> for the message to go through, but it is not a high traffic group, so
> it should be easy to find the thread once it is there).

Thanks. At first I thought that was the reason, but then I noticed it
was always my mails that seemed to get this "bug", so I decided it was
too much of a coincidence.

Hopefully it's indeed a bug.

-- 
Felipe Contreras
--
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: [puzzled and solved] "shortlog" not quite understanding all "log" options

Jeff King  writes:

> On Fri, May 30, 2014 at 12:28:47PM -0700, Junio C Hamano wrote:
>
>>  Aaaand, it turns out that the answer is in the big comment at
>> the beginning of handle_revision_pseudo_opt().
>> 
>> -- >8 --
>> Subject: shortlog: allow --exclude= to be passed
>> 
>> e7b432c5 (revision: introduce --exclude= to tame wildcards,
>> 2013-08-30) taught a new option to the command-line parser of "log"
>> and friends, but did not wire it fully so that it can also be used
>> by "shortlog".
>
> FWIW, I think the discussion above the scissors adds a lot to the
> context. It might be nice to add it to the commit message.

OK.  Try to remember when I reroll it.

> I am slightly puzzled why parse_revision_opt does not just call
> handle_revision_pseudo_opt. According to f6aca0dc4, it is because
> pseudo-options need to be acted on in-order, as they affect things like
> subsequent "--not" options, etc. But if we are using parse_options_step,
> shouldn't we be handling the options in order?
>
> I am sure I am just missing something obvious, so do not trouble
> yourself if you do not know the answer offhand.

Sorry, I don't know ;-)
--
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: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

Jeff King  writes:

> On Fri, May 30, 2014 at 02:58:47PM -0500, Nathan Neulinger wrote:
>
>> Not really, since there are lots of servers,...
>
> But I think anything you could set up in the environment could be set up
> in an on-the-fly $HOME. For example, instead of:
>
>   GIT_WEAK_AUTHOR_NAME=$name
>   GIT_WEAK_AUTHOR_EMAIL=$email
>
> do:
>
>   HOME=$(mktemp -d gitenv.XX")
>   trap 'rm -rf "$HOME"' 0
>   git config --global user.name "$name"
>   git config --global user.email "$email"
>
> You'd want to link in anything else you actually _want_ in $HOME, but
> that also gives an opportunity to set up application-specific options
> based on the user (e.g., if you could pull their .vimrc from some shared
> storage or something).

Yes.  I agree that "Not really, ..." was not very convincing to me.

Another thing that might be useful in general (i.e. I am not
particularly trying to help the "shared" configuration on the topic
of this thread) is to allow environment variable substitution in
expand_user_path().

Nathan's installation can set a "GIT_MYSELF" and then have something
like this in the shared $HOME/.gitconfig

[include]
path = /usr/local/users/$GIT_MYSELF/ident

we could even make the whole thing fail when GIT_MYSELF is not set
but I haven't thought things through ;-)
--
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] fetch doc: update introductory part for clarity

Marc Branchaud  writes:

> The docs say that all the fetched refs are written to FETCH_HEAD (perhaps a
> more accurate name would have been FETCH_HEADS?).  If that's truly the case,
> it seems weird to use FETCH_HEAD in log and merge commands.  (My FETCH_HEAD
> file currently has 1434 lines in it -- what does that mean, and what does it
> imply for those log and merge commands?)

The "fetch" that was run by "pull" would have arranged the single
remote ref that your "pull" merged to your then-current branch to
the very beginning of FETCH_HEAD, so "git log FETCH_HEAD" would show
the line of development from that ref, and "git merge FETCH_HEAD"
would also merge what your "pull" would have merged.

> Perhaps FETCH_HEAD shouldn't be mentioned at all in the introductory part of
> fetch's man page.

A possible downside is that unreasonable people can use the lack
of mention of FETCH_HEAD as an excuse to start making noises about
removing the feature.

Also, a natural way to peek into somebody else's history without
making a permanent damage to your own repository, is:

$ git fetch $repository_of_marc master && git log FETCH_HEAD

As such a one-shot fetch from a random place does not use (and does
not want to use) any remote-tracking branch, knowing that FETCH_HEAD
is available for such a purpose would help people who want to script
such a thing.
--
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: [RFC PATCH] git log: support "auto" decorations

Jeff King  writes:

> I wonder if it would be sane to remove or quote NULs when attaching the
> buffer to commit->buffer. That would _break_ signatures, but that is a
> good thing. I do not think there is a reason to have NULs in your commit
> message unless you are doing something malicious (or using utf16, but
> that already is horribly broken).

Ahh, our messages crossed.  I do not think we are quite ready to
depart from our traditional position: the payload of a commit object
can be any bytestream, even though we do expect and encourage them
to be human readable text in a reasonable encoding.  And there is no
fundamental reason why we should forbid signing the payload that
happens to be a structured binary blob.

The user may need some way other than "log --show-signature" that
can be used to validate, because "log" itself will be useless for
such a payload with or without signature.  But I think that may be
a more or less orthogonal issue.

--
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 0/2] replace signal() with sigaction()

2014-05-30 Thread Jeremiah Mahler

This is the second revision to the patch set to replace signal(2) with
sigaction(2) [1].

As Johannes pointed out [2], replacing signal with sigaction would break
MinGW compatibility.  The first patch in this series addresses this
problem by expanding the faux sigaction function in compat/mingw.c to
support signals other than just SIGALRM.  Details are in the patch
description.

The second patch is a proof of concept.  It converts signal to sigaction
in a case where signal SIGCHILD was used.  Previously this would have
failed with MinGW since the faux sigaction function only supported
SIGALRM.  Now it works as expected.

I have tested these changes under Linux and under Windows 7 using Msysgit.

[1]: http://marc.info/?l=git&m=140125769223552&w=2
[2]: http://marc.info/?l=git&m=140126288325213&w=2

Jeremiah Mahler (2):
  compat/mingw.c: expand MinGW support for sigaction
  connect.c: replace signal() with sigaction()

 compat/mingw.c | 9 +
 connect.c  | 5 -
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.0.0.2.g1d11d5d

--
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] compat/mingw.c: expand MinGW support for sigaction

2014-05-30 Thread Jeremiah Mahler

Due to portability issues across UNIX versions sigaction(2) should be used
instead of signal(2).

>From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Unfortunately MinGW under Windows has limited support for signal and no
support for sigaction.  And this prevents sigaction from being used across
the entire Git project.

In compat/mingw.c there is a faux sigaction function but it only supports
SIGALARM.  Hence the need for continuing to use signal() in other cases.

This patch expands the faux sigaction function so that it calls signal in
cases other than SIGALRM.  Now sigaction can be used across the entire Git
project and MinGW will still work with signal as it did before.

Signed-off-by: Jeremiah Mahler 
---
 compat/mingw.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index e9892f8..e504cef 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1651,14 +1651,15 @@ int setitimer(int type, struct itimerval *in, struct 
itimerval *out)
 
 int sigaction(int sig, struct sigaction *in, struct sigaction *out)
 {
-   if (sig != SIGALRM)
-   return errno = EINVAL,
-   error("sigaction only implemented for SIGALRM");
if (out != NULL)
return errno = EINVAL,
error("sigaction: param 3 != NULL not implemented");
 
-   timer_fn = in->sa_handler;
+   if (sig == SIGALRM)
+   timer_fn = in->sa_handler;
+   else
+   signal(sig, in->sa_handler);
+
return 0;
 }
 
-- 
2.0.0.2.g1d11d5d

--
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.c: replace signal() with sigaction()

2014-05-30 Thread Jeremiah Mahler

>From signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in connect.c

Signed-off-by: Jeremiah Mahler 
---
 connect.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/connect.c b/connect.c
index a983d06..b2a33c9 100644
--- a/connect.c
+++ b/connect.c
@@ -665,11 +665,14 @@ struct child_process *git_connect(int fd[2], const char 
*url,
enum protocol protocol;
const char **arg;
struct strbuf cmd = STRBUF_INIT;
+   struct sigaction sa;
 
/* Without this we cannot rely on waitpid() to tell
 * what happened to our children.
 */
-   signal(SIGCHLD, SIG_DFL);
+   memset(&sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_DFL;
+   sigaction(SIGCHLD, &sa, 0);
 
protocol = parse_connect_url(url, &hostandport, &path);
if (flags & CONNECT_DIAG_URL) {
-- 
2.0.0.2.g1d11d5d

--
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: [RFC PATCH] git log: support "auto" decorations

Junio C Hamano  writes:

> Jeff King  writes:
>
>> Subject: [PATCH] reuse commit->buffer when parsing signatures
>> ...
>> Signed-off-by: Jeff King 
>
> Hmph, unfortunately this seems to break t7510.

And I think without re-reading the patch I know what is wrong.  The
length of the object and strlen(commit->buffer) would be different,
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: [RFC PATCH] git log: support "auto" decorations

On Fri, May 30, 2014 at 01:44:32PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Subject: [PATCH] reuse commit->buffer when parsing signatures
> > ...
> > Signed-off-by: Jeff King 
> 
> Hmph, unfortunately this seems to break t7510.

Urgh, sorry for not testing more thoroughly.

I imagine it is because of the strlen(commit->buffer) I introduced.
Unfortunately I do not know if we have an easy way to know the length of
commit->buffer, short of hitting sha1_object_info (which is somewhat
expensive to do for every commit).

I wonder if it would be sane to remove or quote NULs when attaching the
buffer to commit->buffer. That would _break_ signatures, but that is a
good thing. I do not think there is a reason to have NULs in your commit
message unless you are doing something malicious (or using utf16, but
that already is horribly broken).

-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: [RFC PATCH] git log: support "auto" decorations

Jeff King  writes:

> Subject: [PATCH] reuse commit->buffer when parsing signatures
> ...
> Signed-off-by: Jeff King 

Hmph, unfortunately this seems to break t7510.
--
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 stash gpg prompting

Jeff King  writes:

> However, I wonder if it is really ever going to be sane to set
> commit.gpgsign and not use something like gpg-agent.
>
> For example, if you
> were to ever "git rebase" a patch series (or even just use "git rebase
> -i" to refactor commits), you would be prompted for your passphrase to
> sign each individual patch.

Correct.

I actually doubt it is sane to set commit.gpgsign to true and trust
gpg-agent, but that is a separate issue.

--
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 stash gpg prompting

On Fri, May 30, 2014 at 01:07:16PM -0700, Eddie Monge wrote:

> Git stash is prompting for passphrase to try to "sign" the changes
> being stashed.
> 
> Reproduce:
> Add to global gitconfig with signing key:
> ```
> [commit]
>   gpgsign = true
> ```
> Go to a repo, make some changes, and then run `git stash`
> 
> Expected: stash the changes as normal
> Actual: git prompts for passphrase (if set)

Well, yeah...stash is making a commit (two, actually), so it wants you
to sign it. :)

I suspect that using "git notes" has a similar problem. I can see an
argument for not signing stashes, as they are meant to be temporary and
not shared. I do think notes probably should be signed.

However, I wonder if it is really ever going to be sane to set
commit.gpgsign and not use something like gpg-agent. For example, if you
were to ever "git rebase" a patch series (or even just use "git rebase
-i" to refactor commits), you would be prompted for your passphrase to
sign each individual patch.

-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

[PATCH alt-v1] Improve function dir.c:trim_trailing_spaces()

2014-05-30 Thread Pasha Bolokhov

Discard unnecessary 'nr_spaces' variable and improve
the 'if' structure. Switch to pointers instead of integers

Slightly more rare occurrences of 'text  \' with a backslash
in between spaces are handled correctly. Namely, the code in
8ba87adad6 does not reset 'last_space' when a backslash is
encountered and the above line stays intact as
a result. Add a test at the end of t/t0008-ignores.sh to
exhibit this behavior

Signed-off-by: Pasha Bolokhov 
---
Instead of the "optimized" version, which had complaints about
reasonability, I am attaching the "correction" of the original version.
'last_space' was not reset after encountering a backslash, and
'nr_spaces' is an unnecessary variable.

I myself am still leaning towards the optimized version (which scans
backwards instead of forward), since it completely ignores strings
which do not have spaces at the end, while these forward implementations
(both the one being attached and the original one) always scan
all strings through. Again, I'm not attaching the optimized one
because its readability is indeed less

 dir.c  | 29 ++---
 t/t0008-ignores.sh | 21 +
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/dir.c b/dir.c
index eb6f581..e67dcc2 100644
--- a/dir.c
+++ b/dir.c
@@ -508,21 +508,20 @@ void clear_exclude_list(struct exclude_list *el)
 
 static void trim_trailing_spaces(char *buf)
 {
-   int i, last_space = -1, nr_spaces, len = strlen(buf);
-   for (i = 0; i < len; i++)
-   if (buf[i] == '\\')
-   i++;
-   else if (buf[i] == ' ') {
-   if (last_space == -1) {
-   last_space = i;
-   nr_spaces = 1;
-   } else
-   nr_spaces++;
-   } else
-   last_space = -1;
-
-   if (last_space != -1 && last_space + nr_spaces == len)
-   buf[last_space] = '\0';
+   char *p, *last_space = NULL;
+
+   for (p = buf; *p; p++)
+   if (*p == ' ') {
+   if (last_space == NULL)
+   last_space = p;
+   } else {
+   if (*p == '\\')
+   p++;
+   last_space = NULL;
+   }
+
+   if (last_space != NULL)
+   *last_space = '\0';
 }
 
 int add_excludes_from_file_to_list(const char *fname,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..7becf96 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -806,4 +806,25 @@ test_expect_success !MINGW 'quoting allows trailing 
whitespace' '
test_cmp err.expect err
 '
 
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+   rm -rf whitespace &&
+   mkdir whitespace &&
+   >"whitespace/trailing 1  "  &&
+   >"whitespace/trailing 2 "   &&
+   >"whitespace/trailing 3 "   &&
+   >"whitespace/trailing 4   \\ "  &&
+   >"whitespace/trailing 5 \\ \\ " &&
+   >whitespace/untracked &&
+   echo "whitespace/trailing 1 \\" >ignore  &&
+   echo "whitespace/trailing 2 "   >>ignore &&
+   echo "whitespace/trailing 3  "  >>ignore &&
+   echo "whitespace/trailing 4   " >>ignore &&
+   echo "whitespace/trailing 5     "   >>ignore &&
+   echo whitespace/untracked >expect &&
+   : >err.expect &&
+   git ls-files -o -X ignore whitespace >actual 2>err &&
+   test_cmp expect actual &&
+   test_cmp err.expect err
+'
+
 test_done
-- 
1.9.1

--
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: [puzzled and solved] "shortlog" not quite understanding all "log" options

On Fri, May 30, 2014 at 12:28:47PM -0700, Junio C Hamano wrote:

>  Aaaand, it turns out that the answer is in the big comment at
> the beginning of handle_revision_pseudo_opt().
> 
> -- >8 --
> Subject: shortlog: allow --exclude= to be passed
> 
> e7b432c5 (revision: introduce --exclude= to tame wildcards,
> 2013-08-30) taught a new option to the command-line parser of "log"
> and friends, but did not wire it fully so that it can also be used
> by "shortlog".

FWIW, I think the discussion above the scissors adds a lot to the
context. It might be nice to add it to the commit message.

I am slightly puzzled why parse_revision_opt does not just call
handle_revision_pseudo_opt. According to f6aca0dc4, it is because
pseudo-options need to be acted on in-order, as they affect things like
subsequent "--not" options, etc. But if we are using parse_options_step,
shouldn't we be handling the options in order?

I am sure I am just missing something obvious, so do not trouble
yourself if you do not know the answer offhand.

-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: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

On Fri, May 30, 2014 at 02:58:47PM -0500, Nathan Neulinger wrote:

> Yes, the behavior currently is:
> 
>   If I can figure out "who", I'll set the EMAIL/attributes based on that.
>   If not, it'll default to the "don't know" behavior that throws up the 
> list
> 
> Ideally, I'd prefer the second option be:
> 
>   Force user to specify --author if a good default can't be determined
> 
> But there doesn't appear to be a way to do that.

Yeah, I don't think there is a blessed way to tell git "I am explicitly
_not_ giving you an identity". You can set user.email to "bogus.(none)"
which git will think "oh, I tried to get the FQDN, but failed". However,
that is not a documented interface, and I would not be surprised if it
changes in the future.

The instructions you get:

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

are probably not helpful either (you do not want the user to run "git
config", as that would interfere with the other shared users).

> >If so, wouldn't it be a better option to use that mechanism to set
> >separate $HOME (or XDG_CONFIG_HOME if you prefer) to these real
> >users who share the account, so that separate $HOME/.gitconfig files
> >can be used by them?
> 
> Not really, since there are lots of servers, and lots of application/service
> accounts. Where filesystem acl'ing can be used reasonably, it is, making
> this moot, but it still boils down to example case of "I have a team of X
> people maintaining Y different applications, each on their own dedicated
> account". I'd just like a good mechanism to set defaults based on
> information other than what is in the home dir on the occasions that users
> log in directly to the app account, as opposed to doing updates offline on
> their own systems/to central repo/etc.

But I think anything you could set up in the environment could be set up
in an on-the-fly $HOME. For example, instead of:

  GIT_WEAK_AUTHOR_NAME=$name
  GIT_WEAK_AUTHOR_EMAIL=$email

do:

  HOME=$(mktemp -d gitenv.XX")
  trap 'rm -rf "$HOME"' 0
  git config --global user.name "$name"
  git config --global user.email "$email"

You'd want to link in anything else you actually _want_ in $HOME, but
that also gives an opportunity to set up application-specific options
based on the user (e.g., if you could pull their .vimrc from some shared
storage or something).

-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

Git stash gpg prompting

2014-05-30 Thread Eddie Monge

Git stash is prompting for passphrase to try to "sign" the changes
being stashed.

Reproduce:
Add to global gitconfig with signing key:
```
[commit]
  gpgsign = true
```
Go to a repo, make some changes, and then run `git stash`

Expected: stash the changes as normal
Actual: git prompts for passphrase (if set)
--
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: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

2014-05-30 Thread Nathan Neulinger




On 05/30/2014 02:48 PM, Junio C Hamano wrote:

Nathan Neulinger  writes:


I wouldn't mind having a GIT_EMAIL envvar with the semantics you mean,
but can you say more about the use case?  What's wrong with the usual
EMAIL environment variable?


EMAIL actually worked for this case for me, but there wasn't any
equivalent for author name, so the commits all look like
"sharedaccount ".


I do not want to go into the discussion on the sanity/insanity of
using such a "shared account", but I am guessing that you already
have a concrete and workable mechanism in mind to allow you to set
these environment variables such as GIT_WEAKER_AUTHOR_NAME to
individual real users who share that account, and I further guess
that that is what you use to set EMAIL.  Am I guessing right?


Yes, the behavior currently is:

If I can figure out "who", I'll set the EMAIL/attributes based on that.
If not, it'll default to the "don't know" behavior that throws up the 
list

Ideally, I'd prefer the second option be:

Force user to specify --author if a good default can't be determined

But there doesn't appear to be a way to do that.


If so, wouldn't it be a better option to use that mechanism to set
separate $HOME (or XDG_CONFIG_HOME if you prefer) to these real
users who share the account, so that separate $HOME/.gitconfig files
can be used by them?


Not really, since there are lots of servers, and lots of application/service accounts. Where filesystem acl'ing can be 
used reasonably, it is, making this moot, but it still boils down to example case of "I have a team of X people 
maintaining Y different applications, each on their own dedicated account". I'd just like a good mechanism to set 
defaults based on information other than what is in the home dir on the occasions that users log in directly to the app 
account, as opposed to doing updates offline on their own systems/to central repo/etc.


-- Nathan


Nathan Neulinger   nn...@neulinger.org
Neulinger Consulting   (573) 612-1412
--
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] refs.c: change read_ref_at to use the reflog iterators

2014-05-30 Thread Ronnie Sahlberg

read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.

Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref '.
This might be a minor useability regression, but I don't really think so, since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.

Adapt the t1400 test to handle the cahnge in log messages.

Signed-off-by: Ronnie Sahlberg 
---
 refs.c| 217 --
 t/t1400-update-ref.sh |   4 +-
 2 files changed, 122 insertions(+), 99 deletions(-)

diff --git a/refs.c b/refs.c
index 6898263..99d4832 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char 
*endp)
return xmemdupz(line, ep - line);
 }
 
+struct read_ref_at_cb {
+   const char *refname;
+   unsigned long at_time;
+   int cnt;
+   int reccnt;
+   unsigned char *sha1;
+   int found_it;
+
+   char osha1[20];
+   char nsha1[20];
+   int tz;
+   unsigned long date;
+   char **msg;
+   unsigned long *cutoff_time;
+   int *cutoff_tz;
+   int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+   const char *email, unsigned long timestamp, int tz,
+   const char *message, void *cb_data)
+{
+   struct read_ref_at_cb *cb = cb_data;
+
+   cb->reccnt++;
+   cb->tz = tz;
+   cb->date = timestamp;
+
+   if (timestamp <= cb->at_time || cb->cnt == 0) {
+   if (cb->msg)
+   *cb->msg = xstrdup(message);
+   if (cb->cutoff_time)
+   *cb->cutoff_time = timestamp;
+   if (cb->cutoff_tz)
+   *cb->cutoff_tz = tz;
+   if (cb->cutoff_cnt)
+   *cb->cutoff_cnt = cb->reccnt - 1;
+
+   /*
+* we have not yet updated cb->[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+   if (!is_null_sha1(cb->osha1)) {
+   hashcpy(cb->sha1, nsha1);
+   if (hashcmp(cb->osha1, nsha1))
+   warning("Log for ref %s has gap after %s.",
+   cb->refname, show_date(cb->date, 
cb->tz, DATE_RFC2822));
+   }
+   else if (cb->date == cb->at_time)
+   hashcpy(cb->sha1, nsha1);
+   else
+   if (hashcmp(nsha1, cb->sha1))
+   warning("Log for ref %s unexpectedly ended on 
%s.",
+   cb->refname, show_date(cb->date, cb->tz,
+  DATE_RFC2822));
+
+   /* 
+* return 1. Not to signal an error but to break the loop
+* since we have found the entry we want.
+*/
+   hashcpy(cb->osha1, osha1);
+   hashcpy(cb->nsha1, nsha1);
+   cb->found_it = 1;
+   return 1;
+   }
+
+   hashcpy(cb->osha1, osha1);
+   hashcpy(cb->nsha1, nsha1);
+   if (cb->cnt > 0)
+   cb->cnt--;
+
+   return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+   struct read_ref_at_cb *cb = cb_data;
+
+   if (cb->msg)
+   *cb->msg = xstrdup(message);
+   if (cb->cutoff_time)
+   *cb->cutoff_time = timestamp;
+   if (cb->cutoff_tz)
+   *cb->cutoff_tz = tz;
+   if (cb->cutoff_cnt)
+   *cb->cutoff_cnt = cb->reccnt - 1;
+
+   hashcpy(cb->sha1, osha1);
+   if (is_null_sha1(cb->sha1))
+   hashcpy(cb->sha1, nsha1);
+
+   /* 
+* return 1. Not to signal an error but to break the loop
+* since we have found the entry we want.
+*/
+   return 1;
+}
+
 int read_ref_at(const char *refname, unsigned long at_time, int cnt,
unsigned char *sha1, char **msg,
unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
-   const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
-   char *tz_c;
-   int logfd, tz, reccnt = 0;
-   struct stat st;
-   unsigned long date;
-   unsigned char logged_sha1[20];
-   void *log_mapped;
-   size_t mapsz;
+   struct read_ref_at_cb cb;
 
-   logfile = git_path("logs/%s", refname);
-   logfd = open(logfile

Re: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

Nathan Neulinger  writes:

>> I wouldn't mind having a GIT_EMAIL envvar with the semantics you mean,
>> but can you say more about the use case?  What's wrong with the usual
>> EMAIL environment variable?
>
> EMAIL actually worked for this case for me, but there wasn't any
> equivalent for author name, so the commits all look like
> "sharedaccount ".

I do not want to go into the discussion on the sanity/insanity of
using such a "shared account", but I am guessing that you already
have a concrete and workable mechanism in mind to allow you to set
these environment variables such as GIT_WEAKER_AUTHOR_NAME to
individual real users who share that account, and I further guess
that that is what you use to set EMAIL.  Am I guessing right?

If so, wouldn't it be a better option to use that mechanism to set
separate $HOME (or XDG_CONFIG_HOME if you prefer) to these real
users who share the account, so that separate $HOME/.gitconfig files
can be used by them?
--
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

[puzzled and solved] "shortlog" not quite understanding all "log" options

This gives the list of all recent commits but not the ones that are
only reachable from the notes hierarchy:

$ git log --exclude=refs/notes/\* --all --no-merges --since=18.hours |
  git shortlog

and we are supposed to be able to write the same with shortlog
itself, i.e.

$ git shortlog --exclude=refs/notes/\* --all --no-merges --since=18.hours

but this does not understand the ref exclusion command-line option.

A funny and puzzling thing is that this is understood:

$ git shortlog --all --no-merges --since=18.hours

What is strange is that --all, --no-merges, etc. and the ref
exclusion option all are handled in revision.c and only one of them
is being rejected.

 Aaaand, it turns out that the answer is in the big comment at
the beginning of handle_revision_pseudo_opt().

-- >8 --
Subject: shortlog: allow --exclude= to be passed

e7b432c5 (revision: introduce --exclude= to tame wildcards,
2013-08-30) taught a new option to the command-line parser of "log"
and friends, but did not wire it fully so that it can also be used
by "shortlog".

Signed-off-by: Junio C Hamano 
---
 revision.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/revision.c b/revision.c
index 71e2337..3818b46 100644
--- a/revision.c
+++ b/revision.c
@@ -1633,6 +1633,7 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
!strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
!strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk") ||
!strcmp(arg, "--bisect") || starts_with(arg, "--glob=") ||
+   starts_with(arg, "--exclude=") ||
starts_with(arg, "--branches=") || starts_with(arg, "--tags=") ||
starts_with(arg, "--remotes=") || starts_with(arg, "--no-walk="))
{






--
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] fetch doc: update introductory part for clarity

2014-05-30 Thread Marc Branchaud

On 14-05-30 01:52 PM, Junio C Hamano wrote:
> Marc Branchaud  writes:
> 
>> On 14-05-29 06:42 PM, Junio C Hamano wrote:
>>> +
>>> +The names of refs that are fetched, together with the object names
>>> +they point at, are written to `.git/FETCH_HEAD`.  This information
>>> +is used by a later merge operation done by 'git merge'.  In addition,
>>
>> Isn't this merge stuff about pull, not fetch?
> 
> It is true that "git pull" uses "git fetch" and .git/FETCH_HEAD is a
> documented mechanism between the two to communicate what commits the
> latter downloaded are to be merged by the former, and that is one of
> the reasons why we had the description here in the original before
> this patch.  A user can also do this to refer to the tip of the
> single branch she fetched:
> 
>   git fetch origin master
> git log -p ..FETCH_HEAD
> git merge FETCH_HEAD
> 
> Perhaps "is used ... by 'git merge'" can be rephrased somehow, like
> "can be used to refer to what was fetched"?  Or we could go in the
> opposite direction and be more explicit, i.e.
> 
>   "git pull" calls "git fetch" internally, and this
>   information is used by the former to learn what commits were
>   fetched by the latter.
> 
> I dunno.

Y'know, I've always been a bit confused by FETCH_HEAD, especially if the
fetch updates several remote-tracking branches.

The docs say that all the fetched refs are written to FETCH_HEAD (perhaps a
more accurate name would have been FETCH_HEADS?).  If that's truly the case,
it seems weird to use FETCH_HEAD in log and merge commands.  (My FETCH_HEAD
file currently has 1434 lines in it -- what does that mean, and what does it
imply for those log and merge commands?)

Perhaps FETCH_HEAD shouldn't be mentioned at all in the introductory part of
fetch's man page.

M.

--
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] Fix a bug in compat/bswap.h endianness detection

Thanks; this should have been caught during the review.

+# if defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN)
+#  ...
+# elif defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN)
+#  ...
+# else

--
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] Improve function dir.c:trim_trailing_spaces()

Jeff King  writes:

> On Thu, May 29, 2014 at 02:34:33PM -0700, Pasha Bolokhov wrote:
>
>> > However, I doubt it makes that much of a difference in practice, so
>> > unless it's measurable, I would certainly go with the version that is
>> > more readable (and correct, of course).
>> 
>> Sorry, just to recap, you would go with the existing version
>> (which needs correction), or with the one that is being suggested? (I
>> agree I can format the style a tiny bit better in the latter one)
>
> I actually think the original left-to-right is a little easier to
> follow, but I do not feel strongly. I mainly meant "argue based on
> readability and correctness, do not argue based on speed".

Sensible.

> I'd be OK with either, though I have a slight preference for the first,
> just because I find the "bslash ^= 1" bit of yours, while clever, a bit
> hard to follow.

FWIW, I think I agree.

--
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: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

2014-05-30 Thread Nathan Neulinger


I wouldn't mind having a GIT_EMAIL envvar with the semantics you mean,
but can you say more about the use case?  What's wrong with the usual
EMAIL environment variable?


EMAIL actually worked for this case for me, but there wasn't any equivalent for author name, so the commits all look 
like "sharedaccount ".



[...]

[include]
path = !/usr/local/bin/gen-git-env


This is scary in terms of privilege escalation attacks.  ("Admin,
could you please take a look at this repo?  When I run 'git status',
...".)


Wouldn't that be pulling from ~sharedaccount/.gitconfig anyway though? (Agreed that it could be an issue if you have 
shared ssh agent, krb5 tgts, etc. - but that would exist any time you connected into a shared account.)


I would probably take the approach of not supporting that syntax in anything other than the per-user gitconfig file 
though as a safety measure - per-repo config could indeed be a problem.


-- Nathan


Nathan Neulinger   nn...@neulinger.org
Neulinger Consulting   (573) 612-1412
--
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: [RFC PATCH] git log: support "auto" decorations

On Fri, May 30, 2014 at 02:34:41PM -0400, Jeff King wrote:

> On Fri, May 30, 2014 at 10:35:14AM -0700, Junio C Hamano wrote:
> 
> > > Do you want me to roll it up with a real commit message?
> > 
> > Yes.  I think the change is sensible.
> 
> Here it is. [...]

By the way, I rather derailed Linus's original patch with this
sub-thread. I think it actually looks fine as-is. The shortcomings he
listed are all there, but I think addressing them would end up even
worse.

-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: [RFC PATCH] git log: support "auto" decorations

On Fri, May 30, 2014 at 10:35:14AM -0700, Junio C Hamano wrote:

> > Do you want me to roll it up with a real commit message?
> 
> Yes.  I think the change is sensible.

Here it is. We may want to make these helper functions available to
other callers so they can use the same trick, but I do not know offhand
of any others that would want it. pretty.c is the obvious place, and it
already uses a similar trick in logmsg_reencode (and I would expect most
users of the commit message would actually want the reencoded version,
and would use that).

-- >8 --
Subject: [PATCH] reuse commit->buffer when parsing signatures

When we call show_signature or show_mergetag, we read the
commit object fresh via read_sha1_file and reparse its
headers. However, in most cases we already have the object
data available as commit->buffer.  This is partially
laziness in dealing with the memory allocation issues, but
partially defensive programming, in that we would always
want to verify a clean version of the buffer (not one that
might have been munged by other users of the commit).

However, we do not currently ever munge commit->buffer, and
not using the already-available buffer carries a fairly big
performance penalty when we are looking at a large number of
commits. Here are timings on linux.git:

  [baseline, no signatures]
  $ time git log >/dev/null
  real0m4.902s
  user0m4.784s
  sys 0m0.120s

  [before]
  $ time git log --show-signature >/dev/null
  real0m14.735s
  user0m9.964s
  sys 0m0.944s

  [after]
  $ time git log --show-signature >/dev/null
  real0m9.981s
  user0m5.260s
  sys 0m0.936s

Note that our user CPU time drops almost in half, close to
the non-signature case, but we do still spend more
wall-clock and system time, presumably from dealing with
gpg.

An alternative to this is to note that most commits do not
have signatures (less than 1% in this repo), yet we pay the
re-parsing cost for every commit just to find out if it has
a mergetag or signature. If we checked that when parsing the
commit initially, we could avoid re-examining most commits
later on. Even if we did pursue that direction, however,
this would still speed up the cases where we _do_ have
signatures. So it's probably worth doing either way.

Signed-off-by: Jeff King 
---
 commit.c   | 44 
 commit.h   |  2 +-
 log-tree.c |  2 +-
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/commit.c b/commit.c
index f479331..9e2abf7 100644
--- a/commit.c
+++ b/commit.c
@@ -1080,14 +1080,42 @@ static int do_sign_commit(struct strbuf *buf, const 
char *keyid)
return 0;
 }

-int parse_signed_commit(const unsigned char *sha1,
+/*
+ * Return the contents of the object pointed to by commit, as
+ * if read by read_sha1_file. However, in cases where the commit's
+ * data is already in memory, return that as an optimization.
+ *
+ * The resulting buffer may or may not be freshly allocated,
+ * and should only be freed by free_commit_buffer.
+ */
+static const char *read_commit_buffer(const struct commit *commit,
+ enum object_type *type,
+ unsigned long *size)
+{
+   if (commit->buffer) {
+   *type = OBJ_COMMIT;
+   *size = strlen(commit->buffer);
+   return commit->buffer;
+   }
+
+   return read_sha1_file(commit->object.sha1, type, size);
+}
+
+static void free_commit_buffer(const char *buffer, const struct commit *commit)
+{
+   if (buffer != commit->buffer)
+   free((char *)buffer);
+}
+
+int parse_signed_commit(const struct commit *commit,
struct strbuf *payload, struct strbuf *signature)
 {
+
unsigned long size;
enum object_type type;
-   char *buffer = read_sha1_file(sha1, &type, &size);
+   const char *buffer = read_commit_buffer(commit, &type, &size);
int in_signature, saw_signature = -1;
-   char *line, *tail;
+   const char *line, *tail;

if (!buffer || type != OBJ_COMMIT)
goto cleanup;
@@ -1098,7 +1126,7 @@ int parse_signed_commit(const unsigned char *sha1,
saw_signature = 0;
while (line < tail) {
const char *sig = NULL;
-   char *next = memchr(line, '\n', tail - line);
+   const char *next = memchr(line, '\n', tail - line);

next = next ? next + 1 : tail;
if (in_signature && line[0] == ' ')
@@ -1120,7 +1148,7 @@ int parse_signed_commit(const unsigned char *sha1,
line = next;
}
  cleanup:
-   free(buffer);
+   free_commit_buffer(buffer, commit);
return saw_signature;
 }

@@ -1211,7 +1239,7 @@ void check_commit_signature(const struct commit* commit, 
struct signature_check

sigc->result = 'N';

-   if (parse_signed_commit(commit->object.sha1,
+   if (parse_signed_commit(commit,

Re: [RFC PATCH] t5538: move http push tests out to t5542

On 30 May 2014, Junio C. Hamano said:

> Nix  writes:
>
>> (signed off with my work email address for paranoia's sake.)
>
> As long as your employer is fine with your patch contributed to us,

My employer encourages it.

> and assuming that the patch was done on your employer's time, it

That is hard to define, but I use this copy of git for work purposes as
well as non-work purposes, so I arbitrarily define it as done on their
time.

> would be the right thing to do ;-).  Just FYI, if your From: address
> on your e-mail does not match that address, you can start your
> message body with "From: Nick Alcock ", a
> blank line and then the true body of your message.

Yeah, I oscillated on that and picked the wrong option in the end. :)
--
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: [RFC PATCH] t5538: move http push tests out to t5542

On 30 May 2014, Jeff King said:

> On Fri, May 30, 2014 at 11:29:10AM +0100, Nix wrote:
>
>> +test_expect_success 'push from shallow repo via http' '
>> +mv "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" shallow-upstream.git &&
>> +git clone --bare --no-local full "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
>> +(
>> +cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
>> +git config http.receivepack true
>> +) &&
>> +commit 1s0 &&
>
> This is a typo, no? The original is just "commit 10".

I have no *idea* where that came from. vi-wreckage I suspect.

> I think we can just drop that "1" from the expected output, though. It
> was created in the original by the "push from full to shallow" test (and
> is rather confusing, anyway, as it shares a name with the original "1").

Right.

-- 
NULL && (void)
--
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: feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

Hi,

Nathan Neulinger wrote:

> Right now, there isn't any way to have a systemwide profile script
> that tries to determine a better default for the user name/email
> values, such as in the case of shared logins. The best I've been
> able to do for now is use the 'EMAIL' var.

I wouldn't mind having a GIT_EMAIL envvar with the semantics you mean,
but can you say more about the use case?  What's wrong with the usual
EMAIL environment variable?

[...]
> [include]
>path = !/usr/local/bin/gen-git-env

This is scary in terms of privilege escalation attacks.  ("Admin,
could you please take a look at this repo?  When I run 'git status',
...".)

That said, smudge/clean filters may have the same problem already.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

feature request - implement a "GIT_AUTHOR_EMAIL" equivalent, but processed BEFORE .gitconfig

2014-05-30 Thread Nathan Neulinger

Four related feature requests/ideas:

1. equivalent to GIT_*_EMAIL/NAME vars, but processed only if git config
doesn't set values

Right now, there isn't any way to have a systemwide profile script that tries to determine a better default for the user
name/email values, such as in the case of shared logins. The best I've been able to do for now is use the 'EMAIL' var.

Use case in my environment - most shared-login accounts are accessed via krb5 login, so it provides a nice way to set a
better default for "who is doing this commit" than just the userid.

2. Setting option to user.name (or other setting) that would BLOCK the commit from occurring at all if it would
otherwise fall back to defaults. I thought this previously worked by setting an empty value, but apparently doesn't work
that way in current versions.

3. Setting option to user.name/user.email to prompt, SVN-style, for the name/email. Yes, this would be
annoying/obnoxious to use normally, but intent is to avoid un-named "root@host" commits that would otherwise occur from
user being lazy.

4. (This would accomplish all of the above) - enhance the include.path option to support the "!" syntax similar to what
aliases can do. i.e.

[include]
path = !/usr/local/bin/gen-git-env

[include]
cmd = /usr/local/bin/gen-git-env

-- Nathan

Nathan Neulinger nn...@neulinger.org
Neulinger Consulting (573) 612-1412
--
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 v11 36/41] refs.c: move the check for valid refname to lock_ref_sha1_basic

Ronnie Sahlberg wrote:

> Move the check for check_refname_format from lock_any_ref_for_update
> to lock_ref_sha1_basic. At some later stage we will get rid of
> lock_any_ref_for_update completely.

Do you know if this will cause any functional change?

What is the potential impact?  Is that impact worth it?  (Given how
broken the recovery codepaths currently are, I suspect this change is
worth it, but it seems worth documenting in the log message.)

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 35/41] refs.c: pack all refs before we start to rename a ref

Ronnie Sahlberg wrote:

> We want to do this to make it easier to handle atomic renames in rename_ref 
> for
> the case 'git branch -m foo/bar foo'.

In an ideal world, a part of me would rather see "git branch -m" doing
something more targeted by only packing the two refs it is working
with, and only when it knows it has to so the normal "git branch -m
foo bar" case doesn't have to suffer.  But:

That would be more complicated.

Packing refs is not very slow and has some good benefits for
performance of later commands.  Once we've committed to writing out a
new pack-refs file, it's not so bad to enumerate all loose refs to get
more benefit from writing out the new packed-refs file.

Renaming refs is something usually done by humans and not by scripts
or remote clients.  I'm not too worried about "git branch -m" in a
tight loop getting slower.

So I think this is an okay thing to do.

>   If we can guarantee that foo/bar does 
> not
> exist as a loose ref we can avoid locking foo/bar.lock during transaction
> commit

That is not quite true --- there's still value in locking foo/bar to
avoid clobbering a concurrent ref update.

If git performed the entire rename transaction in the packed-refs
file, we could avoid that race completely (for 'git branch -m foo/bar
foo': lock refs, make sure the loose refs are pruned, perform the
update in packed-refs, unlock refs.  for 'git branch -m foo foo/bar':
lock foo, prune foo, lock foo/bar, prune foo/bar, perform the update
in packed-refs, unlock refs).

Even without doing that, packing refs first has a benefit in terms of
ordering: if you do (1) delete loose foo/bar, (2) write loose foo, (3)
rewrite packed-refs without foo/bar, then because you've packed refs
first, no objects become unreferenced during step (1), which means
that a hypothetical version of "git gc" that used filesystem snapshots
would not see any relevant objects as safe to clean up no matter when
it runs.

> Signed-off-by: Ronnie Sahlberg 
> ---
>  refs.c| 3 +++
>  t/t3200-branch.sh | 6 +++---
>  2 files changed, 6 insertions(+), 3 deletions(-)

With a clearer commit message,
Reviewed-by: Jonathan Nieder 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] fetch doc: update note on '+' in front of the refspec

Marc Branchaud  writes:

>> +When the remote branch you want to fetch is known to
>> +be rewound and rebased regularly, it is expected that
>> +the tip of it will not be descendant of the commit that
>> +used to be at its tip the last time you fetched it and
>> +stored in your remote-tracking branch.  You would want
>
> I think the second part of that last sentence might be clearer as
>
>   it is expected that its new tip will not be a descendant of
>   its previous tip (as stored in your remote-tracking branch
>   the last time you fetched).

Yeah, that reads better.  Thanks.

>
> Then start the next sentence with
>
>   In this case, you would want 

I somehow find that "in this case" redundant, given that "for such
branches" already limits the scope of the suggestion.  I dunno.

>> +to use the `+` sign to indicate non-fast-forward updates
>> +will be needed for such branches.  There is no way to
>> +determine or declare that a branch will be made available
>> +in a repository with this behavior; the pulling user simply
>>  must know this is the expected usage pattern for a branch.
>>  +
>>  [NOTE]
>> 
--
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] fetch doc: update introductory part for clarity

Marc Branchaud  writes:

> On 14-05-29 06:42 PM, Junio C Hamano wrote:
>>  - "Branches" is a more common way to say "heads" in these days.
>> 
>>  - Remote-tracking branches are used a lot more these days and it is
>>worth mentioning that it is one of the primary side effects of
>>the command to update them.
>> 
>>  - Avoid "X. That means Y."  If Y is easier to understand to
>>readers, just say that upfront.
>> 
>>  - Use of explicit refspec to fetch tags does not have much to do
>>with turning "auto following" on or off.  It is a way to fetch
>>tags that otherwise would not be fetched by auto-following.
>> 
>> Signed-off-by: Junio C Hamano 
>> ---
>>  Documentation/git-fetch.txt | 29 -
>>  1 file changed, 16 insertions(+), 13 deletions(-)
>> 
>> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
>> index 5809aa4..d5f5b54 100644
>> --- a/Documentation/git-fetch.txt
>> +++ b/Documentation/git-fetch.txt
>> @@ -17,20 +17,23 @@ SYNOPSIS
>>  
>>  DESCRIPTION
>>  ---
>> -Fetches named heads or tags from one or more other repositories,
>> -along with the objects necessary to complete them.
>> -
>> -The ref names and their object names of fetched refs are stored
>> -in `.git/FETCH_HEAD`.  This information is left for a later merge
>> -operation done by 'git merge'.
>> -
>> -By default, tags are auto-followed.  This means that when fetching
>> -from a remote, any tags on the remote that point to objects that exist
>> -in the local repository are fetched.  The effect is to fetch tags that
>> +Fetch branches and/or tags (collectively, "refs") from one or more
>> +other repositories, along with the objects necessary to complete the
>> +histories of them.
>
> Phrasing: s/the histories of them/their histories/

Yeah, thanks.

>> +
>> +The names of refs that are fetched, together with the object names
>> +they point at, are written to `.git/FETCH_HEAD`.  This information
>> +is used by a later merge operation done by 'git merge'.  In addition,
>
> Isn't this merge stuff about pull, not fetch?

It is true that "git pull" uses "git fetch" and .git/FETCH_HEAD is a
documented mechanism between the two to communicate what commits the
latter downloaded are to be merged by the former, and that is one of
the reasons why we had the description here in the original before
this patch.  A user can also do this to refer to the tip of the
single branch she fetched:

git fetch origin master
git log -p ..FETCH_HEAD
git merge FETCH_HEAD

Perhaps "is used ... by 'git merge'" can be rephrased somehow, like
"can be used to refer to what was fetched"?  Or we could go in the
opposite direction and be more explicit, i.e.

"git pull" calls "git fetch" internally, and this
information is used by the former to learn what commits were
fetched by the latter.

I dunno.

>> +the remote-tracking branches may be updated (see description on
>> + below for details).
>
> I realize that "may be updated" is strictly correct, in that if the remote's
> branches have not changed since the last fetch then the local tracking
> branches won't change.
>
> But it took me a second or two to think of that.  The "may" kindof tripped me
> up.  The fact is that the local tracking branches are always updated to match
> the remote's branches, it's just that sometimes the remote's branches don't
> change.  So I think it would be clearer to say
>
>   the remote-tracking branches are updated
>
> because this makes it clear that the command always makes your local tracking
> branches match the remote's.

The primary reason behind my "may be" was not "they may not have
done anything in the meantime", but was "we may not have configured
to track at all", but in that case by definition we don't have "the
remote-tracking branches", so now I realize that it is pointless to
say "may be updated".

--
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/2] contrib/subtree: Store subtree metadata in .gittrees file

2014-05-30 Thread Keval Patel

- Most of the changes are taken from helmo’s repository from following
URL:
https://github.com/helmo/git-subtree/blob/master/git-subtree.sh
- Add code to create a file named .gittrees which can store information
on the subtree that is getting merged to the super-repository. The
advantage is ‘subtree push’ and ‘subtree pull’ subcommands can be
called with only the —prefix option and no need to provide remote url
- Add tests for new feature additions. All existing and new tests pass
successfully
Files changed in this commit:
1. git/contrib/subtree/git-subtree.sh
2. git/contrib/subtree/t/t7900-subtree.sh

Signed-off-by: Keval Patel 
---
A selection of updates to git-subtree were offered to the list by couple of 
people
($gmane/196667) by Herman van Rink and ($gmane/217820) by Paul Campbell
For various reasons the work stalled and I would like to pick it up from there

The following patches take a selection of these commits and rebase them 
against the tip of master.
The make test works and I have added more tests to cover the new commands and 
use of .gittrees file for storing the subtree metadata

Thanks-to and Based-on-patch-by:
- Herman van Rink
- Matt Hoffman
- Michael Hart
- Nate Jones
- Paul Campbell
- Paul Cartwright
- Peter Jaros
- bibendi

 contrib/subtree/git-subtree.sh |   98 +
 contrib/subtree/t/t7900-subtree.sh |  106 
 2 files changed, 168 insertions(+), 36 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index db925ca..7d01b4b 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -8,8 +8,7 @@ if [ $# -eq 0 ]; then
 set -- -h
 fi
 OPTS_SPEC="\
-git subtree add   --prefix= 
-git subtree add   --prefix=  
+git subtree add   --prefix=  
 git subtree merge --prefix= 
 git subtree pull  --prefix=  
 git subtree push  --prefix=  
@@ -490,12 +489,6 @@ ensure_clean()
fi
 }
 
-ensure_valid_ref_format()
-{
-   git check-ref-format "refs/heads/$1" ||
-   die "'$1' does not look like a ref"
-}
-
 cmd_add()
 {
if [ -e "$dir" ]; then
@@ -505,22 +498,12 @@ cmd_add()
ensure_clean

if [ $# -eq 1 ]; then
-   git rev-parse -q --verify "$1^{commit}" >/dev/null ||
-   die "'$1' does not refer to a commit"
-
-   "cmd_add_commit" "$@"
+   "cmd_add_commit" "$@"
elif [ $# -eq 2 ]; then
-   # Technically we could accept a refspec here but we're
-   # just going to turn around and add FETCH_HEAD under the
-   # specified directory.  Allowing a refspec might be
-   # misleading because we won't do anything with any other
-   # branches fetched via the refspec.
-   ensure_valid_ref_format "$2"
-
-   "cmd_add_repository" "$@"
+   "cmd_add_repository" "$@"
else
-   say "error: parameters were '$@'"
-   die "Provide either a commit or a repository and commit."
+   say "error: parameters were '$@'"
+   die "Provide either a refspec or a repository and refspec."
fi
 }
 
@@ -533,6 +516,24 @@ cmd_add_repository()
revs=FETCH_HEAD
set -- $revs
cmd_add_commit "$@"
+   
+   revs=$(git rev-parse $default --revs-only "$@") || exit $?
+   set -- $revs
+   rev="$1"
+   
+   subtree_mainline_merge=$(git rev-parse HEAD) || exit $?
+   
+   # now add it to our list of repos 
+   git config -f .gittrees --unset subtree.$dir.url
+   git config -f .gittrees --add subtree.$dir.url $repository
+   git config -f .gittrees --unset subtree.$dir.path
+   git config -f .gittrees --add subtree.$dir.path $dir
+   git config -f .gittrees --unset subtree.$dir.branch
+   git config -f .gittrees --add subtree.$dir.branch $refspec
+   git config -f .gittrees --unset subtree.$dir.subtreeCommit
+   git config -f .gittrees --add subtree.$dir.subtreeCommit $rev
+   git config -f .gittrees --unset subtree.$dir.subtreeMergeCommit
+   git config -f .gittrees --add subtree.$dir.subtreeMergeCommit 
$subtree_mainline_merge
 }
 
 cmd_add_commit()
@@ -598,7 +599,7 @@ cmd_split()
eval "$grl" |
while read rev parents; do
revcount=$(($revcount + 1))
-   say -n "$revcount/$revmax ($createcount)
"
+   say -n "$revcount/$revmax ($createcount)"
debug "Processing commit: $rev"
exists=$(cache_get $rev)
if [ -n "$exists" ]; then
@@ -705,31 +706,56 @@ cmd_merge()
 
 cmd_pull()
 {
-   if [ $# -ne 2 ]; then
-   die "You must provide  "
+   if [ $# -gt 2 ]; then
+   die "You should provide either  or  
"
fi
+   if [ -e "$dir" ]; then
ensure_clean
-   ensure_valid_ref_format "$2"
-   git fetch "$@" || exit $?
+   if [ $# -eq 1 ]; then 
+   r

[PATCH 2/2] contrib/subtree: List subcmd and modify push/pull to use .gittrees

2014-05-30 Thread Keval Patel

- Add subtree list sub-command
- git subtree list - Lists the subtrees in current project
- Changes taken from helmo’s repository from following URL:
https://github.com/helmo/git-subtree/blob/master/git-subtree.sh
- Add tests for subtree list and subtree push/pull using .gittrees
- Files changed in this commit:
1. git/contrib/subtree/git-subtree.sh
2. git/contrib/subtree/t/t7900-subtree.sh

Signed-off-by: Keval Patel 
---
A selection of updates to git-subtree were offered to the list by couple of 
people
($gmane/196667) by Herman van Rink and ($gmane/217820) by Paul Campbell
For various reasons the work stalled and I would like to pick it up from there

The following patches take a selection of these commits and rebase them 
against the tip of master.
The make test works and I have added more tests to cover the new commands and 
use of .gittrees file for storing the subtree metadata

Thanks-to and Based-on-patch-by:
- Herman van Rink
- Matt Hoffman
- Michael Hart
- Nate Jones
- Paul Campbell
- Paul Cartwright
- Peter Jaros
- bibendi

 contrib/subtree/git-subtree.sh |   25 ++--
 contrib/subtree/t/t7900-subtree.sh |   72 
 2 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7d01b4b..1151884 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -10,9 +10,10 @@ fi
 OPTS_SPEC="\
 git subtree add   --prefix=  
 git subtree merge --prefix= 
-git subtree pull  --prefix=  
-git subtree push  --prefix=  
+git subtree pull  --prefix= [ [...]]
+git subtree push  --prefix= [ [...]]
 git subtree split --prefix= 
+git subtree list
 --
 h,helpshow the help
 q quiet
@@ -102,15 +103,16 @@ command="$1"
 shift
 case "$command" in
add|merge|pull) default= ;;
-   split|push) default="--default HEAD" ;;
+   split|push|list) default="--default HEAD" ;;
*) die "Unknown command '$command'" ;;
 esac
 
-if [ -z "$prefix" ]; then
+if [ -z "$prefix" -a "$command" != "list" ]; then
die "You must provide the --prefix option."
 fi
 
 case "$command" in
+   list);;
add) [ -e "$prefix" ] && 
die "prefix '$prefix' already exists." ;;
*)   [ -e "$prefix" ] || 
@@ -759,4 +761,19 @@ cmd_push()
fi
 }
 
+subtree_list() 
+{
+   git config -f .gittrees -l | grep subtree | grep path | grep -o '=.*' | 
grep -o '[^=].*' |
+   while read path; do 
+   repository=$(git config -f .gittrees subtree.$path.url)
+   refspec=$(git config -f .gittrees subtree.$path.branch)
+   echo "$path(merged from $repository branch 
$refspec) "
+   done
+}
+
+cmd_list()
+{
+   subtree_list
+}
+
 "cmd_$command" "$@"
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 05110f7..c29993e 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -571,4 +571,76 @@ test_expect_success 'add another subtree with master 
branch' '
 check_equal ''"$(last_commit_message)"'' "Add sub2 subtree"
 '
 
+# Lets commit the changes we made to .gittrees file
+test_expect_success 'Commit chages to .gittrees for sub1 and sub2 in repo' '
+git add .gittrees &&
+git commit -m "Add .gittrees file"
+'
+# Tests for subtree list
+# Hardcode expected output to a file
+cat >expect <<-\EOF
+sub1(merged from ../shared_projects/subtree1 branch master) 
+sub2(merged from ../shared_projects/subtree2 branch master) 
+EOF
+
+test_expect_success 'check subtree list gives correct output' '
+git subtree list>output &&
+test_cmp expect output
+'
+# Lets commit the changes to parent1 before proceeding
+test_expect_success 'Commit changes to the repository' '
+git add --all &&
+git commit -m "Commit expect and output file additions"
+'
+
+# Tests for individual subtree pull using information in .gittrees
+# Go to subtree1 and make a change
+cd ../shared_projects/subtree1
+
+subtree1_change1="Add_line_to_Sub1_File2"
+
+echo $subtree1_change1>>sub1_file2
+
+# Lets commit the changes to subtree1 before proceeding
+test_expect_success 'Commit changes to the subtree1' '
+git add --all &&
+git commit -m "Commit change to sub1_file2"
+'
+
+# Switch to develop branch for a future test to push changes to master
+test_expect_success 'Switch to branch develop' '
+git checkout -b develop
+'
+
+# Back to parent1
+cd ../../parent1
+
+test_expect_success 'check  git subtree pull  works' '
+git subtree pull -P sub1 master &&
+test_cmp sub1/sub1_file1 ../shared_projects/subtree1/sub1_file1 &&
+test_cmp sub1/sub1_file2 ../shared_projects/subtree1/sub1_file2
+'
+
+# Now lets make local change on subtree and push it to subtree remote
+cd sub1
+
+local_change="Local addition of line to sub1 file 2"
+echo $local_change1>>sub1_f

Re: [PATCH v11 33/41] refs.c: pass the ref log message to _create/delete/update instead of _commit

Ronnie Sahlberg wrote:

> Change the reference transactions so that we pass the reflog message
> through to the create/delete/update function instead of the commit message.
> This allows for individual messages for each change in a multi ref
> transaction.

Nice.

That reminds me: in the future, do we want to have some way to figure
out what ref updates happened together?  E.g., cvsnt introduced commit
identifiers to answer a similar kind of question in CVS per-file
history.  If some backend wants to support that, the API this patch
introduces would handle it fine --- good.

[...]
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -673,10 +673,9 @@ static int store_updated_refs(const char *raw_url, const 
> char *remote_name,
>   }
>   }
>   }
> -
>   if (rc & STORE_REF_ERROR_DF_CONFLICT)
>   error(_("some local refs could not be updated; try running\n"
> -   " 'git remote prune %s' to remove any old, conflicting "
> +   "'git remote prune %s' to remove any old, conflicting "
> "branches"), remote_name);

Unrelated change snuck in?

The rest of the patch is
Reviewed-by: Jonathan Nieder 

diff --git a/builtin/fetch.c b/builtin/fetch.c
index faa1233..55f457c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -673,9 +673,10 @@ static int store_updated_refs(const char *raw_url, const 
char *remote_name,
}
}
}
+
if (rc & STORE_REF_ERROR_DF_CONFLICT)
error(_("some local refs could not be updated; try running\n"
- "'git remote prune %s' to remove any old, conflicting "
+ " 'git remote prune %s' to remove any old, conflicting "
  "branches"), remote_name);
 
  abort:
--
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] t5538: move http push tests out to t5542

From: Nick Alcock 

As 0232852b, but for the push tests instead: this avoids a start_httpd
in the middle of the file, which fails under GIT_TEST_HTTPD=false.

Note that we have to munge the test in a few ways while
moving it:

  1. We drop the `test -z "$GIT_TEST_HTTPD"` check; this is
 too simplistic since 83d842d, and we should let
 lib-httpd.sh handle it.

  2. We have to port over some of the old setup from t5538.

  3. In the final test, we no longer expect the extra commit
 "1" built on top of "4". This was a side effect from an
 earlier test in t5538 which was not ported over.

Signed-off-by: Nick Alcock 
Signed-off-by: Jeff King 
---
Here it is with the fixups I suggested in the last email. This passes
for me, but I'd love to have a sanity check from Duy.

 t/t5538-push-shallow.sh  |  59 -
 t/t5542-push-http-shallow.sh | 100 +++
 2 files changed, 100 insertions(+), 59 deletions(-)
 create mode 100755 t/t5542-push-http-shallow.sh

diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
index 8e54ac5..ceee95b 100755
--- a/t/t5538-push-shallow.sh
+++ b/t/t5538-push-shallow.sh
@@ -120,63 +120,4 @@ EOF
git cat-file blob `echo 1|git hash-object --stdin` >/dev/null
)
 '
-
-if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
-   say 'skipping remaining tests, git built without http support'
-   test_done
-fi
-
-. "$TEST_DIRECTORY"/lib-httpd.sh
-start_httpd
-
-test_expect_success 'push to shallow repo via http' '
-   git clone --bare --no-local shallow 
"$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-   (
-   cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-   git config http.receivepack true
-   ) &&
-   (
-   cd full &&
-   commit 9 &&
-   git push $HTTPD_URL/smart/repo.git +master:refs/remotes/top/master
-   ) &&
-   (
-   cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-   git fsck &&
-   git log --format=%s top/master >actual &&
-   cat actual &&
-   cat tracked &&
+   git add tracked &&
+   git commit -m "$1"
+}
+
+test_expect_success 'setup' '
+   git config --global transfer.fsckObjects true &&
+   commit 1 &&
+   commit 2 &&
+   commit 3 &&
+   commit 4 &&
+   git clone . full &&
+   (
+   git init full-abc &&
+   cd full-abc &&
+   commit a &&
+   commit b &&
+   commit c
+   ) &&
+   git clone --no-local --depth=2 .git shallow &&
+   git --git-dir=shallow/.git log --format=%s >actual &&
+   cat actual &&
+   cat actual &&
+   cat

Re: [RFC PATCH] git log: support "auto" decorations

Jeff King  writes:

> On Fri, May 30, 2014 at 09:55:14AM -0700, Junio C Hamano wrote:
>
> I don't think we need to worry about commit->buffer being mucked with.
> It is always either NULL, or points to the original object contents.
> Encoded log messages are always placed in a separate buffer (and in fact
> we use the same "optionally point to commit->buffer" trick there). And
> things like mucking with parents always happen on the parsed form.
>
> Of course I may be missing a site, and it's certainly a maintenance risk
> for the future. But I'd go so far as to say that anything modifying
> commit->buffer is wrong, and that side should be fixed.

I fully agree, and "that side should be fixed" implying "we should
always be on a look-out for such a change" is something the lazyness
tried to avoid.

> Do you want me to roll it up with a real commit message?

Yes.  I think the change is sensible.
--
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] check_refname_component: Optimize

On Fri, May 30, 2014 at 11:47:33AM +0200, Michael Haggerty wrote:

> > I could guess something like "the writer has a different idea of what a
> > valid refname is than we do". But that applies as well to (2), but just
> > as "the reader who wrote packed-refs.stat has a different idea than we
> > do".
> 
> If we want to be robust to future changes to refname rules, we could add
> a header flag like
> 
> # pack-refs with: peeled fully-peeled check-level=1.0
> 
> which promises that the reference names in the file conform to the
> current ("version 1.0") check_refname_format() rules.

Yeah, I thought about mentioning something like that. But really, this
just seems like a lot of complexity to solve the problem in a wrong way.

It's not running check_refname_format that is the real problem. It's the
fact that we do O(# of refs) work whenever we have to access the
packed-refs file. check_refname_format is part of that, surely, but so
is reading the file, creating all of the refname structs in memory, etc.

I'd much rather see a solution that lets us do O(log N) or O(1) work to
access a ref, and then we don't have to care about optimizing
check_refname_format specifically.

I don't mind internal code speedups to micro-optimize check_refname_format.
They may make the code uglier, but they're fairly contained. But things
like check-level are much more invasive, and we'll need to keep
compatibility with them in future versions.

-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 v11 32/41] refs.c: make delete_ref use a transaction

Ronnie Sahlberg wrote:

> Signed-off-by: Ronnie Sahlberg 
> ---
>  refs.c | 34 +-
>  1 file changed, 13 insertions(+), 21 deletions(-)

Reviewed-by: Jonathan Nieder 

[...]
> +++ b/refs.c
[...]
> @@ -2542,24 +2537,21 @@ static int delete_ref_loose(struct ref_lock *lock, 
> int flag, struct strbuf *err)
>  
>  int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
>  {
[...]
> + if (!transaction ||
> + ref_transaction_delete(transaction, refname, sha1, delopt,
> +sha1 && !is_null_sha1(sha1), &err) ||
> + ref_transaction_commit(transaction, NULL, &err)) {
> + error("%s", err.buf);
> + ref_transaction_free(transaction);
> + strbuf_release(&err);
>   return 1;
> + }
[...]
> - ret |= repack_without_ref(lock->ref_name);

The old return value could be 1 or -1 depending on how the deletion
failed.  Now it's consistently 1.

The only callers I see that care are cmd_symbolic_ref and
cmd_update_ref, for which 1 is better (-1 would result in an exit
status of 255, which means something like "died with signal 127").

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] t5538: move http push tests out to t5542

Nix  writes:

> As 0232852b, but for the push tests instead: this avoids a start_httpd
> in the middle of the file, which fails under GIT_TEST_HTTPD=false.
>
> Signed-off-by: Nick Alcock 
> ---
> On 30 May 2014, Jeff King said:
>> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
>> index be951a4..a980574 100755
>> --- a/t/t5537-fetch-shallow.sh
>> +++ b/t/t5537-fetch-shallow.sh
>
> btw, t5538-push-shallow.sh has the same bug but was not fixed up that I
> can see. (Though I can't see the original fix in the history of master
> either: was it squashed away somehow?)

What do you mean by "the original fix"?

0232852b (t5537: move http tests out to t5539, 2014-02-13) you cited
in your proposed log message?  That is v1.9.1~12^2 and certainly is
reachable from the master branch as master~185^2.

> But something like this is needed.

Thanks for marking the patch not to be applied / still broken.  I
agree that moving http bits into a separate test would be a good way
to go.

> (signed off with my work email address for paranoia's sake.)

As long as your employer is fine with your patch contributed to us,
and assuming that the patch was done on your employer's time, it
would be the right thing to do ;-).  Just FYI, if your From: address
on your e-mail does not match that address, you can start your
message body with "From: Nick Alcock ", a
blank line and then the true body of your message.

>  t/t5538-push-shallow.sh  |  59 -
>  t/t5542-push-http-shallow.sh | 101 
> +++
>  2 files changed, 101 insertions(+), 59 deletions(-)
>  create mode 100755 t/t5542-push-http-shallow.sh
>
> diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
> index 8e54ac5..ceee95b 100755
> --- a/t/t5538-push-shallow.sh
> +++ b/t/t5538-push-shallow.sh
> @@ -120,63 +120,4 @@ EOF
>   git cat-file blob `echo 1|git hash-object --stdin` >/dev/null
>   )
>  '
> -
> -if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
> - say 'skipping remaining tests, git built without http support'
> - test_done
> -fi
> -
> -. "$TEST_DIRECTORY"/lib-httpd.sh
> -start_httpd
> -
> -test_expect_success 'push to shallow repo via http' '
> - git clone --bare --no-local shallow 
> "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - (
> - cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - git config http.receivepack true
> - ) &&
> - (
> - cd full &&
> - commit 9 &&
> - git push $HTTPD_URL/smart/repo.git +master:refs/remotes/top/master
> - ) &&
> - (
> - cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - git fsck &&
> - git log --format=%s top/master >actual &&
> - cat  -9
> -4
> -3
> -EOF
> - test_cmp expect actual
> - )
> -'
> -
> -test_expect_success 'push from shallow repo via http' '
> - mv "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" shallow-upstream.git &&
> - git clone --bare --no-local full "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - (
> - cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - git config http.receivepack true
> - ) &&
> - commit 10 &&
> - git push $HTTPD_URL/smart/repo.git +master:refs/remotes/top/master &&
> - (
> - cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - git fsck &&
> - git log --format=%s top/master >actual &&
> - cat  -10
> -1
> -4
> -3
> -2
> -1
> -EOF
> - test_cmp expect actual
> - )
> -'
> -
> -stop_httpd
>  test_done
> diff --git a/t/t5542-push-http-shallow.sh b/t/t5542-push-http-shallow.sh
> new file mode 100755
> index 000..61a7281
> --- /dev/null
> +++ b/t/t5542-push-http-shallow.sh
> @@ -0,0 +1,101 @@
> +#!/bin/sh
> +
> +test_description='push from/to a shallow clone over http'
> +
> +. ./test-lib.sh
> +
> +if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
> + say 'skipping test, git built without http support'
> + test_done
> +fi
> +
> +. "$TEST_DIRECTORY"/lib-httpd.sh
> +start_httpd
> +
> +commit() {
> + echo "$1" >tracked &&
> + git add tracked &&
> + git commit -m "$1"
> +}
> +
> +test_expect_success 'setup' '
> + git config --global transfer.fsckObjects true &&
> + commit 1 &&
> + commit 2 &&
> + commit 3 &&
> + commit 4 &&
> + git clone . full &&
> + (
> + git init full-abc &&
> + cd full-abc &&
> + commit a &&
> + commit b &&
> + commit c
> + ) &&
> + git clone --no-local --depth=2 .git shallow &&
> + git --git-dir=shallow/.git log --format=%s >actual &&
> + cat  +4
> +3
> +EOF
> + test_cmp expect actual &&
> + git clone --no-local --depth=2 full-abc/.git shallow2 &&
> + git --git-dir=shallow2/.git log --format=%s >actual &&
> + cat  +c
> +b
> +EOF
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'push to shallow repo via http' '
> + git clone --bare --no-local shallow 
> "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> + (
> + cd "$HTTPD_DOCUMENT_R

Re: [RFC PATCH] t5538: move http push tests out to t5542

On Fri, May 30, 2014 at 11:29:10AM +0100, Nix wrote:

> As 0232852b, but for the push tests instead: this avoids a start_httpd
> in the middle of the file, which fails under GIT_TEST_HTTPD=false.
> 
> Signed-off-by: Nick Alcock 

Thanks, this makes sense. I'm really not sure why we didn't notice this
at the same time as t5537.

I'm cc-ing Duy, who wrote the original test, to give input on the new
test setup.

> ---
> On 30 May 2014, Jeff King said:
> > diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
> > index be951a4..a980574 100755
> > --- a/t/t5537-fetch-shallow.sh
> > +++ b/t/t5537-fetch-shallow.sh
> 
> btw, t5538-push-shallow.sh has the same bug but was not fixed up that I
> can see. (Though I can't see the original fix in the history of master
> either: was it squashed away somehow?)

I don't think so. As far as I can tell, it was never fixed.

> +test_description='push from/to a shallow clone over http'
> +
> +. ./test-lib.sh
> +
> +if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
> + say 'skipping test, git built without http support'
> + test_done
> +fi

Ah, this may be why we missed it. We should leave the handling of
$GIT_TEST_HTTPD to lib-httpd.sh. I think the topic to refactor lib-httpd
setup and this shallow topic were developed in isolation, and we never
noticed the semantic conflict during the merge, because there's no
textual conflict.

So when I tested with a broken apache, we silently skipped this test
(but setting GIT_TEST_HTTPD=false does not trigger the above skip).

> +test_expect_success 'push from shallow repo via http' '
> + mv "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" shallow-upstream.git &&
> + git clone --bare --no-local full "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> + (
> + cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> + git config http.receivepack true
> + ) &&
> + commit 1s0 &&

This is a typo, no? The original is just "commit 10".

With that fix, the test still fails:

--- expect  2014-05-30 17:18:08.638496558 +
+++ actual  2014-05-30 17:18:08.638496558 +
@@ -1,5 +1,4 @@
 10
-1
 4
 3
 2
not ok 3 - push from shallow repo via http

I think we can just drop that "1" from the expected output, though. It
was created in the original by the "push from full to shallow" test (and
is rather confusing, anyway, as it shares a name with the original "1").

-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: [RFC PATCH] git log: support "auto" decorations

On Fri, May 30, 2014 at 09:55:14AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Thu, May 29, 2014 at 09:54:10PM -0700, Linus Torvalds wrote:
> >
> >> That said, part of it is just that show-signature is so suboptimal
> >> performance-wise, re-parsing the commit buffer for each commit when
> >> "show_signature" is set. That's just crazy, we've already parsed the
> >> commit text, we already *could* know if it has a signature or not, and
> >> skip it if it doesn't. That would require one of the flag bits in the
> >> object, though, or something, so it's probably not worth doing.
> >
> > Wow, it's really quite bad. Not only do we spend time on commits that we
> > could otherwise know do not have signatures, but we actually pull the
> > buffer from disk, even though we generally have it saved as
> > commit->buffer.
> 
> The one for the signature on the commit itself is me being lazy and
> defensive; I did not want to have to worry about people mucking with
> what is in commit->buffer for whatever reason (e.g. re-encode in
> different charset, etc.) and then asking the signature validated.
> 
> The other one for the merge-tag is me just being lazy, as it is
> unlikely to be corrupt by any reasonable kinds of mucking with
> commit->buffer on a merge.

I don't think we need to worry about commit->buffer being mucked with.
It is always either NULL, or points to the original object contents.
Encoded log messages are always placed in a separate buffer (and in fact
we use the same "optionally point to commit->buffer" trick there). And
things like mucking with parents always happen on the parsed form.

Of course I may be missing a site, and it's certainly a maintenance risk
for the future. But I'd go so far as to say that anything modifying
commit->buffer is wrong, and that side should be fixed.

Do you want me to roll it up with a real commit message?

The other option is to do something like Linus suggested, and note the
presence/absence of signature and mergetag headers with a few bits (we
could even use a commit slab if we don't want to waste bits in the
object struct). That would prevent us hitting this code at all for most
commits, so we would save not only the read_sha1_file cost, but the
extra parsing cost.

However, that does nothing to help the cases where we _do_ have
signatures. A repo where somebody GPG-signed every commit, for example,
would still perform terribly. So even if we go that route, I think we'd
want to apply this technique, too.

-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: [RFC PATCH] git log: support "auto" decorations

Jeff King  writes:

> On Thu, May 29, 2014 at 09:54:10PM -0700, Linus Torvalds wrote:
>
>> That said, part of it is just that show-signature is so suboptimal
>> performance-wise, re-parsing the commit buffer for each commit when
>> "show_signature" is set. That's just crazy, we've already parsed the
>> commit text, we already *could* know if it has a signature or not, and
>> skip it if it doesn't. That would require one of the flag bits in the
>> object, though, or something, so it's probably not worth doing.
>
> Wow, it's really quite bad. Not only do we spend time on commits that we
> could otherwise know do not have signatures, but we actually pull the
> buffer from disk, even though we generally have it saved as
> commit->buffer.

The one for the signature on the commit itself is me being lazy and
defensive; I did not want to have to worry about people mucking with
what is in commit->buffer for whatever reason (e.g. re-encode in
different charset, etc.) and then asking the signature validated.

The other one for the merge-tag is me just being lazy, as it is
unlikely to be corrupt by any reasonable kinds of mucking with
commit->buffer on a merge.
--
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] t5537: re-drop http tests

Jeff King  writes:

> And a final side note. If you retry the merge by:
>
>   m=1ddb4d7
>   git checkout $m^1
>   git merge $m^2
>
> you can see the resulting conflict is quite tricky to understand
> Viewing it with diff3 conflict-style makes it much more obvious.

Yes, that was a clear mismerge on my side; thanks for catching it.

>
>  t/t5537-fetch-shallow.sh | 28 
>  1 file changed, 28 deletions(-)
>
> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
> index be951a4..a980574 100755
> --- a/t/t5537-fetch-shallow.sh
> +++ b/t/t5537-fetch-shallow.sh
> @@ -173,33 +173,6 @@ EOF
>   )
>  '
>  
> -if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
> - say 'skipping remaining tests, git built without http support'
> - test_done
> -fi
> -
> -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'}
> -. "$TEST_DIRECTORY"/lib-httpd.sh
> -start_httpd
> -
> -test_expect_success 'clone http repository' '
> - git clone --bare --no-local shallow 
> "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> - git clone $HTTPD_URL/smart/repo.git clone &&
> - (
> - cd clone &&
> - git fsck &&
> - git log --format=%s origin/master >actual &&
> - cat  -7
> -6
> -5
> -4
> -3
> -EOF
> - test_cmp expect actual
> - )
> -'
> -
>  test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
>   cp -R .git read-only.git &&
>   find read-only.git -print | xargs chmod -w &&
> @@ -213,5 +186,4 @@ EOF
>   test_cmp expect actual
>  '
>  
> -stop_httpd
>  test_done
--
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

MAIL

2014-05-30 Thread Richard Sun

Hello, 
My name is Mr. Richard Sun from Hong Kong. I want you to be my partner in a 
business project. Contact me back via my private e-mail address for more 
details; 
richadt...@ymail.com 
Thank you. 
Mr. Richard Sun.
--
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] Fix a bug in compat/bswap.h endianness detection

2014-05-30 Thread Ben Walton

The changes to make detection of endianness more portable had a bug
that breaks on (at least) Solaris x86. The bug appears to be a simple
copy/paste typo. It checks for _BIG_ENDIAN and not _LITTLE_ENDIAN for
both the case where we would decide the system is big endian and
little endian. Instead, the second test should be for _LITTLE_ENDIAN
and not _BIG_ENDIAN.

Two fixes were possible:
1. Change the negation order of the conditions in the second test.
2. Reverse the order of the conditions in the second test.

Use the second option so that the condition we expect is always a
positive check.

Signed-off-by: Ben Walton 
---
I think that this should be applied to cb/byte-swap and re-merged
with next.

 compat/bswap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/bswap.h b/compat/bswap.h
index c4293db..f6fd9a6 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -120,7 +120,7 @@ static inline uint64_t git_bswap64(uint64_t x)
 
 # if defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN)
 #  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
-# elif defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN)
+# elif defined(_LITTLE_ENDIAN) && !defined(_BIG_ENDIAN)
 #  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
 # else
 #  error "Cannot determine endianness"
-- 
1.9.1

--
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/5] fetch doc: update note on '+' in front of the refspec

2014-05-30 Thread Marc Branchaud

On 14-05-29 06:42 PM, Junio C Hamano wrote:
> While it is not *wrong* per-se to say that pulling a rewound/rebased
> branch will lead to an unnecessary merge conflict, that is not what
> the leading "+" sign to allow non-fast-forward update of remote-tracking
> branch is at all.
> 
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/pull-fetch-param.txt | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/pull-fetch-param.txt 
> b/Documentation/pull-fetch-param.txt
> index 18cffc2..2a7e2b7 100644
> --- a/Documentation/pull-fetch-param.txt
> +++ b/Documentation/pull-fetch-param.txt
> @@ -24,15 +24,15 @@ is updated even if it does not result in a fast-forward
>  update.
>  +
>  [NOTE]
> -If the remote branch from which you want to pull is
> -modified in non-linear ways such as being rewound and
> -rebased frequently, then a pull will attempt a merge with
> -an older version of itself, likely conflict, and fail.
> -It is under these conditions that you would want to use
> -the `+` sign to indicate non-fast-forward updates will
> -be needed.  There is currently no easy way to determine
> -or declare that a branch will be made available in a
> -repository with this behavior; the pulling user simply
> +When the remote branch you want to fetch is known to
> +be rewound and rebased regularly, it is expected that
> +the tip of it will not be descendant of the commit that
> +used to be at its tip the last time you fetched it and
> +stored in your remote-tracking branch.  You would want

I think the second part of that last sentence might be clearer as

it is expected that its new tip will not be a descendant of
its previous tip (as stored in your remote-tracking branch
the last time you fetched).

Then start the next sentence with

In this case, you would want 


M.

> +to use the `+` sign to indicate non-fast-forward updates
> +will be needed for such branches.  There is no way to
> +determine or declare that a branch will be made available
> +in a repository with this behavior; the pulling user simply
>  must know this is the expected usage pattern for a branch.
>  +
>  [NOTE]
> 
--
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] fetch doc: update introductory part for clarity

2014-05-30 Thread Marc Branchaud

On 14-05-29 06:42 PM, Junio C Hamano wrote:
>  - "Branches" is a more common way to say "heads" in these days.
> 
>  - Remote-tracking branches are used a lot more these days and it is
>worth mentioning that it is one of the primary side effects of
>the command to update them.
> 
>  - Avoid "X. That means Y."  If Y is easier to understand to
>readers, just say that upfront.
> 
>  - Use of explicit refspec to fetch tags does not have much to do
>with turning "auto following" on or off.  It is a way to fetch
>tags that otherwise would not be fetched by auto-following.
> 
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/git-fetch.txt | 29 -
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
> index 5809aa4..d5f5b54 100644
> --- a/Documentation/git-fetch.txt
> +++ b/Documentation/git-fetch.txt
> @@ -17,20 +17,23 @@ SYNOPSIS
>  
>  DESCRIPTION
>  ---
> -Fetches named heads or tags from one or more other repositories,
> -along with the objects necessary to complete them.
> -
> -The ref names and their object names of fetched refs are stored
> -in `.git/FETCH_HEAD`.  This information is left for a later merge
> -operation done by 'git merge'.
> -
> -By default, tags are auto-followed.  This means that when fetching
> -from a remote, any tags on the remote that point to objects that exist
> -in the local repository are fetched.  The effect is to fetch tags that
> +Fetch branches and/or tags (collectively, "refs") from one or more
> +other repositories, along with the objects necessary to complete the
> +histories of them.

Phrasing: s/the histories of them/their histories/

> +
> +The names of refs that are fetched, together with the object names
> +they point at, are written to `.git/FETCH_HEAD`.  This information
> +is used by a later merge operation done by 'git merge'.  In addition,

Isn't this merge stuff about pull, not fetch?

> +the remote-tracking branches may be updated (see description on
> + below for details).

I realize that "may be updated" is strictly correct, in that if the remote's
branches have not changed since the last fetch then the local tracking
branches won't change.

But it took me a second or two to think of that.  The "may" kindof tripped me
up.  The fact is that the local tracking branches are always updated to match
the remote's branches, it's just that sometimes the remote's branches don't
change.  So I think it would be clearer to say

the remote-tracking branches are updated

because this makes it clear that the command always makes your local tracking
branches match the remote's.

M.

> +
> +By default, any tag that points into the histories being fetched is
> +also fetched; the effect is to fetch tags that
>  point at branches that you are interested in.  This default behavior
> -can be changed by using the --tags or --no-tags options, by
> -configuring remote..tagopt, or by using a refspec that fetches
> -tags explicitly.
> +can be changed by using the --tags or --no-tags options or by
> +configuring remote..tagopt.  By using a refspec that fetches tags
> +explicitly, you can fetch tags that do not point into branches you
> +are interested in as well.
>  
>  'git fetch' can fetch from either a single named repository,
>  or from several repositories at once if  is given and
> 
--
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

[RFC PATCH] t5538: move http push tests out to t5542

As 0232852b, but for the push tests instead: this avoids a start_httpd
in the middle of the file, which fails under GIT_TEST_HTTPD=false.

Signed-off-by: Nick Alcock 
---
On 30 May 2014, Jeff King said:
> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
> index be951a4..a980574 100755
> --- a/t/t5537-fetch-shallow.sh
> +++ b/t/t5537-fetch-shallow.sh

btw, t5538-push-shallow.sh has the same bug but was not fixed up that I
can see. (Though I can't see the original fix in the history of master
either: was it squashed away somehow?)

This new test fails if the patch is applied as-is because I haven't done
any ref adjustment (I don't honestly know what results are expected, and
the actual results I'm seeing, with a ref named '1s0' intermittently
replacing either 1 or 10 and 1 both, are so bizarre and inconsistent
that I suspect I'm hitting another local config problem of some sort.)

But something like this is needed.

(signed off with my work email address for paranoia's sake.)

 t/t5538-push-shallow.sh  |  59 -
 t/t5542-push-http-shallow.sh | 101 +++
 2 files changed, 101 insertions(+), 59 deletions(-)
 create mode 100755 t/t5542-push-http-shallow.sh

diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
index 8e54ac5..ceee95b 100755
--- a/t/t5538-push-shallow.sh
+++ b/t/t5538-push-shallow.sh
@@ -120,63 +120,4 @@ EOF
git cat-file blob `echo 1|git hash-object --stdin` >/dev/null
)
 '
-
-if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
-   say 'skipping remaining tests, git built without http support'
-   test_done
-fi
-
-. "$TEST_DIRECTORY"/lib-httpd.sh
-start_httpd
-
-test_expect_success 'push to shallow repo via http' '
-   git clone --bare --no-local shallow 
"$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-   (
-   cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-   git config http.receivepack true
-   ) &&
-   (
-   cd full &&
-   commit 9 &&
-   git push $HTTPD_URL/smart/repo.git +master:refs/remotes/top/master
-   ) &&
-   (
-   cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-   git fsck &&
-   git log --format=%s top/master >actual &&
-   cat actual &&
-   cat tracked &&
+   git add tracked &&
+   git commit -m "$1"
+}
+
+test_expect_success 'setup' '
+   git config --global transfer.fsckObjects true &&
+   commit 1 &&
+   commit 2 &&
+   commit 3 &&
+   commit 4 &&
+   git clone . full &&
+   (
+   git init full-abc &&
+   cd full-abc &&
+   commit a &&
+   commit b &&
+   commit c
+   ) &&
+   git clone --no-local --depth=2 .git shallow &&
+   git --git-dir=shallow/.git log --format=%s >actual &&
+   cat actual &&
+   cat actual &&
+   cat

Re: [PATCH] check_refname_component: Optimize

2014-05-30 Thread Michael Haggerty

On 05/30/2014 02:07 AM, Jeff King wrote:
> On Fri, May 30, 2014 at 06:43:18AM +0700, Duy Nguyen wrote:
> 
 The first time we read packed_refs, check_refname_format() is called
 in read_packed_refs()->create_ref_entry() as usual. If we find no
 problem, we store packed_refs stat() info in maybe packed_refs.stat.
 Next time we read packed_refs, if packed_refs.stat is there and
 indicates that packed_refs has not changed, we can make
 create_ref_entry() ignore check_refname_format() completely.
>>>
>>> I'm confused. Why would we re-open packed-refs at all if the stat
>>> information hasn't changed?
>>
>> No, not in the same process. In the next run.
> 
> Ah, I thought "packed_refs.stat" was a struct member, but I guess you
> mean it as a filename.
> 
> But then we're just trusting that the "trust me" flag on disk is
> correct. Why not just trust that packed-refs is correct in the first
> place?
> 
> IOW, consider this progression of changes:
> 
>   1. Check refname format when we read packed-refs (the current
>  behavior).
> 
>   2. Keep a separate file "packed-refs.stat" with stat information. If
>  the packed-refs file matches that stat information, do not bother
>  checking refname formats.
> 
>   3. Put a flag in "packed-refs" that says "trust me, I'm valid". Check
>  the refnames when it is generated.
> 
>   4. Realize that we already check the refnames when we write it out.
>  Don't bother writing "trust me, I'm valid"; readers can assume that
>  it is.
> 
> What is the scenario that option (2) protects against that options (3)
> and (4) do not?
> 
> I could guess something like "the writer has a different idea of what a
> valid refname is than we do". But that applies as well to (2), but just
> as "the reader who wrote packed-refs.stat has a different idea than we
> do".

If we want to be robust to future changes to refname rules, we could add
a header flag like

# pack-refs with: peeled fully-peeled check-level=1.0

which promises that the reference names in the file conform to the
current ("version 1.0") check_refname_format() rules.

If we ever make the rules stricter (a "backwards-compatible" change), we
would increment the check-level to 1.1.  That way, an old reader, who
knows about check-level=1.0 but not check-level=1.1, can still trust
that the refnames in the file conform to its check_refname_format()
rules and avoid the verification step.  (Of course if that version
writes the file again, it would have to set the check-level=1.0 tag, and
newer software would be forced to validate on reading to be sure that
the refnames still conform to check-level=1.1.)

If we make the rules looser (a "backwards-incompatible" change), we
would increment the check-level to 2.0.  In that case readers who only
know about check-level 1.x would have to turn their verification code
back on when reading the file to ensure that they can work with the
refnames that it contains.

Format changes should be infrequent enough, and the cost of verification
is low enough, that sometimes ping-ponging back and forth between
software versions shouldn't be a problem in practice.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 2.0.0 PROFILE=BUILD check-phase problems with ./t5561-http-backend.sh; GIT_TEST_HTTPD=false problems with t5537-fetch-shallow.sh