* Re: grep open pull requests
From: Jeff King @ 2017-01-19 22:24 UTC (permalink / raw)
To: Jack Bates; +Cc: git
In-Reply-To: <9f59c0d7-73d3-1b4c-65ca-700d6e1f4f23@nottheoilrig.com>
On Thu, Jan 19, 2017 at 03:12:53PM -0700, Jack Bates wrote:
> Cool, thanks for all your help! "git log --cherry-pick" works quite well.
> One thing: I expected the following to be equivalent, but found that they're
> not. Is that by accident or design?
>
> $ git rev-list --cherry-pick --right-only master...refs/pull/1112/head
> $ git rev-list --cherry-pick master..refs/pull/1112/head
It's by design. The "left" and "right" notions are defined only for a
three-dot symmetric difference.
In the first command you've asked git to look at commits on _both_
master and the PR, down to their merge base. It marks the tips with a
"left" and "right" bit, and then those bits propagate down.
In the second command, you've only asked for the PR commits, and there
is no left/right bit at all. So --cherry-pick is doing nothing, as it
has no "left" commits to compare to.
-Peff
^ permalink raw reply
* Re: grep open pull requests
From: Jack Bates @ 2017-01-19 22:12 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170119190227.opjisryyqn766tqy@sigill.intra.peff.net>
On 19/01/17 12:02 PM, Jeff King wrote:
> It's much trickier to find from the git topology whether a particular
> history contains rebased versions of commits. You can look at the
> --cherry options to "git log", which use patch-ids to try to equate
> commits. Something like:
>
> git for-each-ref --format='%(refname)' 'refs/pull/*/head' |
> while read refname; do
> if test -z "$(git rev-list --right-only --cherry-pick -1 origin...$refname)
> then
> echo "$refname: not merged"
> fi
> done
>
> That's obviously much less efficient than `--no-merged`, but it should
> generally work. The exception is if the rebase changed the commit
> sufficiently that its patch-id may have changed.
Cool, thanks for all your help! "git log --cherry-pick" works quite
well. One thing: I expected the following to be equivalent, but found
that they're not. Is that by accident or design?
$ git rev-list --cherry-pick --right-only master...refs/pull/1112/head
$ git rev-list --cherry-pick master..refs/pull/1112/head
> I think that's probably the best answer to your "unmerged" question,
> too. Ask the API which PRs are unmerged, and then do whatever git-level
> analysis you want based on that.
Right, that makes sense. Thanks again!
^ permalink raw reply
* Re: [RESEND PATCHv2] contrib: remove git-convert-objects
From: Stefan Beller @ 2017-01-19 21:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Linus Torvalds
In-Reply-To: <xmqqo9z2zs2o.fsf@gitster.mtv.corp.google.com>
On Thu, Jan 19, 2017 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> git-convert-objects, originally named git-convert-cache was used in
>> early 2005 to convert to a new repository format, e.g. adding an author
>> date.
>
> I think this description is not wrong per-se but misses the much
> more important point. In the very early days of Git, the objects
> were named after SHA-1 of deflated loose object representation,
> which meant that tweak in zlib or change of compression level would
> give the same object different names X-<. This program was to
> convert an ancient history with these objects and rewrite them to
> match the new object naming scheme where the name comes from a hash
> of the inflated representation.
ok, in case I reroll again, I'll fixup the message.
>
>> By now the need for conversion of the very early repositories is less
>> relevant, we no longer need to keep it in contrib; remove it.
>
> I am not sure if removal of it matters, and I suspect that we saw no
> reaction from anybody because nobody thought it deserves the
> brain-cycle to decide whether to remove it. I dunno.
I do think removing this would improve contrib/, not just because
it would better align with contribs mission statement in its README, but
also for other reasons. Why would a user look into contrib/ at all?
* to find interesting contemporary bits and pieces
* if they want to find old stuff for educational purposes, they ought to
be looking into contrib/examples instead.
So maybe instead of this patch, just move it to the examples section?
(That way we archive the same goal: a cleaner, fresher contrib/
that doesn't look as stale)
Thanks,
Stefan
^ permalink raw reply
* Re: [RFC for GIT] pull-request: add praise to people doing QA
From: Wolfram Sang @ 2017-01-19 22:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, linux-kernel
In-Reply-To: <xmqq7f5qzqx3.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]
> > I didn't know about trailers before. As I undestand it, I could use
> > "Tested-by" as the key, and the commit subject as the value. This list
> > then could be parsed and brought into proper output shape. It would
> > simplify the subject parsing, but most things my AWK script currently
> > does would still need to stay or to be reimplemented (extracting names
> > from tags, creating arrays of tags given by $name). Am I correct?
>
> That is not exactly what I had in mind. I was wondering if we can
> do without any external script, implementing the logic you added
> inside shortlog with an extra option that triggers the whole thing,
> which may call into the same trailers API as used by the
> interpret-trailers command to do the parsing and picking out parts.
Sorry for being unclear. That's what I meant with "or to be
reimplemented". I should have added "in C".
I am afraid this also requires more time than I am willing to
spend on this issue. Seems my hack is going to stay for a while here.
However, thank you for your time and assisting me with pointers!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Git: new feature suggestion
From: Stefan Beller @ 2017-01-19 22:03 UTC (permalink / raw)
To: Joao Pinto
Cc: Linus Torvalds, Konstantin Khomoutov, Git Mailing List,
CARLOS.PALMINHA@synopsys.com
In-Reply-To: <5207b04e-5e80-7100-4328-7e87ee619aea@synopsys.com>
On Thu, Jan 19, 2017 at 1:51 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
> Às 7:16 PM de 1/19/2017, Linus Torvalds escreveu:
>> On Thu, Jan 19, 2017 at 10:54 AM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>
>>> I am currently facing some challenges in one of Linux subsystems where a rename
>>> of a set of folders and files would be the perfect scenario for future
>>> development, but the suggestion is not accepted, not because it's not correct,
>>> but because it makes the maintainer life harder in backporting bug fixes and new
>>> features to older kernel versions and because it is not easy to follow the
>>> renamed file/folder history from the kernel.org history logs.
>>
>> Honestly, that's less of a git issue, and more of a "patch will not
>> apply across versions" issue.
>>
>> No amount of rename detection will ever fix that, simply because the
>> rename hadn't even _happened_ in the old versions that things get
>> backported to.
>>
>> ("git cherry-pick" can do a merge resolution and thus do "backwards"
>> renaming too, so tooling can definitely help, but it still ends up
>> meaning that even trivial patches are no longer the _same_ trivial
>> patch across versions).
>>
>> So renaming things increases maintainer workloads in those situations
>> regardless of any tooling issues.
>>
>> (You may also be referring to the mellanox mess, where this issue is
>> very much exacerbated by having different groups working on the same
>> thing, and maintainers having very much a "I will not take _anything_
>> from any of the groups that makes my life more complicated" model,
>> because those groups fucked up so much in the past).
>>
>> In other words, quite often issues are about workflows rather than
>> tools. The networking layer probably has more of this, because David
>> actually does the backports himself, so he _really_ doesn't want to
>> complicate things.
>
> I totally understand David' side! Synopsys is a well-known IP Vendor, and for a
> long time its focus was the IP only. Knowadays the strategy has changed and
> Synopsys is very keen to help in Open Source, namelly Linux, developing the
> drivers for new IP Cores and participating in the improvement of existing ones.
> I am part of the team that has that job.
>
> In USB and PCI subystems developers created common Synopsys drivers (focused on
> the HW IP) and so today they are massively used by all the SoC that use Synopsys
> IP.
>
> In the network subsystem, there are some drivers that target the same IP but
> were made by different companies. stmmac is an excelent driver for Synopsys MAC
> 10/100/1000/QOS IPs, but there was another driver made by AXIS driver that also
> targeted the QOS IP. We detected that issue and merged the AXIS specific driver
> ops to stmmac, and nowadays, AXIS uses stmmac. So less drivers to maintain!
>
> The idea that was rejected consisted of renaming stmicro/stmmac to dwc/stmmac
> and to have dwc (designware controllers) as the official driver spot for
> Synopsys Ethernet IPs.
> There is another example of duplication, which is AMD' and Samsung' XGMAC
> driver, targeting the same Synopsys XGMAC IP.
>
> I am giving this examples because although the refactor adds work for
> backporting, it reduces the maintenance since we would have less duplicated
> drivers as we have today.
This sounds as if the code in question would only receive backports
for a specific
time (determined by HW lifecycle, maintenance life cycle and such).
So I wonder if this could be solved by not just renaming but
additionally adding a
symbolic link, such that the files in question seem to appear twice on
the file system.
Then backports ought to be applicable (hoping git-am doesn't choke on symlinks),
and after a while once the there no backports any more (due to life
cycle reasons),
remove the link?
This also sounds like a kind of problem, that others have run into before,
how did they solve it?
Thanks,
Stefan
>
> Thanks,
> Joao
>
>
>> Linus
>>
>
^ permalink raw reply
* Re: merge maintaining history
From: Philip Oakley @ 2017-01-19 21:58 UTC (permalink / raw)
To: Jacob Keller, David J. Bakeman; +Cc: Git mailing list
In-Reply-To: <5880BB23.8030702@comcast.net>
From: "David J. Bakeman" <nakuru@comcast.net>
> On 01/14/2017 10:24 PM, Jacob Keller wrote:
>> On Fri, Jan 13, 2017 at 6:01 PM, David J. Bakeman <nakuru@comcast.net>
>> wrote:
>>> History
>>>
>>> git cloned a remote repository and made many changes pushing them all to
>>> said repository over many months.
>>>
>>> The powers that be then required me to move project to new repository
>>> server did so by pushing local version to new remote saving all history!
>>>
>>> Now have to merge back to original repository(which has undergone many
>>> changes since I split off) but how do I do that without loosing the
>>> history of all the commits since the original move? Note I need to push
>>> changes to files that are already in existence. I found on the web a
>>> bunch of ways to insert a whole new directory structure into an existing
>>> repository but as I said I need to do it on top of existing files. Of
>>> course I can copy all the files from my local working repository to the
>>> cloned remote repository and commit any changes but I loose all the
>>> history that way.
>>>
>>> Thanks.
>> If I understand it.. you have two remotes now:
>>
>> The "origin" remote, which was the original remote you started with.
>>
>> You have now a "new" remote which you created and pushed to.
>>
>> So you want to merge the "new" history into the original tree now, so
>> you checkout the original tree, then "git merge <new-remote>/<branch>"
>> and then fix up any conflicts, and then git commit to create a merge
>> commit that has the new history. Then you could push that to both
>> trees.
>>
>> I would want a bit more information about your setup before providing
>> actual commands.
> Thanks I think that's close but it's a little more complicated I think
> :<( I don't know if this diagram will work but lets try.
>
> original A->B->C->D->E->F
> \
> first branch b->c->d->e
>
> new repo e->f->g->h
>
> Now I need to merge h to F without loosing b through h hopefully. Yes e
> was never merged back to the original repo and it's essentially gone now
> so I can't just merge to F or can I?
a quick bikeshed..
You have both repositories, so nothing is lost.
Do note that 'e' commit sha1 in the first branch will be different from the
sha1 of 'e' commit in the new repo, but both should have the same top level
tree if they are truly identical.
You can fetch both repositories into a single common repo so you will have
an --orphan branch of the new repo.
Consider adding a --allow-empty commit e' to the original repo to note what
happens next (and how to do it), which is to use the replace mechanism to
bridge the gap between the e of the old repo and the e of the new repo by
making e of the new repo replace the e' you just created.
This will make it look like 'h' is the natural development of 'a'. Even if e
& e were not tree-same, you will be able to see a diff. You can now merge
'h' onto 'F' in whatever way you find appropriate to give the right view of
the development.
When you push this back upstream, note that the 'replace' is local, so
upstream sees a gap, but that commit e' has the instructions to rebuild the
link, should others require it (you may have to push the e' commit
separately).
--
Philip
^ permalink raw reply
* Re: Git: new feature suggestion
From: Joao Pinto @ 2017-01-19 21:51 UTC (permalink / raw)
To: Linus Torvalds, Joao Pinto
Cc: Konstantin Khomoutov, Git Mailing List,
CARLOS.PALMINHA@synopsys.com
In-Reply-To: <CA+55aFzGaxhRRHXUcfnUDcgyaAKy4jXLcKMXH8T61x8sxEJT+g@mail.gmail.com>
Às 7:16 PM de 1/19/2017, Linus Torvalds escreveu:
> On Thu, Jan 19, 2017 at 10:54 AM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>
>> I am currently facing some challenges in one of Linux subsystems where a rename
>> of a set of folders and files would be the perfect scenario for future
>> development, but the suggestion is not accepted, not because it's not correct,
>> but because it makes the maintainer life harder in backporting bug fixes and new
>> features to older kernel versions and because it is not easy to follow the
>> renamed file/folder history from the kernel.org history logs.
>
> Honestly, that's less of a git issue, and more of a "patch will not
> apply across versions" issue.
>
> No amount of rename detection will ever fix that, simply because the
> rename hadn't even _happened_ in the old versions that things get
> backported to.
>
> ("git cherry-pick" can do a merge resolution and thus do "backwards"
> renaming too, so tooling can definitely help, but it still ends up
> meaning that even trivial patches are no longer the _same_ trivial
> patch across versions).
>
> So renaming things increases maintainer workloads in those situations
> regardless of any tooling issues.
>
> (You may also be referring to the mellanox mess, where this issue is
> very much exacerbated by having different groups working on the same
> thing, and maintainers having very much a "I will not take _anything_
> from any of the groups that makes my life more complicated" model,
> because those groups fucked up so much in the past).
>
> In other words, quite often issues are about workflows rather than
> tools. The networking layer probably has more of this, because David
> actually does the backports himself, so he _really_ doesn't want to
> complicate things.
I totally understand David' side! Synopsys is a well-known IP Vendor, and for a
long time its focus was the IP only. Knowadays the strategy has changed and
Synopsys is very keen to help in Open Source, namelly Linux, developing the
drivers for new IP Cores and participating in the improvement of existing ones.
I am part of the team that has that job.
In USB and PCI subystems developers created common Synopsys drivers (focused on
the HW IP) and so today they are massively used by all the SoC that use Synopsys
IP.
In the network subsystem, there are some drivers that target the same IP but
were made by different companies. stmmac is an excelent driver for Synopsys MAC
10/100/1000/QOS IPs, but there was another driver made by AXIS driver that also
targeted the QOS IP. We detected that issue and merged the AXIS specific driver
ops to stmmac, and nowadays, AXIS uses stmmac. So less drivers to maintain!
The idea that was rejected consisted of renaming stmicro/stmmac to dwc/stmmac
and to have dwc (designware controllers) as the official driver spot for
Synopsys Ethernet IPs.
There is another example of duplication, which is AMD' and Samsung' XGMAC
driver, targeting the same Synopsys XGMAC IP.
I am giving this examples because although the refactor adds work for
backporting, it reduces the maintenance since we would have less duplicated
drivers as we have today.
Thanks,
Joao
> Linus
>
^ permalink raw reply
* Re: [PATCH v2 2/2] Be more careful when determining whether a remote was configured
From: Jeff King @ 2017-01-19 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Thomas Gummerer, Andrew Arnott
In-Reply-To: <xmqqy3y6yb9i.fsf@gitster.mtv.corp.google.com>
On Thu, Jan 19, 2017 at 01:45:29PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I'm trying to figure out why "fetch --multiple" wouldn't just take a url
> > in the first place. I guess it is because multiple fetch is useless
> > without refspecs (since otherwise you're just writing to FETCH_HEAD,
> > which gets immediately overwritten).
>
> This is probably a tangent, if FETCH_HEAD is overwritten, wouldn't
> that be a bug in the implementation of --multiple? I somehow
> thought we had an option to tell second and subsequent "fetch" to
> append to FETCH_HEAD instead of overwriting it.
Maybe. I was just speculating on the reason.
I just disabled that check and tried it with two local repos:
git init
git fetch --multiple /path/to/one /path/to/two
cat .git/FETCH_HEAD
and indeed it does seem to append.
The logic comes from 9c4a036b3 (Teach the --all option to 'git fetch',
2009-11-09), but it does not seem to give any reasoning. Nor could I
find anything on the mailing list. <shrug>
-Peff
^ permalink raw reply
* Re: Git: new feature suggestion
From: Jakub Narębski @ 2017-01-19 21:48 UTC (permalink / raw)
To: Linus Torvalds, Konstantin Khomoutov
Cc: Joao Pinto, Git Mailing List, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <CA+55aFxAe8bH2xXkx1p5gYN+nc-D-vjNnfUeA_64Q3ttpbHq+w@mail.gmail.com>
W dniu 19.01.2017 o 19:39, Linus Torvalds pisze:
> On Wed, Jan 18, 2017 at 10:33 PM, Konstantin Khomoutov
> <kostix+git@007spb.ru> wrote:
>>
>> Still, I welcome you to read the sort-of "reference" post by Linus
>> Torvalds [1] in which he explains the reasoning behind this approach
>> implemented in Git.
>
> It's worth noting that that discussion was from some _very_ early days
> in git (one week into the whole thing), when none of those
> visualization tools were actually implemented.
>
> Even now, ten years after the fact, plain git doesn't actually do what
> I outlined. Yes, "git blame -Cw" works fairly well, and is in general
> better than the traditional per-file "annotate". And yes, "git log
> --follow" does another (small) part of the outlined thing, but is
> really not very powerful.
It is really a pity that "git log --follow" is so limited; it's
development stopped at early 'good enough' implementation.
For example "git log --follow gitweb/gitweb.perl" would not show
the whole history of a file (which was once independent project),
and "git log --follow" doesn't work for directories or multiple
files.
>
> Some tools on top of git do more, but I think in general this is an
> area that could easily be improved upon. For example, the whole
> iterative and interactive drilling down in history of a particular
> file is very inconvenient to do with "git blame" (you find a commit
> that change the area in some way that you don't find interesting, so
> then you have to restart git blame with the parent of that
> unintersting commit).
>
> You can do it in tig, but I suspect a more graphical tool might be better.
Well, we do have "git gui blame".
[...]
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v2 2/2] Be more careful when determining whether a remote was configured
From: Junio C Hamano @ 2017-01-19 21:45 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, Thomas Gummerer, Andrew Arnott
In-Reply-To: <20170119213100.g72ml7r2khu7bvey@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'm trying to figure out why "fetch --multiple" wouldn't just take a url
> in the first place. I guess it is because multiple fetch is useless
> without refspecs (since otherwise you're just writing to FETCH_HEAD,
> which gets immediately overwritten).
This is probably a tangent, if FETCH_HEAD is overwritten, wouldn't
that be a bug in the implementation of --multiple? I somehow
thought we had an option to tell second and subsequent "fetch" to
append to FETCH_HEAD instead of overwriting it.
^ permalink raw reply
* Re: [PATCH v2 2/2] Be more careful when determining whether a remote was configured
From: Johannes Schindelin @ 2017-01-19 21:44 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Thomas Gummerer, Andrew Arnott
In-Reply-To: <20170119213100.g72ml7r2khu7bvey@sigill.intra.peff.net>
Hi Peff,
On Thu, 19 Jan 2017, Jeff King wrote:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index c85f3471d..9024cfffa 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1014,9 +1014,9 @@ static int add_remote_or_group(const char *name, struct string_list *list)
> git_config(get_remote_group, &g);
> if (list->nr == prev_nr) {
> struct remote *remote;
> - if (!remote_is_configured(name))
> - return 0;
> remote = remote_get(name);
> + if (!remote->fetch_refspec_nr)
> + return 0;
Please note that it is legitimate to fetch from foreign vcs, in which case
the fetch refspecs may not be required (or even make sense).
> It's outside the scope of your patches, so I think we are OK to just
> ignore it. But if you want to pursue it, it avoids having to add the
> extra parameter to remote_is_configured().
Sure, it would avoid that. But that parameter makes semantic sense: some
callers may want to have all remotes, even those configured via the
command-line, and others really are only interested in knowing whether the
current repository config already has settings for a remote of the given
name.
> > Many thanks to Jeff King whose tireless review helped with settling for
> > nothing less than the current strategy.
>
> Just how I wanted to be immortalized in git's commit history. ;)
You are welcome ;-)
Ciao,
Johannes
^ permalink raw reply
* Re: merge maintaining history
From: Junio C Hamano @ 2017-01-19 21:42 UTC (permalink / raw)
To: David J. Bakeman; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <5880BB23.8030702@comcast.net>
"David J. Bakeman" <nakuru@comcast.net> writes:
>> So you want to merge the "new" history into the original tree now, so
>> you checkout the original tree, then "git merge <new-remote>/<branch>"
>> and then fix up any conflicts, and then git commit to create a merge
>> commit that has the new history. Then you could push that to both
>> trees.
>>
>> I would want a bit more information about your setup before providing
>> actual commands.
>
> Thanks I think that's close but it's a little more complicated I think
> :<( I don't know if this diagram will work but lets try.
>
> original A->B->C->D->E->F
> \
> first branch b->c->d->e
>
> new repo e->f->g->h
>
> Now I need to merge h to F without loosing b through h hopefully. Yes e
> was never merged back to the original repo and it's essentially gone now
> so I can't just merge to F or can I?
With the picture, I think you mean 'b' is forked from 'B' and the
first branch built 3 more commits on top, leading to 'e'.
You say "new repo" has 'e' thru 'h', and I take it to mean you
started developing on top of the history that leads to 'e' you built
in the first branch, and "new repo" has the resulting history that
leads to 'h'.
Unless you did something exotic and non-standard, commit 'e' in "new
repo" would be exactly the same as 'e' sitting on the tip of the
"first branch", so the picture would be more like:
> original A->B->C->D->E->F
> \
> first branch b->c->d->e
> \
> new repo f->g->h
no? Then merging 'h' into 'F' will pull everything you did since
you diverged from the history that leads to 'F', resulting in a
history of this shape:
> original A->B->C->D->E->F----------M
> \ /
> first branch b->c->d->e /
> \ /
> new repo f->g->h
If on the other hand you did something non-standard and exotic to
rewrite 'e' at the end of "first branch" and make a different commit
that does not even have any parent in "new repo", and the history of
"new repo" originates in such a commit that is not 'e', things will
become messy. But I didn't think I read you did anything unusual so
a simple "git checkout F && git merge h" should give you what you
want.
^ permalink raw reply
* [PATCH v2 2/2] Be more careful when determining whether a remote was configured
From: Johannes Schindelin @ 2017-01-19 21:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Gummerer, Andrew Arnott, Jeff King
In-Reply-To: <cover.1484860744.git.johannes.schindelin@gmx.de>
One of the really nice features of the ~/.gitconfig file is that users
can override defaults by their own preferred settings for all of their
repositories.
One such default that some users like to override is whether the
"origin" remote gets auto-pruned or not. The user would simply call
git config --global remote.origin.prune true
and from now on all "origin" remotes would be pruned automatically when
fetching into the local repository.
There is just one catch: now Git thinks that the "origin" remote is
configured, even if the repository config has no [remote "origin"]
section at all, as it does not realize that the "prune" setting was
configured globally and that there really is no "origin" remote
configured in this repository.
That is a problem e.g. when renaming a remote to a new name, when Git
may be fooled into thinking that there is already a remote of that new
name.
Let's fix this by paying more attention to *where* the remote settings
came from: if they are configured in the local repository config, we
must not overwrite them. If they were configured elsewhere, we cannot
overwrite them to begin with, as we only write the repository config.
There is only one caller of remote_is_configured() (in `git fetch`) that
may want to take remotes into account even if they were configured
outside the repository config; all other callers essentially try to
prevent the Git command from overwriting settings in the repository
config.
To accommodate that fact, the remote_is_configured() function now
requires a parameter that states whether the caller is interested in all
remotes, or only in those that were configured in the repository config.
Many thanks to Jeff King whose tireless review helped with settling for
nothing less than the current strategy.
This fixes https://github.com/git-for-windows/git/issues/888
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/fetch.c | 2 +-
builtin/remote.c | 14 +++++++-------
remote.c | 12 ++++++++++--
remote.h | 4 ++--
t/t5505-remote.sh | 2 +-
5 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f1570e3464..b5ad09d046 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1177,7 +1177,7 @@ static int add_remote_or_group(const char *name, struct string_list *list)
git_config(get_remote_group, &g);
if (list->nr == prev_nr) {
struct remote *remote = remote_get(name);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 0))
return 0;
string_list_append(list, remote->name);
}
diff --git a/builtin/remote.c b/builtin/remote.c
index e52cf3925b..5339ed6ad1 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -186,7 +186,7 @@ static int add(int argc, const char **argv)
url = argv[1];
remote = remote_get(name);
- if (remote_is_configured(remote))
+ if (remote_is_configured(remote, 1))
die(_("remote %s already exists."), name);
strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
@@ -618,14 +618,14 @@ static int mv(int argc, const char **argv)
rename.remote_branches = &remote_branches;
oldremote = remote_get(rename.old);
- if (!remote_is_configured(oldremote))
+ if (!remote_is_configured(oldremote, 1))
die(_("No such remote: %s"), rename.old);
if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG)
return migrate_file(oldremote);
newremote = remote_get(rename.new);
- if (remote_is_configured(newremote))
+ if (remote_is_configured(newremote, 1))
die(_("remote %s already exists."), rename.new);
strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
@@ -753,7 +753,7 @@ static int rm(int argc, const char **argv)
usage_with_options(builtin_remote_rm_usage, options);
remote = remote_get(argv[1]);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote: %s"), argv[1]);
known_remotes.to_delete = remote;
@@ -1415,7 +1415,7 @@ static int set_remote_branches(const char *remotename, const char **branches,
strbuf_addf(&key, "remote.%s.fetch", remotename);
remote = remote_get(remotename);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
if (!add_mode && remove_all_fetch_refspecs(remotename, key.buf)) {
@@ -1469,7 +1469,7 @@ static int get_url(int argc, const char **argv)
remotename = argv[0];
remote = remote_get(remotename);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
url_nr = 0;
@@ -1537,7 +1537,7 @@ static int set_url(int argc, const char **argv)
oldurl = newurl;
remote = remote_get(remotename);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
if (push_mode) {
diff --git a/remote.c b/remote.c
index ad6c5424ed..8524135de4 100644
--- a/remote.c
+++ b/remote.c
@@ -255,6 +255,7 @@ static void read_remotes_file(struct remote *remote)
if (!f)
return;
+ remote->configured_in_repo = 1;
remote->origin = REMOTE_REMOTES;
while (strbuf_getline(&buf, f) != EOF) {
const char *v;
@@ -289,6 +290,7 @@ static void read_branches_file(struct remote *remote)
return;
}
+ remote->configured_in_repo = 1;
remote->origin = REMOTE_BRANCHES;
/*
@@ -371,6 +373,8 @@ static int handle_config(const char *key, const char *value, void *cb)
}
remote = make_remote(name, namelen);
remote->origin = REMOTE_CONFIG;
+ if (current_config_scope() == CONFIG_SCOPE_REPO)
+ remote->configured_in_repo = 1;
if (!strcmp(subkey, "mirror"))
remote->mirror = git_config_bool(key, value);
else if (!strcmp(subkey, "skipdefaultupdate"))
@@ -714,9 +718,13 @@ struct remote *pushremote_get(const char *name)
return remote_get_1(name, pushremote_for_branch);
}
-int remote_is_configured(struct remote *remote)
+int remote_is_configured(struct remote *remote, int in_repo)
{
- return remote && remote->origin;
+ if (!remote)
+ return 0;
+ if (in_repo)
+ return remote->configured_in_repo;
+ return !!remote->origin;
}
int for_each_remote(each_remote_fn fn, void *priv)
diff --git a/remote.h b/remote.h
index 924881169d..a5bbbe0ef9 100644
--- a/remote.h
+++ b/remote.h
@@ -15,7 +15,7 @@ struct remote {
struct hashmap_entry ent; /* must be first */
const char *name;
- int origin;
+ int origin, configured_in_repo;
const char *foreign_vcs;
@@ -60,7 +60,7 @@ struct remote {
struct remote *remote_get(const char *name);
struct remote *pushremote_get(const char *name);
-int remote_is_configured(struct remote *remote);
+int remote_is_configured(struct remote *remote, int in_repo);
typedef int each_remote_fn(struct remote *remote, void *priv);
int for_each_remote(each_remote_fn fn, void *priv);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 2c03f44c85..ba46e86de0 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -764,7 +764,7 @@ test_expect_success 'rename a remote with name prefix of other remote' '
)
'
-test_expect_failure 'rename succeeds with existing remote.<target>.prune' '
+test_expect_success 'rename succeeds with existing remote.<target>.prune' '
git clone one four.four &&
test_when_finished git config --global --unset remote.upstream.prune &&
git config --global remote.upstream.prune true &&
--
2.11.0.windows.3
^ permalink raw reply related
* Re: [RFC] stash --continue
From: Johannes Schindelin @ 2017-01-19 21:30 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Stephan Beyer, git
In-Reply-To: <aa8104ad-45f4-7bef-f199-6e6021cf0c06@xiplink.com>
Hi Marc,
On Thu, 19 Jan 2017, Marc Branchaud wrote:
> On 2017-01-19 10:49 AM, Johannes Schindelin wrote:
> >
> > On Wed, 18 Jan 2017, Marc Branchaud wrote:
> >
> > > On 2017-01-18 11:34 AM, Johannes Schindelin wrote:
> > > >
> > > > On Wed, 18 Jan 2017, Marc Branchaud wrote:
> > > >
> > > > > On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
> > > > >
> > > > > > On Mon, 16 Jan 2017, Stephan Beyer wrote:
> > > > > >
> > > > > > > a git-newbie-ish co-worker uses git-stash sometimes. Last
> > > > > > > time he used "git stash pop", he got into a merge conflict.
> > > > > > > After he resolved the conflict, he did not know what to do
> > > > > > > to get the repository into the wanted state. In his case, it
> > > > > > > was only "git add <resolved files>" followed by a "git
> > > > > > > reset" and a "git stash drop", but there may be more
> > > > > > > involved cases when your index is not clean before "git
> > > > > > > stash pop" and you want to have your index as before.
> > > > > > >
> > > > > > > This led to the idea to have something like "git stash
> > > > > > > --continue"[1]
> > > > > >
> > > > > > More like "git stash pop --continue". Without the "pop"
> > > > > > command, it does not make too much sense.
> > > > >
> > > > > Why not? git should be able to remember what stash command
> > > > > created the conflict. Why should I have to? Maybe the fire
> > > > > alarm goes off right when I run the stash command, and by the
> > > > > time I get back to it I can't remember which operation I did.
> > > > > It would be nice to be able to tell git to "just finish off (or
> > > > > abort) the stash operation, whatever it was".
> > > >
> > > > That reeks of a big potential for confusion.
> > > >
> > > > Imagine for example a total Git noob who calls `git stash list`,
> > > > scrolls two pages down, then hits `q` by mistake. How would you
> > > > explain to that user that `git stash --continue` does not continue
> > > > showing the list at the third page?
> > >
> > > Sorry, but I have trouble taking that example seriously. It assumes
> > > such a level of "noobness" that the user doesn't even understand how
> > > standard command output paging works, not just with git but with any
> > > shell command.
> >
> > Yeah, well, I thought you understood what I meant.
> >
> > The example was the best I could come up with quickly, and it only
> > tried to show that there are *other* stash operations that one might
> > perceive to happen at the same time as the "pop" operation, so your
> > flimsical comment "why not continue the latest operation" may very
> > well be ambiguous.
> >
> > And if it is not ambiguous in "stash", it certainly will be in other
> > Git operations. And therefore, having a DWIM in "stash" to allow
> > "--continue" without any specific subcommand, but not having it in
> > other Git commands, is just a very poor user interface design. It is
> > prone to confuse users, which is always a hallmark of a bad user
> > interface.
>
> Please don't underestimate the power of syntactic consistency in helping
> users achieve their goals. Having some commands use "git foo
> --continue" while others use "git foo bar --continue" *will* confuse
> people, regardless of how logical the reasons for those differences.
But that ship has already sailed!
By your reasoning, it was a mistake to introduce subcommands such as `git
stash pop` in the first place.
> But in the case of stash, I still don't see the utility in having
> operation-specific continuation.
Stephan already gave one good example where you want it: if `git stash
pop` fails, you may want to continue by *not* dropping the stash via `git
stash apply --continue`.
> Consider the following sequence (as you say, this doesn't work yet, but
> making it work seems reasonable):
>
> git stash pop # creates some conflicts
> git stash apply stash@{4} # creates some other conflicts
> # (User resolves the conflicts created by the pop.)
> git stash pop --continue
Yes, that would make sense: the `pop` would actually drop the stash entry.
> Given the description of the original proposal (do "git reset; git stash
> drop"), what's the state of the index and the working tree?
>
> In particular, what has the user gained by continuing just that pop?
That it was completed.
> Another thing to ask is, how common is such a scenario likely to be?
We have millions of users. Even a 0.001% chance of anything happening will
bite users, who will then come back to bite us.
Let's just not go into the "how likely is this" game.
> I suggest that it will be far more common for users to resolve all the
> conflicts and then want to continue all their interrupted stash
> operations. If so, fussily forcing them to explicitly continue the pop
> and the apply is just a waste.
No, it is not a waste, as we require the user nothing else than to be
precise. Do you want to continue the "stash pop"? Okay, then, call "git
stash pop --continue".
> [... a lot of stuff skipped, as it is basically ignoring my point...]
>
> > But foo *is the operation*! By that reasoning, you should agree that
> > "git stash --continue" is *wrong*!
>
> No, in the user's mind *stash* is the operation!
How can that be true? In the user's mind, the *stash* operation is
equivalent to the *stash save* operation! Because that is what happens
when you call "git stash" without any further command-line parameters.
At this point I will stop commenting on this issue, as I have said all
that I wanted to say about it, at least once. If I failed to get my points
across so far, I simply won't be understood.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] Be more careful when determining whether a remote was configured
From: Jeff King @ 2017-01-19 21:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Thomas Gummerer, Andrew Arnott
In-Reply-To: <1605031b76025f4bd0e485705c34a25557bb75a1.1484860744.git.johannes.schindelin@gmx.de>
On Thu, Jan 19, 2017 at 10:20:02PM +0100, Johannes Schindelin wrote:
> There is only one caller of remote_is_configured() (in `git fetch`) that
> may want to take remotes into account even if they were configured
> outside the repository config; all other callers essentially try to
> prevent the Git command from overwriting settings in the repository
> config.
>
> To accommodate that fact, the remote_is_configured() function now
> requires a parameter that states whether the caller is interested in all
> remotes, or only in those that were configured in the repository config.
Just to make sure I understand the issue, the problem is that:
git config --global remote.foo.url whatever
git fetch --multiple foo bar
would not work without this part of the patch?
I'm trying to figure out why "fetch --multiple" wouldn't just take a url
in the first place. I guess it is because multiple fetch is useless
without refspecs (since otherwise you're just writing to FETCH_HEAD,
which gets immediately overwritten). I wonder if that test really should
be:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c85f3471d..9024cfffa 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1014,9 +1014,9 @@ static int add_remote_or_group(const char *name, struct string_list *list)
git_config(get_remote_group, &g);
if (list->nr == prev_nr) {
struct remote *remote;
- if (!remote_is_configured(name))
- return 0;
remote = remote_get(name);
+ if (!remote->fetch_refspec_nr)
+ return 0;
string_list_append(list, remote->name);
}
return 1;
It's outside the scope of your patches, so I think we are OK to just
ignore it. But if you want to pursue it, it avoids having to add the
extra parameter to remote_is_configured().
> Many thanks to Jeff King whose tireless review helped with settling for
> nothing less than the current strategy.
Just how I wanted to be immortalized in git's commit history. ;)
> builtin/fetch.c | 2 +-
> builtin/remote.c | 14 +++++++-------
> remote.c | 12 ++++++++++--
> remote.h | 4 ++--
> t/t5505-remote.sh | 2 +-
> 5 files changed, 21 insertions(+), 13 deletions(-)
Patch itself looks OK to me.
-Peff
^ permalink raw reply related
* [PATCH v2 0/2] Fix remote_is_configured()
From: Johannes Schindelin @ 2017-01-19 21:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Gummerer, Andrew Arnott, Jeff King
In-Reply-To: <cover.1484687919.git.johannes.schindelin@gmx.de>
A surprising behavior triggered the bug report in
https://github.com/git-for-windows/git/issues/888: the mere existence of
the config setting "remote.origin.prune" (in this instance, configured
via ~/.gitconfig so that it applies to all repositories) fooled `git
remote rename <source> <target>` into believing that the <target> remote
is already there.
This patch pair demonstrates the problem, and then fixes it (along with
potential similar problems, such as setting an HTTP proxy for remotes of
a given name via ~/.gitconfig).
Changes since v1:
- overhauled the strategy to fix the problem: instead of looking at the
config key to determine whether it configures a remote or not, we now
look at the config_source: if the remote setting was configured in
.git/config, `git remote <command>` must not overwrite it. If it was
configured elsewhere, `git remote` cannot overwrite any setting, as
it only touches .git/config.
Johannes Schindelin (2):
remote rename: demonstrate a bogus "remote exists" bug
Be more careful when determining whether a remote was configured
builtin/fetch.c | 2 +-
builtin/remote.c | 14 +++++++-------
remote.c | 12 ++++++++++--
remote.h | 4 ++--
t/t5505-remote.sh | 7 +++++++
5 files changed, 27 insertions(+), 12 deletions(-)
base-commit: ffac48d093d4b518a0cc0e8bf1b7cb53e0c3d7a2
Published-As: https://github.com/dscho/git/releases/tag/rename-remote-v2
Fetch-It-Via: git fetch https://github.com/dscho/git rename-remote-v2
Interdiff vs v1:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f1570e3464..b5ad09d046 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1177,7 +1177,7 @@ static int add_remote_or_group(const char *name, struct string_list *list)
git_config(get_remote_group, &g);
if (list->nr == prev_nr) {
struct remote *remote = remote_get(name);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 0))
return 0;
string_list_append(list, remote->name);
}
diff --git a/builtin/remote.c b/builtin/remote.c
index e52cf3925b..5339ed6ad1 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -186,7 +186,7 @@ static int add(int argc, const char **argv)
url = argv[1];
remote = remote_get(name);
- if (remote_is_configured(remote))
+ if (remote_is_configured(remote, 1))
die(_("remote %s already exists."), name);
strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
@@ -618,14 +618,14 @@ static int mv(int argc, const char **argv)
rename.remote_branches = &remote_branches;
oldremote = remote_get(rename.old);
- if (!remote_is_configured(oldremote))
+ if (!remote_is_configured(oldremote, 1))
die(_("No such remote: %s"), rename.old);
if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG)
return migrate_file(oldremote);
newremote = remote_get(rename.new);
- if (remote_is_configured(newremote))
+ if (remote_is_configured(newremote, 1))
die(_("remote %s already exists."), rename.new);
strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
@@ -753,7 +753,7 @@ static int rm(int argc, const char **argv)
usage_with_options(builtin_remote_rm_usage, options);
remote = remote_get(argv[1]);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote: %s"), argv[1]);
known_remotes.to_delete = remote;
@@ -1415,7 +1415,7 @@ static int set_remote_branches(const char *remotename, const char **branches,
strbuf_addf(&key, "remote.%s.fetch", remotename);
remote = remote_get(remotename);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
if (!add_mode && remove_all_fetch_refspecs(remotename, key.buf)) {
@@ -1469,7 +1469,7 @@ static int get_url(int argc, const char **argv)
remotename = argv[0];
remote = remote_get(remotename);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
url_nr = 0;
@@ -1537,7 +1537,7 @@ static int set_url(int argc, const char **argv)
oldurl = newurl;
remote = remote_get(remotename);
- if (!remote_is_configured(remote))
+ if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
if (push_mode) {
diff --git a/remote.c b/remote.c
index 298f2f93fa..8524135de4 100644
--- a/remote.c
+++ b/remote.c
@@ -255,7 +255,7 @@ static void read_remotes_file(struct remote *remote)
if (!f)
return;
- remote->configured = 1;
+ remote->configured_in_repo = 1;
remote->origin = REMOTE_REMOTES;
while (strbuf_getline(&buf, f) != EOF) {
const char *v;
@@ -290,7 +290,7 @@ static void read_branches_file(struct remote *remote)
return;
}
- remote->configured = 1;
+ remote->configured_in_repo = 1;
remote->origin = REMOTE_BRANCHES;
/*
@@ -373,6 +373,8 @@ static int handle_config(const char *key, const char *value, void *cb)
}
remote = make_remote(name, namelen);
remote->origin = REMOTE_CONFIG;
+ if (current_config_scope() == CONFIG_SCOPE_REPO)
+ remote->configured_in_repo = 1;
if (!strcmp(subkey, "mirror"))
remote->mirror = git_config_bool(key, value);
else if (!strcmp(subkey, "skipdefaultupdate"))
@@ -386,25 +388,21 @@ static int handle_config(const char *key, const char *value, void *cb)
if (git_config_string(&v, key, value))
return -1;
add_url(remote, v);
- remote->configured = 1;
} else if (!strcmp(subkey, "pushurl")) {
const char *v;
if (git_config_string(&v, key, value))
return -1;
add_pushurl(remote, v);
- remote->configured = 1;
} else if (!strcmp(subkey, "push")) {
const char *v;
if (git_config_string(&v, key, value))
return -1;
add_push_refspec(remote, v);
- remote->configured = 1;
} else if (!strcmp(subkey, "fetch")) {
const char *v;
if (git_config_string(&v, key, value))
return -1;
add_fetch_refspec(remote, v);
- remote->configured = 1;
} else if (!strcmp(subkey, "receivepack")) {
const char *v;
if (git_config_string(&v, key, value))
@@ -433,7 +431,6 @@ static int handle_config(const char *key, const char *value, void *cb)
return git_config_string((const char **)&remote->http_proxy_authmethod,
key, value);
} else if (!strcmp(subkey, "vcs")) {
- remote->configured = 1;
return git_config_string(&remote->foreign_vcs, key, value);
}
return 0;
@@ -721,9 +718,13 @@ struct remote *pushremote_get(const char *name)
return remote_get_1(name, pushremote_for_branch);
}
-int remote_is_configured(struct remote *remote)
+int remote_is_configured(struct remote *remote, int in_repo)
{
- return remote && remote->configured;
+ if (!remote)
+ return 0;
+ if (in_repo)
+ return remote->configured_in_repo;
+ return !!remote->origin;
}
int for_each_remote(each_remote_fn fn, void *priv)
diff --git a/remote.h b/remote.h
index 7e6c8067bb..a5bbbe0ef9 100644
--- a/remote.h
+++ b/remote.h
@@ -15,7 +15,7 @@ struct remote {
struct hashmap_entry ent; /* must be first */
const char *name;
- int origin, configured;
+ int origin, configured_in_repo;
const char *foreign_vcs;
@@ -60,7 +60,7 @@ struct remote {
struct remote *remote_get(const char *name);
struct remote *pushremote_get(const char *name);
-int remote_is_configured(struct remote *remote);
+int remote_is_configured(struct remote *remote, int in_repo);
typedef int each_remote_fn(struct remote *remote, void *priv);
int for_each_remote(each_remote_fn fn, void *priv);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 09c9823002..ba46e86de0 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -766,11 +766,9 @@ test_expect_success 'rename a remote with name prefix of other remote' '
test_expect_success 'rename succeeds with existing remote.<target>.prune' '
git clone one four.four &&
- (
- cd four.four &&
- git config remote.upstream.prune true &&
- git remote rename origin upstream
- )
+ test_when_finished git config --global --unset remote.upstream.prune &&
+ git config --global remote.upstream.prune true &&
+ git -C four.four remote rename origin upstream
'
cat >remotes_origin <<EOF
--
2.11.0.windows.3
^ permalink raw reply
* Re: [RFC for GIT] pull-request: add praise to people doing QA
From: Jeff King @ 2017-01-19 21:20 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Junio C Hamano, git, linux-kernel
In-Reply-To: <20170119204343.xtotmjddhbum2mvr@ninjato>
On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang wrote:
> > As to the implementation, I am wondering if we can make this somehow
> > work well with the "trailers" code we already have, instead of
> > inventing yet another parser of trailers.
> >
> > In its current shape, "interpret-trailers" focuses on "editing" an
> > existing commit log message to tweak the trailer lines. That mode
> > of operation would help amending and rebasing, and to do that it
> > needs to parse the commit log message, identify trailer blocks,
> > parse out each trailer lines, etc.
> >
> > There is no fundamental reason why its output must be an edited
> > original commit log message---it should be usable as a filter that
> > picks trailer lines of the selected trailer type, like "Tested-By",
> > etc.
>
> I didn't know about trailers before. As I undestand it, I could use
> "Tested-by" as the key, and the commit subject as the value. This list
> then could be parsed and brought into proper output shape. It would
> simplify the subject parsing, but most things my AWK script currently
> does would still need to stay or to be reimplemented (extracting names
> from tags, creating arrays of tags given by $name). Am I correct?
>
> All under the assumption that trailers work on a range of commits. I
> have to admit that adding this to git is beyond my scope.
This sounds a lot like the shortlog-trailers work I did about a year
ago:
http://public-inbox.org/git/20151229073832.GN8842@sigill.intra.peff.net/
http://public-inbox.org/git/20151229075013.GA9191@sigill.intra.peff.net/
Nobody seemed to really find it useful, so I didn't pursue it.
Some of the preparatory patches in that series bit-rotted in the
meantime, but you can play with a version based on v2.7.0 by fetching
the "shortlog-trailers-historical" branch from
https://github.com/peff/git.git.
And then things like:
git shortlog --ident=tested-by --format='...tested a patch by %an'
work (and you can put whatever commit items you want into the --format,
including just dumping the hash if you want to do more analysis).
-Peff
^ permalink raw reply
* [PATCH v2 1/2] remote rename: demonstrate a bogus "remote exists" bug
From: Johannes Schindelin @ 2017-01-19 21:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Gummerer, Andrew Arnott, Jeff King
In-Reply-To: <cover.1484860744.git.johannes.schindelin@gmx.de>
Some users like to set `remote.origin.prune = true` in their ~/.gitconfig
so that all of their repositories use that default.
However, our code is ill-prepared for this, mistaking that single entry to
mean that there is already a remote of the name "origin", even if there is
not.
This patch adds a test case demonstrating this issue.
Reported by Andrew Arnott.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t5505-remote.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 8198d8eb05..2c03f44c85 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -764,6 +764,13 @@ test_expect_success 'rename a remote with name prefix of other remote' '
)
'
+test_expect_failure 'rename succeeds with existing remote.<target>.prune' '
+ git clone one four.four &&
+ test_when_finished git config --global --unset remote.upstream.prune &&
+ git config --global remote.upstream.prune true &&
+ git -C four.four remote rename origin upstream
+'
+
cat >remotes_origin <<EOF
URL: $(pwd)/one
Push: refs/heads/master:refs/heads/upstream
--
2.11.0.windows.3
^ permalink raw reply related
* Re: [RFC for GIT] pull-request: add praise to people doing QA
From: Junio C Hamano @ 2017-01-19 21:22 UTC (permalink / raw)
To: Wolfram Sang; +Cc: git, linux-kernel
In-Reply-To: <20170119204343.xtotmjddhbum2mvr@ninjato>
Wolfram Sang <wsa@the-dreams.de> writes:
> I didn't know about trailers before. As I undestand it, I could use
> "Tested-by" as the key, and the commit subject as the value. This list
> then could be parsed and brought into proper output shape. It would
> simplify the subject parsing, but most things my AWK script currently
> does would still need to stay or to be reimplemented (extracting names
> from tags, creating arrays of tags given by $name). Am I correct?
That is not exactly what I had in mind. I was wondering if we can
do without any external script, implementing the logic you added
inside shortlog with an extra option that triggers the whole thing,
which may call into the same trailers API as used by the
interpret-trailers command to do the parsing and picking out parts.
^ permalink raw reply
* Re: [PATCH 2/2] Be more careful when determining whether a remote was configured
From: Johannes Schindelin @ 2017-01-19 21:19 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Thomas Gummerer, Andrew Arnott
In-Reply-To: <20170119182721.7y2zzrbaalfqjjn6@sigill.intra.peff.net>
Hi Peff,
On Thu, 19 Jan 2017, Jeff King wrote:
> diff --git a/remote.c b/remote.c
> index 298f2f93f..720d616be 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -373,6 +373,8 @@ static int handle_config(const char *key, const char *value, void *cb)
> }
> remote = make_remote(name, namelen);
> remote->origin = REMOTE_CONFIG;
> + if (current_config_scope() == CONFIG_SCOPE_REPO)
> + remote->configured = 1;
> if (!strcmp(subkey, "mirror"))
> remote->mirror = git_config_bool(key, value);
> else if (!strcmp(subkey, "skipdefaultupdate"))
>
> That doesn't make your test pass, but I think that is only because your
> test is not covering the interesting case (it puts the new config in the
> repo config, not in ~/.gitconfig).
>
> What do you think?
Heh. After skimming the first three paragraphs of your mail, I had a
similar idea and implemented it. It works great!
v2 coming right up,
Johannes
^ permalink raw reply
* Re: [PATCH v4 3/5] name-rev: add support to exclude refs by pattern match
From: Junio C Hamano @ 2017-01-19 21:08 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Johannes Sixt, Johannes Schindelin, Jacob Keller
In-Reply-To: <20170118230608.28030-4-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> Extend git-name-rev to support excluding refs which match shell patterns
> using --exclude. These patterns can be used to limit the scope of refs
> by excluding any ref that matches one of the --exclude patterns. A ref
> will only be used for naming when it matches at least one --ref pattern
s/--ref pattern/--refs pattern/
> but does not match any of the --exclude patterns. Thus, --exlude
s/--exlude/--exclude/
> patterns are given precedence over --ref patterns.
s/--ref pattern/--refs pattern/
No need to resend. Thanks.
^ permalink raw reply
* Re: [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns
From: Junio C Hamano @ 2017-01-19 21:06 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Johannes Sixt, Johannes Schindelin, Jacob Keller
In-Reply-To: <20170118230608.28030-3-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> Teach git name-rev to take multiple --refs stored as a string list of
> patterns. The list of patterns will be matched inclusively, and each ref
> only needs to match one pattern to be included. A ref will only be
> excluded if it does not match any of the given patterns. Additionally,
> if any of the patterns would allow abbreviation, then we will abbreviate
> the ref, even if another pattern is more strict and would not have
> allowed abbreviation on its own.
>
> Add tests and documentation for this change. The tests expected output
> is dynamically generated, but this is in order to avoid hard-coding
> a commit object id in the test results (as the expected output is to
> simply leave the commit object unnamed).
Makes sense.
I do not see anything that requires "... generated, but" there,
though, as if it is a bad thing to do to prepare expected output
dynamically. I'd just reword "generated. This is..." to make it
neutral.
^ permalink raw reply
* Re: [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns
From: Junio C Hamano @ 2017-01-19 21:03 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Johannes Sixt, Johannes Schindelin, Jacob Keller
In-Reply-To: <20170118230608.28030-3-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> + if (data->ref_filters.nr) {
> + struct string_list_item *item;
> + int matched = 0;
> +
> + /* See if any of the patterns match. */
> + for_each_string_list_item(item, &data->ref_filters) {
> + /* Check every pattern even after we found a match so
> + * that we can determine when we should abbreviate the
> + * output. We will abbreviate the output when any of
> + * the patterns match a subpath, even if one of the
> + * patterns matches fully.
> + */
This describe "what" we do, which we can read from the code. What I
asked you to mention was "why", which cannot be read from the code.
/*
* Check all patterns even after finding a match,
* so that we can see if a match with a subpath exists.
* When a user asked for 'refs/tags/v*' and 'v1.*', both
* of which match, the user is showing her willingness
* to accept a shortened output by having the 'v1.*' in
* the acceptable refnames, so we shouldn't stop when seeing
* 'refs/tags/v1.4' matches 'refs/tags/v*'. We should show
* it as 'v1.4'.
*/
or something like that, perhaps?
^ permalink raw reply
* Re: [RESEND PATCHv2] contrib: remove git-convert-objects
From: Junio C Hamano @ 2017-01-19 20:57 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, torvalds
In-Reply-To: <20170119202941.6575-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> git-convert-objects, originally named git-convert-cache was used in
> early 2005 to convert to a new repository format, e.g. adding an author
> date.
I think this description is not wrong per-se but misses the much
more important point. In the very early days of Git, the objects
were named after SHA-1 of deflated loose object representation,
which meant that tweak in zlib or change of compression level would
give the same object different names X-<. This program was to
convert an ancient history with these objects and rewrite them to
match the new object naming scheme where the name comes from a hash
of the inflated representation.
> By now the need for conversion of the very early repositories is less
> relevant, we no longer need to keep it in contrib; remove it.
I am not sure if removal of it matters, and I suspect that we saw no
reaction from anybody because nobody thought it deserves the
brain-cycle to decide whether to remove it. I dunno.
^ permalink raw reply
* Re: [RFC for GIT] pull-request: add praise to people doing QA
From: Wolfram Sang @ 2017-01-19 20:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, linux-kernel
In-Reply-To: <xmqqlgubc04z.fsf@gitster.mtv.corp.google.com>
> So the idea is to have list of those whose names appear on
> Reviewed-by: and Tested-by: collected and listed after the list of
> commit titles and author names. I personally do not see much
> downsides in doing so, but I do not consume that many PRs myself, so
> let's hear from those who actually do process many of them.
Sadly, no further responses so far. Let's see if they will come if I
keep posting my pull requests with that information attached.
> As to the implementation, I am wondering if we can make this somehow
> work well with the "trailers" code we already have, instead of
> inventing yet another parser of trailers.
>
> In its current shape, "interpret-trailers" focuses on "editing" an
> existing commit log message to tweak the trailer lines. That mode
> of operation would help amending and rebasing, and to do that it
> needs to parse the commit log message, identify trailer blocks,
> parse out each trailer lines, etc.
>
> There is no fundamental reason why its output must be an edited
> original commit log message---it should be usable as a filter that
> picks trailer lines of the selected trailer type, like "Tested-By",
> etc.
I didn't know about trailers before. As I undestand it, I could use
"Tested-by" as the key, and the commit subject as the value. This list
then could be parsed and brought into proper output shape. It would
simplify the subject parsing, but most things my AWK script currently
does would still need to stay or to be reimplemented (extracting names
from tags, creating arrays of tags given by $name). Am I correct?
All under the assumption that trailers work on a range of commits. I
have to admit that adding this to git is beyond my scope.
Thanks for looking into it,
Wolfram
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox