Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add --gui option to mergetool
From: David Aguilar @ 2017-02-05 10:18 UTC (permalink / raw)
  To: Denton Liu; +Cc: git
In-Reply-To: <20170204064303.GA14990@arch-attack.localdomain>

On Fri, Feb 03, 2017 at 10:43:03PM -0800, Denton Liu wrote:
> * fix the discrepancy between difftool and mergetool where
>   the former has the --gui flag and the latter does not by adding the
>   functionality to mergetool

Please avoid bullet points in commit messages when a simple
paragraph will suffice.

The implementation of this feature seems ok, but tests are
needed in t/t7610-mergetool.sh.

> * make difftool read 'merge.guitool' as a fallback, in accordance to the
>   manpage for difftool: "git difftool falls back to git mergetool
>   config variables when the difftool equivalents have not been defined"

I did not spot this change in the code.

Nonetheless, this should be split off as a separate patch, and
tests should be added.

> * add guitool-related completions

This should be split off as a separate patch as well.

Generally, 3 bullet points suggests there should be 3 patches in
this series.

Thanks,
-- 
David

^ permalink raw reply

* Re: release notes/ change number discrepancy ? - Documentation/RelNotes/2.10.0.txt "merge b738396..."
From: Junio C Hamano @ 2017-02-05  7:44 UTC (permalink / raw)
  To: Zenaan Harkness; +Cc: git
In-Reply-To: <20170205022156.GA19612@x220-a02>

Zenaan Harkness <zen@freedbms.net> writes:

> Am I missing something in the following:
>
> looking at Documentation/RelNotes/2.10.0.txt I see the following release
> note (~line 35):
>
>  * "upload-pack" allows a custom "git pack-objects" replacement when
>    responding to "fetch/clone" via the uploadpack.packObjectsHook.
>    (merge b738396 jk/upload-pack-hook later to maint).
>
>
> but when I run git show b738396 , I get the following:
>
> commit b738396cfdcc276c0cde0c1a6462c5cc74ba7b76
> Author: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date:   Thu Jul 14 15:58:59 2016 +0200
>
>     mingw: fix regression in t1308-config-set
>
> which seems to be completely unrelated. What am I missing please?

I think the commit is an "oops, we found a regression on a different
platform than the one used when developing the series after its
development completed, and here is a fix on top" commit that is
queued as the tip of a series.  You shouldn't be using "git show" on
it to look ONLY the tip of the series.

Let me show a better way to ask Git what you want to know, with the
excellent "git when-merged" script (google for it).

$ git when-merged b738396 master
refs/heads/master                      75676c8c8b6cbeec7ccb68d97c17db230d9f2659

We merged that commit to 'master' at 75676c8c8.  What does the merge
log say?

$ git show 75676c8c
commit 75676c8c8b6cbeec7ccb68d97c17db230d9f2659
Merge: 79ed43c28f b738396cfd
Author: Junio C Hamano <gitster@pobox.com>
Date:   Thu Jul 14 10:38:57 2016 -0700

    Merge branch 'jk/upload-pack-hook'

    A hot-fix to make a test working in mingw again.

    * jk/upload-pack-hook:
      mingw: fix regression in t1308-config-set

OK, so it was a hot-fix that consists of a single commit.  What did
we need to hot-fix?  A hot-fix is typically queued as a direct
follow-up to what is needed to be fixed.  When did we merge the
parent of the fix?

$ git when-merged b738396^ master
refs/heads/master                      1e4bf907890e094f1c1c8c5086387e7d5fdb0655

And that merge commit on 'master' shows us the series that needed to
be fixed up.

$ git show 1e4bf907890
commit 1e4bf907890e094f1c1c8c5086387e7d5fdb0655
Merge: 7a738b40f6 20b20a22f8
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Jul 6 13:38:11 2016 -0700

    Merge branch 'jk/upload-pack-hook'

    "upload-pack" allows a custom "git pack-objects" replacement when
    responding to "fetch/clone" via the uploadpack.packObjectsHook.

    * jk/upload-pack-hook:
      upload-pack: provide a hook for running pack-objects
      t1308: do not get fooled by symbolic links to the source tree
      config: add a notion of "scope"
      config: return configset value for current_config_ functions
      config: set up config_source for command-line config
      git_config_parse_parameter: refactor cleanup code
      git_config_with_options: drop "found" counting

You learned from the above that the jk/upload-pack-hook topic was
developed as a 7-patch series, reviewed, tested and got merged to
'master' on Jul 6th.  Unfortunately a test in the series had a
portability issue that wasn't discovered while it was being reviewed
and tested, and a hot-fix was queued on top and merged to 'master'
about a week later.  If we wanted to merge the topic to the
maintenance track, we cannot just merge the original 7-patch series.
We need to merge the whole thing, including the 8th one that is the
hot-fix.

^ permalink raw reply

* Re: [PATCH] tag: add a config option for setting --annotate by default
From: David Aguilar @ 2017-02-05  4:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqlgtm8s5k.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 03, 2017 at 09:02:47PM -0800, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> 
> > Make it easier for users to remember to annotate their tags.
> > Allow setting the default value for "--annotate" via the "tag.annotate"
> > configuration variable.
> >
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> 
> I do not care too strongly about this, but I need to point out that
> this will have fallout to tools and scripts.  E.g. if you have this
> configured and try to create a new tag in gitk, wouldn't this part
> 
> 	if {$msg != {}} {
> 	    exec git tag -a -m $msg $tag $id
> 	} else {
> 	    exec git tag $tag $id
> 	}
> 
> try to open an editor somehow to get the message even when $msg is
> an empty string?  I think the same problem already exists for the
> tag.forceSignAnnotated variable we already have added, though.

That's true.  I should have put "RFC" in the subject line.
Let's drop this patch unless there's others that find it useful.

How do you feel about a patch to add "git merge --signoff", for
consistency with "git commit"?

The rationale is that there might be situations (evil merges, or
even regular merges depending on the project) where someone
might want to signoff on their merges.
-- 
David

^ permalink raw reply

* release notes/ change number discrepancy ? - Documentation/RelNotes/2.10.0.txt "merge b738396..."
From: Zenaan Harkness @ 2017-02-05  2:21 UTC (permalink / raw)
  To: git

Am I missing something in the following:

looking at Documentation/RelNotes/2.10.0.txt I see the following release
note (~line 35):

 * "upload-pack" allows a custom "git pack-objects" replacement when
   responding to "fetch/clone" via the uploadpack.packObjectsHook.
   (merge b738396 jk/upload-pack-hook later to maint).


but when I run git show b738396 , I get the following:

commit b738396cfdcc276c0cde0c1a6462c5cc74ba7b76
Author: Johannes Schindelin <johannes.schindelin@gmx.de>
Date:   Thu Jul 14 15:58:59 2016 +0200

    mingw: fix regression in t1308-config-set


which seems to be completely unrelated. What am I missing please?

Thanks
Zenaan

^ permalink raw reply

* [PATCH] completion: fix git svn authorship switches
From: Eric Wong @ 2017-02-05  2:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

--add-author-from and --use-log-author are for "git svn dcommit",
not "git svn (init|clone)"

Signed-off-by: Eric Wong <e@80x24.org>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

 Though, I now wonder if allowing those switches to write changes
 in $GIT_CONFIG at init/clone time makes sense...

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 97d73ad88f..6990e98c44 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2587,14 +2587,14 @@ _git_svn ()
 			--no-metadata --use-svm-props --use-svnsync-props
 			--log-window-size= --no-checkout --quiet
 			--repack-flags --use-log-author --localtime
+			--add-author-from
 			--ignore-paths= --include-paths= $remote_opts
 			"
 		local init_opts="
 			--template= --shared= --trunk= --tags=
 			--branches= --stdlayout --minimize-url
 			--no-metadata --use-svm-props --use-svnsync-props
-			--rewrite-root= --prefix= --use-log-author
-			--add-author-from $remote_opts
+			--rewrite-root= --prefix= $remote_opts
 			"
 		local cmt_opts="
 			--edit --rmdir --find-copies-harder --copy-similarity=
-- 
EW

^ permalink raw reply related

* Re: init --separate-git-dir does not set core.worktree
From: Kyle Meyer @ 2017-02-04 23:34 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller
In-Reply-To: <CACsJy8C=owNPpND4Ab7bFE24kpWBr5fQdob21DEDCckCXu0Mng@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer <kyle@kyleam.com> wrote:

[...]

>>  * COMMIT_EDITMSG in .git/modules/<module>/: set working tree to the
>>    output of "git rev-parse --show-toplevel"
>>
>>  * COMMIT_EDITMSG in .git/worktrees/<wtree>/: set working tree to the
>>    path in .git/worktrees/<wtree>/gitdir, minus the trailing "/.git"
>>
>>  * COMMIT_EDITMSG in .git: set working tree to the parent directory
>>
>> This fails for a repo set up with --separate-git-dir [*2*], where the
>> last step will go out into an unrelated repo.  If core.worktree was set
>> and "git rev-parse --show-toplevel" returned the working tree like it
>> did for submodules, things would work.
>
> OK. If I read this right, given a path of any text file somewhere
> within ".git" directory. you are tasked to find out where the
> associated worktree is?

Right

> I.e. this is not an emacsclient executed as part of "git commit",
> correct?

... but it is from a "git commit" call.  I think you're asking because,
if we know where "git commit" was called from, we know what the working
tree is.  This is true, but with the current Magit design, the function
for determining the top-level of the working tree, magit-toplevel,
doesn't have access to this information.  From Emacs, Magit calls "git
commit", setting GIT_EDITOR for that process so that git invokes the
current Emacs instance for editing the commit message.  From
COMMIT_EDITMSG, we want the magit-toplevel to be able to determine the
working tree location so that commands can use magit-toplevel to set
their pwd.

The only Magit command that currently relies on magit-toplevel to
determine the working tree from ".git/COMMIT_EDITMSG" is a command that
shows a diff with the staged changes in a separate buffer.  (Even though
"git diff --cached" would work from within ".git/", some functionality
in this buffer depends on its pwd being set top-level of the working
tree.)

So, whatever solution we come up with on the Magit end will likely
involve recording where "git commit" was called so that the diff command
can figure out the working tree.

> So you need some sort of back-link to ".git" location. And
> unfortunately there's no such thing for .git file (unless it points to
> .git/worktrees/...). I'm hesitant to set core.worktree unless it's
> absolutely needed since it may have unexpected interaction with
> $GIT_WORK_TREE and others (not sure if it has any interaction with
> submodules too). I think adding a new file "gitdir" would be a safer
> option.
>
> I'm not entirely sure if enforcing "one worktree - one repository" is
> safe though. The first two bullet points are very specific and we can
> assume that, but ".git" files alone can be used for anything. In
> theory you can always create a secondary worktree (that's not managed
> by "git worktree") by setting GIT_WORK_TREE and "git checkout -f"
> somewhere. But I guess those would be temporary and nobody would want
> magic to point back to them.

Right, makes sense.

Unfortunately, magit-toplevel was designed thinking that the
--separate-git-dir / core.worktree behavior in Git 2.8.4 and lower was
intentional [*].  We'll need to rework this on our end.

Thanks for your input and for confirming that "git init
--separate-git-dir" does not set core.worktree by design.

[*] https://github.com/magit/magit/issues/460#issuecomment-36035787.

-- 
Kyle

^ permalink raw reply

* Re: feature request: add -q to "git branch"
From: Pranit Bauva @ 2017-02-04 21:17 UTC (permalink / raw)
  To: Kevin Layer; +Cc: Git List
In-Reply-To: <CAFZEwPOFDT7=1qhg4ygJpVUnfQo3XUjDoNtZ4LJvG5V9+RDNwA@mail.gmail.com>

Hey Kevin,

Sorry for the previous message.

On Sun, Feb 5, 2017 at 2:47 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> Hey Kevin,
>
> On Fri, Feb 3, 2017 at 11:59 PM, Kevin Layer <layer@known.net> wrote:
>> It should be possible to quietly create a branch.

I think `git branch` is already quiet. Are you seeing something else?

Regards,
Pranit Bauva

^ permalink raw reply

* Re: feature request: add -q to "git branch"
From: Pranit Bauva @ 2017-02-04 21:17 UTC (permalink / raw)
  To: Kevin Layer; +Cc: Git List
In-Reply-To: <CAGSZTjLmYCyKZ1BBRv+JVYq4oX7EQcNzyxAnS_3NBUPjr3g8zQ@mail.gmail.com>

Hey Kevin,

On Fri, Feb 3, 2017 at 11:59 PM, Kevin Layer <layer@known.net> wrote:
> It should be possible to quietly create a branch.
>
> Thanks.
>
> Kevin

^ permalink raw reply

* Re: [PATCH v3] parse-remote: remove reference to unused op_prep
From: Pranit Bauva @ 2017-02-04 21:15 UTC (permalink / raw)
  To: Siddharth Kannan; +Cc: Git List
In-Reply-To: <1486218663-31820-1-git-send-email-kannan.siddharth12@gmail.com>

Hey Siddharth,

On Sat, Feb 4, 2017 at 8:01 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> The error_on_missing_default_upstream helper function learned to
> take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> if no upstream specified", 2011-02-09), but as of 045fac5845
> ("i18n: git-parse-remote.sh: mark strings for translation",
>  2016-04-19), the argument is no longer used.  Remove it.
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>

This looks good to me! Thanks :)

Regards,
Pranit Bauva

^ permalink raw reply

* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Stefan Beller @ 2017-02-04 18:43 UTC (permalink / raw)
  To: Carlo Wood; +Cc: Junio C Hamano, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <20170201021040.4f6cc827@hikaru>

On Tue, Jan 31, 2017 at 5:10 PM, Carlo Wood <carlo@alinoe.com> wrote:
> On Tue, 31 Jan 2017 14:08:41 -0800
> Stefan Beller <sbeller@google.com> wrote:
>
>> On Sun, Jan 29, 2017 at 5:00 PM, Junio C Hamano <gitster@pobox.com>
>> wrote:
>> >  2. If the amend is good and ready to go, "git add" to update the
>> >     superproject to make that amended result the one that is needed
>> >     in the submodule.
>>
>> yup.
>
> But that is what I am doing. The amended commit IS already
> added to the superproject (and pushed to the remote).
>
> Please have a look at my script, this happens here:
>
> # Correct that in the parent too:
> pushd parent
> git add subm
> git commit -m 'Updated subm.'
> popd

And if you were to use ammend here, too; there would be no problem;
In the parent there are now two commits, the first one pointing at the
first (unammended) commit in the submodule, the second pointing to
the corrected commit.

>
> The commit from before the amend was added to the super
> project (but never pushed) but has now been completely
> replaced. I still think this is a flaw in git. It shouldn't
> not complain and simply push.

The problem here is in the design.
When "on-demand" is set, the parent repo determines which
sumbodules needs pushing, then runs a plain "git push" in them
and then checks again.

The push operation in the submodule did not push out the un-amended
commit (which is correct IMO).

The parent in the second check determines that there is a commit, that
is not pushed, though. Maybe we need an option there "on-demand-but-no-recheck"
as a weaker promise what Git can deliver there.

>
> --
> Carlo Wood <carlo@alinoe.com>

^ permalink raw reply

* Re: Git clonebundles
From: Shawn Pearce @ 2017-02-04 17:39 UTC (permalink / raw)
  To: Stefan Saasen; +Cc: Git Mailing List
In-Reply-To: <CADoxLGPFgF7W4XJzt0X+xFJDoN6RmfFGx_96MO9GPSSOjDK0EQ@mail.gmail.com>

On Mon, Jan 30, 2017 at 11:00 PM, Stefan Saasen <ssaasen@atlassian.com> wrote:
>
> Bitbucket recently added support for Mercurial’s clonebundle extension
> (http://gregoryszorc.com/blog/2015/10/22/cloning-improvements-in-mercurial-3.6/).
> Mercurial’s clone bundles allow the Mercurial client to seed a repository using
> a bundle file instead of dynamically generating a bundle for the client.
...
> Prior art
> ~~~~~~~~~
>
> Our proof-of-concept is built on top of ideas that have been
> circulating for a while. We are aware of a number of proposed changes
> in this space:
>
>
> * Jeff King's work on network bundles:
> https://github.com/peff/git/commit/17e2409df37edd0c49ef7d35f47a7695f9608900
> * Nguyễn Thái Ngọc Duy's work on "[PATCH 0/8] Resumable clone
> revisited, proof of concept":
> https://www.spinics.net/lists/git/msg267260.html
> * Resumable clone work by Kevin Wern:
> https://public-inbox.org/git/1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com/

I think you missed the most common deployment of prior art, which is
Android using the git-repo tool[1]. The git-repo tool has had
clone.bundle support since Sep 2011[2] and the Android Git servers
have been answering /clone.bundle requests[3] since just before that.
The bundle files are generated with `git bundle create` on a regular
schedule by cron.

[1] https://gerrit.googlesource.com/git-repo/+/04071c1c72437a930db017bd4c562ad06087986a/project.py#2091
[2] https://gerrit.googlesource.com/git-repo/+/f322b9abb4cadc67b991baf6ba1b9f2fbd5d7812
[3] https://android.googlesource.com/platform/frameworks/base/clone.bundle


> Whilst the above mentioned proposals/proposed changes are in a similar
> space, I would be interest to understand whether there is any
> consensus on the general idea of supporting static bundle files as a
> mechanism to seed a repository?

I don't think we have a consensus on how to advertise a bundle file is
available, which is why there are so many instances of prior art. In
2011 I just threw together /clone.bundle on HTTP because it was easy
to make the Python wrapper ask for the file and handle 404 gracefully
as not found and fall back to `git clone`.

^ permalink raw reply

* git-gui ignores core.hookspath
From: matthias.serfling @ 2017-02-04 17:14 UTC (permalink / raw)
  To: git

Hello,
I’m running on 

$ git --version --build-options
git version 2.11.0.windows.3
built from commit: e11df2efb3072fe73153442589129d2eb8d9ea02
sizeof-long: 4
machine: x86_64


and trying to use core.hookspath with git-gui in my local repository in windows cmd shell 
$ cmd.exe /c ver
Microsoft Windows [Version 6.1.7601]
I have defined a pre-commit hook and set the path to it using core.hookspath, but when commiting in git gui it is not considered. 


Commiting directly on the cmd shell using git commit -a -m "ndkfnj" it works perfectly.

Thanks for any advice or bugfix

^ permalink raw reply

* [PATCH v3] parse-remote: remove reference to unused op_prep
From: Siddharth Kannan @ 2017-02-04 14:31 UTC (permalink / raw)
  To: git; +Cc: pranit.bauva, Siddharth Kannan

The error_on_missing_default_upstream helper function learned to
take op_prep argument with 15a147e618 ("rebase: use @{upstream}
if no upstream specified", 2011-02-09), but as of 045fac5845
("i18n: git-parse-remote.sh: mark strings for translation",
 2016-04-19), the argument is no longer used.  Remove it.

Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
---
Thanks a lot for the review, Pranit and Junio! I have made the appropriate
changes, and the edit to the file inside contrib/examples/ has been removed from
this patch.

 git-parse-remote.sh | 3 +--
 git-rebase.sh       | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index d3c3998..9698a05 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -56,8 +56,7 @@ get_remote_merge_branch () {
 error_on_missing_default_upstream () {
 	cmd="$1"
 	op_type="$2"
-	op_prep="$3" # FIXME: op_prep is no longer used
-	example="$4"
+	example="$3"
 	branch_name=$(git symbolic-ref -q HEAD)
 	display_branch_name="${branch_name#refs/heads/}"
 	# If there's only one remote, use that in the suggestion
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..b89f960 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -448,7 +448,7 @@ then
 		then
 			. git-parse-remote
 			error_on_missing_default_upstream "rebase" "rebase" \
-				"against" "git rebase $(gettext '<branch>')"
+				"git rebase $(gettext '<branch>')"
 		fi
 
 		test "$fork_point" = auto && fork_point=t
-- 
2.1.4


^ permalink raw reply related

* Re: Feature request: flagging “volatile” branches for integration/development
From: Duy Nguyen @ 2017-02-04 14:01 UTC (permalink / raw)
  To: Herbert, Marc; +Cc: Git Mailing List, Josh Triplett
In-Reply-To: <70ccb8fc-30f2-5b23-a832-9e470787a945@intel.com>

On Wed, Feb 1, 2017 at 12:12 AM, Herbert, Marc <marc.herbert@intel.com> wrote:
> (Thanks to Josh Triplett[*] for contributing to this message)
>
> Hi,
>
> We often work with development/integration branches that regularly
> rebase, in addition to stable branches that do not. Git is used to share
> two different types of branches:
>   1. Pull requests and merged code with final SHA1s
>   2. Work in progress with volatile SHA1s.
>
> We’d like to have a consistent way to distinguish these two types by
> advertising a branch as “volatile”.

I don't think we have branch metadata (besides reflog). The closet one
is probably config variable branch.<name>.description, which can be
picked up by format-patch to create cover letters. We could do
something similar, e.g. new config branch.<name>.volatile. The
commands can learn about it and apply special treatments if wanted.

But that would be local information only. We don't have ways to
transfer branch metadata (and we definitely don't want to just share
.git/config file with everybody). I suppose extending git protocol for
this is not hard (e.g. appending metadata in the "have" lines). The
hard part may be policy (e.g. what if the user does not want a branch
to be treated volatile by various commands even if it receives such
flag from a git server).
-- 
Duy

^ permalink raw reply

* Re: topological index field for commit objects
From: Jakub Narębski @ 2017-02-04 13:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20160720130231.GB17469@sigill.intra.peff.net>

On 20 July 2016 at 15:02, Jeff King <peff@peff.net> wrote:
> On Wed, Jul 20, 2016 at 02:07:38AM +0200, Jakub Narębski wrote:
>> W dniu 2016-06-30 o 00:00, Jeff King pisze:
>>> On Wed, Jun 29, 2016 at 11:49:35PM +0200, Jakub Narębski wrote:
>>
>>>> Do Git use EWAH / EWOK bitmaps for reachability analysis, or is it still
>>>> limited to object counting?
>>>
>>> At GitHub we are using them for --contains analysis, along with mass
>>> ahead/behind (e.g., as in https://github.com/gitster/git/branches). My
>>> plan is to send patches upstream, but they need some cleanup first.
>>
>> Ping. have you got time to clean up those patches?
>
> No, I haven't. Don't hold your breath; it's something I hope to work on
> in the next 6 months, not the next 6 weeks.

Ping, Was there any progress on this front? It is now almost 6 months
later...

Could those patches be made available in a "dirty" form?

TIA,
-- 
Jakub Narębski


-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH v2 3/4] introduce new format for git stash create
From: Thomas Gummerer @ 2017-02-04 13:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqtw8guwfm.fsf@gitster.mtv.corp.google.com>

On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> >  create_stash () {
> > -	stash_msg="$1"
> > -	untracked="$2"
> > +	stash_msg=
> > +	untracked=
> > +	new_style=
> > ...
> > +	while test $# != 0
> > +	do
> > +		case "$1" in
> > +		-m|--message)
> > +			shift
> > +			stash_msg="$1"
> 
> 	${1?"-m needs an argument"}
> 
> to error check "git stash create -m<Enter>"?
> 
> > +	if test -z "$new_style"
> > +	then
> > +		stash_msg="$*"
> > +	fi
> 
> This breaks external users who do "git stash create" in the old
> fashioned way, I think, but can be easily fixed with something like:
> 
> 		stash_msg=$1 untracked=$2
> 
> If the existing tests did not catch this, I guess there is a
> coverage gap we may want to fill.  Perhaps add a new test to 3903
> that runs "git stash create message untracked" and makes sure it
> still works?

No I don't think this breaks.  It was never possible to add an
untracked argument to git stash create.  The difference is in a part
of this patch that is snipped out in your reply:

@@ -697,7 +739,7 @@ clear)
        ;;
 create)
        shift
-       create_stash "$*" && echo "$w_commit"
+       create_stash "$@" && echo "$w_commit"
        ;;
 store)
        shift

If I understand this piece correctly (I'm not very proficient in
shell, but my testing seems to agree with me), previously we used $*,
which transformed all arguments to git stash create into one argument
in create_stash.  This needed to change to $@, as otherwise we can't
pull the arguments apart for the new calling style.  The two argument
version of create_stash was only used internally in the save_stash
function.

> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> > index 0171b824c9..34e9610bb6 100755
> > --- a/t/t3903-stash.sh
> > +++ b/t/t3903-stash.sh
> > @@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
> >  	test_cmp expect actual
> >  '
> >  
> > +test_expect_success 'deprecated version of stash create stores correct message' '
> > +	>foo &&
> > +	git add foo &&
> > +	STASH_ID=$(git stash create "create test message") &&
> > +	echo "On master: create test message" >expect &&
> > +	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
> > +	test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'new style stash create stores correct message' '
> > +	>foo &&
> > +	git add foo &&
> > +	STASH_ID=$(git stash create -m "create test message new style") &&
> > +	echo "On master: create test message new style" >expect &&
> > +	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
> > +	test_cmp expect actual
> > +'
> > +
> >  test_done

-- 
Thomas

^ permalink raw reply

* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Duy Nguyen @ 2017-02-04 12:36 UTC (permalink / raw)
  To: Philip Oakley; +Cc: Jacob Keller, Git Mailing List, Jacob Keller
In-Reply-To: <E363F108ABEF45CB99C2C721A90B5EA2@PhilipOakley>

On Sat, Feb 4, 2017 at 7:16 PM, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Duy Nguyen" <pclouds@gmail.com>
>>
>> On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com>
>> wrote:
>>>
>>> +------------
>>> +$ git reset HEAD^                           <1>
>>
>>
>> It may be a good idea to add -N here, so that 'add -p' can pick up the
>> new files if they are added in HEAD.
>
>
> When looking at the man page for `reset` [1] it implies that `-N` requires
> `--mixed` also to be given. Is that correct?

Yes. But since --mixed is the default mode, "reset -N" equals "reset --mixed -N"

> Or could the man page be clearer?

If someone makes questions, I guess the answer is yes it should be made clearer.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2 2/4] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-04 12:19 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqy3xsux4g.fsf@gitster.mtv.corp.google.com>

On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Introduce a new git stash push verb in addition to git stash save.  The
> > push verb is used to transition from the current command line arguments
> > to a more conventional way, in which the message is specified after a -m
> > parameter instead of being a positional argument.
> 
> I think the canonical way to express that is "... the message is
> given as an argument to the -m option" (i.e. some options take an
> argument, some others do not, and the "-m" takes one).
> 
> > This allows introducing a new filename argument to stash single files.
> 
> I do not want them to be "a filename argument", and I do not think
> you meant them as such, either.  
> 
>     This allows us to have pathspecs at the end of the command line
>     arguments like other Git commands do, so that the user can say
>     which subset of paths to stash (and leave others behind).

Yeah, this is much better, thanks.

> > +save_stash () {
> > +	push_options=
> > +	while test $# != 0
> > +	do
> > +		case "$1" in
> > +		-k|--keep-index)
> > +...
> > +		esac
> > +		shift
> > +	done
> 
> It is a bit unfortunate that we need to duplicate the above
> case/esac here.  I do not know if doing it this way:
> 
> 	case "$1" in
> 	--)
> 		shift
> 		break 
> 		;;
> 	--help)
> 		show_help
> 		;;
> 	-*)
> 		# pass all options through to push_stash
> 		push_options="$push_options $1"
> 		;;
> 	*)
> 		break
>                 ;;
> 	esac
> 
> and letting push_stash complain for an unknown option is easier to
> maintain.

I think this will work out nicely.  Will try to implement it this way.

> You are reversing the order of the options in the loop.  Don't.

-- 
Thomas

^ permalink raw reply

* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Philip Oakley @ 2017-02-04 12:16 UTC (permalink / raw)
  To: Duy Nguyen, Jacob Keller; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <CACsJy8B2gyw7RQBh6Qfm5HxOyKWted-0ZeDfd2_U3MvWCO1HEA@mail.gmail.com>

From: "Duy Nguyen" <pclouds@gmail.com>
> On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com> 
> wrote:
>> +------------
>> +$ git reset HEAD^                           <1>
>
> It may be a good idea to add -N here, so that 'add -p' can pick up the
> new files if they are added in HEAD.

When looking at the man page for `reset` [1] it implies that `-N` requires 
`--mixed` also to be given. Is that correct? Or could the man page be 
clearer?

>
>> +$ git add -p                                <2>
> -- 
> Duy
>
Philip
[1] https://git-scm.com/docs/git-reset 


^ permalink raw reply

* Re: [PATCH 1/2] doc: add doc for git-push --recurse-submodules=only
From: Cornelius Weig @ 2017-02-04 12:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, bmwill, sbeller
In-Reply-To: <xmqqinotmrhe.fsf@gitster.mtv.corp.google.com>

Shouldn't this be part of v2.12-rc0? I just checked but it's not there.

Cheers,
  Cornelius

^ permalink raw reply

* Re: [PATCH 00/11] nd/worktree-move update
From: Duy Nguyen @ 2017-02-04 11:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <xmqqtw8bf7xn.fsf@gitster.mtv.corp.google.com>

On Sat, Feb 4, 2017 at 1:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Even if you think "the right way" is to add to the iterators, I
> suspect that we can still do incremental fixes?  I agree with the
> order of importance Michael listed in his message (i.e. the index
> and the HEAD first, and then other per-worktree hierarchies at lower
> priority), and I suspect you do, too.  I am not sure that is what
> you called "middle ground", but I think such an incremental approach
> is totally fine.

Yeah index should be fixed independently. Sorry that thought did not
occur to me. I counted HEAD as a ref too (i.e. to be managed/iterated
by refs subsystem) but I guess it's special enough to be dealt with
without ref iterator. I will try to fix these two first.

By middle ground, I was thinking that, now that we have ref iterator
interface, we more or less know what the api should look like for a
ref user like rev-list, so perhaps we could add a custom (ugly)
iterator for per-worktree refs (including reflog). The ugliness is
probably buried in refs/files-backend.c again until we clean it up.
-- 
Duy

^ permalink raw reply

* Re: How to use git show's "%<(<N>[,trunc|ltrunc|mtrunc])"?
From: Duy Nguyen @ 2017-02-04 11:49 UTC (permalink / raw)
  To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi3ThTLSp_0ZJHto4p75Ds5NMGymHrD0ju9axCqA1+NkMA@mail.gmail.com>

On Sat, Feb 4, 2017 at 12:10 AM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
>>> How do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
>>
>> You'll get an ellipsis at the end though.. (i.e. 02:13... | Hilco)
>
> Indeed, that's not very nice. I suppose I can understand the reason
> for it but it mostly defeats the purpose of "trunc", doesn't it?

It depends. When you truncate a commit subject line, you may want an
indicator that you're not seeing the whole line. For fixed fields like
dates when you expect every string to be truncated, then yes there's
not much point to show the ellipsis.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2] parse-remote: Remove reference to unused op_prep
From: siddharth @ 2017-02-04 11:36 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPPyJBD0a+Jkr7EBCurFzvHLvZY9r_SFWS2wyW7QmmP4pg@mail.gmail.com>

On Sat, Feb 04, 2017 at 04:55:45PM +0530, Pranit Bauva wrote:
> Hey SIddharth,
> 
> > Subject: parse-remote: Remove reference to unused op_prep
>                                          ^
> 
> Minor nit: after the colon, we generally don't use the word starting
> with an uppercase letter which I think can be figured out when you run
> `git log -p git-parse-remote.sh`

Oh, I am really sorry to have missed this. I will fix this and send a
third version of this patch.
> 
> On Sat, Feb 4, 2017 at 1:30 PM, Siddharth Kannan
> <kannan.siddharth12@gmail.com> wrote:
> > The error_on_missing_default_upstream helper function learned to
> > take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> > if no upstream specified", 2011-02-09), but as of 045fac5845
> > ("i18n: git-parse-remote.sh: mark strings for translation",
> >  2016-04-19), the argument is no longer used.  Remove it.
> >
> > Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> > ---
> > Thanks a lot, Pranit and Junio for your reviews on the first version of this
> > patch. I have changed the messages accordingly.
> >
> >  contrib/examples/git-pull.sh | 2 +-
> >  git-parse-remote.sh          | 3 +--
> >  git-rebase.sh                | 2 +-
> >  3 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> > index 6b3a03f..1d51dc3 100755
> > --- a/contrib/examples/git-pull.sh
> > +++ b/contrib/examples/git-pull.sh
> > @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
> >                 echo "for your current branch, you must specify a branch on the command line."
> >         elif [ -z "$curr_branch" -o -z "$upstream" ]; then
> >                 . git-parse-remote
> > -               error_on_missing_default_upstream "pull" $op_type $op_prep \
> > +               error_on_missing_default_upstream "pull" $op_type \
> >                         "git pull <remote> <branch>"
> >         else
> >                 echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
> 
> I guess I suggested you to not change the file in contrib/ in my
> earlier email[1] and to which Junio agreed too[2]. Is there any
> confusion?

Oh, you want me to completely remove the contrib/examples/ change
because that's the old shell implementation. Got it, I just checked 
the log for that file and realised that it hasn't been changed 
in a long time.

> 
> > diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> > index d3c3998..9698a05 100644
> > --- a/git-parse-remote.sh
> > +++ b/git-parse-remote.sh
> > @@ -56,8 +56,7 @@ get_remote_merge_branch () {
> >  error_on_missing_default_upstream () {
> >         cmd="$1"
> >         op_type="$2"
> > -       op_prep="$3" # FIXME: op_prep is no longer used
> > -       example="$4"
> > +       example="$3"
> >         branch_name=$(git symbolic-ref -q HEAD)
> >         display_branch_name="${branch_name#refs/heads/}"
> >         # If there's only one remote, use that in the suggestion
> > diff --git a/git-rebase.sh b/git-rebase.sh
> > index 04f6e44..b89f960 100755
> > --- a/git-rebase.sh
> > +++ b/git-rebase.sh
> > @@ -448,7 +448,7 @@ then
> >                 then
> >                         . git-parse-remote
> >                         error_on_missing_default_upstream "rebase" "rebase" \
> > -                               "against" "git rebase $(gettext '<branch>')"
> > +                               "git rebase $(gettext '<branch>')"
> >                 fi
> >
> >                 test "$fork_point" = auto && fork_point=t
> > --
> > 2.1.4
> >
> 
> [1]: http://public-inbox.org/git/CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com/
> [2]: http://public-inbox.org/git/xmqqd1ey8rul.fsf@gitster.mtv.corp.google.com/

^ permalink raw reply

* Re: [PATCH v2] parse-remote: Remove reference to unused op_prep
From: Pranit Bauva @ 2017-02-04 11:25 UTC (permalink / raw)
  To: Siddharth Kannan; +Cc: Git List
In-Reply-To: <1486195204-26901-1-git-send-email-kannan.siddharth12@gmail.com>

Hey SIddharth,

> Subject: parse-remote: Remove reference to unused op_prep
                                         ^

Minor nit: after the colon, we generally don't use the word starting
with an uppercase letter which I think can be figured out when you run
`git log -p git-parse-remote.sh`

On Sat, Feb 4, 2017 at 1:30 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> The error_on_missing_default_upstream helper function learned to
> take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> if no upstream specified", 2011-02-09), but as of 045fac5845
> ("i18n: git-parse-remote.sh: mark strings for translation",
>  2016-04-19), the argument is no longer used.  Remove it.
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---
> Thanks a lot, Pranit and Junio for your reviews on the first version of this
> patch. I have changed the messages accordingly.
>
>  contrib/examples/git-pull.sh | 2 +-
>  git-parse-remote.sh          | 3 +--
>  git-rebase.sh                | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> index 6b3a03f..1d51dc3 100755
> --- a/contrib/examples/git-pull.sh
> +++ b/contrib/examples/git-pull.sh
> @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
>                 echo "for your current branch, you must specify a branch on the command line."
>         elif [ -z "$curr_branch" -o -z "$upstream" ]; then
>                 . git-parse-remote
> -               error_on_missing_default_upstream "pull" $op_type $op_prep \
> +               error_on_missing_default_upstream "pull" $op_type \
>                         "git pull <remote> <branch>"
>         else
>                 echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"

I guess I suggested you to not change the file in contrib/ in my
earlier email[1] and to which Junio agreed too[2]. Is there any
confusion?

> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index d3c3998..9698a05 100644
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -56,8 +56,7 @@ get_remote_merge_branch () {
>  error_on_missing_default_upstream () {
>         cmd="$1"
>         op_type="$2"
> -       op_prep="$3" # FIXME: op_prep is no longer used
> -       example="$4"
> +       example="$3"
>         branch_name=$(git symbolic-ref -q HEAD)
>         display_branch_name="${branch_name#refs/heads/}"
>         # If there's only one remote, use that in the suggestion
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 04f6e44..b89f960 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -448,7 +448,7 @@ then
>                 then
>                         . git-parse-remote
>                         error_on_missing_default_upstream "rebase" "rebase" \
> -                               "against" "git rebase $(gettext '<branch>')"
> +                               "git rebase $(gettext '<branch>')"
>                 fi
>
>                 test "$fork_point" = auto && fork_point=t
> --
> 2.1.4
>

[1]: http://public-inbox.org/git/CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com/
[2]: http://public-inbox.org/git/xmqqd1ey8rul.fsf@gitster.mtv.corp.google.com/

^ permalink raw reply

* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Duy Nguyen @ 2017-02-04 11:06 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <20170203202833.17666-1-jacob.e.keller@intel.com>

On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> +------------
> +$ git reset HEAD^                           <1>

It may be a good idea to add -N here, so that 'add -p' can pick up the
new files if they are added in HEAD.

> +$ git add -p                                <2>
-- 
Duy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox