Re: [PATCH v4 06/16] merge_recursive: abort properly upon errors

[Johannes Schindelin]

Hi Junio,

On Mon, 25 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > There are a couple of places where return values indicating errors
> > are ignored. Let's teach them manners.
> 
> That is because the return value never indicated errors before this
> series, isn't it?  A true error used to be expressed by dying, and
> the return value indicating "cleanliness" of the merge were
> deliberately ignored.
> 
> The world order changed by previous patches in this series and the
> callers need to be updated to take the new kind of return values
> into account.  That is not teaching them manners ;-)

I reworded that commit message.

> > Signed-off-by: Johannes Schindelin 
> > ---
> >  merge-recursive.c | 10 --
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/merge-recursive.c b/merge-recursive.c
> > index dc3182b..2d4cb80 100644
> > --- a/merge-recursive.c
> > +++ b/merge-recursive.c
> > @@ -1949,8 +1949,9 @@ int merge_recursive(struct merge_options *o,
> > saved_b2 = o->branch2;
> > o->branch1 = "Temporary merge branch 1";
> > o->branch2 = "Temporary merge branch 2";
> > -   merge_recursive(o, merged_common_ancestors, iter->item,
> > -   NULL, _common_ancestors);
> > +   if (merge_recursive(o, merged_common_ancestors, iter->item,
> > +   NULL, _common_ancestors) < 0)
> > +   return -1;
> > o->branch1 = saved_b1;
> > o->branch2 = saved_b2;
> > o->call_depth--;
> 
> This hunk feels somewhat wrong as-is.
> 
> There is a comment before the pre-context explaining why cleanness
> flag is ignored.  It needs to be updated.  We still do not care
> about cleanliness, i.e. 0=clean, 1=merged with conflict, but we now
> can get negative values so we need to reject and return early if
> this call indicates an error.

I updated that comment. Hopefully now the hunk makes sense ;-)

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

Re: [PATCH v4 02/16] Report bugs consistently

[Johannes Schindelin]

Hi Junio & Peff,

On Mon, 25 Jul 2016, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Mon, Jul 25, 2016 at 02:44:25PM -0700, Junio C Hamano wrote:
> >
> >> > diff --git a/imap-send.c b/imap-send.c
> >> > index db0fafe..67d67f8 100644
> >> > --- a/imap-send.c
> >> > +++ b/imap-send.c
> >> > @@ -506,12 +506,12 @@ static char *next_arg(char **s)
> >> >  
> >> >  static int nfsnprintf(char *buf, int blen, const char *fmt, ...)
> >> >  {
> >> > -int ret;
> >> > +int ret = -1;
> >> >  va_list va;
> >> >  
> >> >  va_start(va, fmt);
> >> >  if (blen <= 0 || (unsigned)(ret = vsnprintf(buf, blen, fmt, 
> >> > va)) >= (unsigned)blen)
> >> > -die("Fatal: buffer too small. Please report a bug.");
> >> > +die("BUG: buffer too small (%d < %d)", ret, blen);
> >> >  va_end(va);
> >> >  return ret;
> >> >  }
> >> 
> >> If "you gave me this size but you need at least this much" is truly
> >> worth reporting, then this is misleading (ret is shown as -1 but you
> >> do not even know how much is necessary).  In any case, this should
> >> be done as a separate step anyway.
> >
> > Hrm, isn't "ret" going to be the necessary size? According to the
> > standard, it should tell us how many bytes were needed, not "-1" (this
> > is the "your vsnprintf is broken" case handled by the strbuf code).
> 
> Yes.  If blen <= 0, we do not even do vsnprintf() and that is why
> Dscho added "int ret = -1" initialization; otherwise his new die()
> would end up referencing uninitialized ret.

Exactly. While I was fixing this bug message, it occurred to me that it
makes little sense to ask a user to report a bug when it is unknown how
small the buffer was and what would have been the desired buffer size. So
I did a fly-by fix.

However, it is true that this is completely outside the purpose of this
patch series (in fact, most of this patch is completely outside the
purpose, and I am regretting that dearly, as the patch series' submission
already takes almost a month, and I only now get people to review the
critical parts of the changes).

So I simply backed out the more verbose message and *only* do the obvious
thing: replace the "Fatal:" prefix by a "BUG:" one.

> > I do think the numbers are reversed, though. It should be "blen < ret".
> 
> That, too ;-)

True. All the better that I reverted that part of the patch ;-)

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

Re: [PATCH v1 3/3] convert: add filter..useProtocol option

[Jakub Narębski]

W dniu 2016-07-25 o 22:16, Lars Schneider pisze:
> 
> On 24 Jul 2016, at 23:30, Jakub Narębski  wrote:
> 
>> W dniu 2016-07-24 o 22:14, Jakub Narębski pisze:
>>> W dniu 2016-07-24 o 20:36, Lars Schneider pisze:
>>
 I agree that the name is not ideal. "UseProtocol" as it is would be a 
 boolean. 
 I thought about "persistent" but this name wouldn't convey the scope of 
 the 
 persistency ("persistent for one Git operation" vs. "persistent for many 
 Git 
 operations"). What do you think about the protocol as int version idea
 described in $gmane/300155 ?
>>>
>>> You mean the `protocol` as a config variable name (fully name being
>>> `filter..protocol`), being integer-valued, isn't it? Wouldn't
>>> `protocolVersion` be a more explicit?
>>
>> Just throwing out further ideas:
>>
>> Perhaps make `persistent` string-valued variable, with the only value
>> supported for now, namely "per-process" / "operation"?
>>
>> Perhaps require for `pidfile` to be present for it to be daemon,
>> that is persist for possibly many Git operations. Or allow "daemon"
>> or "server" value for `persistent`, then?
> 
> I like the direction of this idea. What if we use a string-valued 
> "filter..protocol" with the following options:
> 
> "simple" / "invocation-per-file" / << empty >> --> current clean/smudge 
> behavior
> "invocation-per-process" --> new, proposed behavior
> 
> If necessary this could be enhanced in the future to support even a "daemon"
> mode (with a pidfile config).

Though, after thinking about it, this solution has the problem
that people might think that they can use their old per-file
filters, just flipping the `filter..protocol`.

I dunno.
-- 
Jakub Narębski

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

Re: [PATCH v4 01/16] Verify that `git pull --rebase` shows the helpful advice when failing

[Johannes Schindelin]

Hi Junio,

On Mon, 25 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > +test_expect_success '--rebase with conflicts shows advice' '
> > +   test_when_finished "git rebase --abort; git checkout -f to-rebase" &&
> > +   git checkout -b seq &&
> > +   printf "1\\n2\\n3\\n4\\n5\\n" >seq.txt &&
> 
> Make this more readble by using test-write-lines, perhaps?

Or even test_seq. Thanks for pointing my nose to this.

> > +   git add seq.txt &&
> > +   test_tick &&
> > +   git commit -m "Add seq.txt" &&
> > +   printf "6\\n" >>seq.txt &&
> > +   test_tick &&
> > +   git commit -m "Append to seq.txt" seq.txt &&
> > +   git checkout -b with-conflicts HEAD^ &&
> > +   printf "conflicting\\n" >>seq.txt &&
> > +   test_tick &&
> > +   git commit -m "Create conflict" seq.txt &&
> > +   test_must_fail git pull --rebase . seq 2>err >out &&
> > +   grep "When you have resolved this problem" out
> > +'
> > +test_expect_success 'failed --rebase shows advice' '
> 
> Need a blank line before this one.

Yep, sorry.

> > +   test_when_finished "git rebase --abort; git checkout -f to-rebase" &&
> > +   git checkout -b diverging &&
> > +   test_commit attributes .gitattributes "* text=auto" attrs &&
> > +   sha1="$(printf "1\\r\\n" | git hash-object -w --stdin)" &&
> > +   git update-index --cacheinfo 0644 $sha1 file &&
> > +   git commit -m v1-with-cr &&
> > +   git checkout -f -b fails-to-rebase HEAD^ &&
> 
> It is unclear what the "-f" is for; is it attempting to clean up a
> potential mess previous steps might have left?  We didn't have it in
> the previous test above.

It is there to clean up a very non-potential mess: forcing a CR/LF into a
file marked with `text=auto` makes a royal mess. Neither `git reset
--hard` nor `git stash` will make that file clean. As a consequence, the
`git checkout` without an `-f` would *always* fail "because of uncommitted
changes".

I clarified that in a comment.

> > +   test_commit v2-without-cr file "2" file2-lf &&
> > +   test_must_fail git pull --rebase . diverging 2>err >out &&
> > +   grep "When you have resolved this problem" out
> > +'
> > +
> >  test_expect_success '--rebase fails with multiple branches' '
> > git reset --hard before-rebase &&
> > test_must_fail git pull --rebase . copy master 2>err &&
> 
> Not worth a reroll but after this series settles we would probably
> want to address some of the above up with a follow-up clean-up patch.

As I re-roll anyway, no big deal.

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

[PATCH v3] i18n: notes: mark comment for translation

[Vasco Almeida]

Mark comment displayed when editing a note for translation.

Signed-off-by: Vasco Almeida 
---
 builtin/notes.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 0572051..aec427b 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {
 };
 
 static const char note_template[] =
-   "\nWrite/edit the notes for the following object:\n";
+   N_("Write/edit the notes for the following object:");
 
 struct note_data {
int given;
@@ -179,8 +179,9 @@ static void prepare_note_data(const unsigned char *object, 
struct note_data *d,
copy_obj_to_fd(fd, old_note);
 
strbuf_addch(, '\n');
-   strbuf_add_commented_lines(, note_template, 
strlen(note_template));
-   strbuf_addch(, '\n');
+   strbuf_add_commented_lines(, "\n", strlen("\n"));
+   strbuf_add_commented_lines(, _(note_template), 
strlen(_(note_template)));
+   strbuf_add_commented_lines(, "\n", strlen("\n"));
write_or_die(fd, buf.buf, buf.len);
 
write_commented_object(fd, object);
-- 
2.7.4

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

Re: [PATCH v2] i18n: notes: mark comment for translation

[Vasco Almeida]

A Seg, 25-07-2016 às 10:49 -0700, Junio C Hamano escreveu:
> Vasco Almeida  writes:
> 
> > 
> >  static const char note_template[] =
> > -   "\nWrite/edit the notes for the following object:\n";
> > +   N_("Write/edit the notes for the following object:");
> >  
> >  struct note_data {
> >     int given;
> > @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned
> > char *object, struct note_data *d,
> >     copy_obj_to_fd(fd, old_note);
> >  
> >     strbuf_addch(, '\n');
> > -   strbuf_add_commented_lines(, note_template,
> > strlen(note_template));
> > +   strbuf_addch(, '\n');
> > +   strbuf_add_commented_lines(, _(note_template),
> > strlen(_(note_template)));
> 
> I do not quite understand why you want the blank lines surrounding
> the message outside add_commented_lines() call.  I think the intent
> is to produce
> 
> #
> # Write/edit the notes for the following object:
> #

If this is what we want, I will send a re-roll accordingly.

> with the single call.  If you pushed the newlines outside the
> message, wouldn't you end up having this instead ( denoting an
> extra empty line each before and after the message)?
> 
> 
> # Write/edit the notes for the following object:
> 
> 
Yes, this was my intention. The original does:

    #
    # Write/edit the notes for the following object:
    

--
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/3] push: add shorthand for --force-with-lease branch creation

[John Keeping]

On Tue, Jul 26, 2016 at 12:30:05PM +0200, Jakub Narębski wrote:
> W dniu 2016-07-25 o 23:59, John Keeping pisze:
> 
> > +test_expect_success 'new branch covered by force-with-lease (explicit)' '
> > +   setup_srcdst_basic &&
> > +   (
> > +   cd dst &&
> > +   git branch branch master &&
> > +   git push --force-with-lease=branch: origin branch
> > +   ) &&
> > +   git ls-remote dst refs/heads/branch >expect &&
> > +   git ls-remote src refs/heads/branch >actual &&
> > +   test_cmp expect actual
> > +'
> 
> Do we need to test the negative, that is that if branch is not
> new it prevents push (e.g. when  is HEAD), or is it
> covered by other tests?

It's covered by a test in patch 3 (at least for the implicit case added
there), but I could pull that forwards.  In fact, converting that test
to the explicit syntax will make it simpler since we won't need to set
up a non-fast-forward push.
--
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 v1 3/3] convert: add filter..useProtocol option

[Jakub Narębski]

W dniu 2016-07-25 o 22:32, Lars Schneider pisze: 
> On 25 Jul 2016, at 01:22, Jakub Narębski  wrote:
>> W dniu 2016-07-25 o 00:36, Ramsay Jones pisze:
>>> On 24/07/16 18:16, Lars Schneider wrote:

 It was a conscious decision to have the `filter` talk first.
 My reasoning was:
 
 (1) I want a reliable way to distinguish the existing filter 
 protocol ("single-shot invocation") from the new one ("long 
 running"). I don't think there would be a situation where the 
 existing protocol would talk first. Therefore the users would 
 not accidentally mix them with a possibly half working, 
 undetermined, outcome.
>>> 
>>> If an 'single-shot' filter were incorrectly configured, instead 
>>> of a new one, then the interaction could last a little while - 
>>> since it would result in deadlock! ;-)
>>> 
>>> [If Git talks first instead, configuring a 'single-shot' filter 
>>> _may_ still result in a deadlock - depending on pipe size, etc.]
>> 
>> Would it be possible to do an equivalent of sending empty file to 
>> the filter? If it is misconfigured old-style script, it would exit 
>> after possibly empty output; if not, we would start new-style 
>> interaction.
> 
> I think we would need to close the pipe to communicate "end" to the 
> filter, no? I would prefer to define the protocol explicitly as this 
> is clearly easier.

Well, we could always close stdin of a script, check if it quits,
then reopen. Or close stdin, and send commands via file descriptor 4.
Or send SIGPIPE. But I don't know if it is a good idea.

> On 25 Jul 2016, at 00:36, Ramsay Jones  wrote:

>> If an 'single-shot' filter were incorrectly configured, instead of
>> a new one, then the interaction could last a little while - since
>> it would result in deadlock! ;-)
>> 
>> [If Git talks first instead, configuring a 'single-shot' filter
>> _may_ still result in a deadlock - depending on pipe size, etc.]
> 
> Do you think this is an issue that needs to be addressed in the first
> version? If yes, I would probably look into "select" to specify a
> timeout for the filter.

This might be a better idea.  Additionally, it would make it possible
to detect buggy v2 filter scripts.

> However, wouldn't the current "single-shot"
> clean/smudge filter block in the same way if they don't write
> anything?

Hmmm... so deadlocking (waiting for user to press ^C) might be
an acceptable solution. It would be good to tell him or her why
there was a deadlock (catch SIGINT), that Git was waiting for
specific command in a specific filter driver, for a specific file.


On the other hand v2 protocol has an additional problem: users
switching to v2, while using old one-shot filters (that worked
correctly).  So in my opinion you need to ensure two things:

(1) name things in such way that it is easy to see that you
need to write filter script specifically for the v2 protocol,

(2) if possible, do not hang but warn the user if he or she
wants to use v1 filter (per-file) with v2 protocol (per-command),
or at least help diagnose the issue.

>> This should be tested, if we agree that detecting misconfigured 
>> filters is a good thing.

[clarified]

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

Re: [PATCH v2 2/3] push: add shorthand for --force-with-lease branch creation

[Jakub Narębski]

W dniu 2016-07-25 o 23:59, John Keeping pisze:

> +test_expect_success 'new branch covered by force-with-lease (explicit)' '
> + setup_srcdst_basic &&
> + (
> + cd dst &&
> + git branch branch master &&
> + git push --force-with-lease=branch: origin branch
> + ) &&
> + git ls-remote dst refs/heads/branch >expect &&
> + git ls-remote src refs/heads/branch >actual &&
> + test_cmp expect actual
> +'

Do we need to test the negative, that is that if branch is not
new it prevents push (e.g. when  is HEAD), or is it
covered by other tests?

-- 
Jakub Narębski

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

[PATCH 1/2] fix passing a name for config from submodules

[Heiko Voigt]

In commit 959b5455 we implemented the initial version of the submodule
config cache. During development of that initial version we extracted
the function gitmodule_sha1_from_commit(). During that process we missed
that the strbuf rev was still used in config_from() and now is left
empty. Lets fix this by also returning this string.

Signed-off-by: Heiko Voigt 
---
This is based on 508a285 in next.

 submodule-config.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 1210b26..853989e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -349,9 +349,9 @@ static int parse_config(const char *var, const char *value, 
void *data)
 }
 
 static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
- unsigned char *gitmodules_sha1)
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev)
 {
-   struct strbuf rev = STRBUF_INIT;
int ret = 0;
 
if (is_null_sha1(commit_sha1)) {
@@ -359,11 +359,10 @@ static int gitmodule_sha1_from_commit(const unsigned char 
*commit_sha1,
return 1;
}
 
-   strbuf_addf(, "%s:.gitmodules", sha1_to_hex(commit_sha1));
-   if (get_sha1(rev.buf, gitmodules_sha1) >= 0)
+   strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
+   if (get_sha1(rev->buf, gitmodules_sha1) >= 0)
ret = 1;
 
-   strbuf_release();
return ret;
 }
 
@@ -375,6 +374,7 @@ static const struct submodule *config_from(struct 
submodule_cache *cache,
const unsigned char *commit_sha1, const char *key,
enum lookup_type lookup_type)
 {
+   struct strbuf rev = STRBUF_INIT;
unsigned long config_size;
char *config;
unsigned char sha1[20];
@@ -397,8 +397,10 @@ static const struct submodule *config_from(struct 
submodule_cache *cache,
return entry->config;
}
 
-   if (!gitmodule_sha1_from_commit(commit_sha1, sha1))
+   if (!gitmodule_sha1_from_commit(commit_sha1, sha1, )) {
+   strbuf_release();
return NULL;
+   }
 
switch (lookup_type) {
case lookup_name:
@@ -408,14 +410,19 @@ static const struct submodule *config_from(struct 
submodule_cache *cache,
submodule = cache_lookup_path(cache, sha1, key);
break;
}
-   if (submodule)
+   if (submodule) {
+   strbuf_release();
return submodule;
+   }
 
config = read_sha1_file(sha1, , _size);
-   if (!config)
+   if (!config) {
+   strbuf_release();
return NULL;
+   }
 
if (type != OBJ_BLOB) {
+   strbuf_release();
free(config);
return NULL;
}
@@ -425,8 +432,9 @@ static const struct submodule *config_from(struct 
submodule_cache *cache,
parameter.commit_sha1 = commit_sha1;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
-   git_config_from_mem(parse_config, "submodule-blob", "",
+   git_config_from_mem(parse_config, "submodule-blob", rev.buf,
config, config_size, );
+   strbuf_release();
free(config);
 
switch (lookup_type) {
-- 
2.4.2.387.gf86f31a

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

[PATCH 2/2] submodule-config: combine error checking if clauses

[Heiko Voigt]

So we have less return handling code.

Signed-off-by: Heiko Voigt 
---
 submodule-config.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 853989e..cb9bf8f 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -416,12 +416,7 @@ static const struct submodule *config_from(struct 
submodule_cache *cache,
}
 
config = read_sha1_file(sha1, , _size);
-   if (!config) {
-   strbuf_release();
-   return NULL;
-   }
-
-   if (type != OBJ_BLOB) {
+   if (!config || type != OBJ_BLOB) {
strbuf_release();
free(config);
return NULL;
-- 
2.4.2.387.gf86f31a

--
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] git subcommand to check if branch is up-to-date with upstream

[Sidhant Sharma]

On Monday 25 July 2016 11:03 PM, Jakub Narębski wrote:
> W dniu 2016-07-25 o 18:58, Junio C Hamano pisze:
>> Sidhant Sharma  writes:
>>
>>> I was wondering if it would be a good idea to have a command to check if a
>>> push or pull is required. Perhaps it can also suggest if changes are
>>> fast-forward or the branches (local and remote) have diverged.
>> Doesn't "branch -v" give that information these days?  You'd need to
>> "fetch" first to get the up-to-date worldview before running it, of
>> course.
> You need "branch -v -v". For current branch, you can simply run "git 
> checkout".
> All this is the information for end user, not scripts.
>
> $ git branch -v -v
> * gitweb-docs   4ebf58d [origin/master: ahead 1] gitweb(1): Document query 
> parameters
>   master08bb350 [origin/master] Sixth batch of topics for 2.10
>
> $ git checkout
> Your branch is ahead of 'origin/master' by 1 commit.
>   (use "git push" to publish your local commits)
>
Nice, didn't know that one. Thanks for the tip.

Regards,
Sidhant Sharma
--
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: [ANN] Pro Git Reedited 2nd Edition

[Manlio Perillo]

On Sun, Jul 24, 2016 at 6:07 AM, Jon Forrest  wrote:
> This an announcement of Pro Git Reedited 2nd Edition, which is
> a substantial edit of Chacon and Straub's Pro Git 2nd Edition.
> I spent a lot of time tightening it up and maybe clearing
> up some explanations.
>
> The pdf is downloadable at:
> https://drive.google.com/open?id=0B-Llso12P94-Ujg5Z1dhWUhhMm8
>

Thanks for your work.

I have noted a problem when reading the PDF with Chromium: the
anchors/links do not work.

I don't know if this is an issue with the conversion to PDF or an
issue with Chromium.


Manlio
--
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/3] push: add shorthand for --force-with-lease branch creation

[John Keeping]

On Mon, Jul 25, 2016 at 03:22:48PM -0700, Junio C Hamano wrote:
> John Keeping  writes:
> 
> > Allow the empty string to stand in for the null SHA-1 when pushing a new
> > branch, like we do when deleting branches.
> >
> > This means that the following command ensures that `new-branch` is
> > created on the remote (that is, is must not already exist):
> >
> > git push --force-with-lease=new-branch: origin new-branch
> >
> > Signed-off-by: John Keeping 
> > ---
> > New in v2.
> >
> >  Documentation/git-push.txt |  3 ++-
> >  remote.c   |  2 ++
> >  t/t5533-push-cas.sh| 12 
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> > index bf7c9a2..927a034 100644
> > --- a/Documentation/git-push.txt
> > +++ b/Documentation/git-push.txt
> > @@ -201,7 +201,8 @@ if it is going to be updated, by requiring its current 
> > value to be
> >  the same as the specified value `` (which is allowed to be
> >  different from the remote-tracking branch we have for the refname,
> >  or we do not even have to have such a remote-tracking branch when
> > -this form is used).
> > +this form is used).  If `` is the empty string, then the named ref
> > +must not already exist.
> >  +
> >  Note that all forms other than `--force-with-lease=:`
> >  that specifies the expected current value of the ref explicitly are
> > diff --git a/remote.c b/remote.c
> > index a326e4e..af94892 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -2294,6 +2294,8 @@ int parse_push_cas_option(struct push_cas_option 
> > *cas, const char *arg, int unse
> > entry = add_cas_entry(cas, arg, colon - arg);
> > if (!*colon)
> > entry->use_tracking = 1;
> > +   else if (!colon[1])
> > +   memset(entry->expect, 0, sizeof(entry->expect));
> 
> hashclr()?

Yes (and in the following patch as well).  I hadn't realised that
function exists.

> > else if (get_sha1(colon + 1, entry->expect))
> > return error("cannot parse expected object name '%s'", colon + 
> > 1);
> > return 0;
> > diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> > index c732012..5e7f6e9 100755
> > --- a/t/t5533-push-cas.sh
> > +++ b/t/t5533-push-cas.sh
> > @@ -191,4 +191,16 @@ test_expect_success 'cover everything with default 
> > force-with-lease (allowed)' '
> > test_cmp expect actual
> >  '
> >  
> > +test_expect_success 'new branch covered by force-with-lease (explicit)' '
> > +   setup_srcdst_basic &&
> > +   (
> > +   cd dst &&
> > +   git branch branch master &&
> > +   git push --force-with-lease=branch: origin branch
> > +   ) &&
> > +   git ls-remote dst refs/heads/branch >expect &&
> > +   git ls-remote src refs/heads/branch >actual &&
> > +   test_cmp expect actual
> > +'
> > +
> >  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

[PATCH] commit: Fix description of no-verify

[Orgad Shaneh]

From: Orgad Shaneh 

include also commit-msg hook.

Signed-off-by: Orgad Shaneh 
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 163dbca..2725712 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1616,7 +1616,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, "interactive", , N_("interactively add 
files")),
OPT_BOOL('p', "patch", _interactive, N_("interactively 
add changes")),
OPT_BOOL('o', "only", , N_("commit only specified files")),
-   OPT_BOOL('n', "no-verify", _verify, N_("bypass pre-commit 
hook")),
+   OPT_BOOL('n', "no-verify", _verify, N_("bypass pre-commit 
and commit-msg hooks")),
OPT_BOOL(0, "dry-run", _run, N_("show what would be 
committed")),
OPT_SET_INT(0, "short", _format, N_("show status 
concisely"),
STATUS_FORMAT_SHORT),
-- 
2.8.2

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

[PATCH] merge: Run commit-msg hook

[Orgad Shaneh]

From: Orgad Shaneh 

commit-msg is needed to either validate the commit message or edit it.
Gerrit for instance uses this hook to append its Change-Id footer.

This is relevant to merge commit just like any other commit.

Signed-off-by: Orgad Shaneh 
---
 Documentation/git-merge.txt | 6 +-
 builtin/merge.c | 7 ++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..59508aa 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
[-s ] [-X ] [-S[]]
-   [--[no-]allow-unrelated-histories]
+   [--[no-]allow-unrelated-histories] [--no-verify]
[--[no-]rerere-autoupdate] [-m ] [...]
 'git merge'  HEAD ...
 'git merge' --abort
@@ -87,6 +87,10 @@ invocations. The automated message can include the branch 
description.
Allow the rerere mechanism to update the index with the
result of auto-conflict resolution if possible.
 
+--no-verify::
+   This option bypasses the commit-msg hook.
+   See also linkgit:githooks[5].
+
 --abort::
Abort the current conflict resolution process, and
try to reconstruct the pre-merge state.
diff --git a/builtin/merge.c b/builtin/merge.c
index b555a1b..30c03c8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -51,7 +51,7 @@ static const char * const builtin_merge_usage[] = {
 static int show_diffstat = 1, shortlog_len = -1, squash;
 static int option_commit = 1;
 static int option_edit = -1;
-static int allow_trivial = 1, have_message, verify_signatures;
+static int allow_trivial = 1, have_message, verify_signatures, no_verify;
 static int overwrite_ignore = 1;
 static struct strbuf merge_msg = STRBUF_INIT;
 static struct strategy **use_strategies;
@@ -228,6 +228,7 @@ static struct option builtin_merge_options[] = {
{ OPTION_STRING, 'S', "gpg-sign", _commit, N_("key-id"),
  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_BOOL(0, "overwrite-ignore", _ignore, N_("update ignored 
files (default)")),
+   OPT_BOOL(0, "no-verify", _verify, N_("bypass commit-msg hook")),
OPT_END()
 };
 
@@ -809,6 +810,10 @@ static void prepare_to_commit(struct commit_list 
*remoteheads)
if (launch_editor(git_path_merge_msg(), NULL, NULL))
abort_commit(remoteheads, NULL);
}
+   if (!no_verify &&
+   run_commit_hook(0 < option_edit, get_index_file(), "commit-msg",
+   git_path_merge_msg(), NULL))
+   abort_commit(remoteheads, NULL);
read_merge_msg();
strbuf_stripspace(, 0 < option_edit);
if (!msg.len)
-- 
2.8.2

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

Re: What's cooking in git.git (Jul 2016, #07; Mon, 25)

[Eric Wong]

Lars Schneider  wrote:
> On 26 Jul 2016, at 00:50, Junio C Hamano  wrote:
> > 
> > * ew/git-svn-http-tests (2016-07-25) 2 commits
> > - git svn: migrate tests to use lib-httpd
> > - t/t91*: do not say how to avoid the tests
> > 
> > Reuse the lib-httpd test infrastructure when testing the subversion
> > integration that interacts with subversion repositories served over
> > the http:// protocol.
> > 
> > Will merge to 'next'.
> 
> Do I understand correctly that this patch fixes $gmane/295291 ?
> Nice! Should we add "GIT_SVN_TEST_HTTPD=true" to the .travis.yml, then?

Maybe :)

Unfortunately, that currently causes the svn file:// tests (using the
same test numbers+files) to be skipped.  We should probably define new
test numbers and figure out a way to be able to always run file:// and
http:// (and svn://) tests.

On a side note, svn:// ought to be easy since svnserve supports --inetd
and we can let the kernel bind a random port.  We could be doing the
same with git-daemon --inetd, even...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Jul 2016, #07; Mon, 25)

[Lars Schneider]

On 26 Jul 2016, at 00:50, Junio C Hamano  wrote:

> [...]
> 
> 
> * ew/git-svn-http-tests (2016-07-25) 2 commits
> - git svn: migrate tests to use lib-httpd
> - t/t91*: do not say how to avoid the tests
> 
> Reuse the lib-httpd test infrastructure when testing the subversion
> integration that interacts with subversion repositories served over
> the http:// protocol.
> 
> Will merge to 'next'.

Do I understand correctly that this patch fixes $gmane/295291 ?
Nice! Should we add "GIT_SVN_TEST_HTTPD=true" to the .travis.yml, then?

Thanks,
Lars
--
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] config.mak.uname: correct perl path on FreeBSD

[Junio C Hamano]

Eric Wong  writes:

> I dug around the freebsd history (git://github.com/freebsd/freebsd.git)
> (~1.5G) and the 3.x and 4.x releases with perl in base still contained
> many references to /usr/local/bin/perl
>
> It also seems FreeBSD 2.x releases were also perl-less in base;
> so yes, I'm alright with Duy's patch :)

Thanks for a quick digging to converge to the simplest solution.

Will replace and merge to 'next' soonish.

--
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/3] t7900-subtree.sh: fix quoting and broken && chains

[Junio C Hamano]

Eric Sunshine  writes:

>> @@ -73,10 +73,10 @@ join_commits()
>>  test_create_commit() (
>> repo=$1
>> commit=$2
>
> Perhaps &&-chain the above two lines also to future-proof against
> someone inserting important code somewhere above the following 'cd'.

Yup, also we can have them on a single line, i.e.

repo=$1 commit=$2 &&
cd "$repo" &&
...

Thanks

>
>> -   cd "$repo"
>> -   mkdir -p $(dirname "$commit") \
>> +   cd "$repo" &&
>> +   mkdir -p "$(dirname "$commit")" \
>> || error "Could not create directory for commit"
>> -   echo "$commit" >"$commit"
>> +   echo "$commit" >"$commit" &&
>> git add "$commit" || error "Could not add commit"
>> git commit -m "$commit" || error "Could not commit"
>>  )
--
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] config.mak.uname: correct perl path on FreeBSD

[Eric Wong]

Junio C Hamano  wrote:
> Eric Wong  writes:
> > Junio C Hamano  wrote:
> >> Duy Nguyen  writes:
> >> > On Mon, Jul 25, 2016 at 6:56 PM, Junio C Hamano  
> >> > wrote:
> >> >> On Mon, Jul 25, 2016 at 9:21 AM, Nguyễn Thái Ngọc Duy 
> >> >>  wrote:
> > about the problem sooner.
> >
> >> >>> Signed-off-by: Nguyễn Thái Ngọc Duy 
> >> >>> ---
> >> >>>  Tested with fbsd 10.3, kvm image. But I suppose it's the same as real
> >> >>>  fbsd.
> >> >>
> >> >> Thanks; and we know that older (but not too old that we no longer care 
> >> >> about)
> >> >> FreeBSD all have /usr/local/bin/perl?
> >> >
> >> > I'm no fbsd expert but from the first sentence in [1] "Perl has been
> >> > removed from base more than ten years ago..." I would assume it meant
> >> > "... removed from base, _to_ ports system" which means /usr/local for
> >> > all package installation (for ten years for perl). So I think we are
> >> > good.
> >> 
> >> I guess we didn't follow through
> >> 
> >> http://public-inbox.org/git/%3C20160720025630.GA71874%40plume%3E/
> >> 
> >> and allowed the thread to drift into a tangent?
> >
> > +Cc Dscho
> >
> > I've been meaning to followup on that, but had connectivity
> > problems to my VM last week.  I still prefer we use numeric
> > comparisons for version numbers since numbers are... numeric.
> > IOW, I prefer we go with my original patch.
> 
> I tend to agree with you if we have to do "systems older than this
> should use /usr/bin, others should use /usr/local/bin", but this
> different incarnation of the same topic seems to claim that older
> ones had /usr/local/bin forever anyway, and that was what made the
> patch interesting.

I dug around the freebsd history (git://github.com/freebsd/freebsd.git)
(~1.5G) and the 3.x and 4.x releases with perl in base still contained
many references to /usr/local/bin/perl

It also seems FreeBSD 2.x releases were also perl-less in base;
so yes, I'm alright with Duy's patch :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] subtree: adjust style to match CodingGuidelines

[Johannes Sixt]

These caught my eye browsing through my inbox. I'm not a subtree user.

Am 26.07.2016 um 06:14 schrieb David Aguilar:

@@ -50,87 +51,145 @@ prefix=

 debug()
 {
-   if [ -n "$debug" ]; then
-   printf "%s\n" "$*" >&2
+   if test -n "$debug"
+   then
+   printf "%s\n" "$@" >&2


Are you sure you want this? It prints each argument of the 'debug' 
invocation on its own line.



fi
 }

 say()
 {
-   if [ -z "$quiet" ]; then
-   printf "%s\n" "$*" >&2
+   if test -z "$quiet"
+   then
+   printf "%s\n" "$@" >&2


Same here.


fi
 }

 progress()
 {
-   if [ -z "$quiet" ]; then
-   printf "%s\r" "$*" >&2
+   if test -z "$quiet"
+   then
+   printf "%s\r" "$@" >&2


But here I'm pretty sure that this is not wanted; the original is 
clearly correct.



fi
 }

...

@@ -139,22 +198,27 @@ debug "command: {$command}"
 debug "quiet: {$quiet}"
 debug "revs: {$revs}"
 debug "dir: {$dir}"
-debug "opts: {$*}"
+debug "opts: {$@}"


When the arguments of a script or function are to be printed for the 
user's entertainment/education, then it is safer (and, therefore, 
idiomatic) to use "$*".



 debug

...

 cache_get()
 {
-   for oldrev in $*; do
-   if [ -r "$cachedir/$oldrev" ]; then
+   for oldrev in "$@"
+   do


It is idiomatic to write this as

for oldrev
do

(But your move from bare $* to quoted "$@" fits better under the "fix 
quoting" topic of this patch.)



+   if test -r "$cachedir/$oldrev"
+   then
read newrev <"$cachedir/$oldrev"
echo $newrev
fi

...

@@ -631,17 +749,19 @@ cmd_split()
debug "  parents: $parents"
newparents=$(cache_get $parents)
debug "  newparents: $newparents"
-   
+
tree=$(subtree_for_commit $rev "$dir")
debug "  tree is: $tree"

check_parents $parents
-   
+
# ugly.  is there no better way to tell if this is a subtree
# vs. a mainline commit?  Does it matter?
-   if [ -z $tree ]; then
+   if test -z $tree


This works by accident. When $tree is empty, this reduces to 'test -z', 
which happens to evaluate to true, just what we want. But it be 
appropriate to put $tree in double-quotes nevertheless.



+   then
set_notree $rev
-   if [ -n "$newparents" ]; then
+   if test -n "$newparents"
+   then
cache_set $rev $rev
fi
continue


-- Hannes

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

Re: [PATCH 1/3] t7900-subtree.sh: fix quoting and broken && chains

[Eric Sunshine]

On Tue, Jul 26, 2016 at 12:14 AM, David Aguilar  wrote:
> Allow whitespace in arguments to subtree_test_create_repo.
> Add missing && chains.
>
> Signed-off-by: David Aguilar 
> ---
> diff --git a/contrib/subtree/t/t7900-subtree.sh 
> b/contrib/subtree/t/t7900-subtree.sh
> @@ -16,16 +16,16 @@ export TEST_DIRECTORY
>  subtree_test_create_repo()
>  {
> -   test_create_repo "$1"
> +   test_create_repo "$1" &&
> (
> -   cd $1
> +   cd "$1" &&

Thanks, I noticed this in December 2015 while reviewing a patch on the
list and have had a patch to fix it sitting in my queue since then but
never found time to formalize it.

> git config log.date relative
> )
>  }
>
>  create()
>  {
> -   echo "$1" >"$1"
> +   echo "$1" >"$1" &&
> git add "$1"
>  }
>
> @@ -73,10 +73,10 @@ join_commits()
>  test_create_commit() (
> repo=$1
> commit=$2

Perhaps &&-chain the above two lines also to future-proof against
someone inserting important code somewhere above the following 'cd'.

> -   cd "$repo"
> -   mkdir -p $(dirname "$commit") \
> +   cd "$repo" &&
> +   mkdir -p "$(dirname "$commit")" \
> || error "Could not create directory for commit"
> -   echo "$commit" >"$commit"
> +   echo "$commit" >"$commit" &&
> git add "$commit" || error "Could not add commit"
> git commit -m "$commit" || error "Could not commit"
>  )
--
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