Re: Extending "extended SHA1" syntax to traverse through gitlinks?

2016-08-23 Thread Junio C Hamano
Jakub Narębski  writes:

> The point is that submodule has it's own object database.  It might
> be the same as superproject's, but you need to handle submodule objects
> being in separate submodule repository anyway.  Common repository is
> just a special case.
>
> By the way, this also means that proposed "extended extended SHA1"
> syntax would be useful to user's of submodules...

Not really.

I think that you gave a prime example why ://
is not a useful thing for submodules.  When the syntax resolves to a
40-hex object name, that object name by itself is not useful.

You also need to carry an additional piece of information that lets
you identify the location of the repository, in which the object
name is valid, in the current user's context (i.e. somewhere in the
superproject where the submodule lives).  In other words, you'd need
to carry : around anyway for the object name to be
useful, so there is no good reason why anybody should insist that
the plumbing level resolve :// directly to an
object name in the first place.
--
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 v3] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Junio C Hamano
Jacob Keller  writes:

> From: Jacob Keller 
>
> Change the default behavior of git-format-patch to generate numbered
> sequence of 0/1 and 1/1 when generating both a cover-letter and a single
> patch. This standardizes the cover letter to have 0/N which helps
> distinguish the cover letter from the patch itself. Since the behavior
> is easily changed via configuration as well as the use of -n and -N this
> should be acceptable default behavior.
>
> Add tests for the new default behavior.
>
> Signed-off-by: Jacob Keller 
> ---
>  builtin/log.c|  8 
>  t/t4021-format-patch-numbered.sh | 17 +
>  2 files changed, 21 insertions(+), 4 deletions(-)

Looks good, thanks.  Queued.
--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Junio C Hamano
Johannes Schindelin  writes:

> The feature in question is also highly unlikely to be used as much by
> non-Windows users as by Windows users due to the unfortunate choice of the
> default setting for core.autocrlf.

My vague recollection from some years ago was that even among those
who were active in msysGit development there were people who
advocated for straight-thru and others who wanted core.autocrlf as
the default, but I do not know the current state of the affairs.

In any case, in the ideal future, I would imagine that we would want
to have "cat-file blob" to enable "--filters" by default; that would
make cat-file and hash-objects a pair of symmetric operations.

That certainly will not happen within 2.x timeframe, and the new
"cat-file --filter" feature can appear in 2.11 at the earliest, but
I think by that time (or with a few more cycles) we may have a
handful other improvements that are backward incompatible lined up
to urge us to think about bumping the version number to 3.0.  I
recall writing "Will keep in next to see if anybody screams" a few
times already, and they are all good excuses to invite a version
bump.

To prepare for that future, we would probably want to start updating
in-tree scripts (including the tests) so that they call "cat-file
--no-filter blob" whereever they currently say "cat-file blob" in or
soon after 2.11.  Of course, if some of them currently pipe
"cat-file blob" output to munge it to produce what --filters would
have done (I didn't check), we would want to rewrite them to use the
new feature "cat-file --filter blob" when we do so.  In short, there
won't be any "cat-file blob" call that does not have either --filters
or --no-filters, except the ones we write specifically to check the
updated default behaviour when that happens.

Would that sound like a good longer-term plan?
--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Dakota Hawkins
On Tue, Aug 23, 2016 at 5:42 PM, Junio C Hamano  wrote:
>
> Junio C Hamano  writes:
>
> > One way to avoid that risk may be to release the new feature as
> > "experimental-and-subject-to-change", so that interested users on
> > Windows can actually try it out to see if the feature itself
> > (whatever its interface to them is) makes sense, and you can gauge
> > the value of upstreaming it, while cautioning these early adopters
> > that it has not fully been through the usual review process and may
> > have to change while becoming part of the official release.  This is
> > no different from various "experimental features" we unleash to the
> > wild, either via 'master' or keeping it in 'next' (we tend to do
> > more of the latter, marking "see if anybody screams").
>
> In case it was not clear, I am _not_ saying that the port to Windows
> must not ship with any feature that is not yet in the upstream (the
> same goes for a port to Macs, where it may want to do more about
> dealing with Unicode "normalization" gotchas).  The differences in
> platforms make it more likely that needs for certain things are felt
> earlier and more strongly on a particular platform, and shipping a
> new thing as an experimental feature to end-users may be the most
> effective way to come up with the best approach to help the users.

What is the practical difference between what happened and releasing a
feature as "experimental-and-subject-to-change"?

I use GFW almost exclusively, but I pretty much always consult the
upstream documentation anyway (because I find it easier).

It doesn't seem likely that many users who weren't in dire need of
this feature will even realize/remember it exists, so it's hard for me
to conclude that anybody will really be harmed by this particular
(temporary?) discontinuity.

At any rate it doesn't seem like you guys are going to persuade one
another and from the outside looking in it seems like this argument is
more ideological than technical, which makes it seem like it should
probably embarrass all involved because of its length and publicity.
But maybe I'm wrong, in which case I'm here to embarrass myself.

-Dakota
--
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: Getting "The following submodule paths contain changes that can not be found on any remote" when they are in the remote

2016-08-23 Thread Stefan Beller
On Tue, Aug 23, 2016 at 11:38 AM, Leandro Lucarella
 wrote:
> On Tue, 23 Aug 2016 20:34:46 +0200
> Leandro Lucarella  wrote:
>> This even happens after doing a:
>>
>> git submodule deinit 
>> rm -fr 
>> rm -fr .git/modules/
>> git submodule update --init
>
> One more thing, doing a clean new clone seems to work, but I have this
> issue in many many repos, which are old clones, from before Git had the
> ability to check for submodules. Could it be some incompatibility in
> old clones with this? Is there anything I can look for in the .git/
> directory or elsewhere to fix them?

Check if push.recurseSubmodules is set?

>
> Thanks again!
>
> --
> Leandro Lucarella
> Technical Development Lead
> Sociomantic Labs GmbH 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Getting "The following submodule paths contain changes that can not be found on any remote" when they are in the remote

2016-08-23 Thread Leandro Lucarella
On Tue, 23 Aug 2016 20:34:46 +0200
Leandro Lucarella  wrote:
> This even happens after doing a:
> 
> git submodule deinit 
> rm -fr 
> rm -fr .git/modules/
> git submodule update --init

One more thing, doing a clean new clone seems to work, but I have this
issue in many many repos, which are old clones, from before Git had the
ability to check for submodules. Could it be some incompatibility in
old clones with this? Is there anything I can look for in the .git/
directory or elsewhere to fix them?

Thanks again!

-- 
Leandro Lucarella
Technical Development Lead
Sociomantic Labs GmbH 
--
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 v10 8/9] submodule: refactor show_submodule_summary with helper function

2016-08-23 Thread Junio C Hamano
Jacob Keller  writes:

> From: Jacob Keller 
>
> A future patch is going to add a new submodule diff format which
> displays an inline diff of the submodule changes. To make this easier,
> and to ensure that both submodule diff formats use the same initial
> header, factor out show_submodule_header() function which will print the
> current submodule header line, and then leave the show_submodule_summary
> function to lookup and print the submodule log format.
>
> This does create one format change in that "(revision walker failed)"
> will now be displayed on its own line rather than as part of the message
> because we no longer perform this step directly in the header display
> flow. However, this is a rare case as most causes of the failure will be
> due to a missing commit which we already check for and avoid previously.
> flow. However, this is a rare case and shouldn't impact much.
>
> Signed-off-by: Jacob Keller 
> ---

Up to this step it all looked sensible.  I'll take a look at 9/9
later.

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


Re: [PATCHv5 8/8] clone: recursive and reference option triggers submodule alternates

2016-08-23 Thread Stefan Beller
>> +
>> +   if (option_recursive) {
>> +   if (option_required_reference.nr &&
>> +   option_optional_reference.nr)
>> +   die(_("clone --recursive is not compatible with "
>> + "both --reference and --reference-if-able"));
>
> So if you have multiple references that don't all match we basically
> just refuse to allow recursive?
>
> Would it be better to simply assume that we want to die on missing
> references instead of failing the clone here?

The new config options are per repo (or even set globally), and not
per alternate. And as we communicate the [if-able] part via the config
options to the submodules it is not feasible to transport both
kinds of (reference-or-die and reference-but-ignore-misses).

That is why I introduced this check in the first place. If we'd go back
to the drawing board and come up with a solution that is on a
"per alternate" basis we could allow such things.

> That is, treat it so
> that multiple reference and reference-if-able will die, and only info
> if we got only reference-if-able?
>
> Probably what's here is fine, and mixing reference and
> reference-if-able doesn't make much sense.

I think the reference-if-able doesn't make sense for one project alone
as you can easily script around that, but is only useful if you have
submodules in a partially checked out superproject that you want
to reference to.

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


Re: [PATCHv5 8/8] clone: recursive and reference option triggers submodule alternates

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> When `--recursive` and `--reference` is given, it is reasonable to
> expect that the submodules are created with references to the submodules
> of the given alternate for the superproject.
>
>   An initial attempt to do this was presented to the mailing list, which
>   used flags that are passed around ("--super-reference") that instructed
>   the submodule clone to look for a reference in the submodules of the
>   referenced superproject. This is not well thought out, as any further
>   `submodule update` should also respect the initial setup.
>
>   When a new  submodule is added to the superproject and the alternate
>   of the superproject does not know about that submodule yet, we rather
>   error out informing the user instead of being unclear if we did or did
>   not use a submodules alternate.
>
> To solve this problem introduce new options that store the configuration
> for what the user wanted originally.
>
> Signed-off-by: Stefan Beller 
> ---
>  Documentation/config.txt   | 12 ++
>  builtin/clone.c| 19 +
>  builtin/submodule--helper.c| 87 
> ++
>  t/t7408-submodule-reference.sh | 43 +
>  4 files changed, 161 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index bc1c433..602f43a 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2837,6 +2837,18 @@ submodule.fetchJobs::
> in parallel. A value of 0 will give some reasonable default.
> If unset, it defaults to 1.
>
> +submodule.alternateLocation::
> +   Specifies how the submodules obtain alternates when submodules are
> +   cloned. Possible values are `no`, `superproject`.
> +   By default `no` is assumed, which doesn't add references. When the
> +   value is set to `superproject` the submodule to be cloned computes
> +   its alternates location relative to the superprojects alternate.
> +
> +submodule.alternateErrorStrategy
> +   Specifies how to treat errors with the alternates for a submodule
> +   as computed via `submodule.alternateLocation`. Possible values are
> +   `ignore`, `info`, `die`.
> +
>  tag.forceSignAnnotated::
> A boolean to specify whether annotated tags created should be GPG 
> signed.
> If `--annotate` is specified on the command line, it takes
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 0182665..404c5e8 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char 
> *prefix)
> else
> fprintf(stderr, _("Cloning into '%s'...\n"), dir);
> }
> +
> +   if (option_recursive) {
> +   if (option_required_reference.nr &&
> +   option_optional_reference.nr)
> +   die(_("clone --recursive is not compatible with "
> + "both --reference and --reference-if-able"));

So if you have multiple references that don't all match we basically
just refuse to allow recursive?

Would it be better to simply assume that we want to die on missing
references instead of failing the clone here? That is, treat it so
that multiple reference and reference-if-able will die, and only info
if we got only reference-if-able?

Probably what's here is fine, and mixing reference and
reference-if-able doesn't make much sense.

Thanks,
Jake
--
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] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Jacob Keller
From: Jacob Keller 

Change the default behavior of git-format-patch to generate numbered
sequence of 0/1 and 1/1 when generating both a cover-letter and a single
patch. This standardizes the cover letter to have 0/N which helps
distinguish the cover letter from the patch itself. Since the behavior
is easily changed via configuration as well as the use of -n and -N this
should be acceptable default behavior.

Add tests for the new default behavior.

Signed-off-by: Jacob Keller 
---
 builtin/log.c|  8 
 t/t4021-format-patch-numbered.sh | 17 +
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 92dc34dcb0cc..49aa534f4a01 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
/* nothing to do */
return 0;
total = nr;
-   if (!keep_subject && auto_number && total > 1)
-   numbered = 1;
-   if (numbered)
-   rev.total = total + start_number - 1;
if (cover_letter == -1) {
if (config_cover_letter == COVER_AUTO)
cover_letter = (total > 1);
else
cover_letter = (config_cover_letter == COVER_ON);
}
+   if (!keep_subject && auto_number && (total > 1 || cover_letter))
+   numbered = 1;
+   if (numbered)
+   rev.total = total + start_number - 1;
 
if (!signature) {
; /* --no-signature inhibits all signatures */
diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh
index 886494b58f67..9be65fd4440a 100755
--- a/t/t4021-format-patch-numbered.sh
+++ b/t/t4021-format-patch-numbered.sh
@@ -36,6 +36,11 @@ test_no_numbered() {
test_num_no_numbered $1 2
 }
 
+test_single_cover_letter_numbered() {
+   grep "^Subject: \[PATCH 0/1\]" $1 &&
+   grep "^Subject: \[PATCH 1/1\]" $1
+}
+
 test_single_numbered() {
grep "^Subject: \[PATCH 1/1\]" $1
 }
@@ -121,4 +126,16 @@ test_expect_success '--start-number && --numbered' '
grep "^Subject: \[PATCH 3/3\]" patch8
 '
 
+test_expect_success 'single patch with cover-letter defaults to numbers' '
+   git format-patch --cover-letter --stdout HEAD~1 >patch9.single &&
+   test_single_cover_letter_numbered patch9.single
+'
+
+test_expect_success 'Use --no-numbered and --cover-letter single patch' '
+   git format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch10 
&&
+   test_no_numbered patch10
+'
+
+
+
 test_done
-- 
2.10.0.rc0.259.g83512d9

--
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] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Jacob Keller
On Tue, Aug 23, 2016 at 3:34 PM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 92dc34dcb0cc..49aa534f4a01 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, 
>> const char *prefix)
>>   /* nothing to do */
>>   return 0;
>>   total = nr;
>> - if (!keep_subject && auto_number && total > 1)
>> - numbered = 1;
>> - if (numbered)
>> - rev.total = total + start_number - 1;
>>   if (cover_letter == -1) {
>>   if (config_cover_letter == COVER_AUTO)
>>   cover_letter = (total > 1);
>>   else
>>   cover_letter = (config_cover_letter == COVER_ON);
>>   }
>> + if (!keep_subject && auto_number && (total > 1 || cover_letter))
>> + numbered = 1;
>> + if (numbered)
>> + rev.total = total + start_number - 1;
>>
>>   if (!signature) {
>>   ; /* --no-signature inhibits all signatures */
>
> That sounds sensible.
>
>> diff --git a/t/t4021-format-patch-numbered.sh 
>> b/t/t4021-format-patch-numbered.sh
>> index 886494b58f67..ea0a388f0191 100755
>> --- a/t/t4021-format-patch-numbered.sh
>> +++ b/t/t4021-format-patch-numbered.sh
>> @@ -36,6 +36,11 @@ test_no_numbered() {
>>   test_num_no_numbered $1 2
>>  }
>>
>> +test_single_cover_letter_numbered() {
>> + grep "^Subject: \[PATCH 0/1\]" $1 &&
>> + grep "^Subject: \[PATCH 1/1\]" $1
>> +}
>> +
>>  test_single_numbered() {
>>   grep "^Subject: \[PATCH 1/1\]" $1
>>  }
>> @@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' 
>> '
>>   test_single_no_numbered patch0.single
>>  '
>>
>> +test_expect_success 'single patch with cover-letter defaults to numbers' '
>> + git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&
>> + test_single_cover_letter_numbered patch0.single
>> +'
>
> The remaining and existing tests seems to expect that the result of
> each test is stored in a distinct file so that the output can be
> inspected after seeing a test failure without running the script
> with the "-i" option.  Perhaps rename this to patch0.single-cover or
> something?  The same for the overwriting of patch1 we can see below.
>

Hmm, ya. I'll rework that.

Thanks,
Jake
--
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: [PATCHv5 6/8] clone: clarify option_reference as required

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> In the next patch we introduce optional references; To better distinguish
> between optional and required references we rename the variable.
>

Makes sense. It's a bit weird to process "option_required_reference"
but I think it is reasonable.

Regards,
Jake
--
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: [PATCHv5 7/8] clone: implement optional references

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> In a later patch we want to try to create alternates for submodules,
> but they might not exist in the referenced superproject. So add a way
> to skip the non existing references and report them.
>

Seems pretty straight forward to me.

> Signed-off-by: Stefan Beller 
> ---
>  Documentation/git-clone.txt |  5 -
>  builtin/clone.c | 35 +--
>  2 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index ec41d3d..e316c4b 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -90,13 +90,16 @@ If you want to break the dependency of a repository 
> cloned with `-s` on
>  its source repository, you can simply run `git repack -a` to copy all
>  objects from the source repository into a pack in the cloned repository.
>
> ---reference ::
> +--reference[-if-able] ::
> If the reference repository is on the local machine,
> automatically setup `.git/objects/info/alternates` to
> obtain objects from the reference repository.  Using
> an already existing repository as an alternate will
> require fewer objects to be copied from the repository
> being cloned, reducing network and local storage costs.
> +   When using the `--reference-if-able`, a non existing
> +   directory is skipped with a warning instead of aborting
> +   the clone.
>  +
>  *NOTE*: see the NOTE for the `--shared` option, and also the
>  `--dissociate` option.
> diff --git a/builtin/clone.c b/builtin/clone.c
> index ef5db7c..0182665 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -51,6 +51,7 @@ static int option_progress = -1;
>  static enum transport_family family;
>  static struct string_list option_config = STRING_LIST_INIT_NODUP;
>  static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
> +static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
>  static int option_dissociate;
>  static int max_jobs = -1;
>
> @@ -81,6 +82,8 @@ static struct option builtin_clone_options[] = {
>N_("directory from which templates will be used")),
> OPT_STRING_LIST(0, "reference", _required_reference, 
> N_("repo"),
> N_("reference repository")),
> +   OPT_STRING_LIST(0, "reference-if-able", _optional_reference,
> +   N_("repo"), N_("reference repository")),
> OPT_BOOL(0, "dissociate", _dissociate,
>  N_("use --reference only while cloning")),
> OPT_STRING('o', "origin", _origin, N_("name"),
> @@ -283,24 +286,36 @@ static void strip_trailing_slashes(char *dir)
>  static int add_one_reference(struct string_list_item *item, void *cb_data)
>  {
> struct strbuf err = STRBUF_INIT;
> -   struct strbuf sb = STRBUF_INIT;
> +   int *required = cb_data;
> char *ref_git = compute_alternate_path(item->string, );
>
> -   if (!ref_git)
> -   die("%s", err.buf);
> -
> -   strbuf_addf(, "%s/objects", ref_git);
> -   add_to_alternates_file(sb.buf);
> +   if (!ref_git) {
> +   if (*required)
> +   die("%s", err.buf);
> +   else
> +   fprintf(stderr,
> +   _("info: Could not add alternate for '%s': 
> %s\n"),
> +   item->string, err.buf);
> +   } else {
> +   struct strbuf sb = STRBUF_INIT;
> +   strbuf_addf(, "%s/objects", ref_git);
> +   add_to_alternates_file(sb.buf);
> +   strbuf_release();
> +   }

I might have done this with a "goto out" instead of the else block,
but this is reasonable as well.

Regards,
Jake
--
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: [PATCHv5 4/8] submodule--helper update-clone: allow multiple references

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> Allow the user to pass in multiple references to update_clone.
> Currently this is only internal API, but once the shell script is
> replaced by a C version, this is needed.
>
> This fixes an API bug between the shell script and the helper.
> Currently the helper accepts "--reference" "--reference=foo"
> as a OPT_STRING whose value happens to be "--reference=foo", and
> then uses
>
> if (suc->reference)
> argv_array_push(>args, suc->reference)
>
> where suc->reference _is_ "--reference=foo" when invoking the
> underlying "git clone", it cancels out.
>
> With this change we omit one of the "--reference" arguments when
> passing references from the shell script to the helper.
>

Yep, I see the API fix, and it looks correct. Makes use of the helper
easier and more likely to be done correctly.

> Signed-off-by: Stefan Beller 
> ---
>  builtin/submodule--helper.c | 14 +-
>  git-submodule.sh|  2 +-
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index ce9b3e9..7096848 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -584,7 +584,7 @@ struct submodule_update_clone {
> /* configuration parameters which are passed on to the children */
> int quiet;
> int recommend_shallow;
> -   const char *reference;
> +   struct string_list references;
> const char *depth;
> const char *recursive_prefix;
> const char *prefix;
> @@ -600,7 +600,8 @@ struct submodule_update_clone {
> int failed_clones_nr, failed_clones_alloc;
>  };
>  #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
> -   SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \
> +   SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \
> +   NULL, NULL, NULL, \
> STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
>
>
> @@ -710,8 +711,11 @@ static int prepare_to_clone_next_submodule(const struct 
> cache_entry *ce,
> argv_array_pushl(>args, "--path", sub->path, NULL);
> argv_array_pushl(>args, "--name", sub->name, NULL);
> argv_array_pushl(>args, "--url", url, NULL);
> -   if (suc->reference)
> -   argv_array_push(>args, suc->reference);

Here, you now no longer pass in the reference as a whole, but assume
that it is actually just the value to pass to --reference. And below
you correctly replace the extra --reference. Ok so I see how you fixed
the API bug.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index c90dc33..3b412f5 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -575,7 +575,7 @@ cmd_update()
> ${wt_prefix:+--prefix "$wt_prefix"} \
> ${prefix:+--recursive-prefix "$prefix"} \
> ${update:+--update "$update"} \
> -   ${reference:+--reference "$reference"} \
> +   ${reference:+"$reference"} \

>From above, I see how you fixed this to assume (as above) that
$reference is "--reference " and pass it directly. It worked
before but was definitely a bit "brittle" in that direct calling of
the helper function would not behave as expected. Nice!

Regards,
Jake
--
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] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Junio C Hamano
Jacob Keller  writes:

> diff --git a/builtin/log.c b/builtin/log.c
> index 92dc34dcb0cc..49aa534f4a01 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, 
> const char *prefix)
>   /* nothing to do */
>   return 0;
>   total = nr;
> - if (!keep_subject && auto_number && total > 1)
> - numbered = 1;
> - if (numbered)
> - rev.total = total + start_number - 1;
>   if (cover_letter == -1) {
>   if (config_cover_letter == COVER_AUTO)
>   cover_letter = (total > 1);
>   else
>   cover_letter = (config_cover_letter == COVER_ON);
>   }
> + if (!keep_subject && auto_number && (total > 1 || cover_letter))
> + numbered = 1;
> + if (numbered)
> + rev.total = total + start_number - 1;
>  
>   if (!signature) {
>   ; /* --no-signature inhibits all signatures */

That sounds sensible.

> diff --git a/t/t4021-format-patch-numbered.sh 
> b/t/t4021-format-patch-numbered.sh
> index 886494b58f67..ea0a388f0191 100755
> --- a/t/t4021-format-patch-numbered.sh
> +++ b/t/t4021-format-patch-numbered.sh
> @@ -36,6 +36,11 @@ test_no_numbered() {
>   test_num_no_numbered $1 2
>  }
>  
> +test_single_cover_letter_numbered() {
> + grep "^Subject: \[PATCH 0/1\]" $1 &&
> + grep "^Subject: \[PATCH 1/1\]" $1
> +}
> +
>  test_single_numbered() {
>   grep "^Subject: \[PATCH 1/1\]" $1
>  }
> @@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '
>   test_single_no_numbered patch0.single
>  '
>  
> +test_expect_success 'single patch with cover-letter defaults to numbers' '
> + git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&
> + test_single_cover_letter_numbered patch0.single
> +'

The remaining and existing tests seems to expect that the result of
each test is stored in a distinct file so that the output can be
inspected after seeing a test failure without running the script
with the "-i" option.  Perhaps rename this to patch0.single-cover or
something?  The same for the overwriting of patch1 we can see below.

>  test_expect_success 'multiple patch defaults to numbered' '
>  
>   git format-patch --stdout HEAD~2 >patch0.multiple &&
> @@ -64,6 +74,11 @@ test_expect_success 'Use --numbered' '
>  
>  '
>  
> +test_expect_success 'Use --no-numbered and --cover-letter single patch' '
> + git format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch1 &&
> + test_no_numbered patch1
> +'
> +
>  test_expect_success 'format.numbered = true' '
>  
>   git config format.numbered true &&
--
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: [PATCHv5 5/8] clone: factor out checking for an alternate path

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> +   if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) {
> +   char *ref_git_git = mkpathdup("%s/.git", ref_git);
> +   free(ref_git);
> +   ref_git = ref_git_git;
> +   } else if (!is_directory(mkpath("%s/objects", ref_git))) {

I realize this was directly copied but the use of !repo to determine
what file path here was definitely not easy to process, because it
made me think we were adding .git twice to the path. Not sure if there
is an easier way to expand this and avoid that confusion?

Also, what happens if repo is false, so we run the first block above,
but is_directory fails? We just continue along even though it appears
like we should fail? I'm not 100% sure I follow the logic here.
Regardless this appears to be a pretty direct copy of what was there
before so I don't think it's worse.

Thanks,
Jake
--
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: [PATCHv5 2/8] t7408: merge short tests, factor out testing method

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> Tests consisting of one line each can be consolidated to have fewer tests
> to run as well as fewer lines of code.
>
> When having just a few git commands, do not create a new shell but
> use the -C flag in Git to execute in the correct directory.
>
> Signed-off-by: Stefan Beller 
> ---

Looks good. The resulting test file is easier to read, and you created
a common function to perform checking if we used alternates instead of
duplicating that part multiple times.

>  t/t7408-submodule-reference.sh | 48 
> ++
>  1 file changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
> index b84c6748..dff47af 100755
> --- a/t/t7408-submodule-reference.sh
> +++ b/t/t7408-submodule-reference.sh
> @@ -8,6 +8,15 @@ test_description='test clone --reference'
>
>  base_dir=$(pwd)
>
> +test_alternate_is_used () {
> +   alternates_file="$1" &&
> +   working_dir="$2" &&
> +   test_line_count = 1 "$alternates_file" &&
> +   echo "0 objects, 0 kilobytes" >expect &&
> +   git -C "$working_dir" count-objects >actual &&
> +   test_cmp expect actual
> +}
> +

This change wasn't mentioned in the description. You updated the tests
to use a stronger check for when alternates is in use. This is good. I
might have mentioned it in the description but it's not worth a
re-roll.

> -test_expect_success 'that reference gets used with update' '
> -   cd super-clone/sub &&
> -   echo "0 objects, 0 kilobytes" >expected &&
> -   git count-objects >current &&
> -   diff expected current

The stronger variant was used once here, but you use it in several
locations. It's good to have extracted this as the new tests are more
readable.

Thanks,
Jake
--
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 6/7] delta_base_cache: use hashmap.h

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> So while the value for that case _isn't_ as good as the
> optimal one measured above (which was 2048 entries), given
> the bouncing I'm hesitant to suggest that 2048 is any kind
> of optimum (not even for linux.git, let alone as a general
> rule). The generic hashmap has the appeal that it drops the
> number of tweakable numbers by one, which means we can focus
> on tuning other elements, like the LRU strategy or the
> core.deltaBaseCacheLimit setting.
>
> And indeed, if we bump the cache limit to 1G (which is
> probably silly for general use, but maybe something people
> with big workstations would want to do), the linux.git log-S
> time drops to 3m32s. That's something you really _can't_ do
> easily with the static hash table, because the number of
> entries needs to grow in proportion to the memory limit (so
> 2048 is almost certainly not going to be the right value
> there).
>
> This patch takes that direction, and drops the static hash
> table entirely in favor of using the hashmap.h API.

Sounds very 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 5/7] delta_base_cache: drop special treatment of blobs

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> Let's run the same numbers without caring about object type
> at all (i.e., one LRU list, and always evicting whatever is
> at the head, regardless of type).
> ...
> So it seems like a clear winner, and that's what this patch
> implements.
>
> Signed-off-by: Jeff King 
> ---

Nice work.  You make your readers expect some clever data structures
that may perform better than the obvious two-separate-list approach
and end up using the simplest way.  Quite nice.



>  sha1_file.c | 8 
>  1 file changed, 8 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index c02e785..33564d6 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2175,14 +2175,6 @@ static void add_delta_base_cache(struct packed_git *p, 
> off_t base_offset,
>   list_entry(lru, struct delta_base_cache_entry, lru);
>   if (delta_base_cached <= delta_base_cache_limit)
>   break;
> - if (f->type == OBJ_BLOB)
> - release_delta_base_cache(f);
> - }
> - list_for_each(lru, _base_cache_lru) {
> - struct delta_base_cache_entry *f =
> - list_entry(lru, struct delta_base_cache_entry, lru);
> - if (delta_base_cached <= delta_base_cache_limit)
> - break;
>   release_delta_base_cache(f);
>   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] gitk tree view: lines messed up

2016-08-23 Thread Raimar Sandner
Hi,

I'm having problems with gitk not displaying the history tree correctly. The 
lines representing branches are messed up, circles for the individual commits 
are missing (screenshot attached).

I tried to delete ~/.config/git/gitk, even tried a completely fresh user 
account without success. The strangest thing: in an almost identical 
installation I cannot reproduce the issue.

Arch Linux
$ pacman -Q git tk tcl  

   
git 2.9.3-1
tk 8.6.6-1
tcl 8.6.6-1

I'm running out of ideas. There is no error reported from gitk. Is there any 
way to turn on more verbose debugging output? Thanks for your help!

Regards
Raimar

Re: [PATCHv5 3/8] submodule--helper module-clone: allow multiple references

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> Allow users to pass in multiple references, just as clone accepts multiple
> references as well.
>

Straight forward and reasonable change.

Regards,
Jake
--
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: [PATCHv5 1/8] t7408: modernize style

2016-08-23 Thread Jacob Keller
On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller  wrote:
> No functional change intended. This commit only changes formatting
> to the style we recently use, e.g. starting the body of a test with a
> single quote on the same line as the header, and then having the test
> indented in the following lines.
>
> Whenever we change directories, we do that in subshells.
>

I looked this over using -w to ignore whitespace changes, and it
appears to have no functional changes. Much cleaner style overall, and
easier to read the new file as it is now. The tests pass fine both
before and after this commit, and I don't see anything that should
functionally change the results.

Thanks,
Jake
--
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 v14 01/27] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

2016-08-23 Thread Pranit Bauva
Hey Junio,

On Wed, Aug 24, 2016 at 1:58 AM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>> `--next-all` is meant to be used as a subcommand to support multiple
>> "operation mode" though the current implementation does not contain any
>> other subcommand along side with `--next-all` but further commits will
>> include some more subcommands.
>>
>> Helped-by: Johannes Schindelin 
>> Mentored-by: Lars Schneider 
>> Mentored-by: Christian Couder 
>> Signed-off-by: Pranit Bauva 
>> ---
>
> A series this size really needs a cover letter that describes what
> changes were made relative to the previous round to help reviewers.

I am actually using SubmitGit these days as I am back in my campus and
IMAP/SMTP connections are blocked here. SubmitGit doesn't have the
option to include a cover letter. Though I can describe the changes by
including a separate email as a reply to 1st patch. I will send the
changes according to v12 since v13 wasn't commented on.

> I also noticed something curious.  All of your messages are dated
>
> Date: Tue, 23 Aug 2016 11:53:53 +
>
> and I think that is the same as all the previous series, but how are
> you sending your patches?  The reason why I ask is because this does
> not allow readers to tell their mail reader to sort messages by the
> sender's timestamp in order to see the messages in order (as a
> matter of fact, git-send-email knows that readers are helped by this
> feature, and backdates the first message of 10 patch series by 10
> seconds, stamps the second message as later than the first by one
> second, etc. to help them).

This might be a bug in SubmitGit. I didn't notice this previously.

Regards,
Pranit Bauva
--
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/7] clear_delta_base_cache_entry: use a more descriptive name

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> The delta base cache entries are stored in a fixed-length
> hash table. So the way to remove an entry is to "clear" the
> slot in the table, and that is what this function does.
>
> However, the name is a leaky abstraction. If we were to
> change the hash table implementation, it would no longer be
> about "clearing". We should name it after _what_ it does,
> not _how_ it does it. I.e., something like "remove" instead
> of "clear".
>
> But that does not tell the whole story, either. The subtle
> thing about this function is that it removes the entry, but
> does not free the entry data. So a more descriptive name is
> "detach"; we give ownership of the data buffer to the
> caller, and remove any other resources.

OK.  Sounds 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: git-config(1) should mention `git -c`

2016-08-23 Thread Jeff King
On Tue, Aug 23, 2016 at 10:16:18AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > That seems like the most sensible place, as that is where we should
> > cover the order of reading and precedence. Perhaps FILES should be
> > renamed to SOURCES or something (though I do not recall if we are
> > restricted to "usual" manpage section names or not).
> >
> > Arguably this is not about git-config the program at all, but the
> > general concept of "configuration for git", because the precedence rules
> > apply equally to all of the git programs that read config.
> 
> True, but that argument leads us to say git(1) is the best place ;-)

Sort of. I agree it is a good place to mention the precedence, but...

> If the user wants to know "how does the configuration values get
> read?", and wishes not having to go around fishing for the
> information in multiple places (and I think that is a reasonable
> thing to wish for), I think adding it to the FILES section of
> git-config(1) is a better option than inventing a separate
> gitconfig(7), which would still require the user to consult two
> places.

The flip side of "fishing for the information in multiple places" is "I
know it is somewhere in git-config(1), but I have to wade through a
bunch of cruft about git-config command-line options to find it".

So I'd argue that the concept of config (overview, precedence, file
syntax, list of options) could be separate from both git-config(1) and
from git(1), and that both of those places could point to it. That
introduces a level of indirection which is annoying the first time ("I
am reading git-config(1), but now I have to jump to another manpage")
but helpful the other times ("I know I want config concepts, not the
config tool; I can immediately jump to the right place").

Anyway. Just my two cents on the matter. I think we can improve David's
complaint without anything so drastic.

-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 3/7] release_delta_base_cache: reuse existing detach function

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> This function drops an entry entirely from the cache,
> meaning that aside from the freeing of the buffer, it is
> exactly equivalent to detach_delta_base_cache_entry(). Let's
> build on top of the detach function, which shortens the code
> and will make it simpler when we change out the underlying
> storage in future patches.
>
> Signed-off-by: Jeff King 
> ---
>  sha1_file.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 1d0810c..8264b39 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2152,10 +2152,7 @@ static inline void release_delta_base_cache(struct 
> delta_base_cache_entry *ent)
>  {
>   if (ent->data) {
>   free(ent->data);
> - ent->data = NULL;
> - ent->lru.next->prev = ent->lru.prev;
> - ent->lru.prev->next = ent->lru.next;
> - delta_base_cached -= ent->size;
> + detach_delta_base_cache_entry(ent);

If we were designing this from scratch, we might have made detach_*
to return the pointer to minimize direct access to ent->data, but I
do not think it is worth it.  Looks very 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


[PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Jacob Keller
From: Jacob Keller 

Change the default behavior of git-format-patch to generate numbered
sequence of 0/1 and 1/1 when generating both a cover-letter and a single
patch. This standardizes the cover letter to have 0/N which helps
distinguish the cover letter from the patch itself. Since the behavior
is easily changed via configuration as well as the use of -n and -N this
should be acceptable default behavior.

Add tests for the new default behavior.

Signed-off-by: Jacob Keller 
---
 builtin/log.c|  8 
 t/t4021-format-patch-numbered.sh | 15 +++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 92dc34dcb0cc..49aa534f4a01 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
/* nothing to do */
return 0;
total = nr;
-   if (!keep_subject && auto_number && total > 1)
-   numbered = 1;
-   if (numbered)
-   rev.total = total + start_number - 1;
if (cover_letter == -1) {
if (config_cover_letter == COVER_AUTO)
cover_letter = (total > 1);
else
cover_letter = (config_cover_letter == COVER_ON);
}
+   if (!keep_subject && auto_number && (total > 1 || cover_letter))
+   numbered = 1;
+   if (numbered)
+   rev.total = total + start_number - 1;
 
if (!signature) {
; /* --no-signature inhibits all signatures */
diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh
index 886494b58f67..ea0a388f0191 100755
--- a/t/t4021-format-patch-numbered.sh
+++ b/t/t4021-format-patch-numbered.sh
@@ -36,6 +36,11 @@ test_no_numbered() {
test_num_no_numbered $1 2
 }
 
+test_single_cover_letter_numbered() {
+   grep "^Subject: \[PATCH 0/1\]" $1 &&
+   grep "^Subject: \[PATCH 1/1\]" $1
+}
+
 test_single_numbered() {
grep "^Subject: \[PATCH 1/1\]" $1
 }
@@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '
test_single_no_numbered patch0.single
 '
 
+test_expect_success 'single patch with cover-letter defaults to numbers' '
+   git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&
+   test_single_cover_letter_numbered patch0.single
+'
+
 test_expect_success 'multiple patch defaults to numbered' '
 
git format-patch --stdout HEAD~2 >patch0.multiple &&
@@ -64,6 +74,11 @@ test_expect_success 'Use --numbered' '
 
 '
 
+test_expect_success 'Use --no-numbered and --cover-letter single patch' '
+   git format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch1 &&
+   test_no_numbered patch1
+'
+
 test_expect_success 'format.numbered = true' '
 
git config format.numbered true &&
-- 
2.10.0.rc0.259.g83512d9

--
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] transport: report missing submodule pushes consistently on stderr

2016-08-23 Thread Stefan Beller
The surrounding advice is printed to stderr, but the list of submodules
is not. Make the report consistent by reporting everything to stderr.

Signed-off-by: Stefan Beller 
---

  This fixes one of the bugs mentioned in
  
https://public-inbox.org/git/cagz79kbkyupbjfvyx3hj_r5zw36+3ufonnlc-dpic40npja...@mail.gmail.com/T/#t
  
  How to fix the other was not as obvious to me as I do not understand the
  philosophy on verbosity in the transport code.
  
  Thanks,
  Stefan

 transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport.c b/transport.c
index cf8de6e..94d6dc3 100644
--- a/transport.c
+++ b/transport.c
@@ -771,7 +771,7 @@ static void die_with_unpushed_submodules(struct string_list 
*needs_pushing)
fprintf(stderr, _("The following submodule paths contain changes that 
can\n"
"not be found on any remote:\n"));
for (i = 0; i < needs_pushing->nr; i++)
-   printf("  %s\n", needs_pushing->items[i].string);
+   fprintf(stderr, "  %s\n", needs_pushing->items[i].string);
fprintf(stderr, _("\nPlease try\n\n"
  " git push --recurse-submodules=on-demand\n\n"
  "or cd to the path and use\n\n"
-- 
2.10.0.rc1.1.g1ceb01a

--
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 v14 01/27] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

2016-08-23 Thread Junio C Hamano
Pranit Bauva  writes:

> `--next-all` is meant to be used as a subcommand to support multiple
> "operation mode" though the current implementation does not contain any
> other subcommand along side with `--next-all` but further commits will
> include some more subcommands.
>
> Helped-by: Johannes Schindelin 
> Mentored-by: Lars Schneider 
> Mentored-by: Christian Couder 
> Signed-off-by: Pranit Bauva 
> ---

A series this size really needs a cover letter that describes what
changes were made relative to the previous round to help reviewers.

I also noticed something curious.  All of your messages are dated

Date: Tue, 23 Aug 2016 11:53:53 +

and I think that is the same as all the previous series, but how are
you sending your patches?  The reason why I ask is because this does
not allow readers to tell their mail reader to sort messages by the
sender's timestamp in order to see the messages in order (as a
matter of fact, git-send-email knows that readers are helped by this
feature, and backdates the first message of 10 patch series by 10
seconds, stamps the second message as later than the first by one
second, etc. to help 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


Re: [PATCH 1/7] cache_or_unpack_entry: drop keep_cache parameter

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> There is only one caller of cache_or_unpack_entry() and it
> always passes 1 for the keep_cache parameter. We can
> simplify it by dropping the "!keep_cache" case.
>
> Another call, which did pass 0, was dropped in abe601b
> (sha1_file: remove recursion in unpack_entry, 2013-03-27),
> as unpack_entry() now does more complicated things than a
> simple unpack when there is a cache miss.
>
> Signed-off-by: Jeff King 
> ---
>  sha1_file.c | 13 +++--
>  1 file changed, 3 insertions(+), 10 deletions(-)

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


Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Junio C Hamano
Junio C Hamano  writes:

> One way to avoid that risk may be to release the new feature as
> "experimental-and-subject-to-change", so that interested users on
> Windows can actually try it out to see if the feature itself
> (whatever its interface to them is) makes sense, and you can gauge
> the value of upstreaming it, while cautioning these early adopters
> that it has not fully been through the usual review process and may
> have to change while becoming part of the official release.  This is
> no different from various "experimental features" we unleash to the
> wild, either via 'master' or keeping it in 'next' (we tend to do
> more of the latter, marking "see if anybody screams").

In case it was not clear, I am _not_ saying that the port to Windows
must not ship with any feature that is not yet in the upstream (the
same goes for a port to Macs, where it may want to do more about
dealing with Unicode "normalization" gotchas).  The differences in
platforms make it more likely that needs for certain things are felt
earlier and more strongly on a particular platform, and shipping a
new thing as an experimental feature to end-users may be the most
effective way to come up with the best approach to help the users.

--
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 v14 01/27] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

2016-08-23 Thread Pranit Bauva
Hey everyone,

Sending a "cover letter" to this series.

Changes with wrt v12[1] are:

 * Rebased on v2.10-rc0
 * Two function signatures had changed while the topic
develop so changed those.
 * Correct the "mark for translation" messages properly as
I had previously used N_() in some places but I had to
actually use _().
 * In the patch 04/27[2], bisect_clean_state() is put in bisect.c
   rather than builtin/bisect--helper.c because I will need it
   further when porting bisect_next() function.
 * The patches from 14th are completely new to the series.
They port even more functions and remove some then unused
subcommands.
 * 14th patch[3] is a tricky one as it changes a lot of things. I am
not sure whether to put this all in one patch or split it.

[1]: 
http://public-inbox.org/git/010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000...@eu-west-1.amazonses.com/
[2]: 
http://public-inbox.org/git/01020156b73fe66f-bfad6316-39d4-4577-8f75-d1b4b2031263-000...@eu-west-1.amazonses.com/
[3]: 
http://public-inbox.org/git/01020156b73fe6ce-3b204354-849b-40fd-93ff-2ebcf77df91c-000...@eu-west-1.amazonses.com/

Regards,
Pranit Bauva
--
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] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Junio C Hamano
Junio C Hamano  writes:

> This obviously changes the behaviour, but I do not think of a reason
> why this change is a bad idea.  

>> diff --git a/builtin/log.c b/builtin/log.c
>> index 92dc34dcb0cc..8e6100fb0c5b 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -1676,7 +1676,7 @@ int cmd_format_patch(int argc, const char **argv, 
>> const char *prefix)
>>  /* nothing to do */
>>  return 0;
>>  total = nr;
>> -if (!keep_subject && auto_number && total > 1)
>> +if (!keep_subject && auto_number && (total > 1 || cover_letter))
>>  numbered = 1;
>>  if (numbered)
>>  rev.total = total + start_number - 1;

Actually there is a very good reason why this patch is not good
(yet).  When the --cover option is not specified on the command
line, cover_letter is -1 (use configuration or turn it on only when
it is a multi-patch series) at this point.

I think you would have noticed it if you ran any format-patch tests.
t/t4021-format-patch-numbered.sh fails at the very beginning.

With the attached SQUASH, existing tests pass, which is a strong
sign that this new feature needs to be protected by a new test in
the t4021 script to make sure other people would not break it in the
future.

 builtin/log.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index e50d361..b7bfeb9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1650,16 +1650,16 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
/* nothing to do */
return 0;
total = nr;
-   if (!keep_subject && auto_number && (total > 1 || cover_letter))
-   numbered = 1;
-   if (numbered)
-   rev.total = total + start_number - 1;
if (cover_letter == -1) {
if (config_cover_letter == COVER_AUTO)
cover_letter = (total > 1);
else
cover_letter = (config_cover_letter == COVER_ON);
}
+   if (!keep_subject && auto_number && (total > 1 || cover_letter))
+   numbered = 1;
+   if (numbered)
+   rev.total = total + start_number - 1;
 
if (!signature) {
; /* --no-signature inhibits all signatures */
--
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 5/7] xdl_change_compact(): introduce the concept of a change group

2016-08-23 Thread Junio C Hamano
Michael Haggerty  writes:

> The idea of xdl_change_compact() is fairly simple:
>
> * Proceed through groups of changed lines in the file to be compacted,
>   keeping track of the corresponding location in the "other" file.
>
> * If possible, slide the group up and down to try to give the most
>   aesthetically pleasing diff. Whenever it is slid, the current location
>   in the other file needs to be adjusted.
>
> But these simple concepts are obfuscated by a lot of index handling that
> is written in terse, subtle, and varied patterns. I found it very hard
> to convince myself that the function was correct.
>
> So introduce a "struct group" that represents a group of changed lines
> in a file. Add some functions that perform elementary operations on
> groups:
>
> * Initialize a group to the first group in a file
> * Move to the next or previous group in a file
> * Slide a group up or down
>
> Even though the resulting code is longer, I think it is easier to
> understand and review.

Yup.  The important thing is that the length of the core logic of
sliding up and down becomes easier to read, because it shrinks; the
mechanics of sliding up and down may need more lines with boilderplate,
but they are isolated "do one thing and do it well" helpers.

Nice.


--
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] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Jacob Keller
On Tue, Aug 23, 2016 at 2:21 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>>>  total = nr;
>>> -if (!keep_subject && auto_number && total > 1)
>>> +if (!keep_subject && auto_number && (total > 1 || cover_letter))
>>>  numbered = 1;
>>>  if (numbered)
>>>  rev.total = total + start_number - 1;
>
> Actually there is a very good reason why this patch is not good
> (yet).  When the --cover option is not specified on the command
> line, cover_letter is -1 (use configuration or turn it on only when
> it is a multi-patch series) at this point.
>
> I think you would have noticed it if you ran any format-patch tests.
> t/t4021-format-patch-numbered.sh fails at the very beginning.
>

Oops! Sorry for not running tests.

> With the attached SQUASH, existing tests pass, which is a strong
> sign that this new feature needs to be protected by a new test in
> the t4021 script to make sure other people would not break it in the
> future.
>

I'll add a new test and squash your fix in.

Thanks,
Jake

>  builtin/log.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index e50d361..b7bfeb9 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1650,16 +1650,16 @@ int cmd_format_patch(int argc, const char **argv, 
> const char *prefix)
> /* nothing to do */
> return 0;
> total = nr;
> -   if (!keep_subject && auto_number && (total > 1 || cover_letter))
> -   numbered = 1;
> -   if (numbered)
> -   rev.total = total + start_number - 1;
> if (cover_letter == -1) {
> if (config_cover_letter == COVER_AUTO)
> cover_letter = (total > 1);
> else
> cover_letter = (config_cover_letter == COVER_ON);
> }
> +   if (!keep_subject && auto_number && (total > 1 || cover_letter))
> +   numbered = 1;
> +   if (numbered)
> +   rev.total = total + start_number - 1;
>
> if (!signature) {
> ; /* --no-signature inhibits all signatures */
--
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: Getting "The following submodule paths contain changes that can not be found on any remote" when they are in the remote

2016-08-23 Thread Stefan Beller
On Tue, Aug 23, 2016 at 11:34 AM, Leandro Lucarella
 wrote:
> Hi, I'm getting very often, but not always, with many different
> projects using submodules, the message:
>
>   The following submodule paths contain changes that can
>   not be found on any remote:
> 
>
>   Please try
>
> git push --recurse-submodules=on-demand

Check if push.recurseSubmodules is set (in the project, globally,
in the home dir?)

git config --list | grep push.recurseSubmodules

>
>   or cd to the path and use
>
> git push
>
>   to push them to a remote.
>
>   fatal: Aborting.
>
> This even happens after doing a:
>
> git submodule deinit 
> rm -fr 
> rm -fr .git/modules/
> git submodule update --init
>
> So I am getting the reference from the remote, but when pushing a new
> change (that doesn't touch the submodules) I keep getting this error.

So the submodule did not change, but you still get the error?
That sounds like a bug.

Can you provide a sample repository that demonstrates this bug?
Maybe there are 'dangling' submodule pointers in past commits
that trigger this problem?

>
> I tried to get more information about why this is happening but I
> couldn't. Googling didn't help either, so I'm resorting to ask here.

If you are sure it is bug and not your fault, you can overwrite
the push.recurseSubmodules setting locally in your repository
to be "no" or "on-demand", see the git config documentation[1].

[1] e.g. https://www.kernel.org/pub/software/scm/git/docs/git-config.html

>
> I would also like to report a tiny bug, when using push --quiet, I do
> get all the message above except for the  name, which is quite
> confusing.

This sounds like a second bug.

See https://github.com/git/git/blob/master/transport.c#L767

So there are a few issues:

* the printf in the loop should be an fprintf(stderr,...)  for consistency.
* die_with_unpushed_submodules that is called from transport_push
  doesn't respect the transport->verbose setting.

The second bug can be explained with these issues; the first one
could hint at a bug in find_unpushed_submodules in submodule.c

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


Re: [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Jacob Keller
On Tue, Aug 23, 2016 at 9:33 AM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> From: Jacob Keller 
>>
>> Change the default behavior of git-format-patch to generate numbered
>> sequence of 0/1 and 1/1 when generating both a cover-letter and a single
>> patch. This standardizes the cover letter to have 0/N which helps
>> distinguish the cover letter from the patch itself. Since the behavior
>> is easily changed via configuration as well as the use of -n and -N this
>> should be acceptable default behavior.
>>
>> Signed-off-by: Jacob Keller 
>> ---
>
> This obviously changes the behaviour, but I do not think of a reason
> why this change is a bad idea.
>

Yes. The basic idea is "number all files outputted if we generate more
than 1 file" Since a cover letter is a separate file, we number 0/1
and 1/1.

Thanks,
Jake
--
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


Getting "The following submodule paths contain changes that can not be found on any remote" when they are in the remote

2016-08-23 Thread Leandro Lucarella
Hi, I'm getting very often, but not always, with many different
projects using submodules, the message:

  The following submodule paths contain changes that can
  not be found on any remote:


  Please try

git push --recurse-submodules=on-demand

  or cd to the path and use

git push

  to push them to a remote.

  fatal: Aborting.

This even happens after doing a:

git submodule deinit 
rm -fr 
rm -fr .git/modules/
git submodule update --init

So I am getting the reference from the remote, but when pushing a new
change (that doesn't touch the submodules) I keep getting this error.

I tried to get more information about why this is happening but I
couldn't. Googling didn't help either, so I'm resorting to ask here.

I would also like to report a tiny bug, when using push --quiet, I do
get all the message above except for the  name, which is quite
confusing.

$ git --version
git version 2.9.3

(running under Ubuntu 14.04)

Please CC me, and thanks a lot in advance!

-- 
Leandro Lucarella
Technical Development Lead
Sociomantic Labs GmbH 
--
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: Extending "extended SHA1" syntax to traverse through gitlinks?

2016-08-23 Thread Jakub Narębski
W dniu 23.08.2016 o 08:53, Josh Triplett pisze:
> On Mon, Aug 22, 2016 at 08:39:19PM +0200, Jakub Narębski wrote:
>> W dniu 21.08.2016 o 16:26, Josh Triplett pisze:
>>> On Sun, Aug 21, 2016 at 03:46:36PM +0200, Jakub Narębski wrote:
 W dniu 21.08.2016 o 00:50, Josh Triplett pisze:
>
[...]
 And with the above manual resolving, you can see the problem with
 implementing it: the git-cat-file (in submodule) and git-rev-parse
 (in supermodule) are across repository boundary.
>>>
>>> Only if the gitlink points to a commit that doesn't exist in the same
>>> repository.  A gitlink can point to a commit you already have.
>>
>> The idea of submodules is that tree object in superproject includes
>> link to commit of subproject (so called gitlink).  Tree object is
>> in superproject repository, while gitlinked commit is in submodule
>> repository.
>>
>> True, with modern Git the submodule repository is embedded in .git
>> area of superproject, with '.git' in submodule being gitling file,
>> but by design those objects are in different repositories, in different
>> object databases.
> 
> git-submodule handles them that way by default, yes.  But a gitlink
> doesn't inherently have to point to a separate repository, and even a
> submodule could point to an object available in the same repository
> (perhaps via another ref).
> 
> git-series creates such gitlinks, for instance.

The point is that submodule has it's own object database.  It might
be the same as superproject's, but you need to handle submodule objects
being in separate submodule repository anyway.  Common repository is
just a special case.

By the way, this also means that proposed "extended extended SHA1"
syntax would be useful to user's of submodules...

 Also the problem with proposed syntax is that is not very visible.
 But perhaps it is all right.  Maybe :/ as separator would be better,
 or using parentheses or braces?
>>>
>>> It seems as visible as the standard commit:path syntax; the second colon
>>> seems just as visible as the first.  :/ already has a different meaning
>>> (text search), so that would introduce inconsistency.
>>
>> Actually ":/" has a special meaning only if it is at beginning:
> 
> True, but it seems inconsistent to have :/ mean search if at the
> beginning, or traversal if not.

Right.  It would also mean that if we have directory or submodule
called 'foo:', then 'foo:/bar' would be ambiguous where it was not
before.

BTW. currently there is not much need for quoting, at least not for
the ':' as separator.  Files with ':' in them, even if they are
named 'HEAD:foo' can be distinguished with ./HEAD:foo, or with
':(top)HEAD:foo'.  This would not be the case if supermodule to
submodule separator was ':'; the '//' is safe-ish.

Also, '//' would have additional meaning, in that left hand side
and right hand side are in [possibly] different repositories.


Sidenote (on MS Windows):
 samsung@notebook MINGW64 ~/test (master)
 $ echo 'HEAD:A..B' >'HEAD:A..B'

 samsung@notebook MINGW64 ~/test (master)
 $ git add 'HEAD:A..B'
 fatal: pathspec 'HEAD:A..B' did not match any files

 samsung@notebook MINGW64 ~/test (master)
 $ ls
 A  A..B  B  HEAD:A..B  file  sub/  subm/


>> But perhaps '//' would be better.
> 
> That does seem unambiguous, and it can't conflict with an existing file.
> Does it seem reasonable to allow that for the initial commit as well
> ('committish//file', as well as 'commit//gitlink//file')?

I don't think we can change this without breaking scripts (because it
would be breaking backward compatibility).  And adding new syntax...

The problem might be shells sanitizing input, that is turning '//'
into '/' before passing it to command; I don't know if it is a problem.
Probably not.

> Also, while that handles traversal into the tree contained in the
> gitlinked commit, what about navigating by commit (using '~' and '^',
> for instance)?  Does it seem reasonable to allow those as well, perhaps
> only if you use // to reach the gitlink?  For instance,
> 'commit//gitlink~3', or 'commit//gitlink^{tree}'?

I don't know which of those work, and which do not:

  HEAD:path/to/submodule~3
  :0:path/to/submodule^{tree}
  HEAD~3:path/to/submodule

But I think the following should work:

  v1.0.1~2^2~4:path/to/submodule~3//inner/subm~4//sub/file


NOTE that the syntax allows to start at revision, at the index state
of superproject, but it only goes to state recorder, or to be recorded
in the superproject.  There is no syntax to find out HEAD or index
version of submodule, unless you are within submodule, isn't it?

Best,
-- 
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: Most recent revision that contains a string

2016-08-23 Thread Junio C Hamano
Nikolaus Rath  writes:

>> Btw, please don't set these headers on kernel.org lists:
>>
>>  Mail-Copies-To: never
>>  Mail-Followup-To: git@vger.kernel.org
>>
>> Like any mail server, vger fails from time-to-time and
>> reply-to-all prevents it from being a single point of failure.
>
> Huh? If the list-server fails only I will receive the message so it's
> still lost for everyone else. But I am more than happy to take this
> little risk if it saves me from the nuisance of getting duplicate
> responses.

On a mailing list, your readers' time is more valuable than yours,
simply because there are a lot more of them.

I am typing this message ONLY because I want to talk to YOU, and I
want your address to be on its To: field, with Eric and the list on
Cc: field.  The recipients can prioritize their incoming messages by
reading what's addressed directly to them first and when they have
leftover time they can read ones that they are CC'ed, but because
you used Mail-Followup-To: I had to correct the headers manually,
which means you stole time from me.  And if I didn't then your
Mail-Followup-to: would have stolen time from the other recipients.

Please don't use it on this list.

--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Junio C Hamano
Johannes Schindelin  writes:

> In case it is not crystal-clear, let me clarify one very important point.
> It seems that some people mistake the work I do for something I do on a
> whim. This is not so.
>
> The patch series that triggered this entire unfortunate discussion
> introduced the --smudge option, which I have subsequently renamed to
> --filters and submitted as a patch series to the Git project.

As the "--filters" is meant as a new feature, it will not land on
the maintenance track.  It is very likely that it won't be in 2.10,
so it won't appear in 2.10.x maintenance track, either.

I do not agree with Duy that the "port to Windows" needs a separate
distinct name, though.  Having said that, aside from the issue of
handling of bugreports has been already meantioned, which mostly
costs for Git developers, whatever new feature you unleash ahead of
upstream to your Windows port has cost to your end-users.  Its
implementation or its external interface may have to change when you
upstream the new feature that has already been in the field, and
your end-users would have to adjust their scripts and/or muscle
memory.

One way to avoid that risk may be to release the new feature as
"experimental-and-subject-to-change", so that interested users on
Windows can actually try it out to see if the feature itself
(whatever its interface to them is) makes sense, and you can gauge
the value of upstreaming it, while cautioning these early adopters
that it has not fully been through the usual review process and may
have to change while becoming part of the official release.  This is
no different from various "experimental features" we unleash to the
wild, either via 'master' or keeping it in 'next' (we tend to do
more of the latter, marking "see if anybody screams").



--
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: Possible bug: git pull --rebase discards local commits

2016-08-23 Thread Junio C Hamano
Joshua Phillips  writes:

> I've found a case where git pull --rebase discards commits in my branch
> if the remote-tracking branch was rewound (and the remote tracking
> branch's reflog contains my branch's latest commit). This is due to
> git-pull's usage of git merge-base --fork-point.
>
> On one hand, this behaviour might be correct since the remote repository
> essentially removed that commit from master by 'reset --hard'. On the
> other hand, I was surprised that git pull --rebase discarded a commit in
> my branch.

Yup, that sounds like a bad way to handle the situation.  After all,
the upstream may have first accepted your first attempt, and then
decided that it was premature and rewound it, expecting you to give
an improved reroll.  But I also agree with you that it may be
correct to drop it because the upstream already rejected it.

Since Git cannot tell between these two cases, we should play safer
than what the current code does, I would think.

--
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-svn bridge and line endings

2016-08-23 Thread Torsten Bögershausen
On 23.08.16 19:50, Lucian Smith wrote:
> On Tue, Aug 23, 2016 at 10:36 AM, Julian Phillips
>  wrote:
>> On 23/08/2016 17:14, Lucian Smith wrote:
>>>
>>> Thanks for the quick responses!
>>>
>>> My situation is that the git side is entirely whatever github.org is
>>> running; presumably the latest stable version?  They provide a URL for
>>> repositories hosted there that can be accessed by an SVN
>>> client--somewhere on github is the 'git-svn bridge' (as I understand
>>> it): something that receives SVN requests, translates them to
>>> git-speak, and replies with what SVN expects.
>>
>>
>> The ability to use a Subversion client is functionality provided by GitHub,
>> and not native to git itself.  So unless someone for the appropriate GitHub
>> team happens to read this thread I expect there isn't much we can do to
>> help.  I don't know if they've even provided any real detail of how they
>> implemented the bridge.
> 
> All right, that makes sense.  And yeah, it was hard to find any
> information about the bridge, which is why I ended up here...
> 
>>> There is indeed a .gitattributes file in the repository, but the SVN
>>> client doesn't know what to do with it.  My hope was that something in
>>> the bridge code, that translated SVN requests to git and back, could
>>> take the SVN request, "Please give me this file; I'm on Windows" look
>>> at the .gitattributes file in the repository, and hand out a file with
>>> CR/LF's in it.  Conversely, when SVN tells git "Here is the new
>>> version of the file to check in," the bridge could look at the file,
>>> realize it had CR/LF's in it, look at the .gitattributes file to know
>>> if it needed to be converted, and then convert it appropriately.
>>>
>>> I can imagine a full-blown bridge that could even translate the SVN
>>> EOL propset back and forth appropriately, but I'm not sure if going
>>> that far is necessary and/or helpful.
>>>
>>> I don't know if this is the right mailing list for that particular bit
>>> of software, but it at least seemed like a good place to start.  Thank
>>> you!
>>
>>
>> Supported properties are listed here:
>> https://help.github.com/articles/subversion-properties-supported-by-github/
>>
>> You'll need to ask GitHub to implement support for the svn:eol-style
>> property I expect.
> 
> Thanks for finding that!  That even has an 'ask a human' button, which
> I expect is my next step.
> 
>> Might be easier to just use Tortoise Git?
> 
> It may be!  But thanks for the responses anyway.
Most text-files have been commited with LF:
/tmp/ttt/sbml-test-suite> git ls-files --eol | grep "i/lf" | wc -l
   10266
Some have been commited with CRLF:
/tmp/ttt/sbml-test-suite> git ls-files --eol | grep "i/crlf" | wc -l
1620


The whole repo deserves to be normalized and equipped with a .gitattributes 
file,
see

https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html


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


Re: [L10N] Kickoff of translation for Git 2.10.0 round 1

2016-08-23 Thread Jean-Noël AVILA
On mardi 23 août 2016 08:58:55 CEST Junio C Hamano wrote:

> > Let's cut it like this : first ten are literally translated, the following
> > ones fall back to a general rule.
> 
> I actually once wrote "It is rare to squash dozens of commits into
> one, so the first ten or dozen messages that spell the ordinals out
> may be a good thing for readability", in the message you are
> responding to, before realizing that the messages actually say
> "1st", "2nd", etc., not "first", "second", etc., and scrapped that
> part of the response.  I do not really see much point in forcing the
> first ten to be translated differently.

In fact, taking some advance on the patch, I made the translations and used 
full words in the translated language.
By majority of repliers, let's just use a single sentence for each case "use" 
or "skip". 

Will rerun the patch set.

JN
--
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] doc: mention `git -c` in git-config(1)

2016-08-23 Thread Junio C Hamano
David Glasser  writes:

> Signed-off-by: David Glasser 
> ---
>  Documentation/git-config.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index f163113..83f86b9 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -263,6 +263,9 @@ The files are read in the order given above, with
> last value found taking

No need to resend but the above context line somehow got line-wrapped.

>  precedence over values read earlier.  When multiple values are taken then all
>  values of a key from all files will be used.
>
> +You may override individual configuration parameters when running any git
> +command by using the `-c` option. See linkgit:git[1] for details.
> +

This is not a new (as linkgit:git[1] also has it) but "override" is
a white lie, and may invite nitpickers.  The -c var=val given from
the "git" command line is merely tacked at the end of the search
order, so it would "override" when the variable is used as a
single-valued variable, but when it is used as multi-valued one, it
does not.

That might be something we want to fix up further in later patches;
the change we see in this patch is good regardless.

Thanks.

>  All writing options will per default write to the repository specific
>  configuration file. Note that this also affects options like `--replace-all`
>  and `--unset`. *'git config' will only ever change one file at a time*.
--
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-svn bridge and line endings

2016-08-23 Thread Julian Phillips

On 23/08/2016 17:14, Lucian Smith wrote:

Thanks for the quick responses!

My situation is that the git side is entirely whatever github.org is
running; presumably the latest stable version?  They provide a URL for
repositories hosted there that can be accessed by an SVN
client--somewhere on github is the 'git-svn bridge' (as I understand
it): something that receives SVN requests, translates them to
git-speak, and replies with what SVN expects.


The ability to use a Subversion client is functionality provided by 
GitHub, and not native to git itself.  So unless someone for the 
appropriate GitHub team happens to read this thread I expect there isn't 
much we can do to help.  I don't know if they've even provided any real 
detail of how they implemented the bridge.



There is indeed a .gitattributes file in the repository, but the SVN
client doesn't know what to do with it.  My hope was that something in
the bridge code, that translated SVN requests to git and back, could
take the SVN request, "Please give me this file; I'm on Windows" look
at the .gitattributes file in the repository, and hand out a file with
CR/LF's in it.  Conversely, when SVN tells git "Here is the new
version of the file to check in," the bridge could look at the file,
realize it had CR/LF's in it, look at the .gitattributes file to know
if it needed to be converted, and then convert it appropriately.

I can imagine a full-blown bridge that could even translate the SVN
EOL propset back and forth appropriately, but I'm not sure if going
that far is necessary and/or helpful.

I don't know if this is the right mailing list for that particular bit
of software, but it at least seemed like a good place to start.  Thank
you!


Supported properties are listed here: 
https://help.github.com/articles/subversion-properties-supported-by-github/


You'll need to ask GitHub to implement support for the svn:eol-style 
property I expect.


Might be easier to just use Tortoise Git?


-Lucian

On Mon, Aug 22, 2016 at 10:54 PM, Eric Wong  wrote:

Alfred Perlstein  wrote:

I hadn't anticipated there be to translation between svn props and
.gitattributes, it sounds a bit messy but possible, that said, is it
not possible to commit .gitattribute files to the svn repo?  Even in
FreeBSD land such small token files are permitted.


I'm not sure if an automatic translation is necessary or
desired (because of a corruption risk).

Perhaps Lucian can clarify the situation for his repo.


As far as documenting svn-properties, most of the properties are
used on the Subversion side either by subversion itself, or by
scripts in the subversion repository.  Perhaps a blurb "see the
subversion documentation and/or your local subversion
administrator's guide for properties and their uses." would suffice?


Yes, perhaps with a workable example Lucian can use today with
any git v2.3.0 or later.

Thanks for the quick response!


Opinions?  Happy to look into it.

--
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


--
Julian
--
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 v10 0/9] submodule inline diff format

2016-08-23 Thread Stefan Beller
On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano  wrote:
> I am not so sure about that.  If there is an existing place that is
> buggy, shouldn't we fix that, instead of spreading the same bug
> (assuming that it is a bug in the first place, which I do not have a
> strong opinion on, at least not yet)?
>
> Can there be .git/modules// repository that is pointed at an
> in-tree .git file when there is no "name" defined?

If you're holding it wrong we can come into that state.
* Checkout the submodule,
* then remove .gitmodules as well as relevant config in .git/config.
Result: Then we have a only a gitlink recorded as well as connected
working tree to a gitdir inside a superprojects .git/modules/.

> I thought we
> errored out in module_name helper function in git-submodule.sh when
> we need a name and only have path (I just checked in the maint-2.6
> track); did we break it recently? submodule--helper.c::module_name()
> seems to error out when submodule_from_path() fails to find one and
> will segfault if it does not have name, so it is not likely.

The name is literally the only thing that is not optional in a struct submodule
(see submodule-config.c:182 In lookup_or_create_by_name, these structs are
added to the internal cache.

Stepping back a bit, I think we'd want to document this expectation more
in the man pages
The name unlike the path of a submodule must not be changed (as the
name is used internally to point at the submodules git dir)
--
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/2] help: introduce option --command-only

2016-08-23 Thread Ralf Thielow
2016-08-19 10:32 GMT+02:00 Johannes Schindelin :

> So how about fixing that? I would suggest to do it this way:
>
> - configure help.format = html (for "man", the current code would always
>   add $(prefix)/share/man to the MANPATH when testing, not what we want,
>   and hacking this code *just* for testing is both ugly and unnecessary).
>
> - configure help.htmlpath to point to a subdirectory that is created and
>   populated in the same test script.
>
> - configure help.browser to point to a script that is created in the same
>   script and whose output we can verify, too.
>
> The last point actually requires a patch that was recently introduced into
> Git for Windows [*1*] (and that did not make it upstream yet) which
> reverts that change whereby web--browse was sidestepped. That sidestepping
> was well-intentioned but turned out to cause more harm than good.
>

So I'll pickup the patch you sent [1] to my series and prepare the test cases
the way you described to verify that the 'help' command works.

Thanks!

[1]
http://public-inbox.org/git/03ae6a9d47cb95a54960bfdc90c5392f890ff1e3.1471595956.git.johannes.schinde...@gmx.de/
--
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-svn bridge and line endings

2016-08-23 Thread Lucian Smith
On Tue, Aug 23, 2016 at 10:36 AM, Julian Phillips
 wrote:
> On 23/08/2016 17:14, Lucian Smith wrote:
>>
>> Thanks for the quick responses!
>>
>> My situation is that the git side is entirely whatever github.org is
>> running; presumably the latest stable version?  They provide a URL for
>> repositories hosted there that can be accessed by an SVN
>> client--somewhere on github is the 'git-svn bridge' (as I understand
>> it): something that receives SVN requests, translates them to
>> git-speak, and replies with what SVN expects.
>
>
> The ability to use a Subversion client is functionality provided by GitHub,
> and not native to git itself.  So unless someone for the appropriate GitHub
> team happens to read this thread I expect there isn't much we can do to
> help.  I don't know if they've even provided any real detail of how they
> implemented the bridge.

All right, that makes sense.  And yeah, it was hard to find any
information about the bridge, which is why I ended up here...

>> There is indeed a .gitattributes file in the repository, but the SVN
>> client doesn't know what to do with it.  My hope was that something in
>> the bridge code, that translated SVN requests to git and back, could
>> take the SVN request, "Please give me this file; I'm on Windows" look
>> at the .gitattributes file in the repository, and hand out a file with
>> CR/LF's in it.  Conversely, when SVN tells git "Here is the new
>> version of the file to check in," the bridge could look at the file,
>> realize it had CR/LF's in it, look at the .gitattributes file to know
>> if it needed to be converted, and then convert it appropriately.
>>
>> I can imagine a full-blown bridge that could even translate the SVN
>> EOL propset back and forth appropriately, but I'm not sure if going
>> that far is necessary and/or helpful.
>>
>> I don't know if this is the right mailing list for that particular bit
>> of software, but it at least seemed like a good place to start.  Thank
>> you!
>
>
> Supported properties are listed here:
> https://help.github.com/articles/subversion-properties-supported-by-github/
>
> You'll need to ask GitHub to implement support for the svn:eol-style
> property I expect.

Thanks for finding that!  That even has an 'ask a human' button, which
I expect is my next step.

> Might be easier to just use Tortoise Git?

It may be!  But thanks for the responses anyway.

-Lucian
--
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/2] help: introduce option --command-only

2016-08-23 Thread Ralf Thielow
2016-08-19 10:39 GMT+02:00 Remi Galan Alfonso
:
> Hi Ralf,
>
> Ralf Thielow  writes:
>> [...]
>> +test_expect_success "works for commands and guides by default" "
>> +git help status &&
>> +git help revisions
>> +"
>> +
>> +test_expect_success "--command-only does not work for guides" "
>> +git help --command-only status &&
>> +cat <<-EOF >expected &&
>> +git: 'revisions' is not a git command. See 'git --help'.
>> +EOF
>> +(git help --command-only revisions 2>actual || true) &&
>
> I think you want to use
>   `test_must_fail git help --command-only revisions 2>actual`
> here to make sure that the command does fail.
>

Thanks!

> Thanks,
> Rémi
--
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] doc: mention `git -c` in git-config(1)

2016-08-23 Thread David Glasser
Signed-off-by: David Glasser 
---
 Documentation/git-config.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index f163113..83f86b9 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -263,6 +263,9 @@ The files are read in the order given above, with
last value found taking
 precedence over values read earlier.  When multiple values are taken then all
 values of a key from all files will be used.

+You may override individual configuration parameters when running any git
+command by using the `-c` option. See linkgit:git[1] for details.
+
 All writing options will per default write to the repository specific
 configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
-- 
2.8.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: git-config(1) should mention `git -c`

2016-08-23 Thread David Glasser
On Tue, Aug 23, 2016 at 10:16 AM, Junio C Hamano  wrote:
> Jeff King  writes:
>
>> That seems like the most sensible place, as that is where we should
>> cover the order of reading and precedence. Perhaps FILES should be
>> renamed to SOURCES or something (though I do not recall if we are
>> restricted to "usual" manpage section names or not).
>>
>> Arguably this is not about git-config the program at all, but the
>> general concept of "configuration for git", because the precedence rules
>> apply equally to all of the git programs that read config.
>
> True, but that argument leads us to say git(1) is the best place ;-)
>
> If the user wants to know "how does the configuration values get
> read?", and wishes not having to go around fishing for the
> information in multiple places (and I think that is a reasonable
> thing to wish for), I think adding it to the FILES section of
> git-config(1) is a better option than inventing a separate
> gitconfig(7), which would still require the user to consult two
> places.

Great, I sent a patch in a new thread. I hope I formatted it
correctly; haven't sent a patch via email in a while.



-- 
glas...@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] help: make option --help open man pages only for Git commands

2016-08-23 Thread Ralf Thielow
Sorry for being late in responding. It's been busy days.

2016-08-18 21:51 GMT+02:00 Junio C Hamano :
> Ralf Thielow  writes:
>
> The same comment applies to 1/2, too, in that the word "command"
> will be interpreted differently by different people.  For example,
> "git co --help" and "git help co" would work, with or without 1/2 in
> place when you have "[alias] co = checkout", so we are calling "Git
> subcommands that we ship, custom commands 'git-$foo' the users have
> in their $PATH, and aliases the users create" collectively "command".
>
> As long as the reader understands that definition, both the log
> messages of 1/2 and 2/2 _and_ the updated description for "git help"
> we have in 1/2 are all very clear.  I do not care too much about the
> commit log message, but we may want to think about the documentation
> a bit more.
>
> Here is what 1/2 adds to "git help" documentation:
>
> +Note that `git --help ...` is almost identical to `git help ...` because
> +the former is internally converted into the latter with option 
> --command-only
> +being added.
>
>  To display the linkgit:git[1] man page, use `git help git`.
>
> @@ -43,6 +44,10 @@ OPTIONS
> Prints all the available commands on the standard output. This
> option overrides any given command or guide name.
>
> +-c::
> +--command-only::
> +   Display help information only for commands.
> +
>
> First, I do not think a short form is unnecessary; the users are not
> expected to use that form, once they started typing "git help...".
> If we flip the polarity and call it --exclude-guides or something,
> would it make it less ambiguous?
>

Sure.  Since "command" is understood as both Git command
and guide in this context, the name --exclude-guides would describe the
behaviour of that option less ambiguous.  I'll rename it.

>> This breaks "git  --help" while "git help " still works.
>
> I wouldn't call that a breakage; "git everyday --help" shouldn't
> have worked in the first place.  It did something useful merely by
> accident ;-).
>

OK, I'll call it "doesn't work anymore".

>> diff --git a/git.c b/git.c
>> index 0f1937f..2cd2e06 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)
>>   strip_extension(argv);
>>   cmd = argv[0];
>>
>> - /* Turn "git cmd --help" into "git help cmd" */
>> + /* Turn "git cmd --help" into "git help --command-only cmd" */
>>   if (argc > 1 && !strcmp(argv[1], "--help")) {
>> + struct argv_array args;
>> + int i;
>> +
>>   argv[1] = argv[0];
>>   argv[0] = cmd = "help";
>> +
>> + argv_array_init();
>> + for (i = 0; i < argc; i++) {
>> + argv_array_push(, argv[i]);
>> + if (!i)
>> + argv_array_push(, "--command-only");
>> + }
>> +
>> + argc++;
>> + argv = argv_array_detach();
>>   }
>>
>>   builtin = get_builtin(cmd);
>
> The code does this after it:
>
> if (builtin)
> exit(run_builtin(...));
>
> and returns.  If we didn't get builtin, we risk leaking args.argv
> here, but we assume argv[0] = cmd = "help" is always a builtin,
> which I think is a safe assumption, so the code is OK.  Static
> checkers that are only half intelligent may yell at you for not
> releasing the resources, though.  I wonder if it is worth doing
>
> static void handle_builtin(int argc, const char **argv)
> {
> struct argv_array args = ARGV_ARRAY_INIT;
> ...
> if (argc > 1 && !strcmp(argv[1], "--help")) {
> ...
> argv = args.argv;
> }
> builtin = get_builtin(cmd);
> if (builtin)
> exit(run_builtin(...));
> argv_array_clear();
> }
>
> to help unconfuse them.
>

I'll do it this way.

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


Re: git-svn bridge and line endings

2016-08-23 Thread Lucian Smith
Windows doesn't have any problem reading files with any form of crlf.
The main issue we have run into thus far is that if I create a new
file in Windows, it's going to have crlf, and if I check it in that
way through this svn process, the file is then stored with crlfs,
which git does not expect.  A wide variety of problems results.

-Lucian

On Tue, Aug 23, 2016 at 10:26 AM, Junio C Hamano  wrote:
> Lucian Smith  writes:
>
>> The setup is that we have a git repository on github, and I've checked
>> out a branch on my Windows machine using Tortoise svn.  I make
>> changes, commit them, and the branch is updated.  In general, this
>> works fine.
>
> Hmph, doesn't Windows port ship with core.autocrlf these days?
--
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-svn bridge and line endings

2016-08-23 Thread Junio C Hamano
Lucian Smith  writes:

> The setup is that we have a git repository on github, and I've checked
> out a branch on my Windows machine using Tortoise svn.  I make
> changes, commit them, and the branch is updated.  In general, this
> works fine.

Hmph, doesn't Windows port ship with core.autocrlf these days?
--
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 v10 0/9] submodule inline diff format

2016-08-23 Thread Junio C Hamano
Stefan Beller  writes:

> On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller  
> wrote:
>> From: Jacob Keller 
>>
>> A few suggestions from Stefan in regards to falling back to
>> .git/modules/ being a bad idea. I've chosen I think to avoid using
>> die() as we just stick with the current path if we can't find its name.
>
> Which makes the existing bug more subtle :(
>
>> I think this should be safe since we already do this today.
>
> It's a bug today already. Thanks for spotting!
>
>> The new flow
>> only changes if we are able to lookup the submodule, so I don't think
>> it's worth adding a die() call.
>
> Well this series improves the buggy-ness as it is only buggy when the name
> is not found, and we fall back on the path.

I am not so sure about that.  If there is an existing place that is
buggy, shouldn't we fix that, instead of spreading the same bug
(assuming that it is a bug in the first place, which I do not have a
strong opinion on, at least not yet)?

Can there be .git/modules// repository that is pointed at an
in-tree .git file when there is no "name" defined?  I thought we
errored out in module_name helper function in git-submodule.sh when
we need a name and only have path (I just checked in the maint-2.6
track); did we break it recently? submodule--helper.c::module_name()
seems to error out when submodule_from_path() fails to find one and
will segfault if it does not have name, so it is not likely.


--
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-config(1) should mention `git -c`

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> That seems like the most sensible place, as that is where we should
> cover the order of reading and precedence. Perhaps FILES should be
> renamed to SOURCES or something (though I do not recall if we are
> restricted to "usual" manpage section names or not).
>
> Arguably this is not about git-config the program at all, but the
> general concept of "configuration for git", because the precedence rules
> apply equally to all of the git programs that read config.

True, but that argument leads us to say git(1) is the best place ;-)

If the user wants to know "how does the configuration values get
read?", and wishes not having to go around fishing for the
information in multiple places (and I think that is a reasonable
thing to wish for), I think adding it to the FILES section of
git-config(1) is a better option than inventing a separate
gitconfig(7), which would still require the user to consult two
places.

--
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-svn bridge and line endings

2016-08-23 Thread Lucian Smith
On Tue, Aug 23, 2016 at 9:43 AM, Torsten Bögershausen  wrote:
> On Mon, Aug 22, 2016 at 05:04:47PM -0700, Lucian Smith wrote:
>> I'm attempting to use the git-svn bridge, and am having problems with
>> line endings on Windows.
>>
>> The setup is that we have a git repository on github, and I've checked
>> out a branch on my Windows machine using Tortoise svn.  I make
>> changes, commit them, and the branch is updated.  In general, this
>> works fine.
>
> Just to make sure:
> The repo is in git format.
> Is it a public repo ?

Yes, it is.  In fact, it's https://github.com/sbmlteam/sbml-test-suite

> Or could you make a piblic demo repo ?
> Do I understand it right: Tortoise SVN talks directly to the Git server ?

This is the part that is a little bit of black magic for me, but yes:
I point TortoiseSVN at
https://github.com/sbmlteam/test-suite.git/branches/develop and tell
it to check that out, update, commit, etc.

> Isn't Tortoise SVN a client to talk to SVN server?
> What goes over the wire to the remote Git server, git or SVN ?

That's a good question ;-)  I think SVN!

> To my understanding, "git svn" can use Git locally, and talk to an SVN server.
> What do I miss ?

I assume something is using "git svn" on the github side of things,
but I don't know for sure, and I'm not sure who's in charge of that,
either.

-Lucian

>> If this was just SVN, I could set the 'eol-style' for files to
>> 'native' to let it know to expect Windows/linux/mac line endings for
>> particular files.  This seems to be handled in git by using the
>> '.gitattributes' file instead.  Unfortunately, the git/svn bridge
>> doesn't seem to be translate the information in the .gitattributes
>> file to appropriate eol-style settings in SVN.  Checking out a file
>> using SVN on Windows leaves me with a file without CRLF's, and if I
>> check in a CRLF file, that's the way it goes into the repository.
>> Differences in CRLF alone show up as 'real' differences that can be
>> checked in, and, if this happens, this causes problems with other
>> people's repositories.
>>
>> Am I doing something wrong; is there another way to handle this; or
>> can I file this as a bug report/feature request?
>
--
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 7/7] blame: actually use the diff opts parsed from the command line

2016-08-23 Thread Junio C Hamano
Michael Haggerty  writes:

> Somebody who knows more about how diff operations are configured
> should please review this. I'm not certain that the change as
> implemented won't have other unwanted side-effects, though of course
> I checked that the test suite runs correctly.

Generally, I think this is a bad idea.  

We run "diff-tree" internally for multiple purposes:

 - When the same path we are drilling down appears, we use diff-tree
   to compare that path alone, to obtain textual diff, so that we
   can say "what did not appear in the textual diff output are
   attributable to the parent commit, we can exonerate this child".

   Even if the command line to "git blame" had "-Sfoo", you do not
   want to pass it down to this invocation.  If the child did not
   change the number of occurrences of string "foo" in the path
   being drilled down, it will pass down the blame for all lines to
   the parent.  And copying -R from the command line to this
   invocation does not make any sense.  Copying -C -C from the
   command line will defeat the whole purpose of having find_origin
   vs find_rename distinction, I would think, for this step.

 - When the path we have been drilling down for no longer appears in
   the parent, we use diff-tree with rename detection to find where
   the file _could_ have come from.

   It is a good idea to allow the similarity threshold to be tweaked
   from the command line.  Copying -R from the command line to this
   invocation may make sense, but I think you would break this step
   if you copied -C/-C -C

 - When -C/-C -C/-C -C -C is given from the command line, after
   running the "pass the blame to our primary suspect", we run
   tree-level diff-tree with find-copy option, only to enumerate
   paths that appear in the parent.  We pick pieces from these paths
   by doing blob-level diffs in diff_hunks() ourselves.  Copying -C
   from the command line would be useful if you are only doing a
   single '-C' (because it would allow you to tweak the similarity
   threshold), but otherwise it would probably break the command.

The design principle taken in "git blame" is to leave the decision
on what diff options do or do not make sense for these particular
invocations of the internal "diff-tree", and have "blame" give the
selected options to the internal "diff-tree" invocations.  "-w"
happens to use xdl_opts that will eventually be passed down to diff
machinery but you should consider it an accidental optimization
(i.e. "blame" designer considered that it makes sense to give
whitespace-ignoring comparison at the leaf-level "diff between
blobs", and found that the most efficient way to do so is to just
take "-w" from the command line and set the ignore-whitespace bit
used to drive the internal "diff-tree" invocation).  If we _were_
adding -R to allow the users to affect similarity threshold,
we would parse it in "git blame" command line option processing,
and would give that _ONLY_ to the second invocation above, i.e. the
one done in find_rename() but not in others like in find_origin.





--
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] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

2016-08-23 Thread Junio C Hamano
Jacob Keller  writes:

> From: Jacob Keller 
>
> Change the default behavior of git-format-patch to generate numbered
> sequence of 0/1 and 1/1 when generating both a cover-letter and a single
> patch. This standardizes the cover letter to have 0/N which helps
> distinguish the cover letter from the patch itself. Since the behavior
> is easily changed via configuration as well as the use of -n and -N this
> should be acceptable default behavior.
>
> Signed-off-by: Jacob Keller 
> ---

This obviously changes the behaviour, but I do not think of a reason
why this change is a bad idea.  

>  builtin/log.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 92dc34dcb0cc..8e6100fb0c5b 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1676,7 +1676,7 @@ int cmd_format_patch(int argc, const char **argv, const 
> char *prefix)
>   /* nothing to do */
>   return 0;
>   total = nr;
> - if (!keep_subject && auto_number && total > 1)
> + if (!keep_subject && auto_number && (total > 1 || cover_letter))
>   numbered = 1;
>   if (numbered)
>   rev.total = total + start_number - 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: git-svn bridge and line endings

2016-08-23 Thread Torsten Bögershausen
On Mon, Aug 22, 2016 at 05:04:47PM -0700, Lucian Smith wrote:
> I'm attempting to use the git-svn bridge, and am having problems with
> line endings on Windows.
> 
> The setup is that we have a git repository on github, and I've checked
> out a branch on my Windows machine using Tortoise svn.  I make
> changes, commit them, and the branch is updated.  In general, this
> works fine.

Just to make sure:
The repo is in git format.
Is it a public repo ?
Or could you make a piblic demo repo ?
Do I understand it right: Tortoise SVN talks directly to the Git server ?
Isn't Tortoise SVN a client to talk to SVN server?
What goes over the wire to the remote Git server, git or SVN ?

To my understanding, "git svn" can use Git locally, and talk to an SVN server.
What do I miss ?

> 
> If this was just SVN, I could set the 'eol-style' for files to
> 'native' to let it know to expect Windows/linux/mac line endings for
> particular files.  This seems to be handled in git by using the
> '.gitattributes' file instead.  Unfortunately, the git/svn bridge
> doesn't seem to be translate the information in the .gitattributes
> file to appropriate eol-style settings in SVN.  Checking out a file
> using SVN on Windows leaves me with a file without CRLF's, and if I
> check in a CRLF file, that's the way it goes into the repository.
> Differences in CRLF alone show up as 'real' differences that can be
> checked in, and, if this happens, this causes problems with other
> people's repositories.
> 
> Am I doing something wrong; is there another way to handle this; or
> can I file this as a bug report/feature request?

--
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


Smart HTTP push permissions failure

2016-08-23 Thread David McGough
Hi Git Community!

I'm trying to get Git on the server.  I installed Git and httpd on Centos 7 and 
configred for smart http.  I created a project on my local git and I cloned it 
to a base repository:
git clone --bare DataConversion DataConversion.git then I scp it to the server: 
scp -r DataConversion g...@xx.xx.xx.xx:/opt/git/repository/product/tools.  Then 
on the server for the project I ran git config core.sharedRepository group  

I added a remote server to my local project: git remote add origin 
http://xx.xx.xx.xx/git/product/tools/DataConversion.git

git remote -v shows:
origin  http://xx.xx.xx.xx/git/product/tools/DataConversion.git (fetch)
origin  http://xx.xx.xx.xx/git/product/tools/DataConversion.git (push)

When I try to push to the server I get this message:
remote: error: insufficient permission for adding an object to repository 
database ./objects
remote: fatal: failed to write object

Fwiw I can clone the project from the server to my local.

Here are the permssions on the project and the objects folder.

[git@services-git DataConversion.git]$ pwd
/opt/git/repos/product/tools/DataConversion.git
[git@services-git DataConversion.git]$ ll
total 24
-rwxrwxr-x.  1 git staff  196 Aug 23 11:24 config
-rwxrwxr-x.  1 git staff   73 Aug 22 15:28 description
-rwxrwxr-x.  1 git staff   23 Aug 22 15:28 HEAD
drwxrwxr-x.  2 git staff 4096 Aug 22 15:28 hooks
drwxrwxr-x.  2 git staff   20 Aug 22 15:28 info
drwxrwxr-x. 65 git staff 4096 Aug 22 16:50 objects
-rwxrwxr-x.  1 git staff   98 Aug 22 15:29 packed-refs
drwxrwxr-x.  4 git staff   29 Aug 22 15:29 refs
[git@services-git DataConversion.git]$
[git@services-git DataConversion.git]$ cd objects
[git@services-git objects]$ ll
total 12
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 06
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 08
drwxrwxr-x. 2 git staff   96 Aug 22 15:28 0a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 17
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 19
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 1c
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 24
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 29
drwxrwxr-x. 2 git staff 4096 Aug 22 15:28 30
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 32
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 33
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 3d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 3f
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 41
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 4b
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 57
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 5a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 5d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 5f
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 64
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 65
drwxrwxr-x. 2 git staff 4096 Aug 22 15:28 69
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 6d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 70
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 74
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 7a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 7b
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 7c
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 84
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 89
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 8a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 8c
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 93
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 9d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 a0
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 a2
drwxrwxr-x. 2 git staff 4096 Aug 22 15:28 a3
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 a6
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 ab
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 af
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 b1
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 b7
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 b8
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 c3
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 c8
drwxrwxr-x. 2 git staff   96 Aug 22 15:28 c9
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 cb
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 cf
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 d1
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 d8
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 d9
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 db
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 dc
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 e1
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 e7
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 e9
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 ea
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 ed
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 f0
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 f3
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 f5
drwxrwxr-x. 2 git staff6 Aug 22 15:29 info
drwxrwxr-x. 2 git staff6 Aug 22 15:29 pack

apache and git users are both in the staff group, and staff group is their 
default group.  I have also tried to use the set group id bit but to no avail. 
http://www.gnu.org/software/coreutils/manual/html_node/Directory-Setuid-and-Setgid.html

[root@services-git DataConversion.git]# groups apache
apache : staff git
[root@services-git DataConversion.git]# groups git
git : staff apache

So I am pretty confused about what the issue.  Which 

Re: Extending "extended SHA1" syntax to traverse through gitlinks?

2016-08-23 Thread Junio C Hamano
Jakub Narębski  writes:

> Especially that for submodules you need:
>
> $ git --git-dir=subdir/.git cat-file -p $(git rev-parse HEAD:subdir):file
>
> (or something like that), assuming that you start in supermodule.
>   ...
>
> But perhaps '//' would be better.

If the users have to know where they need to use different
separator, I do not think it is worth complicating the plumbing to
do this for them.  I'd rather keep things simple, and let the users
build complex stuff on top of the plumbing.
--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Johannes Schindelin
Hi Michael,

On Tue, 23 Aug 2016, Johannes Schindelin wrote:

> On Tue, 23 Aug 2016, Michael J Gruber wrote:
> 
> > Maybe you want to re-read what you wrote to trigger his response, and
> > consider adjusting your attitude ("I want this now so I'll release it in
> > Git4Win") rather than the downstream name.
> 
> I am working *very* hard on improving the user experience of Git for
> Windows. And yes, sometimes I have to include something in Git for Windows
> versions that upstream Git does not include in the corresponding version
> number.
> 
> I am really at a loss why you see fit to attack me about that.

In case it is not crystal-clear, let me clarify one very important point.
It seems that some people mistake the work I do for something I do on a
whim. This is not so.

The patch series that triggered this entire unfortunate discussion
introduced the --smudge option, which I have subsequently renamed to
--filters and submitted as a patch series to the Git project.

So it is an altogether unfair misrepresentation to state that I introduce
features that deviate so much from upstream Git as to require a new name.

The feature in question is also highly unlikely to be used as much by
non-Windows users as by Windows users due to the unfortunate choice of the
default setting for core.autocrlf. Basically, Windows users have to use
those --filters all the time, and in many cases, git cat-file --batch
without --filters is simply useless. This is nothing, say, Linux users
would care about, of course, but Windows folks like me care a great deal
about it.

It is this need that literally guarantees that I will get more useful
feedback from Windows users about this feature (and in this context I mean
application developers) than from Linux or MacOSX users. And as a matter
of fact, I got exactly that: great feedback. This feedback resulted in the
addition of the --path option, and of the work I did on making --filters
compatible with the --batch mode.

So I take great exception at this criticism. I think these comments were
not really thought through, and I also would consider this discussion in
and of itself ("is Git for Windows really Git? Should it not be renamed,
despite Dscho's best efforts to get them in sync?") to be much more
harmful than any feature I introduced into Git for Windows before trying
to get it integrated into upstream Git.

Thank you very much for your attention,
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: [ANNOUNCE] Git v2.10.0-rc1

2016-08-23 Thread Junio C Hamano
Johannes Schindelin  writes:

> On Fri, 19 Aug 2016, Junio C Hamano wrote:
>
>> [...]
>>  * A new run-command API function pipe_command() is introduced to
> ...
> You probably want a '*' in front of this paragraph, too.
>
>>  * Squelch compiler warnings for netmalloc (in compat/) library.
>
> s/netmalloc/nedmalloc/

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


Re: [PATCH v8 7/8] cache: add empty_tree_oid object

2016-08-23 Thread Junio C Hamano
Jeff King  writes:

> I suspect we need more than just the "is_empty" query. At least for the
> blob case, we do hashcpy() it into a struct (which should eventually
> become oidcpy). The empty-tree case even more so, as we pass it to
> random functions like lookup_tree().
>
> Our EMPTY_TREE_SHA1_BIN_LITERAL effectively ends up as a singleton
> in-core, too; it's just handled transparently by the compiler, since
> it's a literal. This effectively gives us _two_ singletons. We could do:
>
>   const struct object_id empty_blob_oid = {
> "\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b"
> "\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
>   };
>   #define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
>
> It's possible the use of an actual string literal lets the compiler do
> more optimizations, but I'd doubt it matters in practice. Probably it is
> just sticking that literal somewhere in BSS and filling in the pointer
> to it.

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


Possible bug: git pull --rebase discards local commits

2016-08-23 Thread Joshua Phillips
I've found a case where git pull --rebase discards commits in my branch
if the remote-tracking branch was rewound (and the remote tracking
branch's reflog contains my branch's latest commit). This is due to
git-pull's usage of git merge-base --fork-point.

On one hand, this behaviour might be correct since the remote repository
essentially removed that commit from master by 'reset --hard'. On the
other hand, I was surprised that git pull --rebase discarded a commit in
my branch.

Tested on 1.9.1, 2.7.4 and 2.10-rc1.

Steps to reproduce:

# Set up initial repository
git init source
cd source
git config receive.denyCurrentBranch no # make 'git push' work - not
relevant to bug
echo hello > test
git add test
git commit -m "Initial commit."

# Clone repository, make and push two commits.
cd ..
git clone source clone
cd clone
echo greetings >> test
git commit -a -m "This commit is rewritten."
echo something >> test
git commit -a -m "This commit disappears."
git push origin master

# Discard the second and rewrite the first commit.
cd ../source
git reset --hard HEAD~ # remove "This commit disappears"
git commit --amend -m "This commit is a rewrite."
cd ../clone

# Observe that "This commit disappears" is still in our branch but
is not in origin/master
git fetch && git log --graph master origin/master

# Now git pull --rebase gets confused because origin/master used to
point to "This commit disappears",
# so it assumes that that was the fork point for master, and "This
commit disappears" is discarded.
git pull --rebase
git log --graph # "This commit disappears" is completely gone!
--
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 05/15] sequencer: lib'ify read_and_refresh_cache()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index b90294f..a8c3a48 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -638,18 +638,21 @@ static int prepare_revs(struct replay_opts *opts)
return 0;
 }
 
-static void read_and_refresh_cache(struct replay_opts *opts)
+static int read_and_refresh_cache(struct replay_opts *opts)
 {
static struct lock_file index_lock;
int index_fd = hold_locked_index(_lock, 0);
if (read_index_preload(_index, NULL) < 0)
-   die(_("git %s: failed to read the index"), action_name(opts));
+   return error(_("git %s: failed to read the index"),
+   action_name(opts));
refresh_index(_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, 
NULL);
if (the_index.cache_changed && index_fd >= 0) {
if (write_locked_index(_index, _lock, COMMIT_LOCK))
-   die(_("git %s: failed to refresh the index"), 
action_name(opts));
+   return error(_("git %s: failed to refresh the index"),
+   action_name(opts));
}
rollback_lock_file(_lock);
+   return 0;
 }
 
 static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
@@ -977,7 +980,8 @@ static int pick_commits(struct commit_list *todo_list, 
struct replay_opts *opts)
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || opts->edit));
-   read_and_refresh_cache(opts);
+   if (read_and_refresh_cache(opts))
+   return -1;
 
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur, opts);
@@ -1041,7 +1045,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)
if (opts->subcommand == REPLAY_NONE)
assert(opts->revs);
 
-   read_and_refresh_cache(opts);
+   if (read_and_refresh_cache(opts))
+   return -1;
 
/*
 * Decide what to do depending on the arguments; a fresh
-- 
2.10.0.rc1.99.gcd66998


--
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 12/15] sequencer: lib'ify save_opts()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 17f2c8b..bac32ea 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -954,37 +954,39 @@ static int save_todo(struct commit_list *todo_list, 
struct replay_opts *opts)
return 0;
 }
 
-static void save_opts(struct replay_opts *opts)
+static int save_opts(struct replay_opts *opts)
 {
const char *opts_file = git_path_opts_file();
+   int res = 0;
 
if (opts->no_commit)
-   git_config_set_in_file(opts_file, "options.no-commit", "true");
+   res |= git_config_set_in_file_gently(opts_file, 
"options.no-commit", "true");
if (opts->edit)
-   git_config_set_in_file(opts_file, "options.edit", "true");
+   res |= git_config_set_in_file_gently(opts_file, "options.edit", 
"true");
if (opts->signoff)
-   git_config_set_in_file(opts_file, "options.signoff", "true");
+   res |= git_config_set_in_file_gently(opts_file, 
"options.signoff", "true");
if (opts->record_origin)
-   git_config_set_in_file(opts_file, "options.record-origin", 
"true");
+   res |= git_config_set_in_file_gently(opts_file, 
"options.record-origin", "true");
if (opts->allow_ff)
-   git_config_set_in_file(opts_file, "options.allow-ff", "true");
+   res |= git_config_set_in_file_gently(opts_file, 
"options.allow-ff", "true");
if (opts->mainline) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(, "%d", opts->mainline);
-   git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+   res |= git_config_set_in_file_gently(opts_file, 
"options.mainline", buf.buf);
strbuf_release();
}
if (opts->strategy)
-   git_config_set_in_file(opts_file, "options.strategy", 
opts->strategy);
+   res |= git_config_set_in_file_gently(opts_file, 
"options.strategy", opts->strategy);
if (opts->gpg_sign)
-   git_config_set_in_file(opts_file, "options.gpg-sign", 
opts->gpg_sign);
+   res |= git_config_set_in_file_gently(opts_file, 
"options.gpg-sign", opts->gpg_sign);
if (opts->xopts) {
int i;
for (i = 0; i < opts->xopts_nr; i++)
-   git_config_set_multivar_in_file(opts_file,
+   res |= git_config_set_multivar_in_file_gently(opts_file,

"options.strategy-option",
opts->xopts[i], "^$", 
0);
}
+   return res;
 }
 
 static int pick_commits(struct commit_list *todo_list, struct replay_opts 
*opts)
@@ -1128,9 +1130,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return -1;
if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
return error(_("Can't revert as initial commit"));
-   if (save_head(sha1_to_hex(sha1)))
+   if (save_head(sha1_to_hex(sha1)) ||
+   save_opts(opts))
return -1;
-   save_opts(opts);
return pick_commits(todo_list, opts);
 }
 
-- 
2.10.0.rc1.99.gcd66998


--
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 14/15] sequencer: do not die() in do_pick_commit()

2016-08-23 Thread Johannes Schindelin
Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 324463f..c29de64 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -589,12 +589,14 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
 * However, if the merge did not even start, then we don't want to
 * write it at all.
 */
-   if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res 
== 1))
-   update_ref(NULL, "CHERRY_PICK_HEAD", commit->object.oid.hash, 
NULL,
-  REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
-   if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || 
res == 1))
-   update_ref(NULL, "REVERT_HEAD", commit->object.oid.hash, NULL,
-  REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+   if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res 
== 1) &&
+   update_ref(NULL, "CHERRY_PICK_HEAD", commit->object.oid.hash, NULL,
+  REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
+   res = -1;
+   if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || 
res == 1) &&
+   update_ref(NULL, "REVERT_HEAD", commit->object.oid.hash, NULL,
+  REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
+   res = -1;
 
if (res) {
error(opts->action == REPLAY_REVERT
-- 
2.10.0.rc1.99.gcd66998


--
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 09/15] sequencer: lib'ify create_seq_dir()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1bcea84..1706ef4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -839,8 +839,8 @@ static int create_seq_dir(void)
return -1;
}
else if (mkdir(git_path_seq_dir(), 0777) < 0)
-   die_errno(_("Could not create sequencer directory %s"),
- git_path_seq_dir());
+   return error(_("Could not create sequencer directory %s (%s)"),
+ git_path_seq_dir(), strerror(errno));
return 0;
 }
 
-- 
2.10.0.rc1.99.gcd66998


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


[PATCH 11/15] sequencer: lib'ify save_todo()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 3e07aa0..17f2c8b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -929,24 +929,29 @@ static int sequencer_rollback(struct replay_opts *opts)
return -1;
 }
 
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static int save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 {
static struct lock_file todo_lock;
struct strbuf buf = STRBUF_INIT;
int fd;
 
-   fd = hold_lock_file_for_update(_lock, git_path_todo_file(), 
LOCK_DIE_ON_ERROR);
+   fd = hold_lock_file_for_update(_lock, git_path_todo_file(), 0);
+   if (fd < 0)
+   return error_errno(_("Could not lock '%s'"),
+  git_path_todo_file());
if (format_todo(, todo_list, opts) < 0)
-   die(_("Could not format %s."), git_path_todo_file());
+   return error(_("Could not format %s."), git_path_todo_file());
if (write_in_full(fd, buf.buf, buf.len) < 0) {
strbuf_release();
-   die_errno(_("Could not write to %s"), git_path_todo_file());
+   return error_errno(_("Could not write to %s"),
+  git_path_todo_file());
}
if (commit_lock_file(_lock) < 0) {
strbuf_release();
-   die(_("Error wrapping up %s."), git_path_todo_file());
+   return error(_("Error wrapping up %s."), git_path_todo_file());
}
strbuf_release();
+   return 0;
 }
 
 static void save_opts(struct replay_opts *opts)
@@ -995,7 +1000,8 @@ static int pick_commits(struct commit_list *todo_list, 
struct replay_opts *opts)
return -1;
 
for (cur = todo_list; cur; cur = cur->next) {
-   save_todo(cur, opts);
+   if (save_todo(cur, opts))
+   return -1;
res = do_pick_commit(cur->item, opts);
if (res)
return res;
-- 
2.10.0.rc1.99.gcd66998


--
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 10/15] sequencer: lib'ify save_head()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1706ef4..3e07aa0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -844,18 +844,22 @@ static int create_seq_dir(void)
return 0;
 }
 
-static void save_head(const char *head)
+static int save_head(const char *head)
 {
static struct lock_file head_lock;
struct strbuf buf = STRBUF_INIT;
int fd;
 
-   fd = hold_lock_file_for_update(_lock, git_path_head_file(), 
LOCK_DIE_ON_ERROR);
+   fd = hold_lock_file_for_update(_lock, git_path_head_file(), 0);
+   if (fd < 0)
+   return error_errno(_("Could not lock HEAD"));
strbuf_addf(, "%s\n", head);
if (write_in_full(fd, buf.buf, buf.len) < 0)
-   die_errno(_("Could not write to %s"), git_path_head_file());
+   return error_errno(_("Could not write to %s"),
+  git_path_head_file());
if (commit_lock_file(_lock) < 0)
-   die(_("Error wrapping up %s."), git_path_head_file());
+   return error(_("Error wrapping up %s."), git_path_head_file());
+   return 0;
 }
 
 static int reset_for_rollback(const unsigned char *sha1)
@@ -1118,7 +1122,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return -1;
if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
return error(_("Can't revert as initial commit"));
-   save_head(sha1_to_hex(sha1));
+   if (save_head(sha1_to_hex(sha1)))
+   return -1;
save_opts(opts);
return pick_commits(todo_list, opts);
 }
-- 
2.10.0.rc1.99.gcd66998


--
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-svn bridge and line endings

2016-08-23 Thread Lucian Smith
Thanks for the quick responses!

My situation is that the git side is entirely whatever github.org is
running; presumably the latest stable version?  They provide a URL for
repositories hosted there that can be accessed by an SVN
client--somewhere on github is the 'git-svn bridge' (as I understand
it): something that receives SVN requests, translates them to
git-speak, and replies with what SVN expects.

There is indeed a .gitattributes file in the repository, but the SVN
client doesn't know what to do with it.  My hope was that something in
the bridge code, that translated SVN requests to git and back, could
take the SVN request, "Please give me this file; I'm on Windows" look
at the .gitattributes file in the repository, and hand out a file with
CR/LF's in it.  Conversely, when SVN tells git "Here is the new
version of the file to check in," the bridge could look at the file,
realize it had CR/LF's in it, look at the .gitattributes file to know
if it needed to be converted, and then convert it appropriately.

I can imagine a full-blown bridge that could even translate the SVN
EOL propset back and forth appropriately, but I'm not sure if going
that far is necessary and/or helpful.

I don't know if this is the right mailing list for that particular bit
of software, but it at least seemed like a good place to start.  Thank
you!

-Lucian

On Mon, Aug 22, 2016 at 10:54 PM, Eric Wong  wrote:
> Alfred Perlstein  wrote:
>> I hadn't anticipated there be to translation between svn props and
>> .gitattributes, it sounds a bit messy but possible, that said, is it
>> not possible to commit .gitattribute files to the svn repo?  Even in
>> FreeBSD land such small token files are permitted.
>
> I'm not sure if an automatic translation is necessary or
> desired (because of a corruption risk).
>
> Perhaps Lucian can clarify the situation for his repo.
>
>> As far as documenting svn-properties, most of the properties are
>> used on the Subversion side either by subversion itself, or by
>> scripts in the subversion repository.  Perhaps a blurb "see the
>> subversion documentation and/or your local subversion
>> administrator's guide for properties and their uses." would suffice?
>
> Yes, perhaps with a workable example Lucian can use today with
> any git v2.3.0 or later.
>
> Thanks for the quick response!
>
>> Opinions?  Happy to look into it.
--
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 08/15] sequencer: lib'ify walk_revs_populate_todo()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 808cd53..1bcea84 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -816,17 +816,19 @@ static int read_populate_opts(struct replay_opts **opts)
return 0;
 }
 
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static int walk_revs_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
 {
struct commit *commit;
struct commit_list **next;
 
-   prepare_revs(opts);
+   if (prepare_revs(opts))
+   return -1;
 
next = todo_list;
while ((commit = get_revision(opts->revs)))
next = commit_list_append(commit, next);
+   return 0;
 }
 
 static int create_seq_dir(void)
@@ -,8 +1113,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 * progress
 */
 
-   walk_revs_populate_todo(_list, opts);
-   if (create_seq_dir() < 0)
+   if (walk_revs_populate_todo(_list, opts) ||
+   create_seq_dir() < 0)
return -1;
if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
return error(_("Can't revert as initial commit"));
-- 
2.10.0.rc1.99.gcd66998


--
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: [PATCHv8 8/8] clone: recursive and reference option triggers submodule alternates

2016-08-23 Thread Junio C Hamano
Stefan Beller  writes:

> On Wed, Aug 17, 2016 at 5:17 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>>  and now with error handling of invalid options.
>>
>> Thanks.
>
> Well this was not the end of the story. I sent that version out,
> before doing a final test run, because changing a few lines is trivial right? 
> :(
>
> I'll resend with:

Heh, my integration test caught it before pushing it out ;-).

>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a1da3ea..a366757 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -562,7 +562,7 @@ static void prepare_possible_alternates(const char 
> *sm_name,
>
> if (!strcmp(sm_alternate, "superproject"))
> foreach_alt_odb(add_possible_reference_from_superproject, 
> );
> -   else if (!strcmp(sm_alternate, "no")
> +   else if (!strcmp(sm_alternate, "no"))
> ; /* do nothing */
> else
> die(_("Value '%s' for submodule.alternateLocation is
> not recognized"), sm_alternate);
--
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 02/15] sequencer: lib'ify do_recursive_merge()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index c75296c..0c8c955 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -303,7 +303,8 @@ static int do_recursive_merge(struct commit *base, struct 
commit *next,
if (active_cache_changed &&
write_locked_index(_index, _lock, COMMIT_LOCK))
/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
-   die(_("%s: Unable to write new index file"), action_name(opts));
+   return error(_("%s: Unable to write new index file"),
+   action_name(opts));
rollback_lock_file(_lock);
 
if (opts->signoff)
-- 
2.10.0.rc1.99.gcd66998


--
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 13/15] sequencer: lib'ify sequencer_pick_revisions()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index bac32ea..324463f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1093,10 +1093,11 @@ int sequencer_pick_revisions(struct replay_opts *opts)
if (!get_sha1(name, sha1)) {
if (!lookup_commit_reference_gently(sha1, 1)) {
enum object_type type = sha1_object_info(sha1, 
NULL);
-   die(_("%s: can't cherry-pick a %s"), name, 
typename(type));
+   return error(_("%s: can't cherry-pick a %s"),
+   name, typename(type));
}
} else
-   die(_("%s: bad revision"), name);
+   return error(_("%s: bad revision"), name);
}
 
/*
@@ -1112,10 +1113,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
!opts->revs->cmdline.rev->flags) {
struct commit *cmit;
if (prepare_revision_walk(opts->revs))
-   die(_("revision walk setup failed"));
+   return error(_("revision walk setup failed"));
cmit = get_revision(opts->revs);
if (!cmit || get_revision(opts->revs))
-   die("BUG: expected exactly one commit from walk");
+   return error("BUG: expected exactly one commit from 
walk");
return single_pick(cmit, opts);
}
 
-- 
2.10.0.rc1.99.gcd66998


--
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 15/15] sequencer: do not die() in fast_forward_to()

2016-08-23 Thread Johannes Schindelin
A fast-forward may fail e.g. when it would overwrite an untracked
file. We still must not exit() in that case: the sequencer is
supposedly providing a library function.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index c29de64..9ddd054 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -226,7 +226,7 @@ static int fast_forward_to(const unsigned char *to, const 
unsigned char *from,
 
read_cache();
if (checkout_fast_forward(from, to, 1))
-   exit(128); /* the callee should have complained already */
+   return -1; /* the callee should have complained already */
 
strbuf_addf(, _("%s: fast-forward"), action_name(opts));
 
-- 
2.10.0.rc1.99.gcd66998
--
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 03/15] sequencer: lib'ify do_pick_commit()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 0c8c955..6ac2187 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -464,7 +464,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
 * to work on.
 */
if (write_cache_as_tree(head, 0, NULL))
-   die (_("Your index file is unmerged."));
+   return error (_("Your index file is unmerged."));
} else {
unborn = get_sha1("HEAD", head);
if (unborn)
-- 
2.10.0.rc1.99.gcd66998


--
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 00/15] Lib'ify quite a few functions in sequencer.c

2016-08-23 Thread Johannes Schindelin
This patch series is one of the half dozen patch series left to move the
bulk of rebase -i into a builtin.

The purpose of this patch series is to switch the functions in
sequencer.c from die()ing to returning errors instead, as proper library
functions should do, to give callers a chance to clean up after an
error.


Johannes Schindelin (15):
  sequencer: lib'ify write_message()
  sequencer: lib'ify do_recursive_merge()
  sequencer: lib'ify do_pick_commit()
  sequencer: lib'ify prepare_revs()
  sequencer: lib'ify read_and_refresh_cache()
  sequencer: lib'ify read_populate_todo()
  sequencer: lib'ify read_populate_opts()
  sequencer: lib'ify walk_revs_populate_todo()
  sequencer: lib'ify create_seq_dir()
  sequencer: lib'ify save_head()
  sequencer: lib'ify save_todo()
  sequencer: lib'ify save_opts()
  sequencer: lib'ify sequencer_pick_revisions()
  sequencer: do not die() in do_pick_commit()
  sequencer: do not die() in fast_forward_to()

 sequencer.c | 168 
 1 file changed, 101 insertions(+), 67 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/libify-sequencer-v1
Fetch-It-Via: git fetch https://github.com/dscho/git libify-sequencer-v1

-- 
2.10.0.rc1.99.gcd66998

base-commit: 2632c897f74b1cc9b5533f467da459b9ec725538
--
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 06/15] sequencer: lib'ify read_populate_todo()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index a8c3a48..5f6b020 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -746,7 +746,7 @@ static int parse_insn_buffer(char *buf, struct commit_list 
**todo_list,
return 0;
 }
 
-static void read_populate_todo(struct commit_list **todo_list,
+static int read_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
 {
struct strbuf buf = STRBUF_INIT;
@@ -754,18 +754,21 @@ static void read_populate_todo(struct commit_list 
**todo_list,
 
fd = open(git_path_todo_file(), O_RDONLY);
if (fd < 0)
-   die_errno(_("Could not open %s"), git_path_todo_file());
+   return error(_("Could not open %s (%s)"),
+   git_path_todo_file(), strerror(errno));
if (strbuf_read(, fd, 0) < 0) {
close(fd);
strbuf_release();
-   die(_("Could not read %s."), git_path_todo_file());
+   return error(_("Could not read %s."), git_path_todo_file());
}
close(fd);
 
res = parse_insn_buffer(buf.buf, todo_list, opts);
strbuf_release();
if (res)
-   die(_("Unusable instruction sheet: %s"), git_path_todo_file());
+   return error(_("Unusable instruction sheet: %s"),
+   git_path_todo_file());
+   return 0;
 }
 
 static int populate_opts_cb(const char *key, const char *value, void *data)
@@ -1015,7 +1018,8 @@ static int sequencer_continue(struct replay_opts *opts)
if (!file_exists(git_path_todo_file()))
return continue_single_pick();
read_populate_opts();
-   read_populate_todo(_list, opts);
+   if (read_populate_todo(_list, opts))
+   return -1;
 
/* Verify that the conflict has been resolved */
if (file_exists(git_path_cherry_pick_head()) ||
-- 
2.10.0.rc1.99.gcd66998


--
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] i18n: fix typos for translation

2016-08-23 Thread Junio C Hamano
Jean-Noel Avila  writes:

> Signed-off-by: Jean-Noel Avila 
> ---
>  bisect.c| 10 +-
>  sequencer.c |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/bisect.c b/bisect.c
> index 6f512c2..b9a0701 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -760,7 +760,7 @@ static void handle_skipped_merge_base(const unsigned char 
> *mb)
>   char *bad_hex = oid_to_hex(current_bad_oid);
>   char *good_hex = join_sha1_array_hex(_revs, ' ');
>  
> - warning(_("the merge base between %s and [%s] "
> + warning(_("The merge base between %s and [%s] "

I _think_ most these are not typos and must stay as they are.  From
time to time we try to clean things up, cf. 8c3ca351 (config:
lower-case first word of error strings, 2016-04-09).

The removal of "a" from "a another reviert" is good.  If you are
fixing the cases, you would want to do the correction the other way.

>   "must be skipped.\n"
>   "So we cannot be sure the first %s commit is "
>   "between %s and %s.\n"
> @@ -846,7 +846,7 @@ static void check_good_are_ancestors_of_bad(const char 
> *prefix, int no_checkout)
>   int fd;
>  
>   if (!current_bad_oid)
> - die(_("a %s revision is needed"), term_bad);
> + die(_("A %s revision is needed"), term_bad);
>  
>   /* Check if file BISECT_ANCESTORS_OK exists. */
>   if (!stat(filename, ) && S_ISREG(st.st_mode))
> @@ -863,7 +863,7 @@ static void check_good_are_ancestors_of_bad(const char 
> *prefix, int no_checkout)
>   /* Create file BISECT_ANCESTORS_OK. */
>   fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
>   if (fd < 0)
> - warning_errno(_("could not create file '%s'"),
> + warning_errno(_("Could not create file '%s'"),
> filename);
>   else
>   close(fd);
> @@ -914,7 +914,7 @@ void read_bisect_terms(const char **read_bad, const char 
> **read_good)
>   *read_good = "good";
>   return;
>   } else {
> - die_errno(_("could not read file '%s'"), filename);
> + die_errno(_("Could not read file '%s'"), filename);
>   }
>   } else {
>   strbuf_getline_lf(, fp);
> @@ -944,7 +944,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
>  
>   read_bisect_terms(_bad, _good);
>   if (read_bisect_refs())
> - die(_("reading bisect refs failed"));
> + die(_("Reading bisect refs failed"));
>  
>   check_good_are_ancestors_of_bad(prefix, no_checkout);
>  
> diff --git a/sequencer.c b/sequencer.c
> index 2e9c7d0..3804fa9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -702,7 +702,7 @@ static struct commit *parse_insn_line(char *bol, char 
> *eol, struct replay_opts *
>   if (action != opts->action) {
>   if (action == REPLAY_REVERT)
> error((opts->action == REPLAY_REVERT)
> - ? _("Cannot revert during a another revert.")
> + ? _("Cannot revert during another revert.")
>   : _("Cannot revert during a cherry-pick."));
>   else
> error((opts->action == REPLAY_REVERT)
--
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 07/15] sequencer: lib'ify read_populate_opts()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5f6b020..808cd53 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -806,12 +806,14 @@ static int populate_opts_cb(const char *key, const char 
*value, void *data)
return 0;
 }
 
-static void read_populate_opts(struct replay_opts **opts_ptr)
+static int read_populate_opts(struct replay_opts **opts)
 {
if (!file_exists(git_path_opts_file()))
-   return;
-   if (git_config_from_file(populate_opts_cb, git_path_opts_file(), 
*opts_ptr) < 0)
-   die(_("Malformed options sheet: %s"), git_path_opts_file());
+   return 0;
+   if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) 
< 0)
+   return error(_("Malformed options sheet: %s"),
+   git_path_opts_file());
+   return 0;
 }
 
 static void walk_revs_populate_todo(struct commit_list **todo_list,
@@ -1017,8 +1019,8 @@ static int sequencer_continue(struct replay_opts *opts)
 
if (!file_exists(git_path_todo_file()))
return continue_single_pick();
-   read_populate_opts();
-   if (read_populate_todo(_list, opts))
+   if (read_populate_opts() ||
+   read_populate_todo(_list, opts))
return -1;
 
/* Verify that the conflict has been resolved */
-- 
2.10.0.rc1.99.gcd66998


--
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 01/15] sequencer: lib'ify write_message()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 2e9c7d0..c75296c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -180,17 +180,20 @@ static void print_advice(int show_hint, struct 
replay_opts *opts)
}
 }
 
-static void write_message(struct strbuf *msgbuf, const char *filename)
+static int write_message(struct strbuf *msgbuf, const char *filename)
 {
static struct lock_file msg_file;
 
-   int msg_fd = hold_lock_file_for_update(_file, filename,
-  LOCK_DIE_ON_ERROR);
+   int msg_fd = hold_lock_file_for_update(_file, filename, 0);
+   if (msg_fd < 0)
+   return error_errno(_("Could not lock '%s'"), filename);
if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
-   die_errno(_("Could not write to %s"), filename);
+   return error_errno(_("Could not write to %s"), filename);
strbuf_release(msgbuf);
if (commit_lock_file(_file) < 0)
-   die(_("Error wrapping up %s."), filename);
+   return error(_("Error wrapping up %s."), filename);
+
+   return 0;
 }
 
 static struct tree *empty_tree(void)
@@ -564,16 +567,16 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
 head, , opts);
if (res < 0)
return res;
-   write_message(, git_path_merge_msg());
+   res |= write_message(, git_path_merge_msg());
} else {
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
 
-   write_message(, git_path_merge_msg());
+   res = write_message(, git_path_merge_msg());
 
commit_list_insert(base, );
commit_list_insert(next, );
-   res = try_merge_command(opts->strategy, opts->xopts_nr, 
opts->xopts,
+   res |= try_merge_command(opts->strategy, opts->xopts_nr, 
opts->xopts,
common, sha1_to_hex(head), remotes);
free_commit_list(common);
free_commit_list(remotes);
-- 
2.10.0.rc1.99.gcd66998


--
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 04/15] sequencer: lib'ify prepare_revs()

2016-08-23 Thread Johannes Schindelin
To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 6ac2187..b90294f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -621,7 +621,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
return res;
 }
 
-static void prepare_revs(struct replay_opts *opts)
+static int prepare_revs(struct replay_opts *opts)
 {
/*
 * picking (but not reverting) ranges (but not individual revisions)
@@ -631,10 +631,11 @@ static void prepare_revs(struct replay_opts *opts)
opts->revs->reverse ^= 1;
 
if (prepare_revision_walk(opts->revs))
-   die(_("revision walk setup failed"));
+   return error(_("revision walk setup failed"));
 
if (!opts->revs->commits)
-   die(_("empty commit set passed"));
+   return error(_("empty commit set passed"));
+   return 0;
 }
 
 static void read_and_refresh_cache(struct replay_opts *opts)
-- 
2.10.0.rc1.99.gcd66998


--
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: [L10N] Kickoff of translation for Git 2.10.0 round 1

2016-08-23 Thread Junio C Hamano
Jean-Noël AVILA  writes:

>> > 3. git-rebase--interactive, in this_nth_commit_message and
>> > skip_nth_commit_message are not localizable,
>> 
>> As the "TRANSLATORS" comment alludes to, "This is the Nth thing" can
>> be rephrased to "This is the thing N" or "This is the thing #N"
>> easily, and if that form without ordinal is acceptable for many
>> languages, we should say that it is also OK in C-locale without
>> translation.  So I agree that the recent change was pointless (even
>> though the result may be localizable).
> ...
>> so I do not think we cannot do it with ngettext().
>
> Let's cut it like this : first ten are literally translated, the following 
> ones 
> fall back to a general rule.

I actually once wrote "It is rare to squash dozens of commits into
one, so the first ten or dozen messages that spell the ordinals out
may be a good thing for readability", in the message you are
responding to, before realizing that the messages actually say
"1st", "2nd", etc., not "first", "second", etc., and scrapped that
part of the response.  I do not really see much point in forcing the
first ten to be translated differently.


--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Johannes Schindelin
Hi Michael,

On Tue, 23 Aug 2016, Michael J Gruber wrote:

> Maybe you want to re-read what you wrote to trigger his response, and
> consider adjusting your attitude ("I want this now so I'll release it in
> Git4Win") rather than the downstream name.

I am working *very* hard on improving the user experience of Git for
Windows. And yes, sometimes I have to include something in Git for Windows
versions that upstream Git does not include in the corresponding version
number.

I am really at a loss why you see fit to attack me about that.

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: Bug with git worktrees and git init

2016-08-23 Thread Michael J Gruber
Max Nordlund venit, vidit, dixit 23.08.2016 14:35:
> Hi,
> 
> I've been using multiple worktrees for months without issue (it's a
> great feature, thanks), until recently when I wanted to add hooks to
> them. So, when I added a template for the hooks, everything was fine
> until I did a git reset --hard in the original repo which both applied
> those changes to the other worktrees' working tree (the files on
> disk), and made my master branch kinda lose it's connection to the
> remote/think it was a kinda bare repo.
> 
> To reproduce this:
> 
> ```
> mkdir source-repo
> cd source-repo
> git init
> touch foo
> git add foo
> git commit -m 'Add foo'
> git worktree add ../worktree # which also creates a new branch 'worktree'
> touch bar
> git add bar
> git commit -m 'Add bar'
> cd ../worktree
> git init

This is where the problem is created already: git init does not quite
notice that it is in a linked worktree. It treats

source-repo/.git/worktrees/worktree

as a proper gitdir and creates the full gitdir structure in there
(branches, hooks, info) which is usually shared among worktrees; also,
it adds the config setting

core.worktree = /worktree

to the main config (source-repo/.git/config) which is why "git status"
thinks that bar is missing - it indeed is in that worktree.

I've cc'ed the master of worktrees.

> cd ../source-repo
> git reset --hard master
> cd ../worktree
> git status # Suddenly `bar` has appear the working tree and is not tracked
> ```
> 
> I don't really now what is up with this, but I did notice that it is
> the last worktree in which git init has been run that is affected. I
> only ran git init to copy the hooks from the template, but if that is
> not something you should do in a worktree then a check would have been
> nice.
> 
> Thanks for this awesome tool, and I hope this helps
> Max Nordlund
> 

--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Michael J Gruber
Johannes Schindelin venit, vidit, dixit 23.08.2016 15:54:
> Hi Duy,
> 
> On Mon, 22 Aug 2016, Duy Nguyen wrote:
> 
>> On Thu, Aug 18, 2016 at 3:37 PM, Johannes Schindelin
>>  wrote:
>>> Hi Junio,
>>>
>>> On Wed, 17 Aug 2016, Junio C Hamano wrote:
>>>
 Johannes Schindelin  writes:

>> And then your "git cat-file" patch can be upstreamed with the option
>> renamed to (or with an additional synonym) "--filters", which would make
>> things consistent.
>
> Right. I would like to ask for a `--smudge` synonym nevertheless, just
> because I already use this. On the other hand, it is early enough to tell
> everybody who knows about this feature to change their invocation (anybody
> who would know about `--smudge` would be in that 1% of users that have
> read the release notes, so most likely would read the next release notes,
> too).

 It is OK if it were your private edition, but you end up hurting
 your users if you need to redo the feature differently.
>>>
>>> Unfortunately, this is the situation of Git for Windows from its
>>> beginning: there has not been a single time that Git for Windows could
>>> live with unpatched upstream Git's source code.
>>>
>>> Business as usual, though.
>>
>> Bug fixes is one thing, features is completely different.
> 
> Oh? Completely?
> 
> So the core.hideDotFiles feature should have forced me to rename Git for
> Windows to, say, DschoGit on Windows?
> 
> Let's just stop here. This is getting too silly.

I see more truth than silliness in Duy's suggestion. Maybe you want to
re-read what you wrote to trigger his response, and consider adjusting
your attitude ("I want this now so I'll release it in Git4Win") rather
than the downstream name.

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


hello git

2016-08-23 Thread Duncan Ferguson
good evening git


http://www.philadelphiadisabilityattorney.com/Germany.php?farmer=fahvw17vge56g0g



Duncan Ferguson
--
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-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

2016-08-23 Thread Johannes Schindelin
Hi Duy,

On Mon, 22 Aug 2016, Duy Nguyen wrote:

> On Thu, Aug 18, 2016 at 3:37 PM, Johannes Schindelin
>  wrote:
> > Hi Junio,
> >
> > On Wed, 17 Aug 2016, Junio C Hamano wrote:
> >
> >> Johannes Schindelin  writes:
> >>
> >> >> And then your "git cat-file" patch can be upstreamed with the option
> >> >> renamed to (or with an additional synonym) "--filters", which would make
> >> >> things consistent.
> >> >
> >> > Right. I would like to ask for a `--smudge` synonym nevertheless, just
> >> > because I already use this. On the other hand, it is early enough to tell
> >> > everybody who knows about this feature to change their invocation 
> >> > (anybody
> >> > who would know about `--smudge` would be in that 1% of users that have
> >> > read the release notes, so most likely would read the next release notes,
> >> > too).
> >>
> >> It is OK if it were your private edition, but you end up hurting
> >> your users if you need to redo the feature differently.
> >
> > Unfortunately, this is the situation of Git for Windows from its
> > beginning: there has not been a single time that Git for Windows could
> > live with unpatched upstream Git's source code.
> >
> > Business as usual, though.
> 
> Bug fixes is one thing, features is completely different.

Oh? Completely?

So the core.hideDotFiles feature should have forced me to rename Git for
Windows to, say, DschoGit on Windows?

Let's just stop here. This is getting too silly.

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] config: add conditional include

2016-08-23 Thread Johannes Schindelin
Hi Duy,

On Mon, 22 Aug 2016, Duy Nguyen wrote:

> On Mon, Aug 22, 2016 at 8:22 PM, Matthieu Moy
>  wrote:
> >>> I think the syntax should be design to allow arbitrary boolean
> >>> expression later if needed.
> >>
> >> I would be against that. We may extend it more in future, but it
> >> should be under control, not full boolean expressions.
> >
> > Why?
> >
> > I'm not saying we absolutely need it, but if we allow several kinds of
> > conditions (gitdir-is:... and others in the future), then it's natural
> > to allow combining them, and arbitrary boolean expression is both simple
> > and powerful (operators and/or/not and parenthesis and you have
> > everything you'll ever need).
> 
> For starter, we don't want another debate "python vs ruby vs lua vs
> ..." as the scripting language :) (for the record I vote Scheme! maybe
> with infix syntax)

FWIW I do not think that Matthieu implied that this has to be implemented.
But it does not make sense to slam the door shut prematurely, either.

Meaning: the more doors you can keep open with the new syntax, the better.

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 v14 11/27] bisect--helper: `bisect_next_check` & bisect_voc shell function in C

2016-08-23 Thread Pranit Bauva
Reimplement `bisect_next_check` shell function in C and add
`bisect-next-check` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

Also reimplement `bisect_voc` shell function in C and call it from
`bisect_next_check` implementation in C.

Using `--bisect-next-check` is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired but its implementation will
be called by some other methods.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 103 ++-
 git-bisect.sh|  60 ++-
 2 files changed, 106 insertions(+), 57 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 450426c..aca3908 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -6,6 +6,7 @@
 #include "dir.h"
 #include "argv-array.h"
 #include "run-command.h"
+#include "prompt.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-reset []"),
N_("git bisect--helper --bisect-write
 []"),
N_("git bisect--helper --bisect-check-and-set-terms  
 "),
+   N_("git bisect--helper --bisect-next-check []  
term_bad.buf);
+   char *good_glob = xstrfmt("%s-*", terms->term_good.buf);
+   char *bad_syn, *good_syn;
+
+   if (ref_exists(bad_ref))
+   missing_bad = 0;
+   free(bad_ref);
+
+   for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+(void *) _good);
+   free(good_glob);
+
+   if (!missing_good && !missing_bad)
+   return 0;
+
+   if (!current_term)
+   return -1;
+
+   if (missing_good && !missing_bad && current_term &&
+   !strcmp(current_term, terms->term_good.buf)) {
+   char *yesno;
+   /*
+* have bad (or new) but not good (or old). We could bisect
+* although this is less optimum.
+*/
+   fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
+   terms->term_bad.buf);
+   if (!isatty(0))
+   return 0;
+   /*
+* TRANSLATORS: Make sure to include [Y] and [n] in your
+* translation. The program will only accept English input
+* at this point.
+*/
+   yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
+   if (starts_with(yesno, "N") || starts_with(yesno, "n"))
+   return -1;
+
+   return 0;
+   }
+   bad_syn = xstrdup(bisect_voc("bad"));
+   good_syn = xstrdup(bisect_voc("good"));
+   if (!is_empty_or_missing_file(git_path_bisect_start())) {
+   error(_("You need to give me at least one %s and "
+   "%s revision. You can use \"git bisect %s\" "
+   "and \"git bisect %s\" for that. \n"),
+   bad_syn, good_syn, bad_syn, good_syn);
+   free(bad_syn);
+   free(good_syn);
+   return -1;
+   }
+   else {
+   error(_("You need to start by \"git bisect start\". You "
+   "then need to give me at least one %s and %s "
+   "revision. You can use \"git bisect %s\" and "
+   "\"git bisect %s\" for that.\n"),
+   good_syn, bad_syn, bad_syn, good_syn);
+   free(bad_syn);
+   free(good_syn);
+   return -1;
+   }
+   free(bad_syn);
+   free(good_syn);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -262,7 +353,8 @@ int cmd_bisect__helper(int argc, 

Re: [ANNOUNCE] Git v2.10.0-rc1

2016-08-23 Thread Johannes Schindelin
Hi Junio,

[dropping LKML]

On Fri, 19 Aug 2016, Junio C Hamano wrote:

> [...]
>  * A new run-command API function pipe_command() is introduced to
>sanely feed data to the standard input while capturing data from
>the standard output and the standard error of an external process,
>which is cumbersome to hand-roll correctly without deadlocking.
> 
>The codepath to sign data in a prepared buffer with GPG has been
>updated to use this API to read from the status-fd to check for
>errors (instead of relying on GPG's exit status).
>(merge efee955 jk/gpg-interface-cleanup later to maint).

You probably want a '*' in front of this paragraph, too.

>  * Squelch compiler warnings for netmalloc (in compat/) library.

s/netmalloc/nedmalloc/

Thank you so much for these extensive release notes. They do take time to
read, but they really cover everything quite nicely.

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 v14 22/27] bisect--helper: `bisect_replay` shell function in C

2016-08-23 Thread Pranit Bauva
Reimplement the `bisect_replay` shell function in C and also add
`--bisect-replay` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--bisect-replay` subcommand is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other method.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 100 ++-
 git-bisect.sh|  32 +--
 2 files changed, 100 insertions(+), 32 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b57b0c8..9c1108d 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -32,6 +32,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-autostart"),
N_("git bisect--helper --bisect-state (bad|new) []"),
N_("git bisect--helper --bisect-state (good|old) [...]"),
+   N_("git bisect--helper --bisect-replay "),
NULL
 };
 
@@ -879,6 +880,93 @@ static int bisect_log(void)
return 0;
 }
 
+static int get_next_word(struct strbuf *line, struct strbuf *word)
+{
+   int i;
+   for (i = 0; line->buf[i] != ' ' && line->buf[i] != '\0'; i++)
+   strbuf_addch(word, line->buf[i]);
+
+   return 0;
+}
+
+static int bisect_replay(struct bisect_terms *terms, const char *filename)
+{
+   struct strbuf line = STRBUF_INIT;
+   FILE *fp;
+
+   if (is_empty_or_missing_file(filename))
+   die(_("no such file with name '%s' exists"), filename);
+
+   if (bisect_reset(NULL))
+   return -1;
+
+   fp = fopen(filename, "r");
+
+   while (strbuf_getline(, fp) != EOF) {
+   struct strbuf command = STRBUF_INIT;
+   if (starts_with(line.buf, "git bisect ") ||
+   starts_with(line.buf, "git-bisect "))
+   strbuf_remove(, 0, 11);
+   else
+   continue;
+
+   get_terms(terms);
+   get_next_word(, );
+   if (check_and_set_terms(terms, command.buf)) {
+   strbuf_release();
+   strbuf_release();
+   }
+
+   if (line.buf[command.len] != '\0')
+   strbuf_remove(, 0, command.len + 1);
+   else
+   strbuf_remove(, 0, command.len);
+
+   if (!strcmp(command.buf, "start")) {
+   struct argv_array argv = ARGV_ARRAY_INIT;
+   sq_dequote_to_argv_array(line.buf, );
+   if (bisect_start(terms, 0, argv.argv, argv.argc)) {
+   strbuf_release();
+   strbuf_release();
+   argv_array_clear();
+   return -1;
+   }
+   argv_array_clear();
+   continue;
+   }
+
+   if (one_of(command.buf, terms->term_good.buf,
+   terms->term_bad.buf, "skip", NULL)) {
+   if (bisect_write(command.buf, line.buf, terms, 0)) {
+   strbuf_release();
+   strbuf_release();
+   return -1;
+   }
+   continue;
+   }
+
+   if (!strcmp(command.buf, "terms")) {
+   struct argv_array argv = ARGV_ARRAY_INIT;
+   sq_dequote_to_argv_array(line. buf, );
+   if (bisect_terms(terms, argv.argv, argv.argc)) {
+   strbuf_release();
+   strbuf_release();
+   argv_array_clear();
+   return -1;
+   }
+   argv_array_clear();
+   continue;
+   }
+
+   strbuf_release();
+   strbuf_release();
+   die(_("?? what are you talking about?"));
+   }
+   strbuf_release();
+
+   return bisect_auto_next(terms, NULL);
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -892,7 +980,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
BISECT_AUTO_NEXT,
BISECT_AUTOSTART,
BISECT_STATE,
-   BISECT_LOG
+   BISECT_LOG,
+   BISECT_REPLAY
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -918,6 +1007,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 

Bug with git worktrees and git init

2016-08-23 Thread Max Nordlund
Hi,

I've been using multiple worktrees for months without issue (it's a
great feature, thanks), until recently when I wanted to add hooks to
them. So, when I added a template for the hooks, everything was fine
until I did a git reset --hard in the original repo which both applied
those changes to the other worktrees' working tree (the files on
disk), and made my master branch kinda lose it's connection to the
remote/think it was a kinda bare repo.

To reproduce this:

```
mkdir source-repo
cd source-repo
git init
touch foo
git add foo
git commit -m 'Add foo'
git worktree add ../worktree # which also creates a new branch 'worktree'
touch bar
git add bar
git commit -m 'Add bar'
cd ../worktree
git init
cd ../source-repo
git reset --hard master
cd ../worktree
git status # Suddenly `bar` has appear the working tree and is not tracked
```

I don't really now what is up with this, but I did notice that it is
the last worktree in which git init has been run that is affected. I
only ran git init to copy the hooks from the template, but if that is
not something you should do in a worktree then a check would have been
nice.

Thanks for this awesome tool, and I hope this helps
Max Nordlund
--
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 v14 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C

2016-08-23 Thread Pranit Bauva
Reimplement `is_expected_rev` & `check_expected_revs` shell function in
C and add a `--check-expected-revs` subcommand to `git bisect--helper` to
call it from git-bisect.sh .

Using `--check-expected-revs` subcommand is a temporary measure to port
shell functions to C so as to use the existing test suite. As more
functions are ported, this subcommand would be retired but its
implementation will be called by some other method.

Helped-by: Eric Sunshine 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 33 -
 git-bisect.sh| 20 ++--
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 9aba094..711be75 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -123,13 +123,40 @@ static int bisect_reset(const char *commit)
return bisect_clean_state();
 }
 
+static int is_expected_rev(const char *expected_hex)
+{
+   struct strbuf actual_hex = STRBUF_INIT;
+   int res = 0;
+   if (strbuf_read_file(_hex, git_path_bisect_expected_rev(), 0) >= 
0) {
+   strbuf_trim(_hex);
+   res = !strcmp(actual_hex.buf, expected_hex);
+   }
+   strbuf_release(_hex);
+   return res;
+}
+
+static int check_expected_revs(const char **revs, int rev_nr)
+{
+   int i;
+
+   for (i = 0; i < rev_nr; i++) {
+   if (!is_expected_rev(revs[i])) {
+   unlink_or_warn(git_path_bisect_ancestors_ok());
+   unlink_or_warn(git_path_bisect_expected_rev());
+   return 0;
+   }
+   }
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
NEXT_ALL = 1,
WRITE_TERMS,
BISECT_CLEAN_STATE,
-   BISECT_RESET
+   BISECT_RESET,
+   CHECK_EXPECTED_REVS
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
@@ -141,6 +168,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
OPT_CMDMODE(0, "bisect-reset", ,
 N_("reset the bisection state"), BISECT_RESET),
+   OPT_CMDMODE(0, "check-expected-revs", ,
+N_("check for expected revs"), CHECK_EXPECTED_REVS),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -167,6 +196,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
if (argc > 1)
die(_("--bisect-reset requires either zero or one 
arguments"));
return bisect_reset(argc ? argv[0] : NULL);
+   case CHECK_EXPECTED_REVS:
+   return check_expected_revs(argv, argc);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 442397b..c3e43248 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -237,22 +237,6 @@ bisect_write() {
test -n "$nolog" || echo "git bisect $state $rev" 
>>"$GIT_DIR/BISECT_LOG"
 }
 
-is_expected_rev() {
-   test -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
-   test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV")
-}
-
-check_expected_revs() {
-   for _rev in "$@"; do
-   if ! is_expected_rev "$_rev"
-   then
-   rm -f "$GIT_DIR/BISECT_ANCESTORS_OK"
-   rm -f "$GIT_DIR/BISECT_EXPECTED_REV"
-   return
-   fi
-   done
-}
-
 bisect_skip() {
all=''
for arg in "$@"
@@ -280,7 +264,7 @@ bisect_state() {
rev=$(git rev-parse --verify "$bisected_head") ||
die "$(eval_gettext "Bad rev input: \$bisected_head")"
bisect_write "$state" "$rev"
-   check_expected_revs "$rev" ;;
+   git bisect--helper --check-expected-revs "$rev" ;;
2,"$TERM_BAD"|*,"$TERM_GOOD"|*,skip)
shift
hash_list=''
@@ -294,7 +278,7 @@ bisect_state() {
do
bisect_write "$state" "$rev"
done
-   check_expected_revs $hash_list ;;
+   git bisect--helper --check-expected-revs $hash_list ;;
*,"$TERM_BAD")
die "$(eval_gettext "'git bisect \$TERM_BAD' can take only one 
argument.")" ;;
*)

--
https://github.com/git/git/pull/287
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More 

[PATCH v14 02/27] bisect: rewrite `check_term_format` shell function in C

2016-08-23 Thread Pranit Bauva
Reimplement the `check_term_format` shell function in C and add
a `--check-term-format` subcommand to `git bisect--helper` to call it
from git-bisect.sh

Using `--check-term-format` subcommand is a temporary measure to port
shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and its
implementation will be called by some other method/subcommand. For
eg. In conversion of write_terms() of git-bisect.sh, the subcommand will
be removed and instead check_term_format() will be called in its C
implementation while a new subcommand will be introduced for write_terms().

Helped-by: Johannes Schindelein 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 60 +++-
 git-bisect.sh| 31 ++---
 2 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8111c91..a47f1f2 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,19 +2,73 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "refs.h"
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
+   N_("git bisect--helper --check-term-format  "),
NULL
 };
 
+/*
+ * Check whether the string `term` belongs to the set of strings
+ * included in the variable arguments.
+ */
+LAST_ARG_MUST_BE_NULL
+static int one_of(const char *term, ...)
+{
+   int res = 0;
+   va_list matches;
+   const char *match;
+
+   va_start(matches, term);
+   while (!res && (match = va_arg(matches, const char *)))
+   res = !strcmp(term, match);
+   va_end(matches);
+
+   return res;
+}
+
+static int check_term_format(const char *term, const char *orig_term)
+{
+   int res;
+   char *new_term = xstrfmt("refs/bisect/%s", term);
+
+   res = check_refname_format(new_term, 0);
+   free(new_term);
+
+   if (res)
+   return error(_("'%s' is not a valid term"), term);
+
+   if (one_of(term, "help", "start", "skip", "next", "reset",
+   "visualize", "replay", "log", "run", NULL))
+   return error(_("can't use the builtin command '%s' as a term"), 
term);
+
+   /*
+* In theory, nothing prevents swapping completely good and bad,
+* but this situation could be confusing and hasn't been tested
+* enough. Forbid it for now.
+*/
+
+   if ((strcmp(orig_term, "bad") && one_of(term, "bad", "new", NULL)) ||
+(strcmp(orig_term, "good") && one_of(term, "good", "old", 
NULL)))
+   return error(_("can't change the meaning of the term '%s'"), 
term);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-   enum { NEXT_ALL = 1 } cmdmode = 0;
+   enum {
+   NEXT_ALL = 1,
+   CHECK_TERM_FMT
+   } cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
+   OPT_CMDMODE(0, "check-term-format", ,
+N_("check format of the term"), CHECK_TERM_FMT),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -29,6 +83,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
switch (cmdmode) {
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
+   case CHECK_TERM_FMT:
+   if (argc != 2)
+   die(_("--check-term-format requires two arguments"));
+   return check_term_format(argv[0], argv[1]);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index ae3cb01..a727c59 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -564,38 +564,11 @@ write_terms () {
then
die "$(gettext "please use two different terms")"
fi
-   check_term_format "$TERM_BAD" bad
-   check_term_format "$TERM_GOOD" good
+   git bisect--helper --check-term-format "$TERM_BAD" bad || exit
+   git bisect--helper --check-term-format "$TERM_GOOD" good || exit
printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
 }
 
-check_term_format () {
-   term=$1
-   git check-ref-format refs/bisect/"$term" ||
-   die "$(eval_gettext "'\$term' is not a valid term")"
-   case "$term" in
-   help|start|terms|skip|next|reset|visualize|replay|log|run)
-   die 

[PATCH v14 26/27] bisect--helper: retire `--bisect-auto-next` subcommand

2016-08-23 Thread Pranit Bauva
The `--bisect-auto-next` subcommand is no longer used in the shell
script and the function `bisect_auto_next` is called from the C
implementation.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a89e2f7..7577b69e 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -26,7 +26,6 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect start [--term-{old,good}= 
--term-{new,bad}=]"
  "[--no-checkout] [ 
[...]] [--] [...]"),
N_("git bisect--helper --bisect-next"),
-   N_("git bisect--helper --bisect-auto-next"),
N_("git bisect--helper --bisect-state (bad|new) []"),
N_("git bisect--helper --bisect-state (good|old) [...]"),
N_("git bisect--helper --bisect-replay "),
@@ -972,7 +971,6 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
BISECT_TERMS,
BISECT_START,
BISECT_NEXT,
-   BISECT_AUTO_NEXT,
BISECT_STATE,
BISECT_LOG,
BISECT_REPLAY
@@ -989,8 +987,6 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("start the bisect session"), BISECT_START),
OPT_CMDMODE(0, "bisect-next", ,
 N_("find the next bisection commit"), BISECT_NEXT),
-   OPT_CMDMODE(0, "bisect-auto-next", ,
-N_("verify the next bisection state then find the next 
bisection state"), BISECT_AUTO_NEXT),
OPT_CMDMODE(0, "bisect-state", ,
 N_("mark the state of ref (or refs)"), BISECT_STATE),
OPT_CMDMODE(0, "bisect-log", ,
@@ -1040,12 +1036,6 @@ int cmd_bisect__helper(int argc, const char **argv, 
const char *prefix)
get_terms();
res = bisect_next(, prefix);
break;
-   case BISECT_AUTO_NEXT:
-   if (argc)
-   die(_("--bisect-auto-next requires 0 arguments"));
-   get_terms();
-   res = bisect_auto_next(, prefix);
-   break;
case BISECT_STATE:
if (argc == 0)
die(_("--bisect-state requires at least 1 argument"));

--
https://github.com/git/git/pull/287
--
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


  1   2   >