Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Junio C Hamano
Duy Nguyen  writes:

>> Granted, "git worktree remove" should be the work horse. But why not have
>> two ways to skin the same cat? I, again, would prefer the short 'n sweet
>> "git branch -d -w pull-rebase-prefix" invocation.
>
> If you put effort into making it happen, I'm not stopping you :)

I actually would ;-)  If I had to choose between the two, I'd prefer
to see this new feature as part of the "worktree" subcommand, simply
because "branch" is a fairly basic feature that can be used by those
who are new to Git without knowing that the "worktree" feature even
exists (hence not having to have one extra option description in its
documentation that talks about "worktree" is a big plus).

But that is only if I had to choose between the two.

Personally, I think you are better off implementing this set of
features as a set of shell aliases (be it bash or tcsh).  For one
thing, you are likely to want to "chdir" to the newly created
worktree built for the branch (or an existing one for the named
branch) for the "take me to a worktree to work on this branch"
feature.  The last action you would want that command to take is to
take you there, and "git anything" subcommand would not let you do
that (unless you tell your users to run "eval `git something`", a
way similar to "ssh-agent -s", that is).

That approach to implement the UI that directly faces the end users
via scripting would let your users choose layouts more flexibly.
Some people may want a repository and its worktrees next to each
other, some others may want the worktrees embedded inside the main
repository, yet some others may want layouts other than those two.
--
To unsubscribe from this list: send the line "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] commit: add a commit.verbose config variable

2016-03-10 Thread Eric Sunshine
[+cc:Roberto Tyley]

On Fri, Mar 11, 2016 at 05:45:27AM +0530, Pranit Bauva wrote:
> On Fri, Mar 11, 2016 at 4:31 AM, Eric Sunshine  
> wrote:
> > As a convenience to reviewers, please use this area below the "---"
> > line to provide links and explain what changed since the previous
> > round rather than doing so in a separate email.
> 
> Actually I am sending the patches with submitGit herokuapp because my
> institute proxy does not allow IMAP/POP3 connections.

That's unfortunate. Your separate "cover letter" often arrives hours
later than the patch itself. Perhaps Roberto can comment on submitGit
and per-patch commentary.

> >> +   echo content >file &&
> >> +   git add file &&
> >> +   test_config commit.verbose true &&
> >> +   (
> >> +   GIT_EDITOR=cat &&
> >> +   export GIT_EDITOR &&
> >> +   test_must_fail git commit >output
> >> +   ) &&
> >> +   test_i18ngrep "diff --git" output
> >> +'
> >
> > Making git-commit fail unconditionally with "aborting due to empty
> > commit message" is a rather sneaky way to perform this test. I would
> > have expected to see these new tests re-use the existing machinery
> > provided by this script (the check-for-diff "editor") rather than
> > inventing an entirely new and unintuitive mechanism. Doing so would
> > also reduce the size of each new test.
> 
> I agree on the fact that making git-commit fail unconditionally is not
> a good way to perform the test. "check-for-diff" is not really an
> "editor" and it checks for the commit message after it has been
> written to the history. The verbose output is stripped when it is
> written to the history so we won't be able to test whether this patch
> works.

It's a bit tricky if you're not used to it, but check-for-diff
actually does what you want, and does so in a more direct way. While
it's true that it's not an "editor" per se, it does get access to the
entire block of text that would normally appear in your editor during
an interactive commit. And, this is happening before the commit has
been written to history. So, check-for-diff gets a chance to look at
the full text that would appear in your editor, and can therefore
check if it contains the expected "diff --git" string.

> This is where purposely breaking the code is required as when
> the commit fails, it gives the output of the contents present at that
> time (which will contain the verbose output). More over the
> 'check-for-diff' uses grep which is not preferred. Many tests are now
> using test_i18ngrep (eg. f79ce8db).

'test_i18ngrep' is intended for strings which may be translated,
however, since the expected "diff --git" string should never be
translated, check-for-diff's use of 'grep' is correct, whereas
'test_i18ngrep' would be misleading (if not actively wrong).

> I had planned on using
> 'check-for-diff' before but it took me some time to figure out this
> behavior and thus I began searching for another mechanism (breaking
> code).

As an experiment, I rewrote the four new tests in terms of
check-for-diff (with "test_set_editor check-for-diff" already in
effect). Here's what they look like, and they function as expected:

test_expect_success 'commit.verbose true and --verbose omitted' '
git -c commit.verbose=true commit --amend
'

test_expect_success 'commit.verbose true and --no-verbose' '
test_must_fail git -c commit.verbose=true commit --amend --no-verbose
'

test_expect_success 'commit.verbose false and --verbose' '
git -c commit.verbose=false commit --amend --verbose
'

test_expect_success 'commit.verbose false and --verbose omitted' '
test_must_fail git -c commit.verbose=false commit --amend
'

These are modeled after the "initial commit shows verbose diff" test
earlier in the script.
--
To unsubscribe from this list: send the line "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] t/t7502 : drop duplicate test

2016-03-10 Thread Pranit Bauva
The older version of this patch :
 - [v1] http://thread.gmane.org/gmane.comp.version-control.git/288662

The changes between the patches :
 - Improved the language construct of the commit message
 - Provided more details about the cited commit in the commit message

Regards,
Pranit Bauva
IIT Kharagpur
--
To unsubscribe from this list: send the line "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 v7 2/2] pull --rebase: add --[no-]autostash flag

2016-03-10 Thread Paul Tan
On Wed, Mar 9, 2016 at 12:18 PM, Mehul Jain  wrote:
> If rebase.autoStash configuration variable is set, there is no way to
> override it for "git pull --rebase" from the command line.
>
> Teach "git pull --rebase" the --[no-]autostash command line flag which
> overrides the current value of rebase.autoStash, if set. As "git rebase"
> understands the --[no-]autostash option, it's just a matter of passing
> the option to underlying "git rebase" when "git pull --rebase" is called.
>
> Helped-by: Matthieu Moy 
> Helped-by: Junio C Hamano 
> Helped-by: Paul Tan 
> Helped-by: Eric Sunshine 
> Signed-off-by: Mehul Jain 
> ---
> Previous patches: $gname287709
>
> Changes:
> - Slight change is documentation.
>
>  Documentation/git-pull.txt |  9 +
>  builtin/pull.c | 16 ++--
>  t/t5520-pull.sh| 39 +++
>  3 files changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index a62a2a6..da89be6 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -128,6 +128,15 @@ unless you have read linkgit:git-rebase[1] carefully.
>  --no-rebase::
> Override earlier --rebase.
>
> +--autostash::
> +--no-autostash::
> +   Before starting rebase, stash local modifications away (see
> +   linkgit:git-stash.txt[1]) if needed, and apply the stash when
> +   done (this option is only valid when "--rebase" is used).
> ++
> +`--no-autostash` is useful to override the `rebase.autoStash`
> +configuration variable (see linkgit:git-config[1]).
> +
>  Options related to fetching
>  ~~~
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 8a318e9..a01058a 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -86,6 +86,7 @@ static char *opt_commit;
>  static char *opt_edit;
>  static char *opt_ff;
>  static char *opt_verify_signatures;
> +static int opt_autostash = -1;
>  static int config_autostash = -1;

Hmm, why can't config_autostash just default to 0?

>  static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
>  static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
> @@ -150,6 +151,8 @@ static struct option pull_options[] = {
> OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
> N_("verify that the named commit has a valid GPG signature"),
> PARSE_OPT_NOARG),
> +   OPT_BOOL(0, "autostash", _autostash,
> +   N_("automatically stash/stash pop before and after rebase")),
> OPT_PASSTHRU_ARGV('s', "strategy", _strategies, N_("strategy"),
> N_("merge strategy to use"),
> 0),
> @@ -801,6 +804,10 @@ static int run_rebase(const unsigned char *curr_head,
> argv_array_pushv(, opt_strategy_opts.argv);
> if (opt_gpg_sign)
> argv_array_push(, opt_gpg_sign);
> +   if (opt_autostash == 1)
> +   argv_array_push(, "--autostash");
> +   else if (opt_autostash == 0)
> +   argv_array_push(, "--no-autostash");

The precise testing for specific values of -1, 0 and 1 throughout the
code makes me uncomfortable. Ordinarily, I would expect a simple

argv_array_push(, opt_autostash ? "--autostash" : "--no-autostash");

Stepping back a bit, the only reason why we introduced opt_autostash =
-1 as a possible value is because we need to detect if
--[no-]autostash is being used with git-merge. I consider that a
kludge, because if git-merge supports --autostash as well (possibly in
the future), then we will not need this -1 value.

So, from that point of view, a -1 value is okay as a workaround, but
kludges, and hence the -1 value, should be gotten rid off as soon as
possible.

>
> argv_array_push(, "--onto");
> argv_array_push(, sha1_to_hex(merge_head));
> @@ -851,12 +858,17 @@ int cmd_pull(int argc, const char **argv, const char 
> *prefix)
> if (is_null_sha1(orig_head) && !is_cache_unborn())
> die(_("Updating an unborn branch with changes added 
> to the index."));
>
> -   if (config_autostash != 1)
> +   if (opt_autostash == -1)
> +   opt_autostash = config_autostash;

So, if config_autostash defaults to zero, we can be certain that
opt_autostash will be either true or false.

> +
> +   if (opt_autostash != 1)

And then we can do if (!opt_autostash) here, instead of making readers
wonder why we are testing for the value "1" specifically.

> die_on_unclean_work_tree(prefix);
>
> if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
> hashclr(rebase_fork_point);
> -   }
> +   } else
> +   if (opt_autostash != -1)
> +

Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Mikael Magnusson
On Thu, Mar 10, 2016 at 2:21 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Thu, 10 Mar 2016, Duy Nguyen wrote:
>
>> On Thu, Mar 10, 2016 at 6:34 PM, Johannes Schindelin
>>  wrote:
>> > One possible improvement would be to add "/xyz/" to the parent
>> > repository's .git/info/exclude, but this developer hesitates to
>> > introduce that feature without the "delete" counterpart: those exclude
>> > entries would likely go stale very quickly. Besides, there might be a
>> > plan in the working to exclude worktrees automagically?
>>
>> That's needed because you add a worktree inside another worktree? I
>> know that feeling, but I've changed my layout from ~/w/git as main
>> worktree (and ~/w/git/.git as repo) to ~/w/git as a non-worktree dir
>> that contains all worktrees, e.g. ~/w/git/reinclude-dir,
>> ~/w/git/worktree-config, ~/w/git/lmdb... My typical worktree add
>> command is "git worktree add ../" then move there and do
>> stuff. No nested worktrees, no need to update exclude file (and no
>> messing up emacs' rgrep command, which does not understand .gitignore
>> anyway)
>
> This feels to me like it is working around the problem rather than solving
> it. My worktrees are inside the corresponding top-level project for a
> reason: I work with multiple projects, and having all of their worktrees
> in a single $HOME/w/ directory would be rather confusing to me.
>
> I really want to keep my Git worktrees inside /usr/src/git/ (in Git for
> Windows' SDK).

You can have /usr/src/git/master, /usr/src/git/some-work-tree, etc,
and /usr/src/git itself is not a git repository at all. That way
/usr/src only has one git-related directory and no worktrees are
nested. The only downside is if you work in master most of the time,
you have to type "/master" more. I think this is what Duy suggested
too, but you interpreted it as having /usr/src/git-master,
/usr/src/git-some-work-tree etc?

-- 
Mikael Magnusson
--
To unsubscribe from this list: send the line "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[2]: Possible bug: --ext-diff ignored with --cc in git log

2016-03-10 Thread Vadim Zeitlin
On Thu, 10 Mar 2016 14:33:55 -0800 Junio C Hamano  wrote:

JCH> Vadim Zeitlin  writes:
JCH> 
JCH> > I.e. the
JCH> > command "git log --ext-diff -p --cc" still outputs the real diff even for
JCH> > the generated files, as if "--ext-diff" were not given. ...
JCH> > Is the current behaviour intentional? I see it with all the git versions 
I
JCH> > tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why
JCH> > would it need to work like this, so I hope it's an oversight and could be
JCH> > corrected.
JCH> 
JCH> I think this is "intentional" in the sense that "--cc" feature is
JCH> fundamentally and conceptually incompatible with "--ext-diff".

 Thank you for your reply, Junio, I hadn't realized that --cc was dependent
on textual diff output format before, but now I understand why it can't
respect --ext-diff.

JCH> I haven't tried it myself, but if the contents you are using
JCH> ext-diff on can be compared in a format that is easy-to-read for
JCH> humans by passing them first to "textconv" filter and then running
JCH> the normal "diff" on, that may be a viable approach to do what you
JCH> are trying to do, as "textconv" feature is meant to still produce
JCH> the output that still follows the usual "diff" convention.  Its
JCH> output should be usable by any tool (e.g. diffstat) meant to
JCH> post-process patch output, and would be a better match for the
JCH> "--cc" mechanism.

 I can't think of a way to make the output as concise as it is now (i.e.
just a single line saying that a generated file has been modified but the
changes to it are not being shown) with this approach.

 Maybe I'm clutching at straws here, but I wonder if it could be possible
to have a file attribute specifying whether --cc or -m should be used for
it when showing merges? Because this is, basically, what I want here: --cc
for normal files for readability but -m for the files I'm not interested
in. It's probably too specific to my particular hack^H^H^H^H use case to
add support for it to Git itself, but I wanted to mention it on a chance
that somebody else might think it's a good idea.

 Anyhow, thanks again for your explanation,
VZ


pgpjJrCnFWw1V.pgp
Description: PGP signature


Re: [PATCH 04/19] index-helper: new daemon for caching index and related stuff

2016-03-10 Thread Duy Nguyen
On Fri, Mar 11, 2016 at 3:22 AM, David Turner  wrote:
> Reads (ignoring SHA verification) will be slightly slower (due to the
> btree overhead).  If, in general, we only had to read part of the
> index, that would be faster. But a fair amount of git code is written
> to assume that it is reasonable to iterate over the whole index.  So I
> don't see a win from just switching to LMDB without other changes.

I didn't think of the btree overhead. Will need to run some test.. I
think lmdb just gives us pointers to mmap'd file, so an entire index
reading can be fast. We just record where the on-disk data is. We need
to make index-on-lmdb use native byte endian though to avoid
conversion and actually reading all index entries because of that.

> (I guess we could parallelize index deserialization more easily with
> lmdb, but I don't know
>
> The original intent of the index was to be a "staging area" to build up
> a tree before committing it.  This implies an alternate view of the
> index as a diff against some (possibly-empty) tree.  An index diff or
> status operation, then, just requires iterating over the changes.
>
> I haven't really dug into merges to understand whether it would be
> reasonable for them to use a representation like this, and what that
> would do to speed there.

I think we keep the same internal representation of the index, a list
of paths with extra info like stat and some flags. Minimal impacts to
the code base that way. There are some cases (I think) where we just
need to read a portion of the index (filtered by pathspec). If we
don't update the index afterwards, lmdb partial reads definitely help.
I need to check if those cases are common (and worth optimizing for)
though.

> In general, git takes the commit side of the commit-diff duality. This
> is generally a good thing, because commits are simpler to reason about.
> But I wonder if we could do better for this specific case.
>
> But I don't think switching to LMDB would replace index-helper.

The point of index-helper is reduce reading cost (let's leave watchman
out for now). But it looks like we're not certain if using lmdb can
reduce reading cost at lower complexity. I'll give it some thought
over the weekend. Maybe in the end we better just go with
index-helper...
-- 
Duy
--
To unsubscribe from this list: send the line "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] t/t7502 : drop duplicate test

2016-03-10 Thread Pranit Bauva
This extra test was introduced erroneously by
f9c0181 (t7502: test commit.status, --status and
--no-status, 2010-01-13)

Signed-off-by: Pranit Bauva 
---
 t/t7502-commit.sh | 5 -
 1 file changed, 5 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index b39e313..725687d 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -527,11 +527,6 @@ try_commit_status_combo () {
test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
'
 
-   test_expect_success 'commit' '
-   try_commit "" &&
-   test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
-   '
-
test_expect_success 'commit --status' '
try_commit --status &&
test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG

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


Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 8:21 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Thu, 10 Mar 2016, Duy Nguyen wrote:
>
>> On Thu, Mar 10, 2016 at 6:34 PM, Johannes Schindelin
>>  wrote:
>> > The invention of the `git worktree` command changed this developer's
>> > working style dramatically. Rather than switching between branches all
>> > the time, topic branches are created and checked out in newly-added
>> > worktrees, to be reworked and refined until the topic branch is either
>> > merged into `master` or abandoned.
>> >
>> > It gets rather tiresome, and also typo-prone, to call "git branch xyz
>> > upstream/master && git worktree add xyz xyz" all the time.
>>
>> You can actually do "git worktree -b xyz xyz upstream/master" for the
>> same effect. Maybe we can avoid "xyz" duplication with "-b -" or a new
>> option name?
>
> My branch names are usually longer, such as pull-rebase-prefix. And I
> really like the short 'n sweet "git branch -w pull-rebase-prefix master"
> invocation.
>
>> > Hence this
>> > proposal: "git branch -w xyz upstream/master" to do the same.

This assumes the new worktree is always placed at `cwd`. Perhaps it's
too specific?

>> > The plan is to also support "git branch -d -w xyz" once the `git
>> > worktree` command learned a `remove` (or `delete`) subcommand.
>>
>> "git worktree remove" is coming (will be resent after -rc period). And
>> I agree it's convenient for it to remove the branch as well because
>> that happened to (and annoyed) me a few times. I still think it should
>> be centered around git-worktree than git-branch though.
>
> Granted, "git worktree remove" should be the work horse. But why not have
> two ways to skin the same cat? I, again, would prefer the short 'n sweet
> "git branch -d -w pull-rebase-prefix" invocation.

If you put effort into making it happen, I'm not stopping you :)

>> > One possible improvement would be to add "/xyz/" to the parent
>> > repository's .git/info/exclude, but this developer hesitates to
>> > introduce that feature without the "delete" counterpart: those exclude
>> > entries would likely go stale very quickly. Besides, there might be a
>> > plan in the working to exclude worktrees automagically?
>>
>> That's needed because you add a worktree inside another worktree? I
>> know that feeling, but I've changed my layout from ~/w/git as main
>> worktree (and ~/w/git/.git as repo) to ~/w/git as a non-worktree dir
>> that contains all worktrees, e.g. ~/w/git/reinclude-dir,
>> ~/w/git/worktree-config, ~/w/git/lmdb... My typical worktree add
>> command is "git worktree add ../" then move there and do
>> stuff. No nested worktrees, no need to update exclude file (and no
>> messing up emacs' rgrep command, which does not understand .gitignore
>> anyway)
>
> This feels to me like it is working around the problem rather than solving
> it. My worktrees are inside the corresponding top-level project for a
> reason: I work with multiple projects, and having all of their worktrees
> in a single $HOME/w/ directory would be rather confusing to me.
>
> I really want to keep my Git worktrees inside /usr/src/git/ (in Git for
> Windows' SDK).

I avoided it because adapting git alone was not enough, emacs' rgrep
is just an example that other tools may have problems with this setup
too. But if you still want to make git exclude them, I think you
should hook into the submodule peeking code instead of updating
exclude file. It sounds neater to me.

But if you go with exclude auto update, it's about time we introduce
multiple exclude files in $GIT_DIR/info. So you can separate auto
stuff from manual exclude rules.
-- 
Duy
--
To unsubscribe from this list: send the line "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?] fetch into shallow sends a large number of objects

2016-03-10 Thread Duy Nguyen
On Fri, Mar 11, 2016 at 4:40 AM, Jeff King  wrote:
> On Thu, Mar 10, 2016 at 01:26:08PM -0800, Junio C Hamano wrote:
>
>> > IMHO, that is the right thing. They asked for "C" as a shallow cut-off
>> > point, so anything that is a parent of "C" should be omitted as shallow,
>> > too. It has nothing to do with the numeric depth, which was just the
>> > starting point for generating the shallow cutoffs.
>>
>> I think that is the right mental model.  The statement that "C and D
>> are current cut points" does not make much sense.  As you cannot
>> rewrite parents of commits after the fact, you cannot construct a
>> case like "when the shallow clone originally was made, two histories
>> were forked long time before B and D, and the cloner ended up with C
>> and D as the cutoff point, but now that we have the ancestry linkage
>> between B and D (and C and E), we need to make E a new cutoff".  The
>> original "shallow" implementation does not store "starting point +
>> number of depth" and instead translates that to the cut-off point
>> for this exact reason.

Well, assume again that F and G are ref heads, and their respective
distance to C and D are the same (like the below graph), then "fetch
--deptch=" can mark C and D as shallow cut points because
--depth traverses from refs until the distance is met, it does not do
total exclusion ^C like rev-list.

   --- B  C  H  F
  /  /
 --- D  E  G


> OK, good. Now there are at least two of us who view it that way. :)

But going that direction just gives me more headache. If you two are
ok with this and nobody else complains, I'm ok too :-D I guess it's a
corner case anyway that's probably hard to occur in practice.

>> > So what next? I think there's some protocol work here, and I think the
>> > overall design of that needs to be considered alongside the other
>> > "deepen" options your topic in pu adds (and of which I'm largely
>> > ignorant). Does this sufficiently interest you to pick up and roll into
>> > your other shallow work?
>>
>> I can pick it up if you are busy with other stuff. But I'm also having
>> a couple other topics at the moment, so it may not progress very fast.
>
> Thanks. I don't think it is too urgent; it has been that way for a
> while. I certainly have plenty of other things to work on, but mostly I
> just feel a bit out of my depth on the shallow stuff. I haven't given it
> any real thought, and you obviously have.

More people understanding this code is always better though. But don't
worry I'll take care of this.

>> > Yeah, we definitely need an extension. I'm not sure if the extension
>> > should be "I know about spontaneous shallow/deepen responses; it's OK to
>> > send them to me" or "I want you to include the shallow points I send as
>> > boundary cutoffs for further shallow-ing of newly fetched history".
>> >
>> > They amount to the same thing when implementing _this_ feature, but the
>> > latter leaves us room in the future for a client to say "sure, I
>> > understand your spontaneous responses, but I explicitly _don't_ want you
>> > to do the boundary computation". I don't know if that is useful or not,
>> > but it might not hurt to have later on (and by adding it now, it "just
>> > works" later on with older servers/clients).
>>
>> I am not sure what distinction you are worried about.  An updated
>> client that is capable of saying "you may give shallow/deepen
>> responses to me" can optionally be told not to say it to the server,
>> and that is equivalent to saying "I don't want you to send them", no?
>
> Mostly, I wondered if we would need to send spontaneous shallow lines
> for any other cases, and the client would not be able to say "I
> understand them and want them in case A, but not in case B".
>
> I do not have any case A in mind; it was just a general sense of "let's
> make feature flags as specific as possible to avoid painting ourselves
> into a corner". I'm OK with implementing it either way.

Another note to myself is, we do not want these spontaneous cutoff
requests to cut client's history even shorter (possibly by accident
because of buggy servers). As long as they are about sealing commits
that lead to non-existing objects, i think it's ok to make it a
general feature.
-- 
Duy
--
To unsubscribe from this list: send the line "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] commit: add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
On Fri, Mar 11, 2016 at 4:31 AM, Eric Sunshine  wrote:

> Add commit.verbose configuration variable as a convenience
> for those who always prefer --verbose.
>
> or something.

Sure!

> As a convenience to reviewers, please use this area below the "---"
> line to provide links and explain what changed since the previous
> round rather than doing so in a separate email.

Actually I am sending the patches with submitGit herokuapp because my
institute proxy does not allow IMAP/POP3 connections.


> The "permanently" bit sounds scary. A more concise way to state this might be:
>
> See the `commit.verbose` configuration variable in
> linkgit:git-config[1].
>
> which doesn't bother spelling out what the intelligent reader should
> infer from the reference.
> Style: space before {

Sure!

>> +test_expect_success 'commit with commit.verbose true and no arguments' '
>
> "no arguments" doesn't convey much; how about "--verbose omitted" or
> something? Ditto for the titles of other tests.

Will change the language construct.
>> +   echo content >file &&
>> +   git add file &&
>> +   test_config commit.verbose true &&
>> +   (
>> +   GIT_EDITOR=cat &&
>> +   export GIT_EDITOR &&
>> +   test_must_fail git commit >output
>> +   ) &&
>> +   test_i18ngrep "diff --git" output
>> +'
>
> Making git-commit fail unconditionally with "aborting due to empty
> commit message" is a rather sneaky way to perform this test. I would
> have expected to see these new tests re-use the existing machinery
> provided by this script (the check-for-diff "editor") rather than
> inventing an entirely new and unintuitive mechanism. Doing so would
> also reduce the size of each new test.

I agree on the fact that making git-commit fail unconditionally is not
a good way to perform the test. "check-for-diff" is not really an
"editor" and it checks for the commit message after it has been
written to the history. The verbose output is stripped when it is
written to the history so we won't be able to test whether this patch
works. This is where purposely breaking the code is required as when
the commit fails, it gives the output of the contents present at that
time (which will contain the verbose output). More over the
'check-for-diff' uses grep which is not preferred. Many tests are now
using test_i18ngrep (eg. f79ce8db). I had planned on using
'check-for-diff' before but it took me some time to figure out this
behavior and thus I began searching for another mechanism (breaking
code).

> Some additional tests[1][2] are probably warranted.
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/288648
> [2]: http://article.gmane.org/gmane.comp.version-control.git/288657

I think these tests also are better included in this file as this
patch triggers it and it would not make much of a difference between
t7507 and t7502 but in fact improve its readability.
>>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t/t7502-commit.sh : remove a repeated test

2016-03-10 Thread Pranit Bauva
On Fri, Mar 11, 2016 at 4:51 AM, Eric Sunshine  wrote:
>> t/t7502-commit.sh : remove a repeated test
>
> Or:
>
> t7502: drop duplicate test
>

Sure!

>> This extra test was introducted in the commit f9c01817
>
> We normally add some parenthetical context when mentioning commits:
>
> This extra test was introduced erroneously by
> f9c0181 (t7502: test commit.status, --status and
> --no-status, 2010-01-13)

Seems like I have to yet get comfortable with the language used here.
I will start reading more commits and stuff.

I will resend the patch with the specified edits.
--
To unsubscribe from this list: send the line "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] t/t7502-commit.sh : remove a repeated test

2016-03-10 Thread Eric Sunshine
On Thu, Mar 10, 2016 at 5:51 PM, Pranit Bauva  wrote:
> t/t7502-commit.sh : remove a repeated test

Or:

t7502: drop duplicate test

> This extra test was introducted in the commit f9c01817

We normally add some parenthetical context when mentioning commits:

This extra test was introduced erroneously by
f9c0181 (t7502: test commit.status, --status and
--no-status, 2010-01-13)

The patch itself makes sense. Thanks.

> Signed-off-by: Pranit Bauva 
> ---
>  t/t7502-commit.sh | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index b39e313..725687d 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -527,11 +527,6 @@ try_commit_status_combo () {
> test_i18ngrep "^# Changes to be committed:" 
> .git/COMMIT_EDITMSG
> '
>
> -   test_expect_success 'commit' '
> -   try_commit "" &&
> -   test_i18ngrep "^# Changes to be committed:" 
> .git/COMMIT_EDITMSG
> -   '
> -
> test_expect_success 'commit --status' '
> try_commit --status &&
> test_i18ngrep "^# Changes to be committed:" 
> .git/COMMIT_EDITMSG
>
> --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[ANNOUNCE] Git v2.8.0-rc2

2016-03-10 Thread Junio C Hamano
A release candidate Git v2.8.0-rc2 is now available for testing
at the usual places.  It is comprised of 459 non-merge commits
since v2.7.0, contributed by 60 people, 19 of which are new faces.

The tarballs are found at:

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

The following public repositories all have a copy of the
'v2.8.0-rc2' tag and the 'master' branch that the tag points at:

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

New contributors whose contributions weren't in v2.7.0 are as follows.
Welcome to the Git development community!

  마누엘, Andrew Wheeler, Changwoo Ryu, Christoph Egger,
  Dan Aloni, Dave Ware, David A. Wheeler, Dickson Wong, Felipe
  Gonçalves Assis, GyuYong Jung, Jon Griffiths, Kazutoshi Satoda,
  Lars Vogel, Martin Amdisen, Matthew Kraai, Paul Wagland, Rob
  Mayoff, Romain Picard, and Victor Leschuk.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Alexander Kuleshov, Alex Henrie, brian m. carlson, Christian
  Couder, David A. Greene, David Turner, Dennis Kaarsemaker,
  Edmundo Carmona Antoranz, Elia Pinto, Eric Wong, Jacob Keller,
  Jeff King, Jiang Xin, Johannes Schindelin, Johannes Sixt,
  John Keeping, Jonathan Nieder, Junio C Hamano, Karsten Blees,
  Karthik Nayak, Knut Franke, Lars Schneider, Matthieu Moy, Matt
  McCutchen, Michael J Gruber, Mike Hommey, Nguyễn Thái Ngọc
  Duy, Øyvind A. Holm, Patrick Steinhardt, Pat Thoyts, Sebastian
  Schuberth, Shawn O. Pearce, Stefan Beller, Stephen P. Smith,
  SZEDER Gábor, Thomas Ackermann, Thomas Braun, Thomas Gummerer,
  Tobias Klauser, Torsten Bögershausen, and Will Palmer.



Git 2.8 Release Notes (draft)
=

Backward compatibility note
---

The rsync:// transport has been removed.


Updates since v2.7
--

UI, Workflows & Features

 * It turns out "git clone" over rsync transport has been broken when
   the source repository has packed references for a long time, and
   nobody noticed nor complained about it.

 * "branch --delete" has "branch -d" but "push --delete" does not.

 * "git blame" learned to produce the progress eye-candy when it takes
   too much time before emitting the first line of the result.

 * "git grep" can now be configured (or told from the command line)
   how many threads to use when searching in the working tree files.

 * Some "git notes" operations, e.g. "git log --notes=", should
   be able to read notes from any tree-ish that is shaped like a notes
   tree, but the notes infrastructure required that the argument must
   be a ref under refs/notes/.  Loosen it to require a valid ref only
   when the operation would update the notes (in which case we must
   have a place to store the updated notes tree, iow, a ref).

 * "git grep" by default does not fall back to its "--no-index"
   behaviour outside a directory under Git's control (otherwise the
   user may by mistake end up running a huge recursive search); with a
   new configuration (set in $HOME/.gitconfig--by definition this
   cannot be set in the config file per project), this safety can be
   disabled.

 * "git pull --rebase" has been extended to allow invoking
   "rebase -i".

 * "git p4" learned to cope with the type of a file getting changed.

 * "git format-patch" learned to notice format.outputDirectory
   configuration variable.  This allows "-o " option to be
   omitted on the command line if you always use the same directory in
   your workflow.

 * "interpret-trailers" has been taught to optionally update a file in
   place, instead of always writing the result to the standard output.

 * Many commands that read files that are expected to contain text
   that is generated (or can be edited) by the end user to control
   their behaviour (e.g. "git grep -f ") have been updated
   to be more tolerant to lines that are terminated with CRLF (they
   used to treat such a line to contain payload that ends with CR,
   which is usually not what the users expect).

 * "git notes merge" used to limit the source of the merged notes tree
   to somewhere under refs/notes/ hierarchy, which was too limiting
   when inventing a workflow to exchange notes with remote
   repositories using remote-tracking notes trees (located in e.g.
   refs/remote-notes/ or somesuch).

 * "git ls-files" learned a new "--eol" option to help diagnose
   end-of-line problems.

 * "ls-remote" learned an option to show which branch the remote
   repository advertises as its primary by pointing its HEAD at.

 * New http.proxyAuthMethod configuration variable can be used to
   specify what authentication method to use, as a way to work around
   proxies that do not give 

[PATCH] t/t7502-commit.sh : remove a repeated test

2016-03-10 Thread Pranit Bauva
This extra test was introducted in the commit f9c01817

Signed-off-by: Pranit Bauva 
---
 t/t7502-commit.sh | 5 -
 1 file changed, 5 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index b39e313..725687d 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -527,11 +527,6 @@ try_commit_status_combo () {
test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
'
 
-   test_expect_success 'commit' '
-   try_commit "" &&
-   test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
-   '
-
test_expect_success 'commit --status' '
try_commit --status &&
test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG

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


[ANNOUNCE] Git v2.7.3

2016-03-10 Thread Junio C Hamano
The latest maintenance release Git v2.7.3 is now available at
the usual places.  This is primarily to sync the maintenance track
with miscellaneous fixes that are scheduled to be part of upcoming
v2.8.0 release.

The tarballs are found at:

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

The following public repositories all have a copy of the 'v2.7.3'
tag and the 'maint' branch that the tag points at:

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



Git v2.7.3 Release Notes


Fixes since v2.7.2
--

 * Traditionally, the tests that try commands that work on the
   contents in the working tree were named with "worktree" in their
   filenames, but with the recent addition of "git worktree"
   subcommand, whose tests are also named similarly, it has become
   harder to tell them apart.  The traditional tests have been renamed
   to use "work-tree" instead in an attempt to differentiate them.

 * Many codepaths forget to check return value from git_config_set();
   the function is made to die() to make sure we do not proceed when
   setting a configuration variable failed.

 * Handling of errors while writing into our internal asynchronous
   process has been made more robust, which reduces flakiness in our
   tests.

 * "git show 'HEAD:Foo[BAR]Baz'" did not interpret the argument as a
   rev, i.e. the object named by the the pathname with wildcard
   characters in a tree object.

 * "git rev-parse --git-common-dir" used in the worktree feature
   misbehaved when run from a subdirectory.

 * The "v(iew)" subcommand of the interactive "git am -i" command was
   broken in 2.6.0 timeframe when the command was rewritten in C.

 * "git merge-tree" used to mishandle "both sides added" conflict with
   its own "create a fake ancestor file that has the common parts of
   what both sides have added and do a 3-way merge" logic; this has
   been updated to use the usual "3-way merge with an empty blob as
   the fake common ancestor file" approach used in the rest of the
   system.

 * The memory ownership rule of fill_textconv() API, which was a bit
   tricky, has been documented a bit better.

 * The documentation did not clearly state that the 'simple' mode is
   now the default for "git push" when push.default configuration is
   not set.

 * Recent versions of GNU grep are pickier when their input contains
   arbitrary binary data, which some of our tests uses.  Rewrite the
   tests to sidestep the problem.

 * A helper function "git submodule" uses since v2.7.0 to list the
   modules that match the pathspec argument given to its subcommands
   (e.g. "submodule add  ") has been fixed.

 * "git config section.var value" to set a value in per-repository
   configuration file failed when it was run outside any repository,
   but didn't say the reason correctly.

 * The code to read the pack data using the offsets stored in the pack
   idx file has been made more carefully check the validity of the
   data in the idx.

Also includes documentation and test updates.



Changes since v2.7.2 are as follows:

Alexander Kuleshov (2):
  exec_cmd.c: use find_last_dir_sep() for code simplification
  git.c: simplify stripping extension of a file in handle_builtin()

David Turner (1):
  refs: document transaction semantics

Jeff King (37):
  checkout: reorder check_filename conditional
  check_filename: tighten dwim-wildcard ambiguity
  get_sha1: don't die() on bogus search strings
  reflog_expire_cfg: NUL-terminate pattern field
  add helpers for detecting size_t overflow
  tree-diff: catch integer overflow in combine_diff_path allocation
  diff: clarify textconv interface
  harden REALLOC_ARRAY and xcalloc against size_t overflow
  add helpers for allocating flex-array structs
  argv-array: add detach function
  convert manual allocations to argv_array
  convert trivial cases to ALLOC_ARRAY
  use xmallocz to avoid size arithmetic
  convert trivial cases to FLEX_ARRAY macros
  use st_add and st_mult for allocation size computation
  prepare_{git,shell}_cmd: use argv_array
  write_untracked_extension: use FLEX_ALLOC helper
  fast-import: simplify allocation in start_packfile
  fetch-pack: simplify add_sought_entry
  test-path-utils: fix normalize_path_copy output buffer size
  sequencer: simplify memory allocation of get_message
  git-compat-util: drop mempcpy compat code
  transport_anonymize_url: use xstrfmt
  diff_populate_gitlink: use a strbuf
  convert ewah/bitmap code to use xmalloc
  ewah: convert to REALLOC_ARRAY, etc
  

Re: [PATCH v4] commit: add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
> + if (!strcmp(k, "commit.verbose")){

> v3 did this line correctly but you somehow lost the SP between
> "){".  What happened?

I will include the SP between )) and { .

> Don't you need a test that status is not broken when the variable is
> set?

I will include the test for status too. But I am a bit confused where
should I place them? This patch triggers the possibility of a breakage
in status related to verbose but other tests related to status are in
different files. Could you tell me the filename where I should place
these test?
--
To unsubscribe from this list: send the line "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] commit: add a commit.verbose config variable

2016-03-10 Thread Eric Sunshine
On Thu, Mar 10, 2016 at 5:12 PM, Pranit Bauva  wrote:
> Since many people always run the command with this option, it would be
> preferrable to specify it in the configuration file instead of passing
> the option with `git commit` again and again.

Perhaps drop the unsubstantiated "many people always" and just say:

Add commit.verbose configuration variable as a convenience
for those who always prefer --verbose.

or something.

> Signed-off-by: Pranit Bauva 
> ---

As a convenience to reviewers, please use this area below the "---"
line to provide links and explain what changed since the previous
round rather than doing so in a separate email.

> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> @@ -290,7 +290,8 @@ configuration variable documented in 
> linkgit:git-config[1].
> what changes the commit has.
> Note that this diff output doesn't have its
> lines prefixed with '#'. This diff will not be a part
> -   of the commit message.
> +   of the commit message. To activate this option permanently, the
> +   configuration variable `commit.verbose` can be set to true.

The "permanently" bit sounds scary. A more concise way to state this might be:

See the `commit.verbose` configuration variable in
linkgit:git-config[1].

which doesn't bother spelling out what the intelligent reader should
infer from the reference.

> diff --git a/builtin/commit.c b/builtin/commit.c
> @@ -1505,6 +1505,10 @@ static int git_commit_config(const char *k, const char 
> *v, void *cb)
> sign_commit = git_config_bool(k, v) ? "" : NULL;
> return 0;
> }
> +   if (!strcmp(k, "commit.verbose")){

Style: space before {

> +   verbose = git_config_bool(k, v);
> +   return 0;
> +   }
>
> status = git_gpg_config(k, v, NULL);
> if (status)
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> @@ -96,4 +96,52 @@ test_expect_success 'verbose diff is stripped out with set 
> core.commentChar' '
> test_i18ngrep "Aborting commit due to empty commit message." err
>  '
>
> +test_expect_success 'commit with commit.verbose true and no arguments' '

"no arguments" doesn't convey much; how about "--verbose omitted" or
something? Ditto for the titles of other tests.

> +   echo content >file &&
> +   git add file &&
> +   test_config commit.verbose true &&
> +   (
> +   GIT_EDITOR=cat &&
> +   export GIT_EDITOR &&
> +   test_must_fail git commit >output
> +   ) &&
> +   test_i18ngrep "diff --git" output
> +'

Making git-commit fail unconditionally with "aborting due to empty
commit message" is a rather sneaky way to perform this test. I would
have expected to see these new tests re-use the existing machinery
provided by this script (the check-for-diff "editor") rather than
inventing an entirely new and unintuitive mechanism. Doing so would
also reduce the size of each new test.

More below...

> +test_expect_success 'commit with commit.verbose true and --no-verbose' '
> +   echo content >file &&
> +   git add file &&
> +   test_config commit.verbose true &&
> +   (
> +   GIT_EDITOR=cat &&
> +   export GIT_EDITOR &&
> +   test_must_fail git commit --no-verbose >output
> +   ) &&
> +   ! test_i18ngrep "diff --git" output
> +'
> +
> +test_expect_success 'commit with commit.verbose false and -v' '
> +   echo content >file &&
> +   git add file &&
> +   test_config commit.verbose false &&
> +   (
> +   GIT_EDITOR=cat &&
> +   export GIT_EDITOR &&
> +   test_must_fail git commit -v >output
> +   ) &&
> +   test_i18ngrep "diff --git" output
> +'
> +
> +test_expect_success 'commit with commit.verbose false no arguments' '
> +   echo content >file &&
> +   git add file &&
> +   test_config commit.verbose false &&
> +   (
> +   GIT_EDITOR=cat &&
> +   export GIT_EDITOR &&
> +   test_must_fail git commit >output
> +   ) &&
> +   ! test_i18ngrep "diff --git" output
> +'

Some additional tests[1][2] are probably warranted.

[1]: http://article.gmane.org/gmane.comp.version-control.git/288648
[2]: http://article.gmane.org/gmane.comp.version-control.git/288657

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


Re: [PATCH v4] commit: add a commit.verbose config variable

2016-03-10 Thread Junio C Hamano
Pranit Bauva  writes:

> + if (!strcmp(k, "commit.verbose")){

v3 did this line correctly but you somehow lost the SP between
"){".  What happened?

> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> index 2ddf28c..4e123a5 100755
> --- a/t/t7507-commit-verbose.sh
> +++ b/t/t7507-commit-verbose.sh
> @@ -96,4 +96,52 @@ test_expect_success 'verbose diff is stripped out with set 
> core.commentChar' '
>   test_i18ngrep "Aborting commit due to empty commit message." err
>  '
>  
> +test_expect_success 'commit with commit.verbose true and no arguments' '
> +test_expect_success 'commit with commit.verbose true and --no-verbose' '
> +test_expect_success 'commit with commit.verbose false and -v' '
> +test_expect_success 'commit with commit.verbose false no arguments' '

Don't you need a test that status is not broken when the variable is
set?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2

2016-03-10 Thread Marcus Kida
Fair enough,

thank you. I’m going to take a look at the previous threads.
I’d also be keen to help working on those issues.

> On 11 Mar 2016, at 9:41 AM, Jeff King  wrote:
> 
> On Fri, Mar 11, 2016 at 05:30:00AM +1100, Marcus Kida wrote:
> 
>> thank you for the feedback.
>> I will fix this, test it and send a patch.
> 
> Unfortunately, I think this issue is a little more complicated.
> 
> There's some prior discussion in
> 
>  http://thread.gmane.org/gmane.comp.version-control.git/284022
> 
> and
> 
>  http://thread.gmane.org/gmane.comp.version-control.git/276456/focus=276506
> 
> The latter, in particular, shows a case where this approach will do the
> wrong thing. The fundamental issue is that refs are potentially stored
> in _two_ places: the filesystem, and the packed-refs file. And the
> latter is always case-sensitive, while the former sometimes is and
> sometimes isn't. But because the storage all happens behind the scenes,
> the user has no way of reliably disambiguating (e.g., does "foo" refer
> to your checked-out "FOO", or are you intentionally trying to delete an
> extraneous "FOO" that ended up in the packed-refs file?).
> 
> -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: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2

2016-03-10 Thread Jeff King
On Fri, Mar 11, 2016 at 05:30:00AM +1100, Marcus Kida wrote:

> thank you for the feedback.
> I will fix this, test it and send a patch.

Unfortunately, I think this issue is a little more complicated.

There's some prior discussion in

  http://thread.gmane.org/gmane.comp.version-control.git/284022

and

  http://thread.gmane.org/gmane.comp.version-control.git/276456/focus=276506

The latter, in particular, shows a case where this approach will do the
wrong thing. The fundamental issue is that refs are potentially stored
in _two_ places: the filesystem, and the packed-refs file. And the
latter is always case-sensitive, while the former sometimes is and
sometimes isn't. But because the storage all happens behind the scenes,
the user has no way of reliably disambiguating (e.g., does "foo" refer
to your checked-out "FOO", or are you intentionally trying to delete an
extraneous "FOO" that ended up in the packed-refs file?).

-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 v4] commit: add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
Older versions of this patch can be found at :-

[v3] : http://thread.gmane.org/gmane.comp.version-control.git/288634

[v2] : http://thread.gmane.org/gmane.comp.version-control.git/288569

[v1] : http://thread.gmane.org/gmane.comp.version-control.git/287540

The changes with respect to last version are :
 - put the code in git_commit_config() instead of git_status_config()
 - Add test to cover all possible scenarios

Regards,
Pranit Bauva
IIT Kharagpur
--
To unsubscribe from this list: send the line "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: --ext-diff ignored with --cc in git log

2016-03-10 Thread Junio C Hamano
Vadim Zeitlin  writes:

> I.e. the
> command "git log --ext-diff -p --cc" still outputs the real diff even for
> the generated files, as if "--ext-diff" were not given. ...
> Is the current behaviour intentional? I see it with all the git versions I
> tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why
> would it need to work like this, so I hope it's an oversight and could be
> corrected.

I think this is "intentional" in the sense that "--cc" feature is
fundamentally and conceptually incompatible with "--ext-diff".

 - The "external diff" feature is to allow third-party tools to
   produce output that is vastly different from the usual "diff"
   output, e.g. unlike the usual "diff", the output may not even be
   line-oriented, and certainly would not have to follow the
   convention of denoting the contents on old and new lines with "-"
   and "+" prefixes.

 - The "--cc" feature is to show multiple "diff" outputs in the
   usual format with post-processing to coalesce them into a more
   concise form, and fundamentally depends on (1) the output being
   line-oriented and (2) the contents of old and new lines denoted
   by "-"/"+" prefixes to be able to do so.

I haven't tried it myself, but if the contents you are using
ext-diff on can be compared in a format that is easy-to-read for
humans by passing them first to "textconv" filter and then running
the normal "diff" on, that may be a viable approach to do what you
are trying to do, as "textconv" feature is meant to still produce
the output that still follows the usual "diff" convention.  Its
output should be usable by any tool (e.g. diffstat) meant to
post-process patch output, and would be a better match for the
"--cc" mechanism.
--
To unsubscribe from this list: send the line "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 v4] commit: add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
Since many people always run the command with this option, it would be
preferrable to specify it in the configuration file instead of passing
the option with `git commit` again and again.

Signed-off-by: Pranit Bauva 
---
 Documentation/config.txt |  4 
 Documentation/git-commit.txt |  3 ++-
 builtin/commit.c |  4 
 t/t7507-commit-verbose.sh| 48 
 4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 01cca0a..9b93f6c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1110,6 +1110,10 @@ commit.template::
"`~/`" is expanded to the value of `$HOME` and "`~user/`" to the
specified user's home directory.
 
+commit.verbose::
+   A boolean to specify whether to always include the verbose option
+   with `git commit`. See linkgit:git-commit[1].
+
 credential.helper::
Specify an external helper to be called when a username or
password credential is needed; the helper may consult external
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9ec6b3c..3dcaac7 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -290,7 +290,8 @@ configuration variable documented in linkgit:git-config[1].
what changes the commit has.
Note that this diff output doesn't have its
lines prefixed with '#'. This diff will not be a part
-   of the commit message.
+   of the commit message. To activate this option permanently, the
+   configuration variable `commit.verbose` can be set to true.
 +
 If specified twice, show in addition the unified diff between
 what would be committed and the worktree files, i.e. the unstaged
diff --git a/builtin/commit.c b/builtin/commit.c
index b3bd2d4..55e9a82 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,6 +1505,10 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
}
+   if (!strcmp(k, "commit.verbose")){
+   verbose = git_config_bool(k, v);
+   return 0;
+   }
 
status = git_gpg_config(k, v, NULL);
if (status)
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 2ddf28c..4e123a5 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -96,4 +96,52 @@ test_expect_success 'verbose diff is stripped out with set 
core.commentChar' '
test_i18ngrep "Aborting commit due to empty commit message." err
 '
 
+test_expect_success 'commit with commit.verbose true and no arguments' '
+   echo content >file &&
+   git add file &&
+   test_config commit.verbose true &&
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git commit >output
+   ) &&
+   test_i18ngrep "diff --git" output
+'
+
+test_expect_success 'commit with commit.verbose true and --no-verbose' '
+   echo content >file &&
+   git add file &&
+   test_config commit.verbose true &&
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git commit --no-verbose >output
+   ) &&
+   ! test_i18ngrep "diff --git" output
+'
+
+test_expect_success 'commit with commit.verbose false and -v' '
+   echo content >file &&
+   git add file &&
+   test_config commit.verbose false &&
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git commit -v >output
+   ) &&
+   test_i18ngrep "diff --git" output
+'
+
+test_expect_success 'commit with commit.verbose false no arguments' '
+   echo content >file &&
+   git add file &&
+   test_config commit.verbose false &&
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git commit >output
+   ) &&
+   ! test_i18ngrep "diff --git" output
+'
+
 test_done

--
https://github.com/git/git/pull/205
--
To unsubscribe from this list: send the line "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 2/2] mergetool: honor tempfile configuration when resolving delete conflicts

2016-03-10 Thread Junio C Hamano
David Aguilar  writes:

> Teach resolve_deleted_merge() to honor the mergetool.keepBackup and
> mergetool.keepTemporaries configuration knobs.
>
> This ensures that the worktree is kept pristine when resolving deletion
> conflicts with the variables both set to false.
>
> Signed-off-by: David Aguilar 
> ---
> Rebased to include tests and test fixes.

Thanks.  Will queue after applying a single fix-up again.

>
>  git-mergetool.sh | 11 ++-
>  t/t7610-mergetool.sh | 25 +
>  2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index b06ae78..f67bab5 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -126,7 +126,12 @@ resolve_deleted_merge () {
>   case "$ans" in
>   [mMcC]*)
>   git add -- "$MERGED"
> - cleanup_temp_files --save-backup
> + if test "$merge_keep_backup" = "true"
> + then
> + cleanup_temp_files --save-backup
> + else
> + cleanup_temp_files
> + fi
>   return 0
>   ;;
>   [dD]*)
> @@ -135,6 +140,10 @@ resolve_deleted_merge () {
>   return 0
>   ;;
>   [aA]*)
> + if test "$merge_keep_temporaries" = "false"
> + then
> + cleanup_temp_files
> + fi
>   return 1
>   ;;
>   esac
> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
> index 39469d9..db723e8 100755
> --- a/t/t7610-mergetool.sh
> +++ b/t/t7610-mergetool.sh
> @@ -279,6 +279,31 @@ test_expect_success 'mergetool produces no errors when 
> keepBackup is used' '
>   : >expect &&
>   echo d | git mergetool a/a/file.txt 2>actual &&
>   test_cmp expect actual &&
> + ! test -d a &&
> + git reset --hard HEAD
> +'
> +
> +test_expect_success 'mergetool honors tempfile config for deleted files' '
> + test_config mergetool.keepTemporaries false &&
> + test_must_fail git merge move-to-b &&
> + echo d | git mergetool a/a/file.txt &&
> + ! test -d a &&
> + git reset --hard HEAD
> +'
> +
> +test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
> + test_config mergetool.keepTemporaries true &&
> + test_must_fail git merge move-to-b &&
> + ! (echo a; echo n) | git mergetool a/a/file.txt &&
> + test -d a/a &&
> + cat  >expect <<-\EOF &&
> + file_BASE_.txt
> + file_LOCAL_.txt
> + file_REMOTE_.txt
> + EOF
> + ls -1 a/a | sed -e "s/[0-9]*//g" >actual &&
> + test_cmp expect actual &&
> + git clean -fdx &&
>   git reset --hard HEAD
>  '
--
To unsubscribe from this list: send the line "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 smudge filter fails

2016-03-10 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Mar 10, 2016 at 09:45:19AM -0500, Stephen Morton wrote:
>
>> I am a bit confused because this is basically the example used in
>> ProGit [1] and it is fundamentally broken. In fact, if I understand
>> correctly, this means that smudge filters cannot be relied upon to
>> provide any 'keyword expansion' type tasks because they will all by
>> nature have to query the file with 'git log'.
>
> Interesting. Perhaps I am missing something (I am far from an expert in
> clean/smudge filters, which I do not generally use myself), but the
> example in ProGit looks kind of bogus to me. I don't think it ever would
> have worked reliably, under any version of git.
>
>> (Note that although in my example I used 'git checkout', with an only
>> slightly more complicated example I can make it fail on 'git pull'
>> which is perhaps a much more realistic use case. That was probably
>> implied in your answer, I just wanted to mention it.)
>
> Yeah, I think the issue is basically the same for several commands which
> update the worktree and the HEAD. Most of them are going to do the
> worktree first.

You can have a pair of branches A and B that have forked long time
ago, and have a path F that has been changed identically since they
forked, perhaps by cherry-picking the same change.  This happens all
the time.

If there were some mechanism that modifies the checked out version
of F with information that depends on the history that leads to A
(e.g. "which commit that is an ancestor of A last modified F?")
when you check out branch A, it will become invalid when you do "git
checkout B", because the path F will not change because they are the
same between the branches.  In short, CVS $Id$-style substitutions
that depend on the history fundamentally does not work, unless you
are willing to always rewrite the whole working tree every time you
switch branches.

The smudge and clean filters are given _only_ the blob contents and
nothing else, not "which commit (or tree) the blob is taken from",
not "which path this blob sits in that tree-ish", not "what branch
am I on" and this is a very much deliberate design decision made in
order to avoid leading people to a misguided attempt to mimick CVS
$Id$-style substitutions.

--
To unsubscribe from this list: send the line "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/RFC v3] add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
I figured it out, It first runs status_init_config(). I might have
thought "status" as status of the options and thus I may have edited
there.

On Fri, Mar 11, 2016 at 3:08 AM, Pranit Bauva  wrote:
> On Fri, Mar 11, 2016 at 3:04 AM, Junio C Hamano 
> wrote:
>> But doesn't this belong to git_commit_config(), not
>> git_STATUS_config()?  Should "commit.verbose" make output from "git
>> status" verbose?
>
> True. It should belong to git_commit_config(). My bad. But
> surprisingly this code works. I have no idea why. I will update the
> Patch and I have also finished writing test so I will include that
> also.
--
To unsubscribe from this list: send the line "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/RFC v3] add a commit.verbose config variable

2016-03-10 Thread Junio C Hamano
Pranit Bauva  writes:

> On Fri, Mar 11, 2016 at 3:04 AM, Junio C Hamano 
> wrote:
>> But doesn't this belong to git_commit_config(), not
>> git_STATUS_config()?  Should "commit.verbose" make output from "git
>> status" verbose?
>
> True. It should belong to git_commit_config(). My bad. But
> surprisingly this code works. I have no idea why. I will update the
> Patch and I have also finished writing test so I will include that
> also.

If that is surprising to you, that indicates that you only tested
"commit" and not "status", I think.  If you choose commit.verbose to
apply only to "commit", but not "status", then your test should
cover both commands, i.e. when the configuration is set to true, the
default verbosity level for "git commit" should be raised, and the
verbosity level for "git status" should not be--both must be
verified in your test script.


--
To unsubscribe from this list: send the line "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: Sample pre-push hook can crash

2016-03-10 Thread Junio C Hamano
Stephen Morton  writes:

> The sample pre-push hook provided with git [1] will crash if the local
> repo is not up to date with the remote as $remote_sha is not present
> in the local repo. I'm not sure if this patch is exactly correct, it's
> just provided as an example.
>
> Given that people are likely crafting their own solutions based on the
> examples, it's probably good to get right.

It's probably good to get right, but I do not think use of @{u} is
making it right, unfortunately.  You may not necessarily have @{u}
configured, and you may not even pushing to the configured remote
branch.

The spirit of the sample hook, I think, is to validate the new
commits you are publishing, so if you cannot even determine which
ones are new and which ones are not, failing the "push" by exiting
with non-zero status is the right behaviour for this sample.

So perhaps something like this may be more appropriate as an
example.

 templates/hooks--pre-push.sample | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
index 6187dbf..7ef6780 100755
--- a/templates/hooks--pre-push.sample
+++ b/templates/hooks--pre-push.sample
@@ -41,7 +41,12 @@ do
fi
 
# Check for WIP commit
-   commit=`git rev-list -n 1 --grep '^WIP' "$range"`
+   commit=`git rev-list -n 1 --grep '^WIP' "$range"` || {
+   # we do not even know about the range...
+   echo >&2 "Non-ff update to $remote_ref"
+   echo >&2 "fetch from there first"
+   exit 1
+   }
if [ -n "$commit" ]
then
echo >&2 "Found WIP commit in $local_ref, not pushing"
--
To unsubscribe from this list: send the line "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?] fetch into shallow sends a large number of objects

2016-03-10 Thread Jeff King
On Thu, Mar 10, 2016 at 01:26:08PM -0800, Junio C Hamano wrote:

> > IMHO, that is the right thing. They asked for "C" as a shallow cut-off
> > point, so anything that is a parent of "C" should be omitted as shallow,
> > too. It has nothing to do with the numeric depth, which was just the
> > starting point for generating the shallow cutoffs.
> 
> I think that is the right mental model.  The statement that "C and D
> are current cut points" does not make much sense.  As you cannot
> rewrite parents of commits after the fact, you cannot construct a
> case like "when the shallow clone originally was made, two histories
> were forked long time before B and D, and the cloner ended up with C
> and D as the cutoff point, but now that we have the ancestry linkage
> between B and D (and C and E), we need to make E a new cutoff".  The
> original "shallow" implementation does not store "starting point +
> number of depth" and instead translates that to the cut-off point
> for this exact reason.

OK, good. Now there are at least two of us who view it that way. :)

> > Yeah, we definitely need an extension. I'm not sure if the extension
> > should be "I know about spontaneous shallow/deepen responses; it's OK to
> > send them to me" or "I want you to include the shallow points I send as
> > boundary cutoffs for further shallow-ing of newly fetched history".
> >
> > They amount to the same thing when implementing _this_ feature, but the
> > latter leaves us room in the future for a client to say "sure, I
> > understand your spontaneous responses, but I explicitly _don't_ want you
> > to do the boundary computation". I don't know if that is useful or not,
> > but it might not hurt to have later on (and by adding it now, it "just
> > works" later on with older servers/clients).
> 
> I am not sure what distinction you are worried about.  An updated
> client that is capable of saying "you may give shallow/deepen
> responses to me" can optionally be told not to say it to the server,
> and that is equivalent to saying "I don't want you to send them", no?

Mostly, I wondered if we would need to send spontaneous shallow lines
for any other cases, and the client would not be able to say "I
understand them and want them in case A, but not in case B".

I do not have any case A in mind; it was just a general sense of "let's
make feature flags as specific as possible to avoid painting ourselves
into a corner". I'm OK with implementing it either way.

-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


Sample pre-push hook can crash

2016-03-10 Thread Stephen Morton
The sample pre-push hook provided with git [1] will crash if the local
repo is not up to date with the remote as $remote_sha is not present
in the local repo. I'm not sure if this patch is exactly correct, it's
just provided as an example.

Given that people are likely crafting their own solutions based on the
examples, it's probably good to get right.

diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
index 6187dbf..99ed984 100755
--- a/templates/hooks--pre-push.sample
+++ b/templates/hooks--pre-push.sample
@@ -36,9 +36,9 @@ do
# New branch, examine all commits
range="$local_sha"
else
# Update to existing branch, examine new commits
-   range="$remote_sha..$local_sha"
+   range="@{u}..$local_sha"
fi

# Check for WIP commit
commit=`git rev-list -n 1 --grep '^WIP' "$range"`


(This is just something I noticed and thought you might be interested
in, but is not important to me. I actually do care about the smudge
filter issue.)

Stephen

[1] https://github.com/git/git/blob/master/templates/hooks--pre-push.sample
--
To unsubscribe from this list: send the line "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: git branch -D can be used to delete branch which is currently checked out - Part 2

2016-03-10 Thread Marcus Kida
Hi Johannes,
Hi Junio,

here you’ll find a patch to hotfix the “delete-a-branch-you’re-on" issue.
As Junio already stated there’s many more places where case (in)sensitivity is 
not handled correctly but this patch would at least prevent you from deleting 
the branch you’re currently working on (which happened to me yesterday and lead 
to quite some additional stress).

If you think this isn’t worth patching now, please discard my patch.
If you think it’s worth patching and implementing a more advanced filesystem 
backend at a later time, please apply it.

If tested this and validate it works @ 2.8.0-rc1. 

Thank you.

---
diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6b..46bde61 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -215,7 +215,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
int flags = 0;
 
strbuf_branchname(, argv[i]);
-   if (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {
+   if (kinds == FILTER_REFS_BRANCHES && !strcasecmp(head, 
bname.buf)) {
error(_("Cannot delete the branch '%s' "
  "which you are currently on."), bname.buf);
ret = 1;
---

Cheers,
Marcus

> On 10 Mar 2016, at 11:15 PM, Johannes Schindelin  
> wrote:
> 
> Hi Marcus,
> 
> On Thu, 10 Mar 2016, Marcus Kida wrote:
> 
>> Proposed solution:
>> 
>> Use `strcasecmp`, `stricmp`, `strcmpi` here: 
>> https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218
>> 
>> Not sure if/which one of this will work on POSIX as well as MS too though.
> 
> This is not quite a solution (it is not a patch). And you *definitely*
> want to make the use of strcasecmp() contingent on ignore_case. You are
> not alone on this world, after all, and other people have case-sensitive
> filesystems. It's totally doable, of course.
> 
> So, do you feel up to the task?
> 
> Ciao,
> Johannes

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


Re: [BUG?] fetch into shallow sends a large number of objects

2016-03-10 Thread Jeff King
On Thu, Mar 10, 2016 at 07:20:20PM +0700, Duy Nguyen wrote:

> > +   else if (shallows.nr > 0) {
> > +   struct rev_info revs;
> > +   struct argv_array av = ARGV_ARRAY_INIT;
> > +   struct commit *c;
> > +   int i;
> > +
> > +   argv_array_push(, "rev-list");
> > +   argv_array_push(, "--boundary");
> 
> Nice. I didn't know about --boundary. But will it work correctly in
> this case?
> 
>--- B  C  F
>   /  /
>  --- D  E  G
> 
> C and D will be current shallow cut points. People "want" F and G.
> "rev-list --boundary F G ^C ^D" would mark E as boundary/shallow too,
> correct? If so the history from G will be one depth short on a normal
> fetch.

IMHO, that is the right thing. They asked for "C" as a shallow cut-off
point, so anything that is a parent of "C" should be omitted as shallow,
too. It has nothing to do with the numeric depth, which was just the
starting point for generating the shallow cutoffs.

That's just my mental model, though. I admit I don't actually use
shallow clones myself, and maybe people would expect something else.

> > _But_, the client is not prepared to handle this. We send "shallow"
> > lines that it is not expecting, since it did not ask for any depth. So I
> > think this logic would have to kick in only when the client tells us to
> > do so.
> 
> Urgh.. not good. Perhaps a new extension to let the server know the
> client can handle spontaneous "deepen" commands and only activate new
> mode when the extension is present?

Yeah, we definitely need an extension. I'm not sure if the extension
should be "I know about spontaneous shallow/deepen responses; it's OK to
send them to me" or "I want you to include the shallow points I send as
boundary cutoffs for further shallow-ing of newly fetched history".

They amount to the same thing when implementing _this_ feature, but the
latter leaves us room in the future for a client to say "sure, I
understand your spontaneous responses, but I explicitly _don't_ want you
to do the boundary computation". I don't know if that is useful or not,
but it might not hurt to have later on (and by adding it now, it "just
works" later on with older servers/clients).

> > So what next? I think there's some protocol work here, and I think the
> > overall design of that needs to be considered alongside the other
> > "deepen" options your topic in pu adds (and of which I'm largely
> > ignorant). Does this sufficiently interest you to pick up and roll into
> > your other shallow work?
> 
> I can pick it up if you are busy with other stuff. But I'm also having
> a couple other topics at the moment, so it may not progress very fast.

Thanks. I don't think it is too urgent; it has been that way for a
while. I certainly have plenty of other things to work on, but mostly I
just feel a bit out of my depth on the shallow stuff. I haven't given it
any real thought, and you obviously have.

-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: git smudge filter fails

2016-03-10 Thread Jeff King
On Thu, Mar 10, 2016 at 09:45:19AM -0500, Stephen Morton wrote:

> I am a bit confused because this is basically the example used in
> ProGit [1] and it is fundamentally broken. In fact, if I understand
> correctly, this means that smudge filters cannot be relied upon to
> provide any 'keyword expansion' type tasks because they will all by
> nature have to query the file with 'git log'.

Interesting. Perhaps I am missing something (I am far from an expert in
clean/smudge filters, which I do not generally use myself), but the
example in ProGit looks kind of bogus to me. I don't think it ever would
have worked reliably, under any version of git.

> (Note that although in my example I used 'git checkout', with an only
> slightly more complicated example I can make it fail on 'git pull'
> which is perhaps a much more realistic use case. That was probably
> implied in your answer, I just wanted to mention it.)

Yeah, I think the issue is basically the same for several commands which
update the worktree and the HEAD. Most of them are going to do the
worktree first.

-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] Disown ssh+git and git+ssh

2016-03-10 Thread Eric Sunshine
On Wed, Mar 9, 2016 at 4:56 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>> It might be helpful to cite some reference to support the claim that
>> they are "silly" since it's not necessarily obvious to readers who did
>> not following the discussion.
>> ...
>>> || starts_with(url, "ssh://")
>>> +   /*
>>> +* These ways to spell the ssh transport remain supported 
>>> for
>>> +* compat but are undocumented and their use is discouraged
>>> +*/
>>> || starts_with(url, "git+ssh://")
>>> || starts_with(url, "ssh+git://")) {
>>
>> A little "comment" bikeshedding: Aside from undesirably interrupting
>> the code flow, these large comment blocks draw far too much attention
>> from the reader than these deprecated spellings of "ssh" deserve, thus
>> making them seem overly important.
>
> I've been waiting for an update for it but got tired of it.
> Instead of discarding the topic, let's amend it like so:

Minor redundancy[1] aside, this looks good to me. Thanks.

[1]: "do not use" is already implied by "deprecated"

> -- >8 --
> From: Carlos Martín Nieto 
> Date: Mon, 15 Feb 2016 15:29:06 +0100
> Subject: [PATCH] Disown ssh+git and git+ssh
>
> Some people argue that these were silly from the beginning (see
> http://thread.gmane.org/gmane.comp.version-control.git/285590/focus=285601
> for example), but we have to support them for compatibility.
>
> That doesn't mean we have to show them in the documentation.  These
> were already left out of the main list, but a reference in the main
> manpage was left, so remove that.
>
> Also add a note to discourage their use if anybody goes looking for them
> in the source code.
>
> Signed-off-by: Carlos Martín Nieto 
> Signed-off-by: Junio C Hamano 
> ---
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index d987ad2..2f90635 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -1122,7 +1122,7 @@ of clones and fetches.
> connection (or proxy, if configured)
>
>   - `ssh`: git over ssh (including `host:path` syntax,
> -   `git+ssh://`, etc).
> +   `ssh://`, etc).
>
>   - `rsync`: git over rsync
>
> diff --git a/connect.c b/connect.c
> index fd7ffe1..3babb81 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -267,9 +267,9 @@ static enum protocol get_protocol(const char *name)
> return PROTO_SSH;
> if (!strcmp(name, "git"))
> return PROTO_GIT;
> -   if (!strcmp(name, "git+ssh"))
> +   if (!strcmp(name, "git+ssh")) /* deprecated - do not use */
> return PROTO_SSH;
> -   if (!strcmp(name, "ssh+git"))
> +   if (!strcmp(name, "ssh+git")) /* deprecated - do not use */
> return PROTO_SSH;
> if (!strcmp(name, "file"))
> return PROTO_FILE;
> diff --git a/transport.c b/transport.c
> index 67f3666..908e08b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1001,8 +1001,9 @@ struct transport *transport_get(struct remote *remote, 
> const char *url)
> || starts_with(url, "file://")
> || starts_with(url, "git://")
> || starts_with(url, "ssh://")
> -   || starts_with(url, "git+ssh://")
> -   || starts_with(url, "ssh+git://")) {
> +   || starts_with(url, "git+ssh://") /* deprecated - do not use 
> */
> +   || starts_with(url, "ssh+git://") /* deprecated - do not use 
> */
> +   ) {
> /*
>  * These are builtin smart transports; "allowed" transports
>  * will be checked individually in git_connect.
> --
> 2.8.0-rc1-142-g215006a
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Eric Sunshine
On Thu, Mar 10, 2016 at 6:51 AM, Duy Nguyen  wrote:
> On Thu, Mar 10, 2016 at 6:34 PM, Johannes Schindelin
>  wrote:
>> The invention of the `git worktree` command changed this developer's
>> working style dramatically. Rather than switching between branches all
>> the time, topic branches are created and checked out in newly-added
>> worktrees, to be reworked and refined until the topic branch is either
>> merged into `master` or abandoned.
>>
>> It gets rather tiresome, and also typo-prone, to call "git branch xyz
>> upstream/master && git worktree add xyz xyz" all the time.
>
> You can actually do "git worktree -b xyz xyz upstream/master" for the
> same effect. Maybe we can avoid "xyz" duplication with "-b -" or a new
> option name?

There's also the even shorter form:

git worktree add pull-rebase-prefix

which creates both a branch and a worktree named "pull-rebase-prefix".
This assumes that you want the new branch based upon HEAD, which won't
work if your use-case is different and that you're not already at the
desired commit ("origin/master" in your example) when invoking the
command, though.
--
To unsubscribe from this list: send the line "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 04/19] index-helper: new daemon for caching index and related stuff

2016-03-10 Thread David Turner
On Thu, 2016-03-10 at 18:17 +0700, Duy Nguyen wrote:
> On Thu, Mar 10, 2016 at 6:21 AM, Junio C Hamano 
> wrote:
> > Junio C Hamano  writes:
> > 
> > > David Turner  writes:
> > > 
> > > > From: Nguyễn Thái Ngọc Duy 
> > > > 
> > > > Instead of reading the index from disk and worrying about disk
> > > > corruption, the index is cached in memory (memory bit-flips
> > > > happen
> > > > too, but hopefully less often). The result is faster read. Read
> > > > time
> > > > is reduced by 70%.
> > > > 
> > > > The biggest gain is not having to verify the trailing SHA-1,
> > > > which
> > > > takes lots of time especially on large index files.
> > 
> > Come to think of it, wouldn't it be far less intrusive change to
> > just put the index on a ramdisk and skip the trailing SHA-1
> > verification, to obtain a similar result with the same trade off
> > (i.e. blindly trusting memory instead of being paranoid)?
> 
> I'm straying off-topic again because this reminded me about lmdb :)
> For the record when I looked at lmdb I thought of using it as an
> index
> format too. It has everything we wanted in index-v5. Good enough that
> we would not need index-helper and split-index to work around current
> index format. Though I finally decided it didn't fit well.
> 
> On the good side, lmdb is b+ trees, we can update directly on disk
> (without rewriting whole file), read directly from mmap'd file and
> not
> worry about corrupting it. There's no SHA-1 verification either (and
> we can do crc32 per entry if we want too). It's good.
> 
> But, it does not fit well the our locking model (but we can change
> this). And we sometimes want to create temporary throw-away indexes
> (e.g. partial commits) which I don't think is easy to do. And the
> reading directly from mmap does not give us much because we have to
> do
> byte endian conversion  anyway.
> 
> Hmm.. on second thought, even though lmdb can't be the default format
> (for a bunch of other limitations), it can still be an option for
> super big worktrees, just like index-helper being an optional
> optimization. Hm.. hm.. if we can support lmdb as index format in
> addition to the current format, bringing back some work from Thomas..
> We may still need a daemon or something to deal with watchman, but
> the
> underlying mechanism is exactly the same... David, Junio, what do you
> think?

LMDB makes writes faster, since we only have to write the stuff that's
changed.  That's good.

Reads (ignoring SHA verification) will be slightly slower (due to the
btree overhead).  If, in general, we only had to read part of the
index, that would be faster. But a fair amount of git code is written
to assume that it is reasonable to iterate over the whole index.  So I
don't see a win from just switching to LMDB without other changes.

(I guess we could parallelize index deserialization more easily with
lmdb, but I don't know 

The original intent of the index was to be a "staging area" to build up
a tree before committing it.  This implies an alternate view of the
index as a diff against some (possibly-empty) tree.  An index diff or
status operation, then, just requires iterating over the changes. 

I haven't really dug into merges to understand whether it would be
reasonable for them to use a representation like this, and what that
would do to speed there.

In general, git takes the commit side of the commit-diff duality. This
is generally a good thing, because commits are simpler to reason about.
But I wonder if we could do better for this specific case.

But I don't think switching to LMDB would replace index-helper.
--
To unsubscribe from this list: send the line "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/RFC v3] add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
Older versions of this patch can be found at :-
[v2] : http://thread.gmane.org/gmane.comp.version-control.git/288569
[v1] : http://thread.gmane.org/gmane.comp.version-control.git/287540

The changes are :
 - Remove the concept of override-verbose
 - Add the git_config_bool to the method git_status_config instead of cmd_commit

I have to yet figure out how to write the tests for this. I will see
the examples that Matthieu Moy mentioned for their implementation.

Regards,
Pranit Bauva,
IIT Kharagpur
--
To unsubscribe from this list: send the line "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/RFC v3] add a commit.verbose config variable

2016-03-10 Thread Pranit Bauva
Since many people always run the command with this option, it would be
preferrable to specify it in the configuration file instead of passing
the option with `git commit` again and again.

Signed-off-by: Pranit Bauva 
---
 Documentation/config.txt | 4 
 Documentation/git-commit.txt | 3 ++-
 builtin/commit.c | 4 
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 01cca0a..9b93f6c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1110,6 +1110,10 @@ commit.template::
"`~/`" is expanded to the value of `$HOME` and "`~user/`" to the
specified user's home directory.
 
+commit.verbose::
+   A boolean to specify whether to always include the verbose option
+   with `git commit`. See linkgit:git-commit[1].
+
 credential.helper::
Specify an external helper to be called when a username or
password credential is needed; the helper may consult external
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9ec6b3c..3dcaac7 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -290,7 +290,8 @@ configuration variable documented in linkgit:git-config[1].
what changes the commit has.
Note that this diff output doesn't have its
lines prefixed with '#'. This diff will not be a part
-   of the commit message.
+   of the commit message. To activate this option permanently, the
+   configuration variable `commit.verbose` can be set to true.
 +
 If specified twice, show in addition the unified diff between
 what would be committed and the worktree files, i.e. the unstaged
diff --git a/builtin/commit.c b/builtin/commit.c
index b3bd2d4..63ee0f2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1310,6 +1310,10 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
return error(_("Invalid untracked files mode '%s'"), v);
return 0;
}
+   if (!strcmp(k, "commit.verbose")) {
+   verbose = git_config_bool(k, v);
+   return 0;
+   }
return git_diff_ui_config(k, v, NULL);
 }
 

--
https://github.com/git/git/pull/205
--
To unsubscribe from this list: send the line "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: git branch -D can be used to delete branch which is currently checked out

2016-03-10 Thread Marcus Kida
Thank you,
I get your point.

Well this proposed solution will exceed my current knowledge of the git code at 
this point. (Which is basically null because I've never built it before)

> On 11 Mar 2016, at 5:23 AM, Junio C Hamano  wrote:
> 
> Junio C Hamano  writes:
> 
>> It is a possibility to teach the files backend of refs API that some
>> filesystems are case insensitive and do something special about them,
>> but I think in the longer term a more productive solution would be
>> to use the upcoming "pluggable ref backend" subsystem and either
>> 
>> - use a backend that is not the "files" backend (e.g. lmdb backend,
>>   or the tree-object based backend);
>> 
>> - add a variant of "files" backend but encodes the refnames in a
>>   way that is safe on case insensitive filesystems.
> 
> A typofix s/but encodes/that encodes/ is needed to make this
> sentence make any sense.  Sorry for a typo.
> 
> Just to elaborate a bit more, here is what I mean:
> 
> - Thanks to recent work by David, Ronnie and Michael, we eradicated
>   most if not all code that assume that the result of checking
>   "test -f .git/refs/heads/foo" tells us if a branch 'foo' exists
>   [*1*].  They all go through the API designed to allow different
>   backends to access refs.
> 
> - The traditional code that implemented 'foo' branch as writing a
>   file '.git/refs/heads/foo' has been moved to a "files" backend.
>   When used on a platform with case insensitive filesystem, it can
>   answer "it exists" when asked about a branch 'Foo' (notice the
>   case difference).
> 
> - We could add a new backend that is still based largely on the
>   existing "files" backend code, but stores 'foo' branch as a file
>   ".git/refs/6865616473/666f6f" while storing another branch 'Foo'
>   as ".git/refs/6865616473/466f6f" (I just used byte values in hex
>   in this example, but of course you can use a more efficient and
>   mostly human readable representations).
> 
> That way, even on a platform with case insensitive filesystem, you
> do not have to worry about getting confused by the filesystem when
> you have 'foo' and 'Foo' branches.
> 
> 
> [Footnote]
> 
> *1* This is not strictly true even in the pre-pluggable ref backend
>world, as your refs may appear in the packed-refs file, but this
>detail does not matter in the larger picture.
> 
> 
--
To unsubscribe from this list: send the line "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: git branch -D can be used to delete branch which is currently checked out - Part 2

2016-03-10 Thread Marcus Kida
Hi Johannes,

thank you for the feedback.
I will fix this, test it and send a patch.

Cheers,
Marcus

> On 10 Mar 2016, at 11:15 PM, Johannes Schindelin  
> wrote:
> 
> Hi Marcus,
> 
>> On Thu, 10 Mar 2016, Marcus Kida wrote:
>> 
>> Proposed solution:
>> 
>> Use `strcasecmp`, `stricmp`, `strcmpi` here: 
>> https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218
>> 
>> Not sure if/which one of this will work on POSIX as well as MS too though.
> 
> This is not quite a solution (it is not a patch). And you *definitely*
> want to make the use of strcasecmp() contingent on ignore_case. You are
> not alone on this world, after all, and other people have case-sensitive
> filesystems. It's totally doable, of course.
> 
> So, do you feel up to the task?
> 
> Ciao,
> Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug: git branch -D can be used to delete branch which is currently checked out

2016-03-10 Thread Junio C Hamano
Junio C Hamano  writes:

> It is a possibility to teach the files backend of refs API that some
> filesystems are case insensitive and do something special about them,
> but I think in the longer term a more productive solution would be
> to use the upcoming "pluggable ref backend" subsystem and either
>
>  - use a backend that is not the "files" backend (e.g. lmdb backend,
>or the tree-object based backend);
>
>  - add a variant of "files" backend but encodes the refnames in a
>way that is safe on case insensitive filesystems.

A typofix s/but encodes/that encodes/ is needed to make this
sentence make any sense.  Sorry for a typo.

Just to elaborate a bit more, here is what I mean:

 - Thanks to recent work by David, Ronnie and Michael, we eradicated
   most if not all code that assume that the result of checking
   "test -f .git/refs/heads/foo" tells us if a branch 'foo' exists
   [*1*].  They all go through the API designed to allow different
   backends to access refs.

 - The traditional code that implemented 'foo' branch as writing a
   file '.git/refs/heads/foo' has been moved to a "files" backend.
   When used on a platform with case insensitive filesystem, it can
   answer "it exists" when asked about a branch 'Foo' (notice the
   case difference).

 - We could add a new backend that is still based largely on the
   existing "files" backend code, but stores 'foo' branch as a file
   ".git/refs/6865616473/666f6f" while storing another branch 'Foo'
   as ".git/refs/6865616473/466f6f" (I just used byte values in hex
   in this example, but of course you can use a more efficient and
   mostly human readable representations).

That way, even on a platform with case insensitive filesystem, you
do not have to worry about getting confused by the filesystem when
you have 'foo' and 'Foo' branches.


[Footnote]

*1* This is not strictly true even in the pre-pluggable ref backend
world, as your refs may appear in the packed-refs file, but this
detail does not matter in the larger picture.


--
To unsubscribe from this list: send the line "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] pull: drop confusing prefix parameter of die_on_unclean_work_tree()

2016-03-10 Thread Junio C Hamano
Junio C Hamano  writes:

> I think this is quite subjective, as I tend to take the presence of
> "prefix" to mean "the callee assumes that the caller has gone up to
> the root level already", and take the absense of use of "prefix" in
> the callee to mean "the callee is working on the whole tree", and
> discarding the parameter is robbing that clue from that point of
> view.
>
> So I am mildly opposed to most parts of this change, including not
> spelling out (void) as the list of parameters for a function that
> does not take any.
>
> I do think not passing "prefix" to init_revisions() would be the
> right thing.  In fact, that prefix is copied to rev, but the current
> end result is correct _only_ because the pathspec limit given by
> that "prefix" parameter to init_revisions() is not automatically
> copied to rev_info.diffopt, and the code is very misleading.

Another reason why it is more sensible to keep the prefix available,
but not use it to limit the extent of diff, to has_*_changes()
functions is that it would be easier for us to change our mind later
to allow the users to ask for more detailed output.  Instead of
"Cannot pull with rebase: You have unstaged changes, period.", you
may be asked to list which paths are dirty in such a case, and in
order to present the list relative to the directory where the user
started the command, you would need "prefix" available to the code
that calls into diff machinery somehow.
--
To unsubscribe from this list: send the line "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] pull: drop confusing prefix parameter of die_on_unclean_work_tree()

2016-03-10 Thread Junio C Hamano
Johannes Schindelin  writes:

> In cmd_pull(), when verifying that there are no changes preventing a
> rebasing pull, we diligently pass the prefix parameter to the
> die_on_unclean_work_tree() function which in turn diligently passes it
> to the has_unstaged_changes() and has_uncommitted_changes() functions.
>
> The casual reader might now be curious (as this developer was) whether
> that means that calling `git pull --rebase` in a subdirectory will
> ignore unstaged changes in other parts of the working directory. And be
> puzzled that `git pull --rebase` (correctly) complains about those
> changes outside of the current directory.

I think this is quite subjective, as I tend to take the presence of
"prefix" to mean "the callee assumes that the caller has gone up to
the root level already", and take the absense of use of "prefix" in
the callee to mean "the callee is working on the whole tree", and
discarding the parameter is robbing that clue from that point of
view.

So I am mildly opposed to most parts of this change, including not
spelling out (void) as the list of parameters for a function that
does not take any.

I do think not passing "prefix" to init_revisions() would be the
right thing.  In fact, that prefix is copied to rev, but the current
end result is correct _only_ because the pathspec limit given by
that "prefix" parameter to init_revisions() is not automatically
copied to rev_info.diffopt, and the code is very misleading.
--
To unsubscribe from this list: send the line "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: git branch -D can be used to delete branch which is currently checked out

2016-03-10 Thread Junio C Hamano
Marcus Kida  writes:

> Testes on: 
>
> Mac OS X 10.11.3 (El Capitan) using Git 2.6.4
>
> Issue:
>
> git branch -D can be used to delete branch which is currently checked out

There are other limitations a filesystem that is incapable of
differentiating two files with names that are only different in case
imposes on your use of Git, e.g.

 - "git fetch origin", when the origin repository has two tags 'v1'
   and 'V1', may not let you have both of these tags locally;

 - "git checkout Another", when the branch you have is 'another',
   may check it out instead of complaining (replace "checkout" with
   any other command that let you use a refname to specify an object).

It is a possibility to teach the files backend of refs API that some
filesystems are case insensitive and do something special about them,
but I think in the longer term a more productive solution would be
to use the upcoming "pluggable ref backend" subsystem and either

 - use a backend that is not the "files" backend (e.g. lmdb backend,
   or the tree-object based backend);

 - add a variant of "files" backend but encodes the refnames in a
   way that is safe on case insensitive filesystems.

That way, those on platforms with case insensitive filesystems do
not have to be artificially limited.  Git is about working together
with people potentially on other systems, and "on this system you
cannot have 'master' and 'Master' at the same time, because we have
a patch to case-fold the refnames" would not be a good longer term
solution.
--
To unsubscribe from this list: send the line "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] rebase -p: avoid grep on potentailly non-ASCII data

2016-03-10 Thread Junio C Hamano
Torsten Bögershausen  writes:

> On 09.03.16 21:26, Junio C Hamano wrote:
>> Anders Kaseorg  writes:
> []
>>  sane_grep () {
>> -GREP_OPTIONS= LC_ALL=C grep "$@"
>> +GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
>>  }
>>  
>>  sane_egrep () {
>> -GREP_OPTIONS= LC_ALL=C egrep "$@"
>> +GREP_OPTIONS= LC_ALL=C egrep @@SANE_TEXT_GREP@@ "$@"
>>  }
>>
>
> I always wondered why we do LC_ALL=C.

It is because, at least back when we started scripted Porcelains of
Git, that is the only thing we could count on available everywhere,
and more importantly, that is how you disable all the i18n gotchas
and ask tools to use the traditional "a file stores a stream of
bytes" semantics.

> Isn't that begging for trouble, when we feed UTF-8, ISO-8895-1
> or other stuff into a program and say LC_ALL=C at the same time ?

Yes and no.  It is true that in "C" ("POSIX"), behaviour on anything
outside the portable and control character sets is undefined, so in
theory, it is bad that we relied on that undefined behaviour to be
to treat the input as just a stream of bytes.  But at the same time,
it is no worse than letting the user's locale take effect or use
hardcoded en_US.UTF-8 and passing unknown end user data that could
be in latin1 and other stuff.  And sensible implementors of "C"
locale seemed to choose the "stream of bytes" back when fa9d3485
(git-am: force egrep to use correct characters set, 2009-09-25) was
written, which is where LC_ALL=C comes from.  I wasn't around when
that patch was accepted, so I cannot quite tell if it was done to
fix a reported bug (i.e. it would reintroduce the bug if we lost
LC_ALL=C) or it was done "just because".

Back when e1622bfc (Protect scripted Porcelains from GREP_OPTIONS
insanity, 2009-11-23) introduced sane_grep, the only caller of it
that wanted to use LC_ALL=C was this callsite in "git am", and we no
longer have that callsite since 783d7e86 (builtin-am: remove
redirection to git-am.sh, 2015-08-04), so I think whatever fa9d3485
wanted to do will not be broken if we removed LC_ALL=C from
sane_grep.  It however is possible that any callsites of sane_grep
added after e1622bfc implicitly depended on having LC_ALL=C in the
implementation.

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


Git subtree stumbles over annotated tags

2016-03-10 Thread Gregor Jasny
Hello,

today I discovered that it's a bad idea to "git subtree pull" from an
annotated tag. This issue got discussed in those two threads:

http://comments.gmane.org/gmane.comp.version-control.git/247503
http://comments.gmane.org/gmane.comp.version-control.git/248395

I was under the impression that it is fixed in recent versions of git
but my homebrew 2.7.0 still behaves badly. If I run the attached script
to reproduce the issue I get the following error message:

> git push using:  sub somebranch
> fatal: 6d621d73ca18dc90424de0929357b5ae62988e00 is not a valid 'commit' object
> Can't copy commit ab38e3fe1ff27f7f87505db37d35adc5c3ceed27

> git ls-remote sub
> 99be40f8e3a4c926d45507be53ab6918789b3a52  HEAD
> 99be40f8e3a4c926d45507be53ab6918789b3a52  refs/heads/master
> 6d621d73ca18dc90424de0929357b5ae62988e00  refs/tags/sometag
> 99be40f8e3a4c926d45507be53ab6918789b3a52  refs/tags/sometag^{}

Besides handling this bug could you please give me a hint how to repair
my main repository? The problematic subtree pull happened some time ago
so I cannot alter the pull itself. I could go the brutal approach and
remove, then re-add the subtree but I guess there is a smarter approach.
Any help is appreciated!

Thanks,
Gregor


reproduce.sh
Description: Bourne shell script


Re: git smudge filter fails

2016-03-10 Thread Stephen Morton
I am a bit confused because this is basically the example used in
ProGit [1] and it is fundamentally broken. In fact, if I understand
correctly, this means that smudge filters cannot be relied upon to
provide any 'keyword expansion' type tasks because they will all by
nature have to query the file with 'git log'.


(Note that although in my example I used 'git checkout', with an only
slightly more complicated example I can make it fail on 'git pull'
which is perhaps a much more realistic use case. That was probably
implied in your answer, I just wanted to mention it.)

Steve

[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes



On Wed, Mar 9, 2016 at 8:59 PM, Jeff King  wrote:
> On Wed, Mar 09, 2016 at 01:29:31PM -0500, Stephen Morton wrote:
>
>> git config --local filter.dater.smudge 'myDate=`git log
>> --pretty=format:"%cd" --date=iso -1 -- %f`; sed -e
>> "s/\(\\$\)Date[^\\$]*\\$/\1Date: $myDate \\$/g"'
>
> Your filter is running "git log" without a revision parameter, which
> means it is looking at HEAD.
>
> And here
>
>> git checkout no_foo
>> git checkout master
>> cat foo.c
>> #observe keyword expansion lost
>
> You are expecting this second one to do:
>
>   1. Switch HEAD to "master".
>
>   2. Checkout files which need updating. Looking at HEAD in your filter
>  then examines "master", and you see the commit timestamp of the
>  destination.
>
> But that isn't how it is implemented. Checkout will handle the file
> checkout _first_, as that is the part that is likely to run into
> problems (e.g., rejecting a switch because it would lose changes in the
> working tree). Only at the very end, after the change to the working
> tree has succeeded, do we update HEAD.
>
> I think the order you are expecting is conceptually cleaner, but I don't
> think we would want to switch it around (for reasons of efficiency and
> robustness). And I don't think we would want to make a promise about the
> ordering to callers either way, as it binds our implementation.
>
> So is there a way to reliably know the destination of a checkout? My
> first thought was that we could add a placeholder similar to "%f" that
> your filter could use. I'm not sure how we would handle the corner cases
> there, though (e.g., do we always have a "destination" to report? If
> not, what do we give the script?).
>
> I suspect you could also hack something together with a post-checkout
> script, though it would probably be a lot less efficient (and might also
> have some weird corner cases).
>
> -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: Git Smart HTTP using Nginx

2016-03-10 Thread Dennis Kaarsemaker
On do, 2016-03-10 at 10:13 -0300, Ben Mezger wrote:

> The git-scm.com only uses apache2 as an example of setting Git's
> Smart
> HTTP, and searching the web for the Nginx's config only gives me old
> configs or not-functional configurations. Has anyone managed to get
> Smart HTTP to work with Nginx and could give me a sample of the
> .conf?

This works for me, using fcgiwrap:

location ~ ^.*/(HEAD|info/refs|objects/info/.*|git-(upload|receive)-pack)$ {
fastcgi_pass unix:/var/run/fcgiwrap.socket;
fastcgi_param SCRIPT_FILENAME   /usr/lib/git-core/git-http-backend;
fastcgi_param PATH_INFO $uri;
fastcgi_param GIT_PROJECT_ROOT  $repo_root;
fastcgi_param REMOTE_USER $remote_user;
include fastcgi_params;
}

-- 
Dennis Kaarsemaker
http://www.kaarsemaker.net


--
To unsubscribe from this list: send the line "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] pull: drop confusing prefix parameter of die_on_unclean_work_tree()

2016-03-10 Thread Johannes Schindelin
In cmd_pull(), when verifying that there are no changes preventing a
rebasing pull, we diligently pass the prefix parameter to the
die_on_unclean_work_tree() function which in turn diligently passes it
to the has_unstaged_changes() and has_uncommitted_changes() functions.

The casual reader might now be curious (as this developer was) whether
that means that calling `git pull --rebase` in a subdirectory will
ignore unstaged changes in other parts of the working directory. And be
puzzled that `git pull --rebase` (correctly) complains about those
changes outside of the current directory.

The puzzle is easily resolved: while we take pains to pass around the
prefix and even pass it to init_revisions(), the fact that no paths are
passed to init_revisions() ensures that the prefix is simply ignored.

That, combined with the fact that we will *always* want a *full* working
directory check before running a rebasing pull, is reason enough to
simply do away with the actual prefix parameter and to pass NULL
instead, as if we were running this from the top-level working directory
anyway.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..4ed46b3 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -308,12 +308,12 @@ static enum rebase_type config_get_rebase(void)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(const char *prefix)
+static int has_unstaged_changes()
 {
struct rev_info rev_info;
int result;
 
-   init_revisions(_info, prefix);
+   init_revisions(_info, NULL);
DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
diff_setup_done(_info.diffopt);
@@ -324,7 +324,7 @@ static int has_unstaged_changes(const char *prefix)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(const char *prefix)
+static int has_uncommitted_changes()
 {
struct rev_info rev_info;
int result;
@@ -332,7 +332,7 @@ static int has_uncommitted_changes(const char *prefix)
if (is_cache_unborn())
return 0;
 
-   init_revisions(_info, prefix);
+   init_revisions(_info, NULL);
DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
add_head_to_pending(_info);
@@ -345,7 +345,7 @@ static int has_uncommitted_changes(const char *prefix)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(const char *prefix)
+static void die_on_unclean_work_tree()
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int do_die = 0;
@@ -355,12 +355,12 @@ static void die_on_unclean_work_tree(const char *prefix)
update_index_if_able(_index, lock_file);
rollback_lock_file(lock_file);
 
-   if (has_unstaged_changes(prefix)) {
+   if (has_unstaged_changes()) {
error(_("Cannot pull with rebase: You have unstaged changes."));
do_die = 1;
}
 
-   if (has_uncommitted_changes(prefix)) {
+   if (has_uncommitted_changes()) {
if (do_die)
error(_("Additionally, your index contains uncommitted 
changes."));
else
@@ -842,7 +842,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
git_config_get_bool("rebase.autostash", );
if (!autostash)
-   die_on_unclean_work_tree(prefix);
+   die_on_unclean_work_tree();
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-- 
2.7.2.windows.1.8.g47d64e6.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Johannes Schindelin
Hi Duy,

On Thu, 10 Mar 2016, Duy Nguyen wrote:

> On Thu, Mar 10, 2016 at 6:51 PM, Duy Nguyen  wrote:
> >> It gets rather tiresome, and also typo-prone, to call "git branch xyz
> >> upstream/master && git worktree add xyz xyz" all the time.
> >
> > You can actually do "git worktree -b xyz xyz upstream/master" for the
> > same effect. Maybe we can avoid "xyz" duplication with "-b -" or a new
> > option name?
> 
> Another option is just do "worktree -b xyz . upstream/master"
> 
> when the destination already exists and is a directory, the real
> worktree location is /. Similar to "mv abc def"
> becomes "mv abc def/abc" when def is already a directory.

Hmm. That sounds too clever to me. It is clever, alright, but it is also
confusing: "Wait, what? Where is my... Darn! It already existed! And now I
have my-cool-worktree/my-cool-worktree/! Blergh."

Please count that as mild opposition to that idea :-)

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 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Johannes Schindelin
Hi Duy,

On Thu, 10 Mar 2016, Duy Nguyen wrote:

> On Thu, Mar 10, 2016 at 6:34 PM, Johannes Schindelin
>  wrote:
> > The invention of the `git worktree` command changed this developer's
> > working style dramatically. Rather than switching between branches all
> > the time, topic branches are created and checked out in newly-added
> > worktrees, to be reworked and refined until the topic branch is either
> > merged into `master` or abandoned.
> >
> > It gets rather tiresome, and also typo-prone, to call "git branch xyz
> > upstream/master && git worktree add xyz xyz" all the time.
> 
> You can actually do "git worktree -b xyz xyz upstream/master" for the
> same effect. Maybe we can avoid "xyz" duplication with "-b -" or a new
> option name?

My branch names are usually longer, such as pull-rebase-prefix. And I
really like the short 'n sweet "git branch -w pull-rebase-prefix master"
invocation.

> > Hence this
> > proposal: "git branch -w xyz upstream/master" to do the same.
> >
> > The plan is to also support "git branch -d -w xyz" once the `git
> > worktree` command learned a `remove` (or `delete`) subcommand.
> 
> "git worktree remove" is coming (will be resent after -rc period). And
> I agree it's convenient for it to remove the branch as well because
> that happened to (and annoyed) me a few times. I still think it should
> be centered around git-worktree than git-branch though.

Granted, "git worktree remove" should be the work horse. But why not have
two ways to skin the same cat? I, again, would prefer the short 'n sweet
"git branch -d -w pull-rebase-prefix" invocation.

> > One possible improvement would be to add "/xyz/" to the parent
> > repository's .git/info/exclude, but this developer hesitates to
> > introduce that feature without the "delete" counterpart: those exclude
> > entries would likely go stale very quickly. Besides, there might be a
> > plan in the working to exclude worktrees automagically?
> 
> That's needed because you add a worktree inside another worktree? I
> know that feeling, but I've changed my layout from ~/w/git as main
> worktree (and ~/w/git/.git as repo) to ~/w/git as a non-worktree dir
> that contains all worktrees, e.g. ~/w/git/reinclude-dir,
> ~/w/git/worktree-config, ~/w/git/lmdb... My typical worktree add
> command is "git worktree add ../" then move there and do
> stuff. No nested worktrees, no need to update exclude file (and no
> messing up emacs' rgrep command, which does not understand .gitignore
> anyway)

This feels to me like it is working around the problem rather than solving
it. My worktrees are inside the corresponding top-level project for a
reason: I work with multiple projects, and having all of their worktrees
in a single $HOME/w/ directory would be rather confusing to me.

I really want to keep my Git worktrees inside /usr/src/git/ (in Git for
Windows' SDK).

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


Git Smart HTTP using Nginx

2016-03-10 Thread Ben Mezger
Hi all,

The git-scm.com only uses apache2 as an example of setting Git's Smart
HTTP, and searching the web for the Nginx's config only gives me old
configs or not-functional configurations. Has anyone managed to get
Smart HTTP to work with Nginx and could give me a sample of the .conf?

Regards,

Ben Mezger



signature.asc
Description: OpenPGP digital signature


Re: [BUG?] fetch into shallow sends a large number of objects

2016-03-10 Thread Duy Nguyen
On Tue, Mar 08, 2016 at 08:25:24AM -0500, Jeff King wrote:
> I think this patch does roughly the right thing:
> 
> diff --git a/upload-pack.c b/upload-pack.c
> index 4859535..da76f70 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -833,12 +833,41 @@ static void receive_needs(void)
>   deepen_by_rev_list(av.argc, av.argv, );
>   argv_array_clear();
>   }
> - else
> - if (shallows.nr > 0) {
> - int i;
> - for (i = 0; i < shallows.nr; i++)
> - 
> register_shallow(shallows.objects[i].item->oid.hash);
> + else if (shallows.nr > 0) {
> + struct rev_info revs;
> + struct argv_array av = ARGV_ARRAY_INIT;
> + struct commit *c;
> + int i;
> +
> + argv_array_push(, "rev-list");
> + argv_array_push(, "--boundary");

Nice. I didn't know about --boundary. But will it work correctly in
this case?

   --- B  C  F
  /  /
 --- D  E  G

C and D will be current shallow cut points. People "want" F and G.
"rev-list --boundary F G ^C ^D" would mark E as boundary/shallow too,
correct? If so the history from G will be one depth short on a normal
fetch.

> + for (i = 0; i < want_obj.nr; i++) {
> + struct object *o = want_obj.objects[i].item;
> + argv_array_push(, oid_to_hex(>oid));
>   }
> + for (i = 0; i < shallows.nr; i++) {
> + struct object *o = shallows.objects[i].item;
> + argv_array_pushf(, "^%s", oid_to_hex(>oid));
> + }
> +
> + init_revisions(, NULL);
> + setup_revisions(av.argc, av.argv, , NULL);
> + if (prepare_revision_walk())
> + die("revision walk setup failed");
> +
> + while ((c = get_revision())) {
> + if (!(c->object.flags & BOUNDARY))
> + continue;
> + register_shallow(c->object.oid.hash);
> + packet_write(1, "shallow %s",
> +  oid_to_hex(>object.oid));
> + }

>  ...
> _But_, the client is not prepared to handle this. We send "shallow"
> lines that it is not expecting, since it did not ask for any depth. So I
> think this logic would have to kick in only when the client tells us to
> do so.

Urgh.. not good. Perhaps a new extension to let the server know the
client can handle spontaneous "deepen" commands and only activate new
mode when the extension is present?

> So what next? I think there's some protocol work here, and I think the
> overall design of that needs to be considered alongside the other
> "deepen" options your topic in pu adds (and of which I'm largely
> ignorant). Does this sufficiently interest you to pick up and roll into
> your other shallow work?

I can pick it up if you are busy with other stuff. But I'm also having
a couple other topics at the moment, so it may not progress very fast.
--
Duy
--
To unsubscribe from this list: send the line "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: git branch -D can be used to delete branch which is currently checked out - Part 2

2016-03-10 Thread Johannes Schindelin
Hi Marcus,

On Thu, 10 Mar 2016, Marcus Kida wrote:

> Proposed solution:
> 
> Use `strcasecmp`, `stricmp`, `strcmpi` here: 
> https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218
> 
> Not sure if/which one of this will work on POSIX as well as MS too though.

This is not quite a solution (it is not a patch). And you *definitely*
want to make the use of strcasecmp() contingent on ignore_case. You are
not alone on this world, after all, and other people have case-sensitive
filesystems. It's totally doable, of course.

So, do you feel up to the task?

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


Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 6:51 PM, Duy Nguyen  wrote:
>> It gets rather tiresome, and also typo-prone, to call "git branch xyz
>> upstream/master && git worktree add xyz xyz" all the time.
>
> You can actually do "git worktree -b xyz xyz upstream/master" for the
> same effect. Maybe we can avoid "xyz" duplication with "-b -" or a new
> option name?

Another option is just do "worktree -b xyz . upstream/master"

when the destination already exists and is a directory, the real
worktree location is /. Similar to "mv abc def"
becomes "mv abc def/abc" when def is already a directory.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 6:34 PM, Johannes Schindelin
 wrote:
> The invention of the `git worktree` command changed this developer's
> working style dramatically. Rather than switching between branches all
> the time, topic branches are created and checked out in newly-added
> worktrees, to be reworked and refined until the topic branch is either
> merged into `master` or abandoned.
>
> It gets rather tiresome, and also typo-prone, to call "git branch xyz
> upstream/master && git worktree add xyz xyz" all the time.

You can actually do "git worktree -b xyz xyz upstream/master" for the
same effect. Maybe we can avoid "xyz" duplication with "-b -" or a new
option name?

> Hence this
> proposal: "git branch -w xyz upstream/master" to do the same.
>
> The plan is to also support "git branch -d -w xyz" once the `git
> worktree` command learned a `remove` (or `delete`) subcommand.

"git worktree remove" is coming (will be resent after -rc period). And
I agree it's convenient for it to remove the branch as well because
that happened to (and annoyed) me a few times. I still think it should
be centered around git-worktree than git-branch though.

> One possible improvement would be to add "/xyz/" to the parent
> repository's .git/info/exclude, but this developer hesitates to
> introduce that feature without the "delete" counterpart: those exclude
> entries would likely go stale very quickly. Besides, there might be a
> plan in the working to exclude worktrees automagically?

That's needed because you add a worktree inside another worktree? I
know that feeling, but I've changed my layout from ~/w/git as main
worktree (and ~/w/git/.git as repo) to ~/w/git as a non-worktree dir
that contains all worktrees, e.g. ~/w/git/reinclude-dir,
~/w/git/worktree-config, ~/w/git/lmdb... My typical worktree add
command is "git worktree add ../" then move there and do
stuff. No nested worktrees, no need to update exclude file (and no
messing up emacs' rgrep command, which does not understand .gitignore
anyway)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/1] branch: allow conveniently adding new worktrees for new branches

2016-03-10 Thread Johannes Schindelin
With the newly-introduced --worktree option, after a new branch was
created we will add a corresponding new worktree with the same name
automatically.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-branch.txt |  5 +++--
 builtin/branch.c | 27 +--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4a7037f..963cdcb 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[--column[=] | --no-column]
[(--merged | --no-merged | --contains) []] [--sort=]
[--points-at ] [...]
-'git branch' [--set-upstream | --track | --no-track] [-l] [-f]  
[]
+'git branch' [--set-upstream | --track | --no-track] [-l] [-f] [-w | 
--worktree]  []
 'git branch' (--set-upstream-to= | -u ) []
 'git branch' --unset-upstream []
 'git branch' (-m | -M) [] 
@@ -46,7 +46,8 @@ which points to the current 'HEAD', or  if given.
 
 Note that this will create the new branch, but it will not switch the
 working tree to it; use "git checkout " to switch to the
-new branch.
+new branch, or use the `--worktree` option to add a new worktree of
+the same name.
 
 When a local branch is started off a remote-tracking branch, Git sets up the
 branch (specifically the `branch..remote` and `branch..merge`
diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6b..c681b2e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -20,6 +20,7 @@
 #include "utf8.h"
 #include "wt-status.h"
 #include "ref-filter.h"
+#include "run-command.h"
 
 static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r | -a] [--merged | --no-merged]"),
@@ -605,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 {
int delete = 0, rename = 0, force = 0, list = 0;
int reflog = 0, edit_description = 0;
-   int quiet = 0, unset_upstream = 0;
+   int quiet = 0, unset_upstream = 0, new_worktree = 0;
const char *new_upstream = NULL;
enum branch_track track;
struct ref_filter filter;
@@ -622,6 +623,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
BRANCH_TRACK_OVERRIDE),
OPT_STRING('u', "set-upstream-to", _upstream, "upstream", 
"change the upstream info"),
OPT_BOOL(0, "unset-upstream", _upstream, "Unset the 
upstream info"),
+   OPT_BIT('w', "worktree", _worktree, N_("add worktree for 
the new branch"), 1),
OPT__COLOR(_use_color, N_("use colored output")),
OPT_SET_INT('r', "remotes", , N_("act on 
remote-tracking branches"),
FILTER_REFS_REMOTES),
@@ -678,6 +680,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if (!delete && !rename && !edit_description && !new_upstream && 
!unset_upstream && argc == 0)
list = 1;
 
+   if (new_worktree) {
+   if (delete || rename || new_upstream || unset_upstream)
+   die("--worktree requires creating a new branch");
+   if (new_worktree && (argc < 1 || argc > 2))
+   die(_("--worktree needs a branch/worktree name"));
+   }
+
if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || 
filter.points_at.nr)
list = 1;
 
@@ -797,7 +806,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
strbuf_release();
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
-   int branch_existed = 0, remote_tracking = 0;
+   int branch_existed = 0, remote_tracking = 0, res = 0;
struct strbuf buf = STRBUF_INIT;
 
if (!strcmp(argv[0], "HEAD"))
@@ -820,6 +829,17 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
  force, reflog, 0, quiet, track);
 
+   if (new_worktree) {
+   const char *child_argv[] = {
+   "worktree", "add", NULL, NULL, NULL
+   };
+   child_argv[2] = child_argv[3] = argv[0];
+   if ((res = run_command_v_opt(child_argv, RUN_GIT_CMD)))
+   error(_("Could not create worktree %s"), 
argv[0]);
+   else
+   fprintf(stderr, _("New worktree set up at 
%s\n"), argv[0]);
+   }
+
/*
 * We only show the instructions if the user gave us
 * one branch which doesn't exist locally, but is the
@@ -828,10 +848,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if (argc == 1 && track == BRANCH_TRACK_OVERRIDE &&
 

[PATCH 0/1] Introduce a way to create a branch and worktree at the same time

2016-03-10 Thread Johannes Schindelin
The invention of the `git worktree` command changed this developer's
working style dramatically. Rather than switching between branches all
the time, topic branches are created and checked out in newly-added
worktrees, to be reworked and refined until the topic branch is either
merged into `master` or abandoned.

It gets rather tiresome, and also typo-prone, to call "git branch xyz
upstream/master && git worktree add xyz xyz" all the time. Hence this
proposal: "git branch -w xyz upstream/master" to do the same.

The plan is to also support "git branch -d -w xyz" once the `git
worktree` command learned a `remove` (or `delete`) subcommand.

One possible improvement would be to add "/xyz/" to the parent
repository's .git/info/exclude, but this developer hesitates to
introduce that feature without the "delete" counterpart: those exclude
entries would likely go stale very quickly. Besides, there might be a
plan in the working to exclude worktrees automagically?


Johannes Schindelin (1):
  branch: allow conveniently adding new worktrees for new branches

 Documentation/git-branch.txt |  5 +++--
 builtin/branch.c | 27 +--
 2 files changed, 28 insertions(+), 4 deletions(-)

-- 
2.7.2.windows.1.8.g47d64e6.dirty

--
To unsubscribe from this list: send the line "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 04/19] index-helper: new daemon for caching index and related stuff

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 6:21 AM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> David Turner  writes:
>>
>>> From: Nguyễn Thái Ngọc Duy 
>>>
>>> Instead of reading the index from disk and worrying about disk
>>> corruption, the index is cached in memory (memory bit-flips happen
>>> too, but hopefully less often). The result is faster read. Read time
>>> is reduced by 70%.
>>>
>>> The biggest gain is not having to verify the trailing SHA-1, which
>>> takes lots of time especially on large index files.
>
> Come to think of it, wouldn't it be far less intrusive change to
> just put the index on a ramdisk and skip the trailing SHA-1
> verification, to obtain a similar result with the same trade off
> (i.e. blindly trusting memory instead of being paranoid)?

I'm straying off-topic again because this reminded me about lmdb :)
For the record when I looked at lmdb I thought of using it as an index
format too. It has everything we wanted in index-v5. Good enough that
we would not need index-helper and split-index to work around current
index format. Though I finally decided it didn't fit well.

On the good side, lmdb is b+ trees, we can update directly on disk
(without rewriting whole file), read directly from mmap'd file and not
worry about corrupting it. There's no SHA-1 verification either (and
we can do crc32 per entry if we want too). It's good.

But, it does not fit well the our locking model (but we can change
this). And we sometimes want to create temporary throw-away indexes
(e.g. partial commits) which I don't think is easy to do. And the
reading directly from mmap does not give us much because we have to do
byte endian conversion  anyway.

Hmm.. on second thought, even though lmdb can't be the default format
(for a bunch of other limitations), it can still be an option for
super big worktrees, just like index-helper being an optional
optimization. Hm.. hm.. if we can support lmdb as index format in
addition to the current format, bringing back some work from Thomas..
We may still need a daemon or something to deal with watchman, but the
underlying mechanism is exactly the same... David, Junio, what do you
think?
-- 
Duy
--
To unsubscribe from this list: send the line "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 01/19] trace.c: add GIT_TRACE_PACK_STATS for pack usage statistics

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 7:05 AM, David Turner  wrote:
> On Wed, 2016-03-09 at 14:58 -0800, Junio C Hamano wrote:
>> David Turner  writes:
> ...
>> > trace_stats() is intended for GIT_TRACE_*_STATS variable group and
>> > GIT_TRACE_PACK_STATS is more like an example how new vars can be
>> > added.
> ...
>
>> > +   pack_access_nr++;
>> >  }
>>
>> This looks rather half-hearted, in that those who are interested in
>> this new number can run "wc -l" on the pack-access trace log without
>> adding a new "stats" anyway.  It may make the "stats" far more
>> useful to keep track of the summary information of what would be
>> written to the pack access log and add to the report_pack_stats()
>> output things like the average and median distance of seeks
>> (i.e. differences in the "obj_offset" into the same pack by two
>> adjacent pack accesse) and the variance, for example?
>
> On reflection, I do not think I need this patch; it was in Duy's series
> and the index stats patch would need to be rewritten slightly to get
> rid of it, but I'm OK with that.  I wanted to make minimal changes to
> Duy's patches, but I'd rather make larger changes than do work on
> patches I don't need.
>
> So unless Duy objects, I'm going to drop this from the series.

I wanted something to verify from the tests and this was the best I
could come up. But since I added trace_printf_key(_exclude,... I
think that could serve better to show what's going on, and we can
easily summarize from the trace if we want to. That's one option. If
you can adjust the tests another way, I'm fine too.
-- 
Duy
--
To unsubscribe from this list: send the line "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: Change in .gitignore handling: intended or bug?

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 7:59 AM, Junio C Hamano  wrote:
> In any case, back to "on topic" part again; I couldn't come up with
> a better rewrite using named rules (partly because you need to
> clearly define each rule before referring them, and some of the
> rules are temporary workarounds for the 2.8 regression that will
> hopefully disappear in near future).  I think you understand the bug
> and the limitation of the current code a lot better than I do, so if
> you can please send a final version of the documentation update in
> the coming 18 hours (I have an option of using what is already
> queued on 'pu' as a fall-back-good-enough version but I want to keep
> the last-resort option as that--if I know a potential source of a
> better version, I'd choose to ask first ;-).

I'm never good with words, especially in a rush. Let's merge yours.
I'll fix the bug and update gitignore.txt in one go.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 01/48] builtin/apply: avoid parameter shadowing 'p_value' global

2016-03-10 Thread Christian Couder
On Thu, Mar 10, 2016 at 1:54 AM, Duy Nguyen  wrote:
> On Thu, Mar 10, 2016 at 6:27 AM, Junio C Hamano  wrote:
>> Christian Couder  writes:
>>
>>> Signed-off-by: Christian Couder 
>>> ---
>>>  builtin/apply.c | 24 
>>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> To be honest, I have to say that I do not like this change from
>> readability's point of view.  Once the global p_value becomes a
>> field in a "apply state" structure, functions will get a pointer to
>> the struct as a parameter, reference to it would be spelled as
>> "astate->p_value", while a local variable or a parameter would be
>> "p_value" and there is no shadowing issue.
>
> We could revert these local var rename patches at the end of the
> conversion, I think.
>
>> Also, you wouldn't be able to catch a misconversion in this patch
>> only from the patch text, if the conversion is done this way, would
>> you?  The patch may change the parameter p_value to p_v, and change
>> two references to p_value also to p_v, but it may leave another
>> reference to p_value that originally referred to the parameter
>> as-is, making it refer to the global, which would be a new bug
>> introduced by this conversion patch, but that remaining reference
>> would not show up in the patch.
>
> Yeah, I wish we have a semantic parser that can help with this sort of
> refactoring (can sparse do that?). Once we get AST tree, it should be
> easy to identify variable scope. Without it, maybe we can still rely
> on the compiler by renaming _both_ global and local vars to something
> else. That forces us (both patch writer and reviewers) to catch and
> examine every reference.

I agree that the change in the patch is not very nice.
I can do one of the following:

1) do as Junio suggests, just discard this change and wait until
p_value is moved into apply_state for the problem to be solved; a
small downside of this is that if you compile with -Wshadow after each
change you will get some warnings until p_value is moved into
apply_state; I could move the patch to move p_value at the beginning
of the series but it is a bit involved and easier to review if it is
done towards the end,

2) revert these local var rename patches at the end of the conversion,
as Duy suggests

3) instead of renaming the local "p_value" variable, rename the global
"p_value" to maybe "state_p_value", so that when I move it into
apply_state I just need to s/state_p_value/state->p_value/.

I'd rather do 3) than 1) or 2) but I am also ok with 1) and 2).

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


[PATCH 3/3] contrib/subtree: list --resolve gets symbolic refs

2016-03-10 Thread Nicola Paolucci
As the 'list' command finds commit ids for subtrees injected into the
checked out branch the --resolve flag tries to look up the repositories
at 'git-subtree-repo' and retrive the symbolic refs associated with the
commit ids found.

Example:

$ git-subtree.sh list --resolve

vim-airline  https://repo/bling/vim-airline.git 4fa37e5e[...]
vim-airline  https://repo/bling/vim-airline.git HEAD
vim-airline  https://repo/bling/vim-airline.git refs/heads/master

Signed-off-by: Nicola Paolucci 
---
 contrib/subtree/git-subtree.sh | 21 +
 contrib/subtree/git-subtree.txt| 31 +++
 contrib/subtree/t/t7900-subtree.sh | 26 ++
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 82f3fce..fe62151 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -14,7 +14,7 @@ git subtree merge --prefix= 
 git subtree pull  --prefix=  
 git subtree push  --prefix=  
 git subtree split --prefix= 
-git subtree list
+git subtree list  [--resolve]
 --
 h,helpshow the help
 q quiet
@@ -29,6 +29,7 @@ onto= try connecting new tree to an existing one
 rejoinmerge the new branch back into HEAD
  options for 'add', 'merge', and 'pull'
 squashmerge subtree changes as a single commit
+resolve   resolves ids to refs when possible
 "
 eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
 
@@ -48,6 +49,7 @@ annotate=
 squash=
 message=
 prefix=
+resolve=
 
 debug()
 {
@@ -102,6 +104,8 @@ while [ $# -gt 0 ]; do
--no-ignore-joins) ignore_joins= ;;
--squash) squash=1 ;;
--no-squash) squash= ;;
+   --resolve) resolve=1 ;;
+   --no-resolve) resolve= ;;
--) break ;;
*) die "Unexpected option: $opt" ;;
esac
@@ -254,12 +258,21 @@ find_subtree_repos()
END)
if [ -n "$sub" ]; then
if [ -n "$main" ]; then
-   # a rejoin commit?
-   # Pretend its sub was a squash.
sq="$sub"
fi
debug "Subtree found: $dir $repo $sub"
-   echo "$dir" "$repo" "$sub"
+   # Strip potential space at the end in 
repo
+   repo=$(echo $repo)
+   if [ -n "$resolve" ] && [ -n "$repo" ]; 
then
+   echo "$dir" "$repo" "$sub"
+   # Retrieve remote refs if repo 
is available
+   resolved_refs=$(git ls-remote 
"$repo" | grep "$sub" | cut -c 42- | xargs)
+   for r in $resolved_refs; do
+   echo "$dir" "$repo" "$r"
+   done
+   else
+   echo "$dir" "$repo" "$sub"
+   fi
fi
sq=
main=
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 60d76cd..ab36951 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git subtree' push  -P   
 'git subtree' merge -P  
 'git subtree' split -P  [OPTIONS] []
+'git subtree' list [--resolve]
 
 
 DESCRIPTION
@@ -106,6 +107,12 @@ split::
contents of  at the root of the project instead
of in a subdirectory.  Thus, the newly created history
is suitable for export as a separate git repository.
+
+list::
+   List all subtrees injected in checked out branch. Run
+   with optional `--resolve` to retrieve remote symbolic refs
+   associated with the subtree. Address of subtree repo is stored
+   in commits as `git-subtree-repo` at the time of `git subtree add`.
 +
 After splitting successfully, a single commit id is printed to stdout.
 This corresponds to the HEAD of the newly created tree, which you can
@@ -240,6 +247,13 @@ split, because you don't want the subproject's history to 
be part of
 your project anyway.
 
 
+OPTIONS FOR list
+--
+--resolve::
+   Resolves 'git-subtree-split' refs by looking up symbolic refs at
+   'git-subtree-repo'.
+
+
 EXAMPLE 1. Add command
 --
 Let's assume that you have a local repository that you would like
@@ -341,6 

[PATCH 2/3] contrib/subtree: new list command to list subtrees

2016-03-10 Thread Nicola Paolucci
Example output:

$ git subtree list
.vim/bundle/fireplace https://github.com/tpope/vim-fireplace.git b999b0

Signed-off-by: Nicola Paolucci 
---
 contrib/subtree/git-subtree.sh | 54 ++
 contrib/subtree/t/t7900-subtree.sh | 18 +
 2 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 278699b..82f3fce 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -14,6 +14,7 @@ git subtree merge --prefix= 
 git subtree pull  --prefix=  
 git subtree push  --prefix=  
 git subtree split --prefix= 
+git subtree list
 --
 h,helpshow the help
 q quiet
@@ -109,19 +110,22 @@ done
 command="$1"
 shift
 case "$command" in
-   add|merge|pull) default= ;;
+   add|merge|pull|list) default= ;;
split|push) default="--default HEAD" ;;
*) die "Unknown command '$command'" ;;
 esac
 
-if [ -z "$prefix" ]; then
-   die "You must provide the --prefix option."
+if [ "$command" != "list" ]; then
+   if [ -z "$prefix" ]; then
+   die "You must provide the --prefix option."
+   fi
 fi
 
 case "$command" in
-   add) [ -e "$prefix" ] && 
+   add) [ -e "$prefix" ] &&
die "prefix '$prefix' already exists." ;;
-   *)   [ -e "$prefix" ] || 
+   list) ;;
+   *)   [ -e "$prefix" ] ||
die "'$prefix' does not exist; use 'git subtree add'" ;;
 esac
 
@@ -230,6 +234,41 @@ try_remove_previous()
fi
 }
 
+find_subtree_repos()
+{
+   debug "Looking for subtree repos..."
+   sq=
+   main=
+   sub=
+   git log --grep="^git-subtree-dir:" \
+   --pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD |
+   while read a b c; do
+   debug "$a $b $c"
+   debug "{{$sq/$main/$sub}}"
+   case "$a" in
+   START) sq="$b" ;;
+   git-subtree-dir:) dir="$b $c" ;;
+   git-subtree-mainline:) main="$b" ;;
+   git-subtree-split:) sub="$b" ;;
+   git-subtree-repo:) repo="$b $c" ;;
+   END)
+   if [ -n "$sub" ]; then
+   if [ -n "$main" ]; then
+   # a rejoin commit?
+   # Pretend its sub was a squash.
+   sq="$sub"
+   fi
+   debug "Subtree found: $dir $repo $sub"
+   echo "$dir" "$repo" "$sub"
+   fi
+   sq=
+   main=
+   sub=
+   ;;
+   esac
+   done
+}
+
 find_latest_squash()
 {
debug "Looking for latest squash ($dir)..."
@@ -536,6 +575,11 @@ get_repository_url()
echo $repo_url
 }
 
+cmd_list()
+{
+   find_subtree_repos "$@"
+}
+
 cmd_add()
 {
if [ -e "$dir" ]; then
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index ed40e73..ce97446 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -196,6 +196,24 @@ test_expect_success 'add --squash stores git-subtree-repo 
value' '
 '
 
 #
+# Tests for 'git subtree list'
+#
+
+next_test
+test_expect_success 'list outputs list of subtrees' '
+   subtree_test_create_repo "$subtree_test_count" &&
+   subtree_test_create_repo "$subtree_test_count/sub proj" &&
+   test_create_commit "$subtree_test_count" main1 &&
+   test_create_commit "$subtree_test_count/sub proj" sub1 &&
+   (
+   cd "$subtree_test_count" &&
+   git fetch ./"sub proj" master &&
+   git subtree add --prefix="sub dir" "./sub proj" HEAD --squash &&
+   check_equal "$(git subtree list | cut -c -19)" "sub dir ./sub 
proj "
+   )
+'
+
+#
 # Tests for 'git subtree merge'
 #
 
-- 
2.7.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


[PATCH 0/3] subtree: add 'git-subtree-repo' and list command

2016-03-10 Thread Nicola Paolucci
To my knowledge 'git subtree' currently lacks a way to
track where injected repositories come from originally.
Adding this information allows for useful extensions to 
the command and makes it easier to use subtrees to track
external dependencies.

In this patch series I propose to add a 'git-subtree-repo'
line to the meta-data stored when injecting a tree
in a repository with 'git subtree add'. The result looks 
like this:

git-subtree-dir: .vim/bundle/fireplace
git-subtree-split: b999b09cd9d69f359fa5668e81b09dcfde455cca
git-subtree-repo: https://repo/user/vim-fireplace.git

Thanks a lot to Mathias Nyman who has cleaned up my idea to
add 'git-subtree-repo' and already submitted it for review.
I added a test and a tiny fix to his patch and I resend it 
here (hence the v3 in the first patch).

Using this extra value a simple 'git subtree list' command can 
be implemented which scans the checked out branch for subtrees
injected:

$ git subtree list
.vim/bundle/fireplace https://github.com/tpope/vim-fireplace.git b999b0

I also added an optional '--resolve' flag to retrieve symbolic
remote refs associated with the commit ids of the remote repository:

$ git-subtree.sh list --resolve

vim-airline  https://repo/bling/vim-airline.git 4fa37e5e[...]
vim-airline  https://repo/bling/vim-airline.git HEAD
vim-airline  https://repo/bling/vim-airline.git refs/heads/master


Nicola Paolucci (3):
  contrib/subtree: 'add' stores 'git-subtree-repo'
  contrib/subtree: new list command to list subtrees
  contrib/subtree: list --resolve gets symbolic refs

 contrib/subtree/git-subtree.sh | 140 +
 contrib/subtree/git-subtree.txt|  31 
 contrib/subtree/t/t7900-subtree.sh |  63 +
 3 files changed, 205 insertions(+), 29 deletions(-)

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


[PATCH v3 1/3] contrib/subtree: 'add' stores 'git-subtree-repo'

2016-03-10 Thread Nicola Paolucci
Git-subtree operations 'add' and 'pull', when called with the 
parameter will add this to the commit message:
git-subtree-repo: 

Other operations that don't have the  information, like
'merge' or 'add' without  are unchanged. Users with such a
workflow will be on their own with the --message parameter, if they'd
like to record where the subtree came from.

For example:

$ git subtree add --prefix .vim/bundle/fireplace \
https://repo/user/vim-fireplace.git master --squash

Will result in a commit like:

commit ce87dab198fecdff6043d88a26c55d7cd95e8bf9
Author: Bob Marley 
Date:   Tue May 12 13:37:03 2015 +0200

Squashed '.vim/bundle/fireplace/' content from commit b999b09

git-subtree-dir: .vim/bundle/fireplace
git-subtree-split: b999b09cd9d69f359fa5668e81b09dcfde455cca
git-subtree-repo: https://repo/user/vim-fireplace.git

This allows new ways to interact with injected trees, for example
a new command 'git subtree list' becomes possible:

$ git subtree list
.vim/bundle/fireplace https://repo/user/vim-fireplace.git b999b0

Signed-off-by: Mathias Nyman 
Signed-off-by: Nicola Paolucci 
Thanks-to: Aleksi Aalto 
---
 contrib/subtree/git-subtree.sh | 73 +-
 contrib/subtree/t/t7900-subtree.sh | 19 ++
 2 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7a39b30..278699b 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -335,18 +335,21 @@ add_msg()
dir="$1"
latest_old="$2"
latest_new="$3"
+   repo="$4" # optional
if [ -n "$message" ]; then
commit_message="$message"
else
commit_message="Add '$dir/' from commit '$latest_new'"
fi
-   cat <<-EOF
-   $commit_message
-   
-   git-subtree-dir: $dir
-   git-subtree-mainline: $latest_old
-   git-subtree-split: $latest_new
-   EOF
+   echo $commit_message
+   echo
+   echo git-subtree-dir: $dir
+   echo git-subtree-mainline: $latest_old
+   echo git-subtree-split: $latest_new
+   if [ -n "$repo" ]; then
+   repo_url=$(get_repository_url "$repo")
+   echo "git-subtree-repo: $repo_url"
+   fi
 }
 
 add_squashed_msg()
@@ -382,8 +385,9 @@ squash_msg()
dir="$1"
oldsub="$2"
newsub="$3"
+   repo="$4" # optional
newsub_short=$(git rev-parse --short "$newsub")
-   
+
if [ -n "$oldsub" ]; then
oldsub_short=$(git rev-parse --short "$oldsub")
echo "Squashed '$dir/' changes from 
$oldsub_short..$newsub_short"
@@ -397,6 +401,10 @@ squash_msg()
echo
echo "git-subtree-dir: $dir"
echo "git-subtree-split: $newsub"
+   if [ -n "$repo" ]; then
+   repo_url=$(get_repository_url "$repo")
+   echo "git-subtree-repo: $repo_url"
+   fi
 }
 
 toptree_for_commit()
@@ -440,12 +448,13 @@ new_squash_commit()
old="$1"
oldsub="$2"
newsub="$3"
+   repo="$4" # optional
tree=$(toptree_for_commit $newsub) || exit $?
if [ -n "$old" ]; then
-   squash_msg "$dir" "$oldsub" "$newsub" | 
+   squash_msg "$dir" "$oldsub" "$newsub" "$repo" |
git commit-tree "$tree" -p "$old" || exit $?
else
-   squash_msg "$dir" "" "$newsub" |
+   squash_msg "$dir" "" "$newsub" "$repo" |
git commit-tree "$tree" || exit $?
fi
 }
@@ -517,6 +526,16 @@ ensure_valid_ref_format()
die "'$1' does not look like a ref"
 }
 
+get_repository_url()
+{
+   repo=$1
+   repo_url=$(git config --get remote.$repo.url)
+   if [ -z "$repo_url" ]; then
+   repo_url=$repo
+   fi
+   echo $repo_url
+}
+
 cmd_add()
 {
if [ -e "$dir" ]; then
@@ -548,19 +567,18 @@ cmd_add()
 cmd_add_repository()
 {
echo "git fetch" "$@"
-   repository=$1
+   repo=$1
refspec=$2
git fetch "$@" || exit $?
revs=FETCH_HEAD
-   set -- $revs
+   set -- $revs $repo
cmd_add_commit "$@"
 }
 
 cmd_add_commit()
 {
-   revs=$(git rev-parse $default --revs-only "$@") || exit $?
-   set -- $revs
-   rev="$1"
+   rev=$(git rev-parse $default --revs-only "$1") || exit $?
+   repo="${@:2}" # optional

debug "Adding $dir as '$rev'..."
git read-tree --prefix="$dir" $rev || exit $?
@@ -575,12 +593,12 @@ cmd_add_commit()
fi

if [ -n "$squash" ]; then
-   rev=$(new_squash_commit "" "" "$rev") || exit $?
+   rev=$(new_squash_commit "" "" "$rev" "$repo") || exit $?
commit=$(add_squashed_msg "$rev" "$dir" |
 

Re: [RFC/PATCH 00/48] Libifying git apply

2016-03-10 Thread Duy Nguyen
On Thu, Mar 10, 2016 at 12:48 AM, Christian Couder
 wrote:
> This is a patch series about libifying "git apply" functionality, to
> be able to use this functionality in "git am" without spawning new
> processes. This should make "git am" and "git rebase" significantly
> faster.
>
> This has been discussed in the following thread:
>
> http://thread.gmane.org/gmane.comp.version-control.git/287236/
>
> This RFC patch series for now just gets rid of the global variables
> and refactors the code around a bit.
>
> As suggested by Junio the global variables in builtin/apply.c are just
> thrown into a single "apply_state" structure that is passed around the
> callchain. A new parameter called "state" that is a pointer to the
> "apply_state" structure comes at the beginning of the helper functions
> that need it.
>
> Before I make further changes to handle erroneous input and make the
> libified functions not die() and properly clean things up, I'd be
> happy to get some feedback.

I didn't review individual patches. Instead I redid the whole thing
(in a slightly different way) and compared my end result to yours. In
general it looks good but..

1) I think you should focus the series on moving global vars to struct
apply_state only. You can save code move patches for the later phase.
Easier to review.

2) Given that there are only four local variables shadowing global
ones, p_value, linenr and options, I think it's ok to drop 1/48 and
2/48 and keep the good old names. You just need to mention about them
and what function they are declared in in the relevant "move global
..." patch so the reviewer is aware of it.

3) Moving lock_file to struct apply_state, then putting the whole
struct on stack, could be problematic. I believe there's a global
linked list keeping references of all lock_file variables until the
end of time, so we can't destroy lock_file/apply_state until we are
certain it's safe to do so. We could simply leave lock_file as a
global var for now (with a note in struct apply_state so we don't
forget). We can always do fancy stuff like malloc() later on if we
need to.

4) I noticed on the interdiff that there are translatable strings in
this file but not marked as such. You're going to spend lots of time
reading this file and are in a very good positioin to determine if a
string should be translated (i.e. wrap _() around it) or not. So maybe
you can have a look at this in the next series too. It's absolutely ok
if you say "no" here.

5) Minor detail, but we can rename prefix_ to prefix in
init_apply_state(). There trailing underscore was there in cmd_apply
because there's the global "prefix", but it's gone now.

Looking forward to seeing you double, triple git-rebase's speed.
-- 
Duy
--
To unsubscribe from this list: send the line "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] http: honor no_http env variable to bypass proxy

2016-03-10 Thread Elia Pinto
2016-03-07 21:33 GMT+01:00 Junio C Hamano :
> Junio C Hamano  writes:
>
> Second call for help.  I hate having to revert 30f302f7 (Merge
> branch 'kf/http-proxy-auth-methods', 2016-02-03) this late in the
> cycle.
Ok. I made the original merge commit for master. So,  if it is
sufficient, for me the patch is correct.

I am sorry for the delay in the response.

Best
>
>> Jiang Xin  writes:
>>
>>> From: Jiang Xin 
>>>
>>> Curl and its families honor several proxy related environment variables:
>>>
>>> * http_proxy and https_proxy define proxy for http/https connections.
>>> * no_proxy (a comma separated hosts) defines hosts bypass the proxy.
>>>
>>> This command will bypass the bad-proxy and connect to the host directly:
>>>
>>> no_proxy=* https_proxy=http://bad-proxy/ \
>>> curl -sk https://google.com/
>>>
>>> Before commit 372370f (http: use credential API to handle proxy auth...),
>>> Environment variable "no_proxy" will take effect if the config variable
>>> "http.proxy" is not set.  So the following comamnd won't fail if not
>>> behind a firewall.
>>>
>>> no_proxy=* https_proxy=http://bad-proxy/ \
>>> git ls-remote https://github.com/git/git
>>>
>>> But commit 372370f not only read git config variable "http.proxy", but
>>> also read "http_proxy" and "https_proxy" environment variables, and set
>>> the curl option using:
>>>
>>> curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host);
>>>
>>> This caused "no_proxy" environment variable not working any more.
>>>
>>> Set extra curl option "CURLOPT_NOPROXY" will fix this issue.
>>>
>>> Signed-off-by: Jiang Xin 
>>> ---
>>>  http.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>
>> Sounds sensible; I am guessing that this is 2.8.0-rc0 regression
>> that we need to fast-track?
>>
>> Knut, does this look good?
>>
>> Thanks.
>>
>>> diff --git a/http.c b/http.c
>>> index 1d5e3bb..69da445 100644
>>> --- a/http.c
>>> +++ b/http.c
>>> @@ -70,6 +70,7 @@ static long curl_low_speed_limit = -1;
>>>  static long curl_low_speed_time = -1;
>>>  static int curl_ftp_no_epsv;
>>>  static const char *curl_http_proxy;
>>> +static const char *curl_no_proxy;
>>>  static const char *http_proxy_authmethod;
>>>  static struct {
>>>  const char *name;
>>> @@ -624,6 +625,11 @@ static CURL *get_curl_handle(void)
>>>  }
>>>
>>>  curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host);
>>> +#if LIBCURL_VERSION_NUM >= 0x071304
>>> +var_override(_no_proxy, getenv("NO_PROXY"));
>>> +var_override(_no_proxy, getenv("no_proxy"));
>>> +curl_easy_setopt(result, CURLOPT_NOPROXY, curl_no_proxy);
>>> +#endif
>>>  }
>>>  init_curl_proxy_auth(result);
> --
> To unsubscribe from this list: send the line "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: [PATCH] http: honor no_http env variable to bypass proxy

2016-03-10 Thread Elia Pinto
2016-02-29 16:16 GMT+01:00 Jiang Xin :
> From: Jiang Xin 
>
> Curl and its families honor several proxy related environment variables:
>
> * http_proxy and https_proxy define proxy for http/https connections.
> * no_proxy (a comma separated hosts) defines hosts bypass the proxy.
>
> This command will bypass the bad-proxy and connect to the host directly:
>
> no_proxy=* https_proxy=http://bad-proxy/ \
> curl -sk https://google.com/
>
> Before commit 372370f (http: use credential API to handle proxy auth...),
> Environment variable "no_proxy" will take effect if the config variable
> "http.proxy" is not set.  So the following comamnd won't fail if not
> behind a firewall.
>
> no_proxy=* https_proxy=http://bad-proxy/ \
> git ls-remote https://github.com/git/git
>
> But commit 372370f not only read git config variable "http.proxy", but
> also read "http_proxy" and "https_proxy" environment variables, and set
> the curl option using:
>
> curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host);
>
> This caused "no_proxy" environment variable not working any more.
>
> Set extra curl option "CURLOPT_NOPROXY" will fix this issue.
>
> Signed-off-by: Jiang Xin 
Signed-off-by: Elia Pinto 
> ---
>  http.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/http.c b/http.c
> index 1d5e3bb..69da445 100644
> --- a/http.c
> +++ b/http.c
> @@ -70,6 +70,7 @@ static long curl_low_speed_limit = -1;
>  static long curl_low_speed_time = -1;
>  static int curl_ftp_no_epsv;
>  static const char *curl_http_proxy;
> +static const char *curl_no_proxy;
>  static const char *http_proxy_authmethod;
>  static struct {
> const char *name;
> @@ -624,6 +625,11 @@ static CURL *get_curl_handle(void)
> }
>
> curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host);
> +#if LIBCURL_VERSION_NUM >= 0x071304
> +   var_override(_no_proxy, getenv("NO_PROXY"));
> +   var_override(_no_proxy, getenv("no_proxy"));
> +   curl_easy_setopt(result, CURLOPT_NOPROXY, curl_no_proxy);
> +#endif
> }
> init_curl_proxy_auth(result);
>
> --
> 2.8.0.rc0.1.g9eb3984.dirty
>
> --
> To unsubscribe from this list: send the line "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


Bug: git branch -D can be used to delete branch which is currently checked out - Part 2

2016-03-10 Thread Marcus Kida
Proposed solution:

Use `strcasecmp`, `stricmp`, `strcmpi` here: 
https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218

Not sure if/which one of this will work on POSIX as well as MS too though.

Thank you.

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


Re: [RFC/PATCH 04/48] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

2016-03-10 Thread Christian Couder
On Wed, Mar 9, 2016 at 11:55 PM, Stefan Beller  wrote:
> On Wed, Mar 9, 2016 at 9:48 AM, Christian Couder
>  wrote:
>
> Some words in the commit message would be nice here as this is one of
> the patches,
> which isn't "obviously" a good thing to to. This comment also applies
> to "builtin/apply:
> introduce 'struct apply_state' to start libifying" where you lay out
> the plan for the next
> ~40 patches.

Ok, I will try to explain better at least those 2 patches.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "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: git branch -D can be used to delete branch which is currently checked out

2016-03-10 Thread Marcus Kida
Testes on: 

Mac OS X 10.11.3 (El Capitan) using Git 2.6.4

Issue:

git branch -D can be used to delete branch which is currently checked out

Steps to reproduce:

inside a git repository:

$ git checkout -b feature/myAwesomeFeature

-> you end up in feature/myAwesomeFeature branch

$ git checkout FEATURE/myAwesomeFeature

-> you end up in FEATURE/myAwesomeFeature branch

$ git branch -D feature/myAwesomeFeature branch

-> BOOM you just deleted the branch you were at. Congrats you repo is dirty 
again.

Actual behaviour:

It seems like checking out / deleting branches is case insensitive thus you can 
delete a branch you are on by just using a different capitalisation when 
specifying the branch to delete.

Expected behaviour:

error: Cannot delete the branch 'FEATURE/myAwesomeFeature' which you are 
currently on.

Thank you, please don't hesitate to contact me in case you need more info or if 
this has already been fixed in the meantime.

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