Re: [problem with name-rev]

[Phil Hord]

On Tue, Aug 20, 2019 at 12:35 PM Uwe Brauer  wrote:
>
> Take the following part of what I did
>
> git init
> echo 1 > 1
> git add 1
> git commit -m 1
> echo 1.1 > 1
> git add .
> git commit -m 1.1
> git checkout -b foo master~1
> echo 1.2 > 1
> git add .
> git commit -m 1.2
> echo 1.2.1 > 1
> git add .
> git commit -m 1.2.1
> git checkout master
>
> There are 4  commits.
>
> But
>
> Git --log --graph --decorate
>
> Returns
> * commit 98922f82932cd1bef58bebf0229367922bca45fc (HEAD -> master)
> | Author: Uwe Brauer 
> | Date:   Tue Aug 20 21:19:59 2019 +0200
> |
> | 1.1
> |
> * commit 8f565d59c356a6038e3d8a7f5dcd2e4a39ae1bb4
>   Author: Uwe Brauer 
>   Date:   Tue Aug 20 21:19:59 2019 +0200

Try adding '--all' to include all refs, not just the current HEAD, to
begin logging from. Here is what I see after running your setup
script.

$  git log --graph --decorate --all
* commit 3262040f2d8d5f31b4918bda9987e6b5f788531f (foo)
| Author: Phil Hord 
| Date:   Tue Aug 20 12:44:32 2019
|
| 1.2.1
|
* commit fc66c4311bf954d455f468581f2913dffa0f9c2b
| Author: Phil Hord 
| Date:   Tue Aug 20 12:44:32 2019
|
| 1.2
|
| * commit 109e5d4baef4e99cf636189ba1499af817ab0bb1 (HEAD -> master)
|/  Author: Phil Hord 
|   Date:   Tue Aug 20 12:44:32 2019
|
|   1.1
|
* commit 5c1e93ed7c5b3b241d5adfadb365a6bca5d60d3a
  Author: Phil Hord 
  Date:   Tue Aug 20 12:44:32 2019

  1


You could also mention only the refs you are interested in instead of
including all of them.
$  git log --graph --decorate foo master

Re: Only track built files for final output?

[Phil Hord]

On Tue, Aug 20, 2019 at 11:01 AM Leam Hall  wrote:
> On 8/20/19 1:46 PM, Pratyush Yadav wrote:

> > So in your case, what's wrong with just tracking the source files needed
> > to generate the other files, and then when you want a release binary,
> > just clone the repo, run your build system, and get the generated files?
> > What benefit do you get by tracking the generated files?
>
> For internal use I agree with you. However, there's an issue.
>
> The generated files are used by another program's build system, and I
> can't guarantee the other build system's build system is built like
> ours. It seems easier to provide them the generated files and decouple
> their build system layout from ours.

It becomes a burden to keep build products in the repo over time, for
the reasons you already mentioned (they don't merge and you shouldn't
try), but also because those build products never go away, leading to
repo-bloat.  Once you realize the cost is too great, it's often too
late to do something about it cheaply.  My advice is to keep your
source repository clean from the beginning, so it contains only source
code.

This means you still have a problem because you want to distribute
certified build artifacts.  I recommend you use some other tool to
handle that, like Artifactory.

I recognize it seems easy to use Git for this because Git already acts
like a reliable, portable, trackable file distribution system. But
that's secondary to Git's purpose; there are better tools for that. If
you must lean on Git for this, I like to isolate the binaries into a
submodule so developers who don't want or need them aren't bothered by
them, and they can stay out of the way of merges.  But submodules
present new workflow challenges and will require some study and
education.  If you want to keep them out of the way of developers, you
can keep your source code repo and your artifact repo completely
separate and make some "superproject" which contains both of those
repos as submodules.  The nice feature about this setup is you can
positively associate the set of build products with the set of source
code that produced them.

[RFC] Relaxing interpret-trailers' interpretation rules

[Phil Hord]

I'm trying to include "Bug" numbers from commit messages in my
one-line log output, like this:
git log --pretty="%h %(trailers:key=Bug,valueonly,separator=;) %s"

But I have a problem.  We have a tool that appends a trailer to commit
messages like this:

   Reviewed at https://reviews.myco.com/012345

Unfortunately, it adds a newline before this line, effectively making
'git interpret-trailers' think there are no other trailers.

foo: add new feature foo

yadda yadda yadda

Bug: PROJ-155485
Testing: https://jenkins/job/foo-functional/1/console

Reviewed at https://reviews.myco.com/012345

I could fix this going forward, but it doesn't help with my existing
project and various users' similar formats.

I'm thinking of adding a switch to interpret-trailers to tell it not
to stop searching for my trailer key at the last blank line in the
message.  This feels like it will cheapen the purpose of
interpret-trailers, though.

Do you have thoughts on this idea?

Another option I considered is adding some extension to --pretty's
format to let me post-process fields in a command.  I don't think such
a thing already exists, but with the myriad log formats available,
maybe I've overlooked something.

[PATCH v2] use delete_refs when deleting tags or branches

[Phil Hord]

From: Phil Hord 

'git tag -d' and 'git branch -d' both accept one or more refs to
delete, but each deletion is done by calling `delete_ref` on each argv.
This is very slow when removing from packed refs as packed-refs is
locked and rewritten each time. Use delete_refs instead so all the
removals can be done inside a single transaction with a single update.

Since delete_refs performs all the packed-refs delete operations
inside a single transaction, if any of the deletes fail then all
them will be skipped. In practice, none of them should fail since
we verify the hash of each one before calling delete_refs, but some
network error or odd permissions problem could have different results
after this change.

Also, since the file-backed deletions are not performed in the same
transaction, those could succeed even when the packed-refs transaction
fails.

After deleting refs, report the deletion's success only if the ref was
actually deleted. For branch deletion, remove the branch config only
if the branch ref is actually removed.

A manual test deleting 24,000 tags took about 30 minutes using
delete_ref.  It takes about 5 seconds using delete_refs.

Signed-off-by: Phil Hord 
---
This reroll adds the same delete_refs change to 'git branch'. It checks 
individual refs after the operation to report correctly on each whether
it was successfully deleted or not. Maybe this is an unnecessary step,
though. This handles the weird case where some file system error
prevented us from deleting refs, leaving us with an error from 
delete_refs but without any idea which refs might have been affected.

 builtin/branch.c | 50 +---
 builtin/tag.c| 45 +--
 2 files changed, 65 insertions(+), 30 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 2ef214632f..2273239f41 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -202,6 +202,9 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
int remote_branch = 0;
struct strbuf bname = STRBUF_INIT;
unsigned allowed_interpret;
+   struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
+   struct string_list_item *item;
+   int refname_pos = 0;
 
switch (kinds) {
case FILTER_REFS_REMOTES:
@@ -209,12 +212,13 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
/* For subsequent UI messages */
remote_branch = 1;
allowed_interpret = INTERPRET_BRANCH_REMOTE;
-
+   refname_pos = 13;
force = 1;
break;
case FILTER_REFS_BRANCHES:
fmt = "refs/heads/%s";
allowed_interpret = INTERPRET_BRANCH_LOCAL;
+   refname_pos = 11;
break;
default:
die(_("cannot use -a with -d"));
@@ -265,30 +269,36 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
goto next;
}
 
-   if (delete_ref(NULL, name, is_null_oid(&oid) ? NULL : &oid,
-  REF_NO_DEREF)) {
-   error(remote_branch
- ? _("Error deleting remote-tracking branch '%s'")
- : _("Error deleting branch '%s'"),
- bname.buf);
-   ret = 1;
-   goto next;
-   }
-   if (!quiet) {
-   printf(remote_branch
-  ? _("Deleted remote-tracking branch %s (was 
%s).\n")
-  : _("Deleted branch %s (was %s).\n"),
-  bname.buf,
-  (flags & REF_ISBROKEN) ? "broken"
-  : (flags & REF_ISSYMREF) ? target
-  : find_unique_abbrev(&oid, DEFAULT_ABBREV));
-   }
-   delete_branch_config(bname.buf);
+   item = string_list_append(&refs_to_delete, name);
+   item->util = xstrdup((flags & REF_ISBROKEN) ? "broken"
+   : (flags & REF_ISSYMREF) ? target
+   : find_unique_abbrev(&oid, DEFAULT_ABBREV));
 
next:
free(target);
}
 
+   delete_refs(NULL, &refs_to_delete, REF_NO_DEREF);
+
+   for_each_string_list_item(item, &refs_to_delete) {
+   char * describe_ref = item->util;
+   char * name = item->string;
+   if (ref_exists(name))
+   ret = 1;
+   else {
+   char * refname = name + refna

Re: What's cooking in git.git (Jul 2019, #06; Thu, 25)

[Phil Hord]

On Fri, Aug 9, 2019 at 10:48 AM Junio C Hamano  wrote:
>
> Jeff King  writes:
>
> > I don't know of any plans for checkout in particular, but I think the
> > docs for restore/switch make it clear that it's way too early to start
> > scripting around them:
> >
> >   $ git grep EXPERIMENTAL Documentation/
> >   Documentation/git-restore.txt:THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR 
> > MAY CHANGE.
> >   Documentation/git-switch.txt:THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR 
> > MAY CHANGE.
>
> Would it ever be OK to script around checkout, restore and/or switch
> Porcelain commands?

Users who wish to get their job done will script around porcelain all
the time.  I would be surprised if even 1% of build scripts use 'git
checkout-index' instead of 'git checkout'.

No, this doesn't make it OK. ;)

Re: What's cooking in git.git (Jul 2019, #06; Thu, 25)

[Phil Hord]

On Fri, Aug 9, 2019 at 4:41 AM Jeff King  wrote:
>
> On Thu, Aug 08, 2019 at 08:07:36PM -0700, Phil Hord wrote:
>
> > The long form you give there is to be used in case the old email
> > address is not a unique key. See 'git help shortlog'.
> >
> > The problem we have at work is that one woman's old email address
> > includes her deadname, like .  I will
> > leave it up to her whether she chooses to be listed explicitly in the
> > mailmap.  I have wondered if we should permit hashed email addresses
> > to be used for this specific case, but this also has its drawbacks.
>
> Since the set of hash inputs is finite and small (i.e., the set of all
> emails in the repository), it would be trivial to generate the plaintext
> mapping from even a cryptographically strong hashed mapping.
>
> Which isn't to say it's _totally_ worthless, since that adds an extra
> step, but it really is just obfuscating the data.

Yes, obfuscation is all I expect. Someone who needs deeper scrubbing
will need to rewrite their history instead.

Re: What's cooking in git.git (Jul 2019, #06; Thu, 25)

[Phil Hord]

The issue of deadnaming aside, turning on log.mailmap by default is
the sensible thing to do given that other Git features already honor
it that way.  Having it ignored-by-default (but only sometimes) just
adds confusion when a mailmap is available.

> > >  - The '.mailmap' provides a list of transgender individuals, along
> > >with their deadname, which can be used to harass them.
> >
> > This is potentially a problem but it's not as bad as you depict.  A
> > mailmap rule can match against e-mail only, which is precisely what I
> > have done in my projects.
>
> Ah, I may be severely mistaken -- my memory was that '.mailmap'
> rewriting could be used to rewrite both name and email, not merely
> email. I thought that records could take:
>
>   A U Thor  -> B C Xyzz 
>
> instead of canonicalizing by email alone. If this is the case, then I
> completely agree and share the opinion that this is not as bad as I
> originally depicted.

The long form you give there is to be used in case the old email
address is not a unique key. See 'git help shortlog'.

The problem we have at work is that one woman's old email address
includes her deadname, like .  I will
leave it up to her whether she chooses to be listed explicitly in the
mailmap.  I have wondered if we should permit hashed email addresses
to be used for this specific case, but this also has its drawbacks.

Phil

Re: [PATCH 1/1] delete multiple tags in a single transaction

[Phil Hord]

On Thu, Aug 8, 2019 at 12:39 PM Junio C Hamano  wrote:
>
> Phil Hord  writes:
>
> > From: Phil Hord 
> >
> > 'git tag -d' accepts one or more tag refs to delete, but each deletion
> > is done by calling `delete_ref` on each argv. This is painfully slow
> > when removing from packed refs. Use delete_refs instead so all the
> > removals can be done inside a single transaction with a single write.
> >
> > I have a repo with 24,000 tags, most of which are not useful to any
> > developers. Having this many refs slows down many operations that
> > would otherwise be very fast. Removing these tags when they've been
> > accidentally fetched again takes about 30 minutes using delete_ref.
> >
> > git tag -l feature/* | xargs git tag -d
> >
> > Removing the same tags using delete_refs takes less than 5 seconds.
>
> Makes sense.  As mentioned elsewhere in the thread already,
> a batched update-ref would open the packed-refs ony once because
> everything is done in a single transaction, so presumably a pipeline
> like this
>
> git tag -l feature/* |
> sed -e 's|^|delete refs/tags/|' |
> git update-ref --stdin
>
> may work well, and "git tag -d" that gets these refs on the command
> line should be capable of doing the same.
>
> > -static int delete_tag(const char *name, const char *ref,
> > -   const struct object_id *oid, const void *cb_data)
> > +struct tag_args {
> > + char *oid_abbrev;
> > + char *refname;
> > +};
> > +
> > +static int make_string_list(const char *name, const char *ref,
> > + const struct object_id *oid, void *cb_data)
>
> Please think about a few more minutes before naming a function like
> this, and make it a habit for your future patches.
>
> We can see that the callback is used to insert more strings into a
> string list, but the type (i.e. string_list) used to represent the
> set is not all that important.  What is more important is why you
> are building that set for, and saying what is in the set (as opposed
> to saying that the container happens to be a string_list) would be a
> good first step.
>
> I presume that you are enumerating the tags to be deleted, together
> with the data necessary for you to report the deletion of the tags?

Hm.  collect_tags?  collect_tags_to_delete?

It's true I didn't put enought thought into that.  I was experimenting
a bit here and was surprised how little code I ended up needing.

> >  {
> > - if (delete_ref(NULL, ref, oid, 0))
> > - return 1;
> > - printf(_("Deleted tag '%s' (was %s)\n"), name,
> > -find_unique_abbrev(oid, DEFAULT_ABBREV));
> > + struct string_list *ref_list = cb_data;
> > + struct tag_args *info = xmalloc(sizeof(struct tag_args));
> > +
> > + string_list_append(ref_list, ref);
> > +
> > + info->oid_abbrev = xstrdup(find_unique_abbrev(oid, DEFAULT_ABBREV));
> > + info->refname = xstrdup(name);
> > + ref_list->items[ref_list->nr - 1].util = info;
> >   return 0;
> >  }
> >
> > +static int delete_tags(const char **argv)
> > +{
> > + int result;
> > + struct string_list ref_list = STRING_LIST_INIT_DUP;
> > + struct string_list_item *ref_list_item;
> > +
> > + result = for_each_tag_name(argv, make_string_list, (void *) 
> > &ref_list);
> > + if (!result)
> > + result = delete_refs(NULL, &ref_list, REF_NO_DEREF);
> > +
> > + for_each_string_list_item(ref_list_item, &ref_list) {
> > + struct tag_args * info = ref_list_item->util;
> > + if (!result)
> > + printf(_("Deleted tag '%s' (was %s)\n"), 
> > info->refname,
> > + info->oid_abbrev);
> > + free(info->oid_abbrev);
> > + free(info->refname);
> > + free(info);
>
> It is not performance critical, but info->refname is computable from
> ref_list_item->string, isn't it?

Oh, I guess it is.  It's a fixed offset into the string, after all.
Thanks.  I did look for a way to avoid the struct noise. Just not
well.

> I am just wondering if we can do
> this without having to allocate the .util field for each of 20,000
> tags.  We still need to remember oid (or oid_abbrev, but if I were
> writing this, I'd record the full oid in .util and make the code
> that prints call find_unique_abbrev() on it), so I gu

Re: [PATCH 1/1] delete multiple tags in a single transaction

[Phil Hord]

On Thu, Aug 8, 2019 at 11:15 AM Elijah Newren  wrote:
>
> On Wed, Aug 7, 2019 at 9:11 PM Phil Hord  wrote:
> >
> > From: Phil Hord 
> >
> > 'git tag -d' accepts one or more tag refs to delete, but each deletion
> > is done by calling `delete_ref` on each argv. This is painfully slow
> > when removing from packed refs. Use delete_refs instead so all the
> > removals can be done inside a single transaction with a single write.
>
> Nice, thanks for working on this.
>
> > I have a repo with 24,000 tags, most of which are not useful to any
> > developers. Having this many refs slows down many operations that
> > would otherwise be very fast. Removing these tags when they've been
> > accidentally fetched again takes about 30 minutes using delete_ref.
>
> I also get really slow times on a repo with ~20,000 tags (though order
> ~3 minutes rather than ~30, probably due to having an SSD on this
> machine) -- but ONLY IF the refs are packed first (git pack-refs
> --all).  If the refs are loose, it's relatively quick to delete a
> dozen thousand or so tags (order of a few seconds).  It might be worth
> mentioning in the commit message that this only makes a significant
> difference in the case where the refs are packed.

I'm also using an SSD but I still see about 10 tags per second being
deleted with the current code (and packed-refs).  I see that I'm
CPU-bound, so I guess most of the time is spent searching through
.git/packed-refs.  Probably it will run faster as it progresses. I
guess the 18,000 branches in my repo keep me on the wrong end of O(N).

My VM is on an all-flash storage array, but I can't say much about its
write throughput since it's one VM among many.

Previously I thought I saw a significant speedup between v2.7.4 (on my
development vm) and v2.22.0 (on my laptop). But this week I saw it was
slow again on my laptop.  I looked for the regression but didn't find
anyone touching that code. Then I wrote this patch.

But it should have occurred to me while I was in the code that there
is a different path for unpacked refs which could explain my previous
speeds.  I didn't think I had any unpacked refs, though, since every
time I look in .git/refs for what I want, I find it relatively empty.
I see 'git pack-refs --help' says that new refs should show up loose,
but I can't say that has happened for me.  Maybe a new clone uses
packed-refs for *everything* and only newly fetched things are loose.
Is that it?  I guess since I seldom fetch tags after the first clone,
it makes sense they would all be packed.

> > git tag -l feature/* | xargs git tag -d
> >
> > Removing the same tags using delete_refs takes less than 5 seconds.
>
> It appears this same bug also affects `git branch -d` when deleting
> lots of branches (or remote tracking branches) and they are all
> packed; could you apply the same fix there?

Will do.

> In constrast, it appears that `git update-ref --stdin` is fast
> regardless of whether the refs are packed, e.g.
>git tag -l feature/* | sed -e 's%^%delete refs/tags/%' | git
> update-ref --stdin
> finishes quickly (order of a few seconds).

Nice!  That trick is going in my wiki for devs to use on their VMs.
Thanks for that.

[PATCH 1/1] delete multiple tags in a single transaction

[Phil Hord]

From: Phil Hord 

'git tag -d' accepts one or more tag refs to delete, but each deletion
is done by calling `delete_ref` on each argv. This is painfully slow
when removing from packed refs. Use delete_refs instead so all the
removals can be done inside a single transaction with a single write.

I have a repo with 24,000 tags, most of which are not useful to any
developers. Having this many refs slows down many operations that
would otherwise be very fast. Removing these tags when they've been
accidentally fetched again takes about 30 minutes using delete_ref.

git tag -l feature/* | xargs git tag -d

Removing the same tags using delete_refs takes less than 5 seconds.

Signed-off-by: Phil Hord 
---
 builtin/tag.c | 52 +--
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e0a4c25382..f652af83e7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -72,10 +72,10 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting,
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const struct object_id *oid, const void 
*cb_data);
+   const struct object_id *oid, void *cb_data);
 
 static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
-const void *cb_data)
+void *cb_data)
 {
const char **p;
struct strbuf ref = STRBUF_INIT;
@@ -97,18 +97,50 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn,
return had_error;
 }
 
-static int delete_tag(const char *name, const char *ref,
- const struct object_id *oid, const void *cb_data)
+struct tag_args {
+   char *oid_abbrev;
+   char *refname;
+};
+
+static int make_string_list(const char *name, const char *ref,
+   const struct object_id *oid, void *cb_data)
 {
-   if (delete_ref(NULL, ref, oid, 0))
-   return 1;
-   printf(_("Deleted tag '%s' (was %s)\n"), name,
-  find_unique_abbrev(oid, DEFAULT_ABBREV));
+   struct string_list *ref_list = cb_data;
+   struct tag_args *info = xmalloc(sizeof(struct tag_args));
+
+   string_list_append(ref_list, ref);
+
+   info->oid_abbrev = xstrdup(find_unique_abbrev(oid, DEFAULT_ABBREV));
+   info->refname = xstrdup(name);
+   ref_list->items[ref_list->nr - 1].util = info;
return 0;
 }
 
+static int delete_tags(const char **argv)
+{
+   int result;
+   struct string_list ref_list = STRING_LIST_INIT_DUP;
+   struct string_list_item *ref_list_item;
+
+   result = for_each_tag_name(argv, make_string_list, (void *) &ref_list);
+   if (!result)
+   result = delete_refs(NULL, &ref_list, REF_NO_DEREF);
+
+   for_each_string_list_item(ref_list_item, &ref_list) {
+   struct tag_args * info = ref_list_item->util;
+   if (!result)
+   printf(_("Deleted tag '%s' (was %s)\n"), info->refname,
+   info->oid_abbrev);
+   free(info->oid_abbrev);
+   free(info->refname);
+   free(info);
+   }
+   string_list_clear(&ref_list, 0);
+   return result;
+}
+
 static int verify_tag(const char *name, const char *ref,
- const struct object_id *oid, const void *cb_data)
+ const struct object_id *oid, void *cb_data)
 {
int flags;
const struct ref_format *format = cb_data;
@@ -511,7 +543,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged options are only allowed in 
list mode"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag, NULL);
+   return delete_tags(argv);
if (cmdmode == 'v') {
if (format.format && verify_ref_format(&format))
usage_with_options(git_tag_usage, options);
-- 
2.23.0.rc1.174.g4cc1b04b4c.dirty

Re: Deadname rewriting

[Phil Hord]

On Sat, Jun 15, 2019 at 1:19 AM Ævar Arnfjörð Bjarmason
 wrote:
> On Sat, Jun 15 2019, Phil Hord wrote:
>
> > At $work we have a long time employee who has changed their name from
> > Alice to Bob.  Bob doesn't want anyone to call him "Alice" anymore and
> > is prone to be offended if they do.  This is called "deadnaming".
...
> What should be done is to extend the .mailmap support to other
> cases. I.e. make tools like blame, shortlog etc. show the equivalent of
> %aN and %aE by default.

It seems that shortlog and blame do use %aE and %aN by default.  Even
log does.  It is only because I didn't know about %aN 10 years ago
that my custom log format does not.

It's a pity the format author has the option to ignore the mailmap. I
think it's a choice commonly made by mistake rather than intention.  I
wonder if anyone would mind a forced-override config.  Maybe a force
flag in the .mailmap file itself.

  
   Other Authornick2 
   Alice Doe--force


> This topic was discussed at the last git contributor summit (brought up
> by CB Bailey) resulting in this patch, which I see didn't make it in &
> needs to be resurrected again:
> https://public-inbox.org/git/20181212171052.13415-1...@hashpling.org/

Thanks for the link.

I didn't know about config options for mailmap.file and log.mailmap
before. These do make this option much more useful, especially when we
can insert default settings for them into /etc/gitconfig across the
company.

Deadname rewriting

[Phil Hord]

I know name-scrubbing is already covered in filter-branch and other
places. But we have a scenario becoming more common that makes it a
more sensitive topic.

At $work we have a long time employee who has changed their name from
Alice to Bob.  Bob doesn't want anyone to call him "Alice" anymore and
is prone to be offended if they do.  This is called "deadnaming".

We are able to convince most of our work tools to expunge the deadname
from usage anywhere, but git stubbornly calls Bob "Alice" whenever
someone asks for "git blame" or checks in "git log".

We could rewrite history with filter-branch, but that's quite
disruptive.  I found some alternatives.

.mailmap seems perfect for this task, but it doesn't work everywhere
(blame, log, etc.).  Also, it requires the deadname to be forever
proclaimed in the .mailmap file itself.

`git replace` works rather nicely, except all of Bob's old commits
show "replaced" in the decorator list. Also, it doesn't propagate well
from the central server since `refs/replaces` namespace isn't fetched
normally.  But in case anyone wants it, here's what I did:

git log --author=alice.smith --format="%h" --all |
   while read hash ; do
  GIT_EDITOR='sed -i -e s/Alice Smith/Bob Smith/g' -e
's/alice.smith/bob.smith/' \
  git replace --edit $hash
   done
git push origin 'refs/replace/*:refs/replace/*'

I'd quite like the .mailmap solution to work, and I might flesh that
out that some day.

It feels like `.git/info/grafts` would work the best if it could be
distributed with the project, but I'm pretty sure that's a non-starter
for many reasons.

Any other ideas?  Has anyone here encountered this already?

Re: cherry-pick strangeness

[Phil Hord]

On Fri, Jun 14, 2019 at 12:29 AM Vincent Legoll
 wrote:
> On Fri, Jun 14, 2019 at 12:56 AM Elijah Newren  wrote:
> > When you cherry-pick a commit, it reapplies its diff on top of a
> > (usually different) commit, preserving the author name/email/date, but
> > throwing away the committer name/email/date -- instead using your
> > name/email and the time of the cherry-pick for the committer.  Since
> > you are transplanting on the same commit, and you created both the
> > original commit and the cherry-pick, the only thing that can be
> > different is the committer timestamp.  Git records timestamps down to
> > 1-second resolution.  If you run in a script, odds are that the
> > original commit and the cherry-pick both run within the same second
> > (though not always), and thus you end up with precisely the same
> > commit.  When you run interactively, you take longer than a second
> > between commands, and thus have a different committer date which
> > naturally will have a different sha1sum.
>
> Thanks for the thorough explanation.
>
> Looks like this has nothing to do with "--[no-]ff" at all.
>
> Shouldn't something about that be added to the man page to avoid
> people scratch their heads ? (I can try to cook something if this is
> deemed acceptable)

Patches to documentation are welcome, but I'm not sure what you'd say.
Is it just a note to point out that the HEAD resulting from
cherry-pick is not guaranteed to be unique?  That seems too noisy for
something which is inconsequential to most users, to me.  But others
may disagree.

Re: [PATCH v4] documentation: add tutorial for first contribution

[Phil Hord]

On Tue, Apr 23, 2019 at 12:35 PM Emily Shaffer  wrote:
>
> This tutorial covers how to add a new command to Git and, in the
> process, everything from cloning git/git to getting reviewed on the
> mailing list. It's meant for new contributors to go through
> interactively, learning the techniques generally used by the git/git
> development community.
>

Thanks for working on this.  It's very nicely done.

> Signed-off-by: Emily Shaffer 
> ---
> Only minor changes from v3, correcting the comments Junio made in his
> review.
>
> - Changed commit subject
> - Stray monospace typos
> - Curly brace style
>
>  Documentation/Makefile|1 +
>  Documentation/MyFirstContribution.txt | 1073 +
>  2 files changed, 1074 insertions(+)
>  create mode 100644 Documentation/MyFirstContribution.txt
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 26a2342bea..fddc3c3c95 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -74,6 +74,7 @@ API_DOCS = $(patsubst %.txt,%,$(filter-out 
> technical/api-index-skel.txt technica
>  SP_ARTICLES += $(API_DOCS)
>
>  TECH_DOCS += SubmittingPatches
> +TECH_DOCS += MyFirstContribution
>  TECH_DOCS += technical/hash-function-transition
>  TECH_DOCS += technical/http-protocol
>  TECH_DOCS += technical/index-format
> diff --git a/Documentation/MyFirstContribution.txt 
> b/Documentation/MyFirstContribution.txt
> new file mode 100644
> index 00..fc4a59a8c6
> --- /dev/null
> +++ b/Documentation/MyFirstContribution.txt
> @@ -0,0 +1,1073 @@
> +My First Contribution to the Git Project
> +
> +
> +== Summary
> +
> +This is a tutorial demonstrating the end-to-end workflow of creating a 
> change to
> +the Git tree, sending it for review, and making changes based on comments.
> +
> +=== Prerequisites
> +
> +This tutorial assumes you're already fairly familiar with using Git to manage
> +source code.  The Git workflow steps will largely remain unexplained.
> +
> +=== Related Reading
> +
> +This tutorial aims to summarize the following documents, but the reader may 
> find
> +useful additional context:
> +
> +- `Documentation/SubmittingPatches`
> +- `Documentation/howto/new-command.txt`
> +
> +== Getting Started
> +
> +=== Pull the Git codebase
> +
> +Git is mirrored in a number of locations. https://git-scm.com/downloads
> +suggests one of the best places to clone from is GitHub.
> +
> +
> +$ git clone https://github.com/git/git git
> +
> +
> +=== Identify Problem to Solve
> +
> +
> +Use + to indicate fixed-width here; couldn't get ` to work nicely with the
> +quotes around "Pony Saying 'Um, Hello'".
> +
> +In this tutorial, we will add a new command, +git psuh+, short for ``Pony 
> Saying
> +`Um, Hello''' - a feature which has gone unimplemented despite a high 
> frequency
> +of invocation during users' typical daily workflow.
> +
> +(We've seen some other effort in this space with the implementation of 
> popular
> +commands such as `sl`.)
> +
> +=== Set Up Your Workspace
> +
> +Let's start by making a development branch to work on our changes. Per
> +`Documentation/SubmittingPatches`, since a brand new command is a new 
> feature,
> +it's fine to base your work on `master`. However, in the future for bugfixes,
> +etc., you should check that document and base it on the appropriate branch.
> +
> +For the purposes of this document, we will base all our work on the `master`
> +branch of the upstream project. Create the `psuh` branch you will use for
> +development like so:
> +
> +
> +$ git checkout -b psuh origin/master
> +
> +
> +We'll make a number of commits here in order to demonstrate how to send a 
> topic
> +with multiple patches up for review simultaneously.
> +
> +== Code It Up!
> +
> +NOTE: A reference implementation can be found at
> +https://github.com/nasamuffin/git/tree/psuh.
> +
> +=== Adding a new command
> +
> +Lots of the subcommands are written as builtins, which means they are
> +implemented in C and compiled into the main `git` executable. Implementing 
> the
> +very simple `psuh` command as a built-in will demonstrate the structure of 
> the
> +codebase, the internal API, and the process of working together as a 
> contributor
> +with the reviewers and maintainer to integrate this change into the system.
> +
> +Built-in subcommands are typically implemented in a function named "cmd_"
> +followed by the name of the subcommand, in a source file named after the
> +subcommand and contained within `builtin/`. So it makes sense to implement 
> your
> +command in `builtin/psuh.c`. Create that file, and within it, write the entry
> +point for your command in a function matching the style and signature:
> +
> +
> +int cmd_psuh(int argc, const char **argv, const char *prefix)
> +
> +
> +We'll also need to add the extern declaration of psuh; open up `builtin.h`,
> +find the declaration for `cmd_push`, and add a new line for `psuh` 
> 

Re: [PATCH] status: add an empty line when there is no hint

[Phil Hord]

You should probably add some test that demonstrates what your change
intends to do.  For that matter, though, your test already breaks at
least two tests in t7508-status.sh:

not ok 14 - status (advice.statusHints false)
not ok 23 - status -uno (advice.statusHints false)

Phil

On Tue, Apr 23, 2019 at 2:21 AM John Lin  wrote:
>
> When typing "git status", there is an empty line between
> the "Changes not staged for commit:" block and the list
> of changed files. However, when typing "git commit" with
> no files added, there are no empty lines between them.
>
> This patch adds empty lines in the above case and some
> similar cases.
>
> Signed-off-by: John Lin 
> ---
>  wt-status.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index 445a36204a..0766e3ee12 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -175,7 +175,7 @@ static void wt_longstatus_print_unmerged_header(struct 
> wt_status *s)
> }
>
> if (!s->hints)
> -   return;
> +   goto conclude;
> if (s->whence != FROM_COMMIT)
> ;
> else if (!s->is_initial)
> @@ -193,6 +193,7 @@ static void wt_longstatus_print_unmerged_header(struct 
> wt_status *s)
> } else {
> status_printf_ln(s, c, _("  (use \"git add/rm ...\" as 
> appropriate to mark resolution)"));
> }
> +conclude:
> status_printf_ln(s, c, "%s", "");
>  }
>
> @@ -202,13 +203,14 @@ static void wt_longstatus_print_cached_header(struct 
> wt_status *s)
>
> status_printf_ln(s, c, _("Changes to be committed:"));
> if (!s->hints)
> -   return;
> +   goto conclude;
> if (s->whence != FROM_COMMIT)
> ; /* NEEDSWORK: use "git reset --unresolve"??? */
> else if (!s->is_initial)
> status_printf_ln(s, c, _("  (use \"git reset %s ...\" 
> to unstage)"), s->reference);
> else
> status_printf_ln(s, c, _("  (use \"git rm --cached 
> ...\" to unstage)"));
> +conclude:
> status_printf_ln(s, c, "%s", "");
>  }
>
> @@ -220,7 +222,7 @@ static void wt_longstatus_print_dirty_header(struct 
> wt_status *s,
>
> status_printf_ln(s, c, _("Changes not staged for commit:"));
> if (!s->hints)
> -   return;
> +   goto conclude;
> if (!has_deleted)
> status_printf_ln(s, c, _("  (use \"git add ...\" to 
> update what will be committed)"));
> else
> @@ -228,6 +230,7 @@ static void wt_longstatus_print_dirty_header(struct 
> wt_status *s,
> status_printf_ln(s, c, _("  (use \"git checkout -- ...\" to 
> discard changes in working directory)"));
> if (has_dirty_submodules)
> status_printf_ln(s, c, _("  (commit or discard the untracked 
> or modified content in submodules)"));
> +conclude:
> status_printf_ln(s, c, "%s", "");
>  }
>
> @@ -238,8 +241,9 @@ static void wt_longstatus_print_other_header(struct 
> wt_status *s,
> const char *c = color(WT_STATUS_HEADER, s);
> status_printf_ln(s, c, "%s:", what);
> if (!s->hints)
> -   return;
> +   goto conclude;
> status_printf_ln(s, c, _("  (use \"git %s ...\" to include in 
> what will be committed)"), how);
> +conclude:
> status_printf_ln(s, c, "%s", "");
>  }
>
> --
> 2.21.0
>

Re: [PATCH/RFC 0/2] rebase: add switches to control todo-list setup

[Phil Hord]

On Mon, Apr 22, 2019 at 6:21 PM Junio C Hamano  wrote:
>
> Phillip Wood  writes:
>
> > Doing "git rebase -i master" and then editing the todo list has the
> > side effect of rebasing the branch. Often I find I want to amend or
> > reword a commit without rebasing (for instance when preparing a
> > re-roll).
>
> I am not sure what you mean by "not rebasing".  Are you talking
> about --keep-base that uses the same --onto as the previous?
>
> I think that is often desired, but I do not think it has much to do
> with the topic of the proposal these two patches raises.
>
> And that (i.e. "this has nothing to do with the choice of 'onto'")
> was why I used the casual "rebase -i master" in my illustrations.

I know exactly what he means, because it usually is exactly what I
want to do here.  In fact, I almost always want `rebase --interactive`
to do this "in-place" editing of the history.  Sure, I may want to
`rebase @{upstream}` someday, but I seldom use --interactive for that.

Rebase invites conflicts.  It's nice to invite as few as possible at once.

P

Re: [PATCH/RFC 0/2] rebase: add switches to control todo-list setup

[Phil Hord]

On Mon, Apr 22, 2019 at 12:16 PM Phil Hord  wrote:
>
> I have the same need.  I plan to have some switch that invokes this
> "in-place rebase" behavior so that git can choose the upstream for me
> as `mergebase $sequence-edits`.  In fact, I want to make that the
> default for these switches, but that feels too surprising for the
> rebase command. I plan to progress like this:
>
> # --in-place switch is not supported; manual upstream is given by user
> git rebase --edit foo foo^
>
>  # --in-place switch is added; now we can say this
>  git rebase --edit foo --in-place

I originally CC'ed Denton on this thread because he recently added
--keep-base.  I initially hoped it would do something similar to
--in-place, but on reading the patch discussion, I think it's for
something different altogether.  :-\   It's similar, though, in the
same way that --fork-point is; which may be another way to say "not
very."

Re: [PATCH/RFC 0/2] rebase: add switches to control todo-list setup

[Phil Hord]

On Mon, Apr 22, 2019 at 7:44 AM Phillip Wood  wrote:
> Doing "git rebase -i master" and then editing the todo list has the side
> effect of rebasing the branch. Often I find I want to amend or reword a
> commit without rebasing (for instance when preparing a re-roll). To do
> this I use a script that runs something like
>
> GIT_SEQUENCE_EDITOR="sed -i s/pick $sha/edit $sha/" git rebase -i $sha^
>
> and I have my shell set up to interactively select a commit[1] so I
> don't have to cut and paste the output from git log. I've found this
> really useful as most of the time I just want to amend or reword a
> commit or squash fixups rather than rearranging commits. The script
> knows how to rewind a running rebase so I can amend several commits
> without having to start a new rebase each time.
>
> So I can see a use for --edit, --reword & --drop if they selected a
> suitable upstream to avoid unwanted rebases (I'm not so sure about the
> others though). If you want to rebase as well then I agree you might as
> well just edit the todo list.

I have the same need.  I plan to have some switch that invokes this
"in-place rebase" behavior so that git can choose the upstream for me
as `mergebase $sequence-edits`.  In fact, I want to make that the
default for these switches, but that feels too surprising for the
rebase command. I plan to progress like this:

# --in-place switch is not supported; manual upstream is given by user
git rebase --edit foo foo^

 # --in-place switch is added; now we can say this
 git rebase --edit foo --in-place

 # prefer in-place edits as default when editing
 git config --add rebase.in-place-edits true
 git rebase --edit foo

This --in-place switch would use `mergebase $sequence-edits` to find
my upstream parameter if I didn't give one explicitly.

This config option set to true would tell git to assume I meant to use
--in-place whenever I use some sequence-edit switch and I don't
specify an upstream.

I have written some of this code, but since I am running into
conflicts with next and pu, I haven't ironed it out yet.

Phil

Re: [PATCH/RFC 0/2] rebase: add switches to control todo-list setup

[Phil Hord]

On Sun, Apr 21, 2019 at 6:13 PM Junio C Hamano  wrote:
>
> Phil Hord  writes:
>
> > Currently it supports these switches:
> >
> > usage: git rebase [-i] [options] [--exec ] ...
> >:
> > --break stop before the mentioned ref
> > --drop  drop the mentioned ref from the todo list
> > --edit  edit the mentioned ref instead of picking it
> > --rewordreword the mentioned ref instead of picking it
> >
> > I have plans to add these, but I don't like how their "onto" will be
> > controlled. More thinking is needed here.
> >
> > --fixup fixup the mentioned ref instead of picking it
> > --squashsquash the mentioned ref instead of picking it
> > --pick  pick the mentioned ref onto the start of the list
>
> Yeah, I can see that it may be very useful to shorten the sequence
> to (1) learn what commits there are and think what you want to do
> with each of them by looking at "git log --oneline master.." output
> and then to (2) look at and edit todo in "git rebase -i master".
>
> I personally would be fine without the step (1), as what "rebase -i"
> gives me in step (2) essentially is "log --oneline master..".

My example of "drop" is probably the worst one for simplification, but
it nicely reduces the operation to one step if --interactive is not
given.  More commonly I discover something I want to improve in commit
message or in the code so I use --edit or --reword to fix it up before
I submit it for review.

> So I
> am not quite getting in what way these command line options would be
> more useful than without them, though, especially since I do not see
> how well an option to reorder commits would fit with the way you
> structured your UI.

I thought I might have "--pick-onto foo bar" but that requires two
arguments to one switch, which I think is confusing and unprecedented.
My current thinking is that --pick could pick some commit onto the
beginning of the todo list, thereby picking it onto 'upstream'.  If
the same commit appears later in the todo-list, I am inclined to drop
it; but I might want to pick it anyway and let it evaporate as an
empty commit, giving me the opportunity to split a commit in two
places.  But this is still more exotic and confusing.

I tried to have --fixup and --squash simply do their actions in-place,
but that seems useless.  So I thought I might treat the same as pick,
picking them onto upstream. But it's meaningless to fixup or squash on
the first step in the todo, so it would have to be on the first child
of the upstream.  This still feels forced and useless.

A compromise that feels nice in practice is to do the fixup and squash
in-place and then to use --interactive to open the editor.  Since I
have syntax highlighting, the fixup and squash lines stand out boldly
and I find it easier to move these into the right place as needed. But
I think this mode could be confusing for users trying to understand
the utility of these switches.

> Having already said that, if I were to get in the habit of looking
> at "log" first to decide and then running "rebase -i" after I made
> up my mind, using a tweaked "log --oneline" output that looks
> perhaps like this:
>
> $ git log --oneline master.. | tac | cat -n
> 1 xx prelim cleanly
> 2 xx implement the feature
> 3 xx document and test the feature
> 4 xx the final step
> 5 xx fixup! implement the feature
>
> I think I may appreciate such a feature in "rebase -i" even more, if
> the UI were done a bit differently, e.g.
>
> $ git rebase -i --edit="1 3 2 b f5 b r4" master..
>
> to mean "pick the first (i.e. bottommost) one, pick the third one
> for testing, pick the second one, then break so that I can test,
> fixup the fifth one, break to test, and finally pick the fourth
> one but reword its log message", to come up with:
>
> pick xx prelim cleanly
> pick xx document and test the feature
> pick xx implement the feature
> break
> fixup xx oops, the second one needs fixing
> break
> reword xx the final step

This kind of gui brings more power and flexibility. I don't think I
would use it since reordering in the editor feels right to me.  Maybe
that's the real problem with reordering at all with these switches,
and I should leave fixup/squash/pick out for good.

> I am guessing that the way you did it, the above would be impossible
> (as it requires reordering) but given that you would leave most of
> the 'pick's intact and only tweak the

[PATCH/RFC 1/2] rebase: add switches for drop, edit and reword

[Phil Hord]

From: Phil Hord 

A common use for rebase is to drop or edit some commit in a
feature branch.  The commit to be changed is known in advance,
so a user will say 'git rebase -i that-commit^' to see the todo
list in her editor.  Then she will change the command on the line
for "that-commit" to edit instead of pick, or delete the line
altogether.

This involves 'git log' to find the commit in the first place,
the cli to start the rebase, the editor to make a single change, and
some mental context switch from "that-commit" to its hash so she can
be sure to edit the correct line.

Introduce some new "edit-todo" switches to the rebase command to
simplify this cycle.  Add 3 new switches to support common
todo-list operations.
'--drop ' to drop a specific commit,
'--edit ' to edit a specific commit,
'--reword ' to reword a specific commit,

Allow each switch to be used mutliple times on the command line so more
than one ref could be dropped, for example.

Complain and abort the rebase if a mentioned ref is not in the
todo-list in the first place so the user doesn't get a wrong idea
from a successful 'git rebase --drop foo' that did nothing since
no foo was encountered.

Signed-off-by: Phil Hord 
---
 builtin/rebase--interactive.c | 46 +++-
 builtin/rebase.c  | 44 
 sequencer.c   | 98 ++-
 sequencer.h   | 21 +++-
 4 files changed, 192 insertions(+), 17 deletions(-)

diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 4535523bf5..9285d05443 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -140,6 +140,34 @@ static int get_revision_ranges(const char *upstream, const 
char *onto,
return 0;
 }

+static int resolve_commit_list(const struct string_list *str,
+  struct commit_list **revs)
+{
+   struct object_id oid;
+   int i;
+   for (i = 0; i < str->nr; i++) {
+   struct commit *r;
+   const char * ref = str->items[i].string;
+   if (get_oid(ref, &oid))
+   return error(_("cannot resolve %s"), ref);
+
+   r = lookup_commit_reference(the_repository, &oid);
+   if (!r)
+   return error(_("%s is not a commit"), ref);
+
+   commit_list_insert(r, revs);
+   str->items[i].util = &(*revs)->item->object.oid;
+   }
+   return 0;
+}
+
+static int resolve_edits_commit_list(struct sequence_edits *edits)
+{
+   return resolve_commit_list(&edits->drop, &edits->revs) ||
+  resolve_commit_list(&edits->edit, &edits->revs) ||
+  resolve_commit_list(&edits->reword, &edits->revs);
+}
+
 static int init_basic_state(struct replay_opts *opts, const char *head_name,
const char *onto, const char *orig_head)
 {
@@ -163,6 +191,7 @@ static int do_interactive_rebase(struct replay_opts *opts, 
unsigned flags,
 const char *onto, const char *onto_name,
 const char *squash_onto, const char *head_name,
 const char *restrict_revision, char 
*raw_strategies,
+struct sequence_edits *edits,
 struct string_list *commands, unsigned 
autosquash)
 {
int ret;
@@ -197,7 +226,7 @@ static int do_interactive_rebase(struct replay_opts *opts, 
unsigned flags,

ret = sequencer_make_script(the_repository, &todo_list.buf,
make_script_args.argc, 
make_script_args.argv,
-   flags);
+   edits, flags);

if (ret)
error(_("could not generate todo list"));
@@ -233,6 +262,7 @@ int cmd_rebase__interactive(int argc, const char **argv, 
const char *prefix)
*squash_onto = NULL, *upstream = NULL, *head_name = NULL,
*switch_to = NULL, *cmd = NULL;
struct string_list commands = STRING_LIST_INIT_DUP;
+   struct sequence_edits edits = SEQUENCE_EDITS_INIT;
char *raw_strategies = NULL;
enum {
NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH,
@@ -272,6 +302,15 @@ int cmd_rebase__interactive(int argc, const char **argv, 
const char *prefix)
   N_("restrict-revision"), N_("restrict revision")),
OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"),
   N_("squash onto")),
+   OPT_STRING_LIST(0, "drop", &edits.drop, N_("

[PATCH/RFC 2/2] rebase: add --break switch

[Phil Hord]

From: Phil Hord 

Expand the rebase edit switches to include the break switch. This
switch lets the user add a "break" instruction to the todo-list
before the mentioned reference.

This switch is a little different from the ones added so far because
it adds a new instruction between commits in the todo-list instead of
changing the command used to include a commit. It is a little like
"exec" in this regard, except it doesn't add the command after every
commit.

It is not immediately clear whether we should add the break command
before or after the referenced commit.  That is, when the user says
'--break ref', does she mean to break after ref is picked or before
it?  The answer comes when we realize that a 'break' after a ref
is functionally the same as '--edit ref'. Since the user didn't
say '--edit ref', clearly she must have wanted to break _before_ ref
is picked.  So, insert the break before the mentioned ref.

Annoyingly, however, when git stops at a break, it declares that the
previous commit is the one we stopped on, which is always different
from the one the user specified. Does anyone care?  Should --break
effectively be an alias for --edit?

'--break ' to stop the rebase before the mentioned commit

Signed-off-by: Phil Hord 
---
 builtin/rebase--interactive.c | 3 +++
 builtin/rebase.c  | 4 
 sequencer.c   | 7 +++
 sequencer.h   | 1 +
 4 files changed, 15 insertions(+)

diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 9285d05443..a81fa9c1c5 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -164,6 +164,7 @@ static int resolve_commit_list(const struct string_list 
*str,
 static int resolve_edits_commit_list(struct sequence_edits *edits)
 {
return resolve_commit_list(&edits->drop, &edits->revs) ||
+  resolve_commit_list(&edits->breaks, &edits->revs) ||
   resolve_commit_list(&edits->edit, &edits->revs) ||
   resolve_commit_list(&edits->reword, &edits->revs);
 }
@@ -302,6 +303,8 @@ int cmd_rebase__interactive(int argc, const char **argv, 
const char *prefix)
   N_("restrict-revision"), N_("restrict revision")),
OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"),
   N_("squash onto")),
+   OPT_STRING_LIST(0, "break", &edits.breaks, N_("revision"),
+   N_("stop before the mentioned ref")),
OPT_STRING_LIST(0, "drop", &edits.drop, N_("revision"),
N_("drop the mentioned ref from the "
   "todo list")),
diff --git a/builtin/rebase.c b/builtin/rebase.c
index a8101630cf..02079c4172 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1053,6 +1053,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
  NULL };
const char *gpg_sign = NULL;
struct string_list exec = STRING_LIST_INIT_NODUP;
+   struct string_list breaks = STRING_LIST_INIT_NODUP;
struct string_list reword = STRING_LIST_INIT_NODUP;
struct string_list edit = STRING_LIST_INIT_NODUP;
struct string_list drop = STRING_LIST_INIT_NODUP;
@@ -1138,6 +1139,8 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
OPT_STRING_LIST('x', "exec", &exec, N_("exec"),
N_("add exec lines after each commit of the "
   "editable list")),
+   OPT_STRING_LIST(0, "break", &breaks, N_("revision"),
+   N_("stop before the mentioned ref")),
OPT_STRING_LIST(0, "drop", &drop, N_("revision"),
N_("drop the mentioned ref from the "
   "todo list")),
@@ -1404,6 +1407,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
options.cmd = xstrdup(buf.buf);
}

+   forward_switches(&options, "--break", &breaks);
forward_switches(&options, "--drop", &drop);
forward_switches(&options, "--edit", &edit);
forward_switches(&options, "--reword", &reword);
diff --git a/sequencer.c b/sequencer.c
index d7384d987c..4a1a371757 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4261,6 +4261,7 @@ static const char *label_oid(struct object_id *oid, const 
char *label,

 void free_sequence

[PATCH/RFC 0/2] rebase: add switches to control todo-list setup

[Phil Hord]

From: Phil Hord 

I have a local patch to rebase--interactive.sh that adds "edit" switches
to rebase, permitting me to say, for example,

git rebase --drop $sha

This command creates a todo-list but drops the mentioned commit from the
list by changing the "pick" to "drop". Other switches let me edit or
reword a commit in my local history, greatly simplifying my branch
grooming workflow.

With the conversion to rebase.c (yay!) my tools are going away. I'm
porting them to the rewritten rebase*.c and I want to submit them here.
But rebase*.c is still in flux, and my changes have many conflicts with
other inflight changes. I'm happy to wait for those, but in the
meantime, I'd appreciate some feedback on the utility and acceptability
of my plan.

Here's my patch series as it stands today. It lacks documentation and
tests, but it mostly works. Errors are not handled gracefully, but this
will be rectified after I rebase onto pw/rebase-i-internal-rfc.

Currently it supports these switches:

usage: git rebase [-i] [options] [--exec ] ...
   :
--break stop before the mentioned ref
--drop  drop the mentioned ref from the todo list
--edit  edit the mentioned ref instead of picking it
--rewordreword the mentioned ref instead of picking it

I have plans to add these, but I don't like how their "onto" will be
controlled. More thinking is needed here.

--fixup fixup the mentioned ref instead of picking it
--squashsquash the mentioned ref instead of picking it
    --pick  pick the mentioned ref onto the start of the list


Phil Hord (2):
  rebase: add switches for drop, edit and reword
  rebase: add --break switch

 builtin/rebase--interactive.c |  49 +++-
 builtin/rebase.c  |  48 
 sequencer.c   | 105 +-
 sequencer.h   |  22 ++-
 4 files changed, 207 insertions(+), 17 deletions(-)

--
2.20.1

Re: Exposing the states of sequencer, etc.

[Phil Hord]

Ah, yes.  Thanks for the reminder, Peff.  Found my original patch:

http://git.661346.n2.nabble.com/PATCHv2-git-status-show-short-sequencer-state-tc7569767.html#a7570756

I seem to recall that my next iteration of it ran into many conflicts
in wt_status.c, and that file seemed like it might be heading in this
API direction anyway, so I put it on a shelf.  I guess I'm just
surprised the state checking is still so manual.  I'll explore more.

On Fri, Apr 19, 2019 at 12:46 PM Jeff King  wrote:
>
> On Fri, Apr 19, 2019 at 10:57:33AM -0700, Phil Hord wrote:
>
> > "Junio C Hamano via vger.kernel.org" writes:
> >
> > > > When cherry-picking or reverting a sequence of commits and if the final
> > > > pick/revert has conflicts and the user uses `git commit` to commit the
> > > > conflict resolution and does not run `git cherry-pick --continue` then
> > > > the sequencer state is left behind. This can cause problems later.
> > > > ...
> > > I've certainly seen this myself.  Do you use command line prompt
> > > support to remind you of the operation in progress?  I do, and I
> > > have a suspicion that it did not help me in this situation by
> > > ceasing to tell me that I have leftover state files after a manual
> > > commit of the final step that conflicted and gave control back to
> > > me.
> >
> > Is there some place today that we explain the many rules Git uses to
> > determine the operations in progress?  I once had a patch to do this
> > in code, but I think I let it die in committee.  It was something
> > like:
> >
> >  $ git status --show-progress-state
> >  cherry-pick, conflicts, untracked
> >
> > It would be helpful first to have an API for this, of course, though I
> > think that's where I got mired before.
> >
> > I'm willing to take it on again, if there's not already some alternative.
>
> Grep for get_state and print_state in wt-status.c. I think we only do so
> for the "long" status output, but it should be possible to define a
> machine-readable version for the porcelain output.
>
> -Peff

Exposing the states of sequencer, etc.

[Phil Hord]

"Junio C Hamano via vger.kernel.org" writes:

> > When cherry-picking or reverting a sequence of commits and if the final
> > pick/revert has conflicts and the user uses `git commit` to commit the
> > conflict resolution and does not run `git cherry-pick --continue` then
> > the sequencer state is left behind. This can cause problems later.
> > ...
> I've certainly seen this myself.  Do you use command line prompt
> support to remind you of the operation in progress?  I do, and I
> have a suspicion that it did not help me in this situation by
> ceasing to tell me that I have leftover state files after a manual
> commit of the final step that conflicted and gave control back to
> me.

Is there some place today that we explain the many rules Git uses to
determine the operations in progress?  I once had a patch to do this
in code, but I think I let it die in committee.  It was something
like:

 $ git status --show-progress-state
 cherry-pick, conflicts, untracked

It would be helpful first to have an API for this, of course, though I
think that's where I got mired before.

I'm willing to take it on again, if there's not already some alternative.

[no subject]

[Phil Hord]

The current url leads to a 404 error. The corrected url was determined
empirically.

Signed-off-by: Phil Hord 
---
 MaintNotes | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MaintNotes b/MaintNotes
index b7fa21f5cc..168f0b0969 100644
--- a/MaintNotes
+++ b/MaintNotes
@@ -63,7 +63,7 @@ available at:
 http://colabti.org/irclogger/irclogger_log/git-devel

 There is a volunteer-run newsletter to serve our community ("Git Rev
-News" http://git.github.io/rev_news/rev_news.html).
+News" http://git.github.io/rev_news/).

 Git is a member project of software freedom conservancy, a non-profit
 organization (https://sfconservancy.org/).  To reach a committee of
-- 
2.21.0.769.g5ae5d24923

Re: Feature request: Allow to update commit ID in messages when rebasing

[Phil Hord]

Wouldn't we need to extend this to cherry-pick, too?  Suppose I do this:

$ git log -2 --oneline --decorate foo
abcd123456  (foo)   Revert 123456
123456  Some useful commit for the future, but not now

$ git checkout bar
$ git cherry-pick foo^ foo

$ git log -2 --oneline --decorate
badc0ffee  (bar)   Revert 123456
babeface0  Some useful commit for the future, but not now

Now when I rebase bar, the revert appears to be untwinned.

Similar problems arise for other history modifying tools like
filter-branch, fast-export, reposurgeon, bfg, etc.

I guess we can use 'git patch-id' to see if the companion commit is
still in our history, but this seems tenuous.  Can we make it work
anyway?


On Thu, Apr 18, 2019 at 10:33 AM Jakub Narebski  wrote:
>
> Junio C Hamano  writes:
> > Ævar Arnfjörð Bjarmason  writes:
> >> On Wed, Apr 17 2019, Giuseppe Crinò wrote:
> >>
> >>> The feature I'm asking is to add an extra-step during rebasing,
> >>> checking whether there's a reference to a commit that's not going to
> >>> be included in history and asks the user whether the heuristics is
> >>> correct and if she wants to update those references.
> >>>
> >>> Scenario: it can happen for a commit message to contain the ID of an
> >>> ancestor commit. A typical example is a commit with the message
> >>> "revert 01a9fe8". If 01a9fe8 and the commit that reverts it are
> >>> involved in a rebase the message "revert 01a9fe8" is no longer valid
> >>> -- the old 01a9fe8 has now a different hash. This will most likely be
> >>> ignored by the person who's rebasing but will let the other people
> >>> reading history confused.
> >>
> >> This would be useful. Done properly we'd need some machinery/command to
> >> extract the commit id parts from the free-text of the commit
> >> message. That would be useful for other parts of git, e.g. as discussed
> >> here:
> >> https://public-inbox.org/git/xmqqvaxp9oyp@gitster.mtv.corp.google.com/
> >
> > That's a helpful input.
> >
> > But in general we do not have an infrastructure to systematically
> > keep track of "this commit was rewritten to produce that other
> > commit", so even if a mention of an old/superseded commit can be
> > identified reliably, there is no reliable source to rewrite it to
> > the name of the corresponding commit in the new world.
> >
> > For that mapping, we'd need something like the "git change/evolve"
> > Stefan Xenos was working on, which hasn't materialized.
>
> Well, what about limiting changes and rewriting only to the commits
> being rewritten by [interactive] rebase?  I mean that we would rewrite
> "revert 01a9fe8" only if:
>
> a.) the commit with this message is undergoing rewrite
> b.) the commit 01a9fe8 is undergoing rewrite in the same command
>
> We could use the infrastructure from git-filter-branch for this.
>
> It is serious limitation, but that might be good enough for Giuseppe
> Crinò use case.  Though for example there is a question what to do if
> referred-to commit (01a9fe8 in the example) is simply dropped, or is
> gets split in two?  Ask user?
>
>
> Another possibility would be to provide a command line option to rebase
> which would automatically generate replacements (in git-replace meaning)
> from old pre-rebase name to new post-rebase name (assuming no splitting,
> no dropping commits).  This would make references just work... well, as
> long as refs/replace/* are in place (they are not copied by default).
>
> On the other hand some of our performance-improving features, like the
> commit-graph, do not work if there are replacements.
>
>
> Best,
> --
> Jakub Narębski

Re: [PATCH] defer expensive load_ref_decorations until needed

[Phil Hord]

On Wed, Nov 22, 2017 at 1:27 PM, Jeff King  wrote:
> On Tue, Nov 21, 2017 at 03:43:36PM -0800, Phil Hord wrote:
>
>> With many thousands of references, a simple `git rev-parse HEAD` may take
>> more than a second to return because it first loads all the refs into
>> memory even though it will never use them.
>
> The overall goal of lazy-loading seems reasonable, but I'm slightly
> confused: how and why does "git rev-parse HEAD" load ref decorations?
>
> Grepping around I find that we mostly load them only when appropriate
> (when the "log" family sees a decorate option, when we see %d/%D in a
> pretty format, or with --simplify-by-decoration in a traversal). And
> poking at "rev-parse HEAD" in gdb seems to confirm that it does not hit
> that function.

Hm. I think I was confused.

I wrote v1 of this patch a few months ago. Clearly I was wrong about
rev-parse being afflicted.  We have a script that was suffering and it
uses both "git log --format=%h" and "git rev-parse" to get hashes; I
remember testing both, but I can't find it in my $zsh_history; my
memory and my commit-message must be faulty.

However, "git log" does not need any --decorate option to trigger this lag.

$ git for-each-ref| wc -l
24172
$ time git log --format=%h -1
git log --format=%h -1   0.47s user 0.04s system 99% cpu 0.509 total

I grepped the code just now, too, and I see the same as you, though;
it seems to hold off unless !!decoration_style.  Nevertheless, gdb
shows me decoration_style=1 with this command:

GIT_CONFIG=/dev/null cgdb --args git log -1 --format="%h"

Here are timing tests on this repo without this change:

git log --format=%h -1 0.54s user 0.05s system 99% cpu
0.597 total
git log --format=%h -1 --decorate  0.54s user 0.04s system 98% cpu
0.590 total
git log --format=%h%d -1   0.53s user 0.05s system 99% cpu
0.578 total

And the same commands with this change:

git log --format=%h -1  0.01s user 0.01s system 71%
cpu 0.017 total
git log --format=%h -1 --decorate   0.00s user 0.01s system 92%
cpu 0.009 total
git log --format=%h%d -10.53s user 0.09s system 88%
cpu 0.699 total

> I have definitely seen "rev-parse HEAD" be O(# of refs), but that is
> mostly attributable to having all the refs packed (and until v2.15.0,
> the packed-refs code would read the whole file into memory).

Hm.  Could this be why rev-parse was slow for me?  My original problem
showed up on v1.9 (build machine) and I patched it on v2.14.0-rc1.
But, no; testing on 1.9, 2.11 and 2.14 still doesn't show me the lag
in rev-parse.  I remain befuddled.

> I've also
> seen unnecessary ref lookups due to replace refs (we load al of the
> packed refs to find out that no, there's nothing in refs/replace).

I haven't seen this in the code, but I have had refs/replace hacks in
the past. Is that enough to wake this up?

Phil

[PATCH v2 2/2] stash-store: add failing test for same-ref

[Phil Hord]

stash-store cannot create a new stash with the same ref as stash@{0}. No
error is returned even though no new stash log is created. Add a failing
test to track.

Signed-off-by: Phil Hord 
---
 t/t3903-stash.sh | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 279e31717..7d511afd3 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -813,6 +813,17 @@ test_expect_success 'push -m also works without space' '
test_cmp expect actual
 '
 
+test_expect_failure 'store same ref twice' '
+   >foo &&
+   git add foo &&
+   STASH_ID=$(git stash create) &&
+   git stash store -m "original message" $STASH_ID &&
+   git stash store -m "custom message" $STASH_ID &&
+   echo "stash@{0}: custom message" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'store -m foo shows right message' '
>foo &&
git add foo &&
-- 
2.15.0.471.g17a719cfe.dirty

[PATCH v2] Teach stash to parse -m/--message like commit does

[Phil Hord]

`git stash push -m foo` uses "foo" as the message for the stash. But
`git stash push -m"foo"` does not parse successfully.  Similarly
`git stash push --message="My stash message"` also fails.  The stash
documentation doesn't suggest this syntax should work, but gitcli
does and my fingers have learned this pattern long ago for `commit`.

Teach `git stash` and `git store` to parse -mFoo and --message=Foo
the same as `git commit` would do.  Even though it's an internal
function, add similar support to create_stash() for consistency.

Reviewd-by: Junio C Hamano 
Signed-off-by: Phil Hord 
---

Added tests for 'stash push' and 'stash store'.
Added a note that create_stash is included but unnecessary.

 git-stash.sh | 18 +++
 t/t3903-stash.sh | 93 
 2 files changed, 111 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
shift
stash_msg=${1?"BUG: create_stash () -m requires an 
argument"}
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-u|--include-untracked)
shift
untracked=${1?"BUG: create_stash () -u requires an 
argument"}
@@ -193,6 +199,12 @@ store_stash () {
shift
stash_msg="$1"
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-q|--quiet)
quiet=t
;;
@@ -251,6 +263,12 @@ push_stash () {
test -z ${1+x} && usage
stash_msg=$1
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
--help)
show_help
;;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3b1ac1971..39c7f2ebd 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -804,6 +804,99 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
 '
 
+test_expect_success 'push -m also works without space' '
+   >foo &&
+   git add foo &&
+   git stash push -m"unspaced test message" &&
+   echo "stash@{0}: On master: unspaced test message" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store -m foo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store -m "store m" $STASH_ID &&
+   echo "stash@{0}: store m" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store -mfoo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store -m"store mfoo" $STASH_ID &&
+   echo "stash@{0}: store mfoo" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store --message=foo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store --message="store message=foo" $STASH_ID &&
+   echo "stash@{0}: store message=foo" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store --message foo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store --message "store message foo" $STASH_ID &&
+   echo "stash@{0}: store message foo" &

Re: [PATCH] defer expensive load_ref_decorations until needed

[Phil Hord]

On Tue, Nov 21, 2017, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
> I am not sure if "maybe_" is a good name here, though.  If anything,
> you are making the semantics of "load_ref_decorations()" to "maybe"
> (but I do not suggest renaming that one).
>
> How about calling it to load_ref_decorations_lazily() or something?

I groped about for something conventional, but "..._gently" didn't fit
the bill, so I went with "maybe".  I like "lazily" better for this
case. I will change it for v2.

>> Other than that, I like what this patch attempts to do.  A nicely
>> identified low-hanging fruit ;-).
>
> Having said that, this will have a bad interaction with another
> topic in flight: <20171121213341.13939-1-rafa.al...@gmail.com>
>
> Perhaps this should wait until the other topic lands and stabilizes.
> We'd need to rethink if the approach taken by this patch, i.e. to
> still pass the info to load() but holding onto it until the time
> lazy_load() actually uses it, is a sensible way forward, or we would
> want to change the calling convention to help making it easier to
> implement the lazy loading.

I noticed that after just after cleaning this one up, but didn't look
closely yet.  I'll hold this in my local queue until ra lands.

P

Re: doc: prefer 'stash push' over 'stash save'

[Phil Hord]

You probably already noticed this was my fault for filtering the patch
through Gmail's GUI.  I did also push a replacement which hopefully
does apply.

On Tue, Nov 21, 2017 at 8:39 PM, Junio C Hamano  wrote:
> Jonathan Nieder  writes:
>
>> Phil Hord wrote:
>>
>>> Although `git stash save` was deprecated recently, some parts of the
>>> documentation still refer to it instead of `push`.
>>>
>>> Signed-off-by: Phil Hord 
>>> ---
>>>  Documentation/git-stash.txt | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Jonathan Nieder 
>> Thanks.
>
> Heh, this does not even apply to 8be661007 that it claims to apply
> on top of, which is contained in fd2ebf14 ("stash: mark "git stash
> save" deprecated in the man page", 2017-10-22).
>
> I've wiggled it in, so there is no need to resend, but next time
> please be careful when sending the patch and also when sending a
> reviewed-by.

[PATCH] defer expensive load_ref_decorations until needed

[Phil Hord]

With many thousands of references, a simple `git rev-parse HEAD` may take
more than a second to return because it first loads all the refs into
memory even though it will never use them.

Defer loading any references until we actually need them.

Signed-off-by: Phil Hord 
---
 log-tree.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 3b904f037..c1509f8b9 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -84,8 +84,10 @@ void add_name_decoration(enum decoration_type type, const 
char *name, struct obj
res->next = add_decoration(&name_decoration, obj, res);
 }
 
+static void maybe_load_ref_decorations();
 const struct name_decoration *get_name_decoration(const struct object *obj)
 {
+   maybe_load_ref_decorations();
return lookup_decoration(&name_decoration, obj);
 }
 
@@ -150,10 +152,13 @@ static int add_graft_decoration(const struct commit_graft 
*graft, void *cb_data)
 
 void load_ref_decorations(int flags)
 {
-   if (!decoration_loaded) {
+   decoration_flags = flags;
+}
 
+static void maybe_load_ref_decorations()
+{
+   if (!decoration_loaded) {
decoration_loaded = 1;
-   decoration_flags = flags;
for_each_ref(add_ref_decoration, NULL);
head_ref(add_ref_decoration, NULL);
for_each_commit_graft(add_graft_decoration, NULL);
-- 
2.15.0.471.g17a719cfe.dirty

[PATCH] doc: prefer 'stash push' instead of 'stash save'

[Phil Hord]

Although `git stash save` was deprecated recently, some parts of the
documentation still refer to it.

Signed-off-by: Phil Hord 
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8be661007..056dfb866 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -175,14 +175,14 @@ create::
return its object name, without storing it anywhere in the ref
namespace.
This is intended to be useful for scripts.  It is probably not
-   the command you want to use; see "save" above.
+   the command you want to use; see "push" above.
 
 store::
 
Store a given stash created via 'git stash create' (which is a
dangling merge commit) in the stash ref, updating the stash
reflog.  This is intended to be useful for scripts.  It is
-   probably not the command you want to use; see "save" above.
+   probably not the command you want to use; see "push" above.
 
 DISCUSSION
 --
-- 
2.15.0.471.g17a719cfe.dirty

[PATCH] stash: Learn to parse -m/--message like commit does

[Phil Hord]

`git stash push -m foo` uses "foo" as the message for the stash. But
`git stash push -m"foo"` does not parse successfully.  Similarly
`git stash push --message="My stash message"` also fails.  Nothing
in the documentation suggests this syntax should work, but it does
work for `git commit`, and my fingers have learned this pattern long
ago.

Teach `git stash` to parse -mFoo and --message=Foo the same as
`git commit` would do.

Signed-off-by: Phil Hord 
---
 git-stash.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
shift
stash_msg=${1?"BUG: create_stash () -m requires an 
argument"}
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-u|--include-untracked)
shift
untracked=${1?"BUG: create_stash () -u requires an 
argument"}
@@ -193,6 +199,12 @@ store_stash () {
shift
stash_msg="$1"
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-q|--quiet)
quiet=t
;;
@@ -251,6 +263,12 @@ push_stash () {
test -z ${1+x} && usage
stash_msg=$1
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
--help)
show_help
;;
-- 
2.15.0.471.g17a719cfe.dirty

Re: stash: learn to parse -m/--message like commit does

[Phil Hord]

Hm..  Sorry about the formatting here.  It's been a while.  I'll try again.

On Tue, Nov 21, 2017 at 3:07 PM, Phil Hord  wrote:
> `git stash push -m foo` uses "foo" as the message for the stash. But
> `git stash push -m"foo"` does not parse successfully.  Similarly
> `git stash push --message="My stash message"` also fails.  Nothing
> in the documentation suggests this syntax should work, but it does
> work for `git commit`, and my fingers have learned this pattern long
> ago.
>
> Teach `git stash` to parse -mFoo and --message=Foo the same as
> `git commit` would do.
>
> Signed-off-by: Phil Hord 
> ---
>  git-stash.sh | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 4b7495144..1114005ce 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -76,6 +76,12 @@ create_stash () {
>   shift
>   stash_msg=${1?"BUG: create_stash () -m requires an argument"}
>   ;;
> + -m*)
> + stash_msg=${1#-m}
> + ;;
> + --message=*)
> + stash_msg=${1#--message=}
> + ;;
>   -u|--include-untracked)
>   shift
>   untracked=${1?"BUG: create_stash () -u requires an argument"}
> @@ -193,6 +199,12 @@ store_stash () {
>   shift
>   stash_msg="$1"
>   ;;
> + -m*)
> + stash_msg=${1#-m}
> + ;;
> + --message=*)
> + stash_msg=${1#--message=}
> + ;;
>   -q|--quiet)
>   quiet=t
>   ;;
> @@ -251,6 +263,12 @@ push_stash () {
>   test -z ${1+x} && usage
>   stash_msg=$1
>   ;;
> + -m*)
> + stash_msg=${1#-m}
> + ;;
> + --message=*)
> + stash_msg=${1#--message=}
> + ;;
>   --help)
>   show_help
>   ;;
> --
> 2.15.0.471.g17a719cfe.dirty

stash: learn to parse -m/--message like commit does

[Phil Hord]

`git stash push -m foo` uses "foo" as the message for the stash. But
`git stash push -m"foo"` does not parse successfully.  Similarly
`git stash push --message="My stash message"` also fails.  Nothing
in the documentation suggests this syntax should work, but it does
work for `git commit`, and my fingers have learned this pattern long
ago.

Teach `git stash` to parse -mFoo and --message=Foo the same as
`git commit` would do.

Signed-off-by: Phil Hord 
---
 git-stash.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
  shift
  stash_msg=${1?"BUG: create_stash () -m requires an argument"}
  ;;
+ -m*)
+ stash_msg=${1#-m}
+ ;;
+ --message=*)
+ stash_msg=${1#--message=}
+ ;;
  -u|--include-untracked)
  shift
  untracked=${1?"BUG: create_stash () -u requires an argument"}
@@ -193,6 +199,12 @@ store_stash () {
  shift
  stash_msg="$1"
  ;;
+ -m*)
+ stash_msg=${1#-m}
+ ;;
+ --message=*)
+ stash_msg=${1#--message=}
+ ;;
  -q|--quiet)
  quiet=t
  ;;
@@ -251,6 +263,12 @@ push_stash () {
  test -z ${1+x} && usage
  stash_msg=$1
  ;;
+ -m*)
+ stash_msg=${1#-m}
+ ;;
+ --message=*)
+ stash_msg=${1#--message=}
+ ;;
  --help)
  show_help
  ;;
-- 
2.15.0.471.g17a719cfe.dirty

doc: prefer 'stash push' over 'stash save'

[Phil Hord]

Although `git stash save` was deprecated recently, some parts of the
documentation still refer to it instead of `push`.

Signed-off-by: Phil Hord 
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8be661007..056dfb866 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -175,14 +175,14 @@ create::
  return its object name, without storing it anywhere in the ref
  namespace.
  This is intended to be useful for scripts.  It is probably not
- the command you want to use; see "save" above.
+ the command you want to use; see "push" above.

 store::

  Store a given stash created via 'git stash create' (which is a
  dangling merge commit) in the stash ref, updating the stash
  reflog.  This is intended to be useful for scripts.  It is
- probably not the command you want to use; see "save" above.
+ probably not the command you want to use; see "push" above.

 DISCUSSION
 --
-- 
2.15.0.471.g17a719cfe.dirty

Re: git-push branch confusion caused by user mistake

[Phil Hord]

On Mon, Mar 13, 2017 at 1:55 AM Jacob Keller  wrote:
> On Fri, Mar 10, 2017 at 2:13 PM, Junio C Hamano  wrote:
> > Phil Hord  writes:
> >> I think git should be smarter about deducing the dest ref from the
> >> source ref if the source ref is in refs/remotes, but I'm not sure how
> >> far to take it.
> >
> > My knee-jerk reaction is "Don't take it anywhere".
> >
> > Giving a refspec from the command line is an established way to
> > defeat the default behaviour when you do not give any and only the
> > remote, and making it do things behind user's back, you would be
> > robbing the escape hatch from people.
>
> It might be worth having some warning or something happen here? I've
> had several  co-workers at $DAYJOB get confused by this sort of thing.

On one very active project at $work, we have 380,000 commits, 4600
branches in refs/heads and 96 branches in refs/remotes.  About half of
the refs/remotes (43) are obviously user errors.  The other half it's
not possible for me to know.

I suggested to our admins to block attempts to push to
'refs/remotes/*' so in the future users don't lose track of commits
they think they pushed.  But I don't know if that will really happen.

Thanks for the counterexample feedback.

git-push branch confusion caused by user mistake

[Phil Hord]

This week a user accidentally did this:

$ git push origin origin/master
Total 0 (delta 0), reused 0 (delta 0)
To parent.git
 * [new branch]  origin/master -> origin/master

He saw his mistake when the "new branch" message appeared, but he was
confused about how to fix it and worried he broke something.

It seems reasonable that git expanded the original args into this one:

git push origin refs/remotes/origin/master

However, since the dest ref was not provided, it was assumed to be the
same as the source ref, so it worked as if he typed this:

git push origin refs/remotes/origin/master:refs/remotes/origin/master

Indeed, git ls-remote origin shows the result:

$ git ls-remote origin
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e HEAD
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/origin/master

Also, I verified that this (otherwise valid) command has similar
unexpected results:
$ git remote add other foo.git && git fetch other && git push
origin other/topic
$ git ls-remote origin
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e HEAD
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/origin/master
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/other/topic

I think git should be smarter about deducing the dest ref from the
source ref if the source ref is in refs/remotes, but I'm not sure how
far to take it.  It feels like we should translate refspecs something
like this for push:

origin/master
=> refs/remotes/origin/master:refs/heads/master

refs/remotes/origin/master
 => refs/remotes/origin/master:refs/heads/master

origin/master:origin/master
 => refs/remotes/origin/master:refs/heads/origin/master

master:refs/remotes/origin/master
 => refs/heads/master:refs/remotes/origin/master

That is, we should not infer a remote refspec of "refs/remotes/*"; we
should only get there if "refs/remotes" was given explicitly by the
user.

Does this seem reasonable?  I can try to work up a patch if so.

Re: Request re git status

[Phil Hord]

On Mon, Feb 6, 2017 at 3:36 PM Ron Pero  wrote:
> I almost got bit by git: I knew there were changes on the remote
> server, but git status said I was uptodate with the remote.
>

Do you mean you almost pushed some changed history with "--force"
which would have lost others' changes?  Use of this option is
discouraged on shared branches for this very reason.  But if you do
use it, the remote will tell you the hash of the old branch so you can
undo the damage.

But if you did not use --force, then you were not in danger of being
bit.  Git would have prevented the push in that case.


> Why ... not design it to [optionally] DO a fetch and THEN declare
> whether it is up to date?

It's because `git status` does not talk to the remote server, by
design.  The only Git commands that do talk to the remote are push,
pull and fetch.  All the rest work off-line and they do so
consistently.

Imagine `git status` did what you requested; that is, it first did a
fetch and then reported the status.  Suppose someone pushed a commit
to the remote immediately after your fetch completed.  Now git will
still report "up to date" but it will be wrong as soon as the remote
finishes adding the new push.  Yet the "up to date" message will
remain on your console, lying to you.  If you leave and come back in
two days, the message will remain there even if it is no longer
correct.

So you should accept that `git status` tells you the status with
respect to your most recent fetch, and that you are responsible for
the timing of the most recent fetch.  To have git try to do otherwise
would be misleading.

> Or change the message to tell what it really
> did, e.g. "Your branch was up-to-date with 'origin/master' when last
> checked at {timestamp}"? Or even just say, "Do a fetch to find out
> whether your branch is up to date"?

These are reasonable suggestions, but i don't think the extra wording
adds anything for most users.  Adding a timestamp seems generally
useful, but it could get us into other trouble since we have to depend
on outside sources for timestamps.  :-\

diff --color-words breaks spacing

[Phil Hord]

I noticed some weird spacing when comparing files with git diff
--color-words.  The space before a colored word disappears sometimes.

$ git --version
git version 2.11.0.485.g4e59582ff

echo "FOO foo; foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b
FOOfoo; foo = barbaz

There should be a space after FOO in the diff, even if git doesn't
think "foo" and "foo;" are the same word.

If I remove the semicolon, it looks better, but in fact it only moves
the error later. The missing space is now between the two "foo" words.

echo "FOO foo foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b
FOO foofoo = barbaz

Here's the same with the color codes changed to text for purposes of this email:

echo "FOO foo; foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b | tr  \\033 E
FOOE[31mfoo;E[m foo = E[31mbarE[mE[32mbazE[m

echo "FOO foo foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b | tr  \\033 E
FOO fooE[31mfooE[m = E[31mbarE[mE[32mbazE[m

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Phil Hord]

On Tue, Jan 24, 2017 at 1:54 AM Stefan Hajnoczi  wrote:
> > The use of "git show" you are demonstrating is still about showing
> > the commit object, whose behaviour is defined to show the log
> > message and the diff relative to its sole parent, limited to the
> > paths that match the pathspec.
> >
> > It is perfectly fine and desirable that "git show :"
> > and "git show  -- " behaves differently.  These are
> > two completely different features.
>
> Thanks for explaining guys.  It all makes logical sense.  I just hope I
> remember the distinctions in that table for everyday git use.


Familiar itch; previous discussion here:

https://public-inbox.org/git/1377528372-31206-1-git-send-email-ho...@cisco.com/

Phil

Re: [PATCH v5 1/4] implement submodule config API for lookup of .gitmodules values

[Phil Hord]

On Mon, Jun 15, 2015 at 5:06 PM, Heiko Voigt  wrote:
> In a superproject some commands need to interact with submodules. They
> need to query values from the .gitmodules file either from the worktree
> of from certain revisions. At the moment this is quite hard since a
> caller would need to read the .gitmodules file from the history and then
> parse the values. We want to provide an API for this so we have one
> place to get values from .gitmodules from any revision (including the
> worktree).
>
> The API is realized as a cache which allows us to lazily read
> .gitmodules configurations by commit into a runtime cache which can then
> be used to easily lookup values from it. Currently only the values for
> path or name are stored but it can be extended for any value needed.
>
> It is expected that .gitmodules files do not change often between
> commits. Thats why we lookup the .gitmodules sha1 from a commit and then
> either lookup an already parsed configuration or parse and cache an
> unknown one for each sha1. The cache is lazily build on demand for each
> requested commit.
>
> This cache can be used for all purposes which need knowledge about
> submodule configurations. Example use cases are:
>
>  * Recursive submodule checkout needs to lookup a submodule name from
>its path when a submodule first appears. This needs be done before
>this configuration exists in the worktree.
>
>  * The implementation of submodule support for 'git archive' needs to
>lookup the submodule name to generate the archive when given a
>revision that is not checked out.
>
>  * 'git fetch' when given the --recurse-submodules=on-demand option (or
>configuration) needs to lookup submodule names by path from the
>database rather than reading from the worktree. For new submodule it
>needs to lookup the name from its path to allow cloning new
>submodules into the .git folder so they can be checked out without
>any network interaction when the user does a checkout of that
>revision.
>
> Signed-off-by: Heiko Voigt 
> ---
>  .gitignore   |   1 +
>  Documentation/technical/api-submodule-config.txt |  46 +++
>  Makefile |   2 +
>  submodule-config.c   | 445 
> +++
>  submodule-config.h   |  27 ++
>  submodule.c  |   1 +
>  submodule.h  |   1 +
>  t/t7411-submodule-config.sh  |  85 +
>  test-submodule-config.c  |  66 
>  9 files changed, 674 insertions(+)
>  create mode 100644 Documentation/technical/api-submodule-config.txt
>  create mode 100644 submodule-config.c
>  create mode 100644 submodule-config.h
>  create mode 100755 t/t7411-submodule-config.sh
>  create mode 100644 test-submodule-config.c


Instead of test-submodule-config.c to test this new module, it could
be useful to implement these as extensions to rev-parse:

git rev-parse --submodule-name [:]
git rev-parse --submodule-path [:]
git rev-parse --submodule-url [:]
git rev-parse --submodule-ignore [:]
git rev-parse --submodule-recurse [:]

Has this already been considered and rejected for some reason?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Submodules as first class citizens (was Re: Moving to subtrees for plugins?)

[Phil Hord]

On Tue, Jun 9, 2015 at 2:40 PM, Jens Lehmann  wrote:
> Am 07.06.2015 um 08:26 schrieb Stefan Beller:
>>
>> On 06.06.2015 12:53, Luca Milanesio wrote:
>>>>
>>>> On 6 Jun 2015, at 18:49, Phil Hord  wrote:
>>>> On Fri, Jun 5, 2015, 2:58 AM lucamilanesio 
>>>> wrote:
>>>>>
>>>>> Ideally, as a "git clone --recursive" already exists, I would like to
>>>>> see a "git diff --recursive" that goes through the submodules as well
>>>>> :-)
>>>>>
>>>>> Something possibly to propose to the Git mailing list?
>
>
> Such an option makes lots of sense to me (though "--recurse-submodules"
> should be its name for consistency reasons). This could be an alias for
> "--submodule=full", as the "--submodule" option controls the format of
> submodule diffs.

To me, --recurse-submodules means submodules are still not first-class
citizens.  But let's put that aside for a moment; I don't care about
the switch name too much as long as I can configure
'diff.recurse-submodules = true'.

[The following is rather long.  I'm sorry for that.  Feel free to look
away when it gets too vague.]

Let me set up a submodule like so:

  $ git init /tmp/Super && cd /tmp/Super
  Super$ git submodule add https://github.com/gitster/git.git Foo

I wish to be able to grep from Super and find matches in all my submodules.

  Super$ git grep --recurse-submodules base--int
  Foo/.gitignore:/git-rebase--interactive
  Foo/Makefile:SCRIPT_LIB += git-rebase--interactive

But I want this to work naturally across git-module boundaries, so I
want this also to work (grepping a super-project from within a
submodule):

  Super$ cd Foo
  Foo$ git grep --recurse-submodules base--int ..
  .gitignore:/git-rebase--interactive
  Makefile:SCRIPT_LIB += git-rebase--interactive

I expect some groans from the audience here, because I think if the
syntax above worked, then so would this:

  $ cd /tmp
  tmp$ git grep base--int /tmp/Super/Foo
  /tmp/Super/Foo/.gitignore:/git-rebase--interactive
  /tmp/Super/Foo/Makefile:SCRIPT_LIB += git-rebase--interactive

This usage has nothing to do with submodules, really, except that it
allows git commands to reach into foreign git directories by virtue of
the path supplied as some argument instead of via $GITDIR, and in
doing so it helps solve some git submodules use cases of mine.

But if that did not turn your stomach, try this one:

  $ cd /tmp/Super
  Super$ printf "Some submodule data">Foo/data.txt
  Super$ git add Foo/data.txt
  fatal: Pathspec 'Foo/data.txt' is in submodule 'Foo'
  Super$ git add --recurse-submodules Foo/data.txt

Some notes on this usage:

1. --recurse-submodules seems like a reasonable name for this switch,
especially when you consider the 'git add --recurse-submodules .' use
case.

2. This recursive 'git add' seems dangerous to me unless git-status
also shows all the changed/untracked files in submodules as well if
the --recurse-submodules switch is included.  This would support the
expectation that 'git add .' is going to add the files shown by 'git
status .'

3. Configuring --recurse-submodules as the default mode for 'git add'
but not for 'git status' seems reckless enough that I think there
should not be separate options for these two commands.  There are
probably many other "cross-command" scenarios with similar coupling.

Moving on, as we have :/ to mean 'workdir root', I wonder how you
would spell "super-project workdir root".  Maybe it would be ::/

I realize the kinds of features I'm talking about require extensive
code changes in Git.  For example, consider the meaning of this:

  Super$ git diff --recurse-submodules origin/next origin/master

Since I created Super just a few minutes ago and it has no remote
named 'origin', this command seems meaningless to me.  But suppose
that origin/next and origin/master did exist in my Super project.
Then, I would expect in my wishlist Git, that

A.  Super$ git diff --recurse-submodules origin/next origin/master
This would include differences in Foo between origin/master:Foo and
origin/next:Foo; that is, the commits referenced from those gitlinks
in Super.

B.  Super$ git diff --recurse-submodules origin/next HEAD
This would include differences in Foo between origin/master:Foo and
HEAD:Foo; that is, the commits referenced from those gitlinks in
Super.

C.  Super$ git diff --recurse-submodules origin/next
This would include differences in Foo between origin/master:Foo and
the current Foo workdir.

D.  Super$ cd Foo && git diff origin/next
This would include differences in Foo between the Foo submodule's
origin/master and the current Foo workdir.

Now, C and D seem conf

Submodules as first class citizens (was Re: Moving to subtrees for plugins?)

[Phil Hord]

On Fri, Jun 5, 2015, 2:58 AM lucamilanesio  wrote:
>>
>> Some devs of my Team complained that with submodules it is
>> difficult to see the “full picture” of the difference
>> between two SHA1 on the root project, as the submodules
>> would just show as different SHA1s. When you Google
>> “subtree submodules” you find other opinions as well:
>>
>> Just to mention a few:
>> -
>> https://codingkilledthecat.wordpress.com/2012/04/28/why-y
>> our-company-shouldnt-use-git-submodules/ -
>> http://blogs.atlassian.com/2013/05/alternatives-to-git-su
>> bmodule-git-subtree/
>>
>> To be honest with you, I am absolutely fine with
>> submodules as I can easily leave with the “extra pain” of
>> diffing by hand recursively on submodules. But it is true
>> that it may happen to either forget to do a git submodule
>> update or otherwise forget you are in a detached branch
>> and start committing “on the air” without a branch.

...

> Ideally, as a "git clone --recursive" already exists, I would like to
> see a "git diff --recursive" that goes through the submodules as well :-)
>
> Something possibly to propose to the Git mailing list?


I've worked on git diff --recursive a bit myself, along with some
simpler use cases (git ls-tree --recursive) as POCs. I think some of
the needs there begin to have ui implications which could be
high-friction. I really want to finish it someday, but I've been too
busy lately at $job, and now my experiments are all rather stale.

It would be a good discussion to have over at the git list (copied).
Heiko and Jens have laid some new groundwork in this area and it may
be a good time to revisit it.  Or maybe they've even moved deeper than
that; I have been distracted for well over a year now.

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

Re: [PATCH] rebase -i: redo tasks that die during cherry-pick

[Phil Hord]

On Wed, Apr 29, 2015 at 1:15 PM, Junio C Hamano  wrote:
>
> Thanks, will queue.
>
> Aside from the "much more invasive" possibility, the patch makes me
> wonder if it would have been a better design to have a static "todo"
> with a "current" pointer as two state files.  Then reschedule would
> have been just the matter of decrementing the number in "current",
> instead of "grab the last line of one file and prepend to the other
> file, and then lose the last line".

That's an interesting idea.  Changing it now would impact anyone who
now depends on the current todo/done behavior, and I imagine there
are many.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regular Rebase Failure

[Phil Hord]

> On Mon, Apr 27, 2015 at 10:07 AM, Adam Steel  wrote:
>> Stefan,
>>
>> So I switched git versions.
>>
>> $ git --version
>> git version 2.3.1
>>
>> I'm still getting the same regular rebase failures.
>>
>> ---
>>
>> fatal: Unable to create
>> '/Users/asteel/Repositories/rails-teespring/.git/index.lock': File
>> exists.

Is the repository located on a mounted network share, or could other
users be accessing it via a network mount?  We had a similar problem
recently on a new Jenkins VM instance which had only NFS-mounted
storage available. I don't remember if it was Git that was failing on
there, and I wasn't directly involved in solving the problem.  But
while researching the issue I found ominous warnings about the dangers
of file-locking on remote shares [1]. Which is to say, I don't know
much, but I heard a rumor...  :-)

Perhaps this is old news and already well covered in Git.  But I am curious...


[1] http://0pointer.de/blog/projects/locking.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] rebase -i: redo tasks that die during cherry-pick

[Phil Hord]

When rebase--interactive processes a task, it removes the item from
the todo list and appends it to another list of executed tasks. If a
pick (this includes squash and fixup) fails before the index has
recorded the changes, take the corresponding item and put it on the todo
list again. Otherwise, the changes introduced by the scheduled commit
would be lost.

That kind of decision is possible since the cherry-pick command
signals why it failed to apply the changes of the given commit. Either
the changes are recorded in the index using a conflict (return value 1)
and rebase does not continue until they are resolved or the changes
are not recorded in the index (return value neither 0 nor 1) and
rebase has to try again with the same task.

Add a test cases for regression testing to the "rebase-interactive"
test suite.

Signed-off-by: Fabian Ruch 
Signed-off-by: Phil Hord 
---

Notes:
Last year in ${gmane}/250126 Fabian Ruch helpfully provided a patch
to fix a rebase bug I complained about. I have simplified it a bit
and merged in the tests which had been in a separate commit.

It has bitten me twice since the original discussion and has also
been reported by others, though I haven't found those emails to
add them to the CC list yet.

CC: Michael Haggerty 

 git-rebase--interactive.sh| 16 +++
 t/t3404-rebase-interactive.sh | 47 +++
 2 files changed, 63 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 08e5d86..bab0dcc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -132,6 +132,16 @@ mark_action_done () {
fi
 }
 
+# Put the last action marked done at the beginning of the todo list
+# again. If there has not been an action marked done yet, leave the list of
+# items on the todo list unchanged.
+reschedule_last_action () {
+   tail -n 1 "$done" | cat - "$todo" >"$todo".new
+   sed -e \$d <"$done" >"$done".new
+   mv -f "$todo".new "$todo"
+   mv -f "$done".new "$done"
+}
+
 append_todo_help () {
git stripspace --comment-lines >>"$todo" <<\EOF
 
@@ -252,6 +262,12 @@ pick_one () {
output eval git cherry-pick \
${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
"$strategy_args" $empty_args $ff "$@"
+
+   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
+   # previous task so this commit is not lost.
+   ret=$?
+   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
+   return $ret
 }
 
 pick_one_preserving_merges () {
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index eed76cc..ac429a0 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1055,4 +1055,51 @@ test_expect_success 'todo count' '
grep "^# Rebase ..* onto ..* ([0-9]" actual
 '
 
+test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
+   git checkout --force branch2 &&
+   git clean -f &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 2" git rebase -i A &&
+   test_cmp_rev HEAD F &&
+   test_path_is_missing file6 &&
+   >file6 &&
+   test_must_fail git rebase --continue &&
+   test_cmp_rev HEAD F &&
+   rm file6 &&
+   git rebase --continue &&
+   test_cmp_rev HEAD I
+'
+
+test_expect_success 'rebase -i commits that overwrite untracked files 
(squash)' '
+   git checkout --force branch2 &&
+   git clean -f &&
+   git tag original-branch2 &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 squash 2" git rebase -i A &&
+   test_cmp_rev HEAD F &&
+   test_path_is_missing file6 &&
+   >file6 &&
+   test_must_fail git rebase --continue &&
+   test_cmp_rev HEAD F &&
+   rm file6 &&
+   git rebase --continue &&
+   test $(git cat-file commit HEAD | sed -ne \$p) = I &&
+   git reset --hard original-branch2
+'
+
+test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' 
'
+   git checkout --force branch2 &&
+   git clean -f &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 2" git rebase -i --no-ff A &&
+   test $(git cat-file commit HEAD | sed -ne \$p) = F &&
+   test_path_is_missing file6 &&
+   >file6 &&
+   test_must_fail git rebase --continue &&
+   test $(git cat-file commit HEAD | sed -ne \$p) = F &&
+   rm file6 &&
+   git rebase --continue &&
+   test $(git cat-file commit HEAD | sed -ne \$p) = I
+'
+
 test_done
-- 
2.4.0.rc3.329.gd1f7d3b

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

[PATCH] git-push.txt: clean up force-with-lease wording

[Phil Hord]

The help text for the --force-with-lease option to git-push
does not parse cleanly.  Clean up the wording and syntax to
be more sensible.  Also remove redundant information in the
"--force-with-lease alone" description.

Signed-off-by: Phil Hord 
---
 Documentation/git-push.txt | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 5171086..863c30c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -157,9 +157,8 @@ already exists on the remote side.
Usually, "git push" refuses to update a remote ref that is
not an ancestor of the local ref used to overwrite it.
 +
-This option bypasses the check, but instead requires that the
-current value of the ref to be the expected value.  "git push"
-fails otherwise.
+This option overrides this restriction if the current value of the
+remote ref is the expected value.  "git push" fails otherwise.
 +
 Imagine that you have to rebase what you have already published.
 You will have to bypass the "must fast-forward" rule in order to
@@ -171,15 +170,14 @@ commit, and blindly pushing with `--force` will lose her 
work.
 This option allows you to say that you expect the history you are
 updating is what you rebased and want to replace. If the remote ref
 still points at the commit you specified, you can be sure that no
-other people did anything to the ref (it is like taking a "lease" on
-the ref without explicitly locking it, and you update the ref while
-making sure that your earlier "lease" is still valid).
+other people did anything to the ref. It is like taking a "lease" on
+the ref without explicitly locking it, and the remote ref is updated
+only if the "lease" is still valid.
 +
 `--force-with-lease` alone, without specifying the details, will protect
 all remote refs that are going to be updated by requiring their
 current value to be the same as the remote-tracking branch we have
-for them, unless specified with a `--force-with-lease=:`
-option that explicitly states what the expected value is.
+for them.
 +
 `--force-with-lease=`, without specifying the expected value, will
 protect the named ref (alone), if it is going to be updated, by
-- 
2.3.3.377.gdac1145

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

Checkout --force without complete overwrite?

[Phil Hord (hordp)]

I have a repo whose workdir tends to get pretty dirty as I jump around from 
branch to branch tending weeds and whatnot.   Sometimes when I try to switch 
branches git refuses because of local file changes.

  git checkout otherbranch
  error: Your local changes to the following files would be overwritten by 
checkout:
foo.txt
bar.c
  Please, commit your changes or stash them before you can switch branches.
  Aborting

I resolve this by examining my local changes and deciding if I want to keep 
them or not.  I keep the changes in the conflicting files that I want, and then 
I discard the rest.

One way to "discard the rest" is to say 
   git checkout HEAD -- foo.txt bar.c && git checkout otherbranch

But sometimes the list of local-change conflicts I want to discard is too long 
and this recipe seems like a good alternative to me:
   git checkout -f otherbranch

But this is disastrous, because I have been focused on examining the 
conflicting local changes in foo.txt and bar.c, but I forgot about my 
non-conflicting changes to baz.c, lost as it is in a sea of untracked files 
making up the majority of my workdir local changes.  So all my untracked files 
make the leap unscathed, but my precious forgotten changes in baz.c get wiped 
out by the checkout --force, even though the baz.c in index and in otherbranch 
are the same.

I've read the documentation for 'git checkout --force' several times and I have 
a hard time deciding what it means to do.  But I'm sure it's doing what it's 
designed to do and what the man page says it will.  (English is my first 
language, btw.)

What I am seeking is some way to checkout the other branch and replace my 
conflicted local changes while ignoring my non-conflicting local changes in 
tracked files.  Something like --force-gently, maybe.  Does such an option 
exist?

I could script something, but it comes up only often enough to bite me, so I'm 
sure I'd forget I had scripted it.

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

Re: squash commits deep down in history

[Phil Hord]

On Thu, Oct 23, 2014 at 8:34 AM, Henning Moll  wrote:
> Hi,
>
> i need to squash several commits into a single one in a automated way. I know 
> that there is interactive rebase, which can also be automated using 
> GIT_SEQUENCE_EDITOR. Unfortunately my history is very large and i need to 
> squash deep down in the history several times. So using interactive rebase 
> seems not to be the right tool.
>
> I wonder if i can solve this task using filter-branch? I have a file that 
> list the SHA1s for each squash operation per line. All SHA1s of a line are in 
> chronological order (youngest to oldest), or in other words: the first SHA1 
> is the child of the second, and so on.
>
> | ab4423e 3432343 3234252
> | 2324342 5232343
> | ...
>
> Lets say there are N lines in that file. Each line means: squash all SHA1s of 
> this line into the first (or last) SHA1 of this line.

I've often felt there should be some simple commands to do these kinds
of edits.  For example, it is easy to amend HEAD, but an
order-of-magnitude more difficult to amend HEAD^.   I imagine commands
like this:

   git rebase --reword HEAD^
   git rebase --edit some-old-commit
   git rebase --squash ab4423e 3432343 3234252

But each time I think of implementing one of these I am daunted by the
many exceptional cases.


> Performing this task with rebase would require N rewritings of the history. 
> So e.g. HEAD (but many others too) would be rewritten N times even if it is 
> not directly part of a line. My thinking is, that a filter-branch can do this 
> in a single rewrite and therefore would be much more performant.
>
> How can i solve this? Any ideas?

I know you solved this already with filter-branch, but that seems like
a complicated solution to me.  I think the --interactive rebase would
have been useful for you.  You should keep in mind that you do not
need to repeat the command multiple times for your multiple squashes.
For example, if your to-do list looks like this simple example:

pick 000
pick ab4423e
pick 3432343
pick 3234252
pick 001
pick 002
pick 2324342
pick 5232343
pick 003

I think you could get the desired effect by changing it to this:

pick 000
pick ab4423e
squash 3432343
squash 3234252
pick 001
pick 002
pick 2324342
squash 5232343
pick 003

And then running that all in one interactive rebase.  Does that make sense?

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

Re: git reflog --date

[Phil Hord]

On Tue, Oct 21, 2014 at 1:24 PM, Junio C Hamano  wrote:
> John Tapsell  writes:
>
>> Hi all,
>>
>>   Could we add a default to "--date" so that:
>>
>> git reflog --date
>>
>> just works?  (Currently you need to do:   git reflog --date=iso)  It
>> should probably obey the default in log.date?
>
> Hmph.  "--date=

Re: Bug in log for path in case of identical commit

[Phil Hord]

On Fri, Oct 31, 2014 at 4:40 AM, Alexandre Garnier  wrote:
> When merging 2 branches with the same modifications on the both sides,
> depending the merge side, one branch disappear from the file history.
>
> To be more clear, there is a script in attachment to reproduce, but
> here is the result :
> $ git log --graph --oneline --all --decorate --name-status
> *   63c807f (HEAD, master) Merge branch 'branch' into 'master'
> |\
> | * 5dc8785 (branch) Change line 15 on branch
> | | M   file
> | * d9cd3ce Change line 25 on branch
> | | M   file
> * | 7220d52 Change line 15 on master
> |/
> |   M   file
> * 7566672 Initial commit
>   A file
>
> $ git log --graph --oneline --all --decorate --name-status -- file
> * 5dc8785 (branch) Change line 15 on branch
> | M file
> * d9cd3ce Change line 25 on branch
> | M file
> * 7566672 Initial commit
>   A file
>
> => The commit 7220d52 modified the file but is not shown in file
> history anymore.
> The expected result would be:
> * 5dc8785 (branch) Change line 15 on branch
> | M file
> * d9cd3ce Change line 25 on branch
> | M file
> | * 7220d52 Change line 15 on master
> |/
> |   M   file
> * 7566672 Initial commit
>   A file
>
> The order between the 2 commits on the branch is not important.
> If you do a 'cherry-pick 7220d52' or a 'merge --squash master' instead
> of applying the same modification for commit 5dc8785, you get the same
> result (cherry-picking was my initial use-case).
> If you do not create the commit d9cd3ce, then the file history show all 
> commits.
> If you merge 'master' into 'branch', then the file history show all commits.

This last line was confusing to me.  But I think you've misinterpreted
the results a bit.  There is no difference between "merge master into
branch" and "merge branch into master" in this case.  The real reason
the "extra" commit is shown in the former case is that you used
'--all' (include all refs as commandline arguments) and the commit
which was being omitted was directly referenced by a ref, 'master'.

When I remove the "--all" from your test script, I get consistent logs
for the two merges.

Maybe this has misled your other tests as well.  Read the "History
Simplification" section of "git help log".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints

[Phil Hord]

Thanks for working on this.

On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch  wrote:
> The to-do list commands `squash` and `fixup` apply the changes
> introduced by the named commit to the tree but instead of creating
> a new commit on top of the current head it replaces the previous
> commit with a new commit that records the updated tree. If the
> result is an empty commit git-rebase stops with the error message
>
>You asked to amend the most recent commit, but doing so would make
>it empty. You can repeat your command with --allow-empty, or you can
>remove the commit entirely with "git reset HEAD^".
>
> This message is not very helpful because neither does git-rebase
> support an option `--allow-empty` nor does the messages say how to
> resume the rebase. Firstly, change the error message to
>
>The squash result is empty and --keep-empty was not specified.
>
>You can remove the squash commit now with
>
>  git reset HEAD^
>
>Once you are down, run

s/down/done

>
>  git rebase --continue
>

I like the direction of this rewording, but it seems to hide the
instructions for the user who wants to keep the empty commit.

I'm not sure how to improve it.  Maybe this:

The squash result is empty and --keep-empty was not specified.

You may remove the squash commit now with

  git reset HEAD^

or keep it by doing nothing extra.

Continue the rebase with

  git rebase --continue

> If the user wishes to squash a sequence of commits into one
> commit, f. i.
>
>pick A
>squash Revert "A"
>squash A'
>
> , it does not matter for the end result that the first squash
> result, or any sub-sequence in general, is going to be empty. The
> squash message is not affected at all by which commits are created
> and only the commit created by the last line in the sequence will
> end up in the final history. Secondly, print the error message
> only if the whole squash sequence produced an empty commit.
>
> Lastly, since an empty squash commit is not a failure to rewrite
> the history as planned, issue the message above as a mere warning
> and interrupt the rebase with the return value zero. The
> interruption should be considered as a notification with the
> chance to undo it on the spot. Specifying the `--keep-empty`
> option tells git-rebase to keep empty squash commits in the
> rebased history without notification.
>
> Add tests.
>
> Reported-by: Peter Krefting 
> Signed-off-by: Fabian Ruch 
> ---
> Hi,
>
> Peter Krefting is cc'd as the author of the bug report "Confusing
> error message in rebase when commit becomes empty" discussed on the
> mailing list in June. Phil Hord and Jeff King both participated in
> the problem discussion which ended with two proposals by Jeff.
>
> Jeff King writes:
>>   1. Always keep such empty commits. A user who is surprised by them
>>  being empty can then revisit them. Or drop them by doing another
>>  rebase without --keep-empty.
>>
>>   2. Notice ourselves that the end-result of the whole squash is an
>>  empty commit, and stop to let the user deal with it.
>
> This patch chooses the second alternative. Either way seems OK. The
> crucial consensus of the discussion was to silently throw away empty
> interim commits.

This patch does _not_ silently throw away empty commits. I wonder if
such behavior could be triggered with something like --no-keep-empty.
Maybe that belongs in another patch.

>
>Fabian
>
>  git-rebase--interactive.sh| 20 +++---
>  t/t3404-rebase-interactive.sh | 62 
> +++
>  2 files changed, 79 insertions(+), 3 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3222bf6..8820eac 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -549,7 +549,7 @@ do_next () {
> squash|s|fixup|f)
> # This is an intermediate commit; its message will 
> only be
> # used in case of trouble.  So use the long version:
> -   do_with_author output git commit 
> --allow-empty-message \
> +   do_with_author output git commit 
> --allow-empty-message --allow-empty \
> --amend --no-verify -F "$squash_msg" \
> ${gpg_sign_opt:+"$gpg_sign_opt"} ||
> die_failed_squash $sha1 "$rest"
> @@ -558,18 +558,32 @@ do_next () {
> # This is the final command of this squash/fixup group
> if test -f &qu

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Sun, Jun 29, 2014 at 5:13 PM, Jason Pyeron  wrote:
>> -Original Message-
>> From: Phil Hord
>> Sent: Sunday, June 29, 2014 16:27
>>
>> On Sun, Jun 29, 2014 at 4:20 PM, Jason Pyeron
>>  wrote:
>> >> -Original Message-
>> >> From: Phil Hord
>> >> Sent: Sunday, June 29, 2014 16:09
>> >>
>> >> On Sun, Jun 29, 2014 at 11:31 AM, Phil Hord
>> >>  wrote:
>> >> > On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron
>> >>  wrote:
>> >> >> Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
>> >> >> 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually
>> >> reconciles the merge),
>> >> >> but it was too long to be readable in the email.
>> >>
>> >> Ok, I think I understand the issue you are trying to solve now.
>> >>
>> >> Git (rather famously[1]) does not record renames or copies.  It is
>> >> expected instead that renames and copies will be detected
>> when it is
>> >> important after the fact. This allows us to ignore rename detection
>> >> and resolution when creating project history; in the future, better
>> >> rename/copy detection will "just work" on existing repositories and
>> >> the repos themselves will not need to be adjusted.
>> >
>> > Looking at http://pastebin.com/1R68v6jt , I have a work around.
>> >
>> > In summary, 7.git cherry-pick -x HEAD..rebranding , then
>> >
>> > git merge $(echo 'Merge of
>> 1ca13ed2271d60ba93d40bcc8db17ced8545f172 branch -
>> > rebranding' |\
>> > git commit-tree -p HEAD -p rebranding \
>> >  $(git cat-file -p HEAD | grep ^tree | sed -e
>> 's/^tree //') )
>> >
>> > Now it is perfect in the blame and log --graph.
>>
>> Yes, but your workaround unnecessarily duplicates commits and
>> complicates the history of your project. You are munging your project
>
> But I want to avoid thet complicating, while still showing that line 42 was
> modified by X. Should this be possible with a merge, without using 
> cherry-pick?

I think it should.  But there are other complications in your project
which may be getting in the way. You are merging two branches with no
common ancestor.  When git walks down either path looking for the
source commit for each line, it finds two sources for it.  For
example, it reaches commit 39ebb06 which appears to be the origin of
all lines in that file since it has no parent. I imagine this could
act as a short-circuit to further searching.  Furthermore, I'm not
sure how git should know any better.

It seems you already have a merge point for these two branches, but I
haven't looked deeply into how that merge was done.  But I think the
multiple disconnected branch histories may be the cause of the
confusion.

> Btw I am not able to pull up https://git.wiki.kernel.org/ or
> http://git.wiki.kernel.org/


That is strange.  It works for me here, and I am just a user like you.

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

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Sun, Jun 29, 2014 at 4:20 PM, Jason Pyeron  wrote:
>> -Original Message-
>> From: Phil Hord
>> Sent: Sunday, June 29, 2014 16:09
>>
>> On Sun, Jun 29, 2014 at 11:31 AM, Phil Hord
>>  wrote:
>> > On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron
>>  wrote:
>> >> Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
>> >> 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually
>> reconciles the merge),
>> >> but it was too long to be readable in the email.
>>
>> Ok, I think I understand the issue you are trying to solve now.
>>
>> Git (rather famously[1]) does not record renames or copies.  It is
>> expected instead that renames and copies will be detected when it is
>> important after the fact. This allows us to ignore rename detection
>> and resolution when creating project history; in the future, better
>> rename/copy detection will "just work" on existing repositories and
>> the repos themselves will not need to be adjusted.
>
> Looking at http://pastebin.com/1R68v6jt , I have a work around.
>
> In summary, 7.git cherry-pick -x HEAD..rebranding , then
>
> git merge $(echo 'Merge of 1ca13ed2271d60ba93d40bcc8db17ced8545f172 branch -
> rebranding' |\
> git commit-tree -p HEAD -p rebranding \
>  $(git cat-file -p HEAD | grep ^tree | sed -e 's/^tree //') )
>
> Now it is perfect in the blame and log --graph.

Yes, but your workaround unnecessarily duplicates commits and
complicates the history of your project. You are munging your project
to compensate for git's current shortcomings.

But it's your project.  Your choice.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Sun, Jun 29, 2014 at 11:31 AM, Phil Hord  wrote:
> On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron  wrote:
>> Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
>> 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually reconciles the merge),
>> but it was too long to be readable in the email.

Ok, I think I understand the issue you are trying to solve now.

Git (rather famously[1]) does not record renames or copies.  It is
expected instead that renames and copies will be detected when it is
important after the fact. This allows us to ignore rename detection
and resolution when creating project history; in the future, better
rename/copy detection will "just work" on existing repositories and
the repos themselves will not need to be adjusted.

What you are encountering now seems to be a shortcoming of Git's
current rename/copy detection.  But you are trying to overcome today's
shortcoming by adjusting your project history to accommodate it.
Instead you should just do the merge like you normally would without
regard to how 'git blame' shows the result.

Maybe there is a bug here still, but it is probably in git-blame.

[1] 
https://git.wiki.kernel.org/index.php/GitFaq#Why_does_Git_not_.22track.22_renames.3F
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron  wrote:
> Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
> 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually reconciles the merge),
> but it was too long to be readable in the email.
>
> git blame HEAD -- src/Main/Forms/CipherShed.fbp | cut -c 1-8 | sort -u
>
> Gives:
> ac812aa3
> b50a2fb1
>
> git blame b60070f4d0879e277f44d174a163bbb292325fea --
> src/Main/Forms/TrueCrypt.fbp | cut -c 1-8 | sort -u
>
> Gives:
> 07b2176f
> 0eb8b4fa
> 12c94add
> a17c95a3
> a757b4d4
> cac6cd14
> d0a9dfa8
> d94128a9
> e6b1437a
> f1bb489c
>
> If I use cherry pick (vs merge), I can maintain the big history in b60070f, 
> but
> loose the small history in 1ca13ed
>
>   [test]
>   / \
>  /   \
> [b60070f] [1ca13ed]
> | |
> | |
> [65efd37] |
> |\|
> | \   |
> [d8da778] [39ebb06]
>
> How do I maintain all the history including the (line) changes in 1ca13ed?

I see the results, but my brain is not able to make sense of your goal
yet.  I'll try again later when I've had my coffee.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Fri, Jun 27, 2014 at 6:39 PM, Jason Pyeron  wrote:
>> On Fri, Jun 27, 2014 at 4:47 PM, Jason Pyeron
>>  wrote:
>> > There are two identical files from the same original
>> parent, but both were
>> > renamed in their own branches. One branch moved the file to
>> a new folder, the
>> > other renamed the file in the same folder.
>>
>> You have not stated what you think the issue is.  You have only stated
>> the setup.
>
> Thanks, I could have said it better.
>
> I think that git should understand that I have moved a file in path only (the
> tree object containing the file's entry change, but not the entry it self) and
> that the branch from which I want to merge back (with common ancestry) has
> renamed the file in the same path ( the tree object is unchanged, but the 
> entry
> is) such that the object is re-parented and renamed in that path.

I think Git's perspective is that you have moved the file in both
contexts. The name of the file includes the path and filename.  The
fact is that you renamed the file in both branches.  If you had
renamed the file in only one branch, Git would have had a better
chance of figuring out the "right" thing to do.  Git tries not to do
something potentially dangerous without getting your involvement.

That said, Git's rename handling is stupid sometimes and could stand
to be improved.

> How can this be done in git or if it cannot what are the chalenges to patching
> git for this issue.

I do not know a better thing for git to do here.  I can imagine cases
where either choice is the wrong one.  If git silently makes the
choice and continues, say, during a rebase, you might not notice until
things have horribly awry.


>> ...
>> Maybe you meant to move the renamed file to the same folder where it
>> exists in the merge target.  I do not get a conflict when I do that.
>
> Are you saying I should git mv src/TrueCrypt.sln CipherShed.sln ?
>
> Then it will be in the wrong path as intended.
>
>>
>>git reset --hard b60070f4d0879e277f44d174a163bbb292325fea
>>git mv src/TrueCrypt.sln CipherShed.sln
>>git commit -m 'renamed to be congruent with a0c84ff'
>>git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68
>>
>> No conflict (on that file, anyway).
>
> Agreed, but not the desired end state.

Git's magic still has limits.

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

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Fri, Jun 27, 2014 at 4:47 PM, Jason Pyeron  wrote:
> There are two identical files from the same original parent, but both were
> renamed in their own branches. One branch moved the file to a new folder, the
> other renamed the file in the same folder.

You have not stated what you think the issue is.  You have only stated
the setup.


I suppose you want Git to merge without conflict in the end, though,
based on your script.  Is that right?


> Steps to reproduce the issue:
> git init
> git fetch https://github.com/pdinc-oss/CipherShed.git
> git fetch https://github.com/srguglielmo/CipherShed.git
> git checkout -b test b60070f4d0879e277f44d174a163bbb292325fea
> git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68
>
> CONFLICT (rename/rename): Rename "TrueCrypt.sln"->"src/TrueCrypt.sln" in 
> branch
> "HEAD" rename "TrueCrypt.sln"->"CipherShed.sln" in
> "a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68"

Git seems to be doing the correct thing here.


> git reset --hard b60070f4d0879e277f44d174a163bbb292325fea
> git mv src/TrueCrypt.sln src/CipherShed.sln
> git commit -m 'renamed to be congruent with a0c84ff'
> git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68
>
> Sill get a CONFLICT (rename/rename): Rename
> "TrueCrypt.sln"->"src/CipherShed.sln" in branch "HEAD" rename
> "TrueCrypt.sln"->"CipherShed.sln" in 
> "a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68"

Git seems to be doing the correct thing here, too.

> I will have many more to come, any suggestions?

Maybe you meant to move the renamed file to the same folder where it
exists in the merge target.  I do not get a conflict when I do that.

   git reset --hard b60070f4d0879e277f44d174a163bbb292325fea
   git mv src/TrueCrypt.sln CipherShed.sln
   git commit -m 'renamed to be congruent with a0c84ff'
   git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

No conflict (on that file, anyway).

Regards,
Phil

> This message is copyright PD Inc, subject to license 20080407P00.

p.s. Maybe you should remove this copyright threat in the future when
you are writing to an open source community seeking help.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: files deleted with no record?

[Phil Hord]

On Mon, Jun 23, 2014 at 9:15 PM, Jeremy Scott  wrote:
> I just encountered a situation where a merge was made, with no
> apparent changes in files (ie no log), but the result was that some
> files were deleted.
>
> person A adds some files
> person B adds some files from the same point

You mean from the same point in history, right?  Not files with the
same filename or path?

> person B commits and pushes.
> person A commits -- now merge is required
> a few more commits on top of person B's commit

I don't understand this step.  Who adds a few more commits on top of B and why?

> person A merges - this removes the files that B added. There is no log
> of the files being deleted

This sounds like an "evil merge" (see man gitglossary, and google),
but it's not clear to me how you arrived here.

When I tried to reproduce your issue with this script, it did not
remove any files unexpectedly:
--->8---
#!/bin/sh

set -e
mkdir foo && cd foo && git init
echo foo > foo && git add foo && git commit -mfoo

git checkout -b PersonA master
echo bar > bar && git add bar
git commit -m"PersonA: bar"

git checkout -b PersonB master
echo baz > baz && git add baz
git commit -m"PersonB: baz"

echo foo-conflict >> foo && git add foo
git commit -m"PersonB: foo"

git checkout PersonA
echo foo-different >> foo && git add foo
git commit -m"PersonA: foo (conflicts with PersonB)"

git log --oneline --all --stat

if ! git merge PersonB ; then
  git checkout PersonA foo
  git commit --no-edit
fi
git log --oneline --graph --stat
--->8---

A sneaky issue with merges is that they do not have a clear way to
show patch information -- the diff between the commit and its ancestor
-- because they have multiple ancestors.  You can show diffs for a
merge relative to each of its parents, but it is not usually done for
you automatically.  This is why making changes in a merge which are
not actually required for the merge is bad ("evil").

> Clearly this is possible - was this a user error?

Probably, but we'd need to see how the user got there.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Confusing error message in rebase when commit becomes empty

[Phil Hord]

On Wed, Jun 11, 2014 at 1:57 PM, Peter Krefting  wrote:
> Phil Hord:
>
>
>> What does it mean when you say it worked as expected?  Did it leave
>> the empty commit, omit the empty commit, or leave some un-squashed
>> commit?
>
>
> Actually, it did not work as expected I noted afterward, it just dropped the
> reversion commit, and did not squash the next commit into it as I had asked,
> so from three commits, "change", "revert", "new-change" I had two, "change",
> "new-change" with the end result being the same (i.e., instead of squashing
> all three into one "new-change", I had "change" and "revert" +
> "new-change").

Did you have a series of three commits being squashed in your to-do
list?  I mean, did you have a list like this:

   pick ...  do foo
   squash ...  revert "do foo"
   squash ...  What I really meant to do.

I suppose the rebase stopped after the first squash failed due to the
emptiness of the proposed result.  Then rebase --continue proceeded,
having decided that you were finished with the 'revert' commit.  Then
... I would expect the next commit would actually be squashed, but I
can only speculate at the reasons it might have decided not to after
your continue.

This actually sounds like another case of a bug I reported a few weeks
ago[1] and which Fabian Ruch was kindly investigating[2] and trying to
fix.  I don't think his fix would have helped in this case, but I do
think it is worthy of consideration for that same patch series.

>> It's not clear to me what --continue _should_ do in this case, but it does
>> seem like the two options here should be
>
> I sort of expect a squashed commit of "change" + "revert" to be an empty
> commit, and of "change" + "revert" + "new-change" to be a commit of
> "new-change".

Yes, but empty commits are discouraged on some projects.  If you want
your "change + revert = empty" commit to appear after the squash, I
would expect you would want to use --keep-empty on your inital rebase
command.  But I'm not sure that will do what you expected either; it
may only keep previously-empty commits during the rebase.

[1] http://article.gmane.org/gmane.comp.version-control.git/245688
[2] http://www.mail-archive.com/git%40vger.kernel.org/msg51703.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Confusing error message in rebase when commit becomes empty

[Phil Hord]

On Wed, Jun 11, 2014 at 8:49 AM, Peter Krefting  wrote:
> I am rebasing a branch to combine a couple of commits. One is a revert of a 
> previous commit. Since there are commits in-between, I do "squash" to make 
> sure I get everything, and then add the actual change on top of that. The 
> problem is that rebase stops with a confusing error message (from commit, 
> presumably):
>
>   $ git rebase --interactive
>   [...]
>   You asked to amend the most recent commit, but doing so would make
>   it empty. You can repeat your command with --allow-empty, or you can
>   remove the commit entirely with "git reset HEAD^".
>   rebase in progress; onto 342b22f
>   You are currently rebasing branch 'mybranch' on '342b22f'.
>
>   No changes
>
>   Could not apply 4682a1f20f6ac29546536921bc6ea0386441e23e... Revert 
> "something"
>
> OK, so I should retry the command with --allow-empty, then:
>
>   $ git rebase --interactive --allow-empty
>   error: unknown option `allow-empty'
>
> Nope, that's not quite right.


The correct switch for rebase is --keep-empty, but it is too late to
choose it once the interactive rebase is underway.  I think the
correct advice might be something like this:

  You asked to squash this commit and its parent, but doing so would make
  it empty. You can drop this empty commit with "git reset HEAD^" , or you can
  keep it with "git commit --amend --allow-empty".

But I have not tested this.


> Running "git rebase --continue" does work as expected, but perhaps it just 
> shouldn't stop in this case?


What does it mean when you say it worked as expected?  Did it leave
the empty commit, omit the empty commit, or leave some un-squashed
commit?  It's not clear to me what --continue _should_ do in this
case, but it does seem like the two options here should be

 1. keep the empty commit
 2. drop the empty commit

I would expect "git rebase --skip" to drop the empty commit, but maybe
it will "skip" the squash instead.  I don't know.  Better advice here
is certainly needed.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/3] sequencer: Signal failed ff as an aborted, not a conflicted merge

[Phil Hord]

On 06/10/2014 01:56 PM, Junio C Hamano wrote:
> Fabian Ruch  writes:
>
>> On 05/27/2014 08:42 PM, Junio C Hamano wrote:
>>> Fabian Ruch  writes:
 [..]

 In order to signal the three possible situations (not only success and
 failure to complete) after a pick through porcelain commands such as
 `cherry-pick`, exit with a return value that is neither 0 nor 1. -1 was
 chosen in line with the other situations in which the sequencer
 encounters an error.
>>> Hmph... do we still pass negative to exit() anywhere in our codebase?
>> No, but I thought `cmd_cherry_pick` would just forward the `return -1` from 
>> the
>> sequencer to the shell. I had another look and found that `cmd_cherry_pick`
>> calls `die` instead. Now, the commit inserts 128 as exit status in
>> `fast_forward_to`.
>>
>> Would it be appropriate to mention the choice of exit status in the coding
>> guidelines? I didn't know that the int-argument to exit(3) gets truncated and
>> that this is why it is a general rule to only use values in the range from 0 
>> to
>> 255 with exit(3).
> I personally do not think of a reason why it is necessary to mention
> how the argument to exit(3) is expected to be used by the system, but
> if it is common not to know it, I guess it would not hurt to have a
> single paragraph with at most two lines.
>
> In any case, I agree that exiting with 1 that signals "failed with
> conflict" can be confusing to the caller.  Can we have a test to
> demonstrate when this fix matters?

I think you are asking for a test and not for clarification.  But a test
was provided in 3/3 in this series.  Was it not related directly enough?

For clarification, this tri-state return value matters when the caller
is planning to do some cleanup and needs to handle the fallout
correctly.  Maybe changing this return value is not the correct way
forward, though.  It might be better if the caller could examine the
result after-the-fact instead.  This would require some reliable state
functions which I recall were somewhat scattered last time I looked. 
Also I cannot think of a reliable test for "the previous cherry-pick
failed during pre-condition checks" and I'm not sure anything should
exist to track this in .git outside of the return value for this function. 

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

Re: [RFC 2/3] rebase -i: Reschedule tasks that failed before the index was touched

[Phil Hord]

Hi Fabian,

Thanks for looking into this.

On 05/27/2014 07:56 AM, Michael Haggerty wrote:
>> +reschedule_last_action () {
>> +tail -n 1 "$done" | cat - "$todo" >"$todo".new
>> +sed -e \$d <"$done" >"$done".new
>> +mv -f "$todo".new "$todo"
>> +mv -f "$done".new "$done"
>> +}
>> +
>>  append_todo_help () {
>>  git stripspace --comment-lines >>"$todo" <<\EOF
>>  
>> @@ -470,11 +480,15 @@ do_pick () {
>> --no-post-rewrite -n -q -C $1 &&
>>  pick_one -n $1 &&
>>  git commit --allow-empty --allow-empty-message \
>> -   --amend --no-post-rewrite -n -q -C $1 ||
>> -die_with_patch $1 "Could not apply $1... $2"
>> +   --amend --no-post-rewrite -n -q -C $1
> "git cherry-pick" indicates its error status specifically as 1 or some
> other value.  But here it could be that pick_one succeeds but "git
> commit" fails; in that case ret is set to the return code of "git
> commit".  So, if "git commit" fails with a retcode different than 1,
> then reschedule_last_action will be called a few lines later.  This
> seems incorrect to me.

I agree.  I was thinking that pick_one should get this new behavior
instead of do_pick, but some callers may not appreciate the new behavior
(i.e. squash/fixup), though I expect those callers have similar problems
as this commit is trying to fix.

I think adding a pick_one_with_reschedule function (to be called in both
places from do_pick) could solve the issue Michael mentioned without
breaking other pick_one callers.

I have not tested doing so, but I think it would look like this:

===

diff --git i/git-rebase--interactive.sh w/git-rebase--interactive.sh
index fe813ba..ae5603a 100644
--- i/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -235,6 +235,17 @@ git_sequence_editor () {
 eval "$GIT_SEQUENCE_EDITOR" '"$@"'
 }
 
+pick_one_with_reschedule () {
+pick_one $1
+ret=$?
+case "$ret" in
+0) ;;
+1) ;;
+*) reschedule_last_action ;;
+esac
+return $ret
+}
+
 pick_one () {
 ff=--ff
 
@@ -474,13 +485,13 @@ do_pick () {
 # rebase --continue.
 git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1 &&
-pick_one -n $1 &&
+pick_one_with_reschedule -n $1 &&
 git commit --allow-empty --allow-empty-message \
--amend --no-post-rewrite -n -q -C $1 \
${gpg_sign_opt:+"$gpg_sign_opt"} ||
 die_with_patch $1 "Could not apply $1... $2"
 else
-pick_one $1 ||
+pick_one_with_reschedule $1 ||
 die_with_patch $1 "Could not apply $1... $2"
 fi
 }

===

I don't much like the name 'pick_one_with_reschedule', but I didn't like
my other choices, either.

I'll try to look at this and your patches more closely when I have a bit
more time.

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

Re: Zsh submodule name completion borked

[Phil Hord]

On Thu, May 1, 2014 at 6:35 PM, Felipe Contreras
 wrote:
> Phil Hord wrote:
>> When I use zsh tab-completion to complete the submodule name in 'git
>> submodule init', I get more than I expected.
>>
>> From the gerrit repository (which has plugins):
>>   $ git submodule init plugins/
>>   plugins/commit-message-length-validator\ \(v1.0-rc1-9-g545000b\)
>>   plugins/reviewnotes\ \(v1.0-rc1-8-ge984300\)
>>   plugins/replication\ \(v1.1-rc0-13-g4c3f4c9\)
>>
>> It works ok in bash.  I tried to bisect the trouble, but it still
>> fails in 1.8.3, so I'm beginning to think it's me.  Does this happen
>> to anyone else?  Is it something in my local configuration causing
>> this?
>

It seems to be something local.  I thought the issue persisted with no
local .zshrc config, but it looks like I only turned off my local
config and not the global settings.  The recent Ubuntu update is a
likely culprit.  I'll investigate locally and turn my reports up to
Ubuntu/Debian/Zshell.

> Define 'works' in bash. From what I can see from the bash completion,
> it's not doing anything special, so the completion you see are simply
> files.

To clarify my description in case anyone else sees it or is
interested, before I load /etc/zsh/zshrc, tab gives me simple filename
expansion.

After I load /etc/zsh/zshrc, tab expands only submodules in HEAD.  But
for some reason it gets the wrong kind of results in the expansion,
returning not just submodule paths, but submodule paths with tag info
appended.

Sample session:
  $ zsh --norcs
  % git submodule init plugins/
   commit-message-length-validator/
   README
   reviewnotes/
   replication/
  ^C
  % source /etc/zsh/zshrc
  % git submodule init plugins/
  plugins/commit-message-length-validator\ \(v1.0-rc1-9-g545000b\)
  plugins/reviewnotes\ \(v1.0-rc1-8-ge984300\)
  plugins/replication\ \(v1.1-rc0-13-g4c3f4c9\)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Zsh submodule name completion borked

[Phil Hord]

When I use zsh tab-completion to complete the submodule name in 'git
submodule init', I get more than I expected.

>From the gerrit repository (which has plugins):
  $ git submodule init plugins/
  plugins/commit-message-length-validator\ \(v1.0-rc1-9-g545000b\)
  plugins/reviewnotes\ \(v1.0-rc1-8-ge984300\)
  plugins/replication\ \(v1.1-rc0-13-g4c3f4c9\)

It works ok in bash.  I tried to bisect the trouble, but it still
fails in 1.8.3, so I'm beginning to think it's me.  Does this happen
to anyone else?  Is it something in my local configuration causing
this?

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

git-rebase-todo gets popped prematurely

[Phil Hord]

During a 'git rebase --continue', I got an error about having left a
file in place which the next commit intended to add as new.  Stupid me.

So I rm'ed the file and tried again.  This time, git rebase --continue
succeeded.  But it accidentally left out the next commit in my rebase-todo.

I looked in the code and it seems that when the "pick" returns an error,
rebase--interactive stops and lets the user clean up.  But it assumes
the index  already tracks a conflicted merge, and so it removes the
commit from the todo list.  In this case, however, the conflicted merge
was avoided by detecting it in advance.  The result is that the "would
be overwritten" conflict evicts the entire commit from the rebase action.

I think the code needs to detect the difference between "merge failed;
conflicted index" and "merge failed; no change".  I think I can do this
with 'git-status -s -uno', maybe.  But I haven't tried it yet and it
feels like I'm missing a case or two also.

I tried to bisect this to some specific change, but it fails the same
way as far back as 1.6.5. 

Test script follows in case anyone has a better idea how to approach
this and wants to understand it better.

#!/bin/sh

set -x
git --version
rm -rf baz
git init baz && cd baz
echo initial>initial && git add initial && git commit -minitial
echo foo>foo && git add foo && git commit -mfoo
echo bar>bar && git add bar && git commit -mbar
git log --oneline

GIT_EDITOR='sed -i -e s/^pick/edit/ -e /^#/d' git rebase -i HEAD^^
touch bar
git rebase --continue
rm bar
git rebase --continue
git log --oneline


And the tail of the output (note the missing "bar" commit even though
"Successfully rebased"):

+ git log --oneline
fcc9b6e bar
8121f15 foo
a521fa1 initial
+ GIT_EDITOR='sed -i -e s/^pick/edit/ -e /^#/d'
+ git rebase -i 'HEAD^^'
Stopped at 8121f1593ea5c66dc7e9af7719100c1fcf4ab721... foo
You can amend the commit now, with

git commit --amend

Once you are satisfied with your changes, run

git rebase --continue

+ touch bar
+ git rebase --continue
error: The following untracked working tree files would be
overwritten by merge:
bar
Please move or remove them before you can merge.
Aborting
Could not apply fcc9b6ef2941e870f88362edbe0f1078cebb20e5... bar
+ rm bar
+ git rebase --continue
Successfully rebased and updated refs/heads/master.
+ git log --oneline
8121f15 foo
a521fa1 initial


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

Re: Borrowing objects from nearby repositories

[Phil Hord]

On Tue, Mar 11, 2014 at 11:37 PM, Andrew Keller  wrote:
> I am considering developing a new feature, and I'd like to poll the group for 
> opinions.
>
> Background: A couple years ago, I wrote a set of scripts that speed up 
> cloning of frequently used repositories.  The scripts utilize a bare Git 
> repository located at a known location, and automate providing a --reference 
> parameter to `git clone` and `git submodule update`.  Recently, some 
> coworkers of mine expressed an interest in using the scripts, so I published 
> the current version of my scripts, called `git repocache`, described at the 
> bottom of .
>
> Slowly, it has occurred to me that this feature, or something similar to it, 
> may be worth adding to Git, so I've been thinking about the best approach.  
> Here's my best idea so far:
>
> 1)  Introduce '--borrow' to `git-fetch`.  This would behave similarly to 
> '--reference', except that it operates on a temporary basis, and does not 
> assume that the reference repository will exist after the operation 
> completes, so any used objects are copied into the local objects database.  
> In theory, this mechanism would be distinct from '--reference', so if both 
> are used, some objects would be copied, and some objects would be accessible 
> via a reference repository referenced by the alternates file.

Interesting.  I do something similar on my CI Server to reduce
workload on Gerrit. Having a built-in to support submodules would be
nice.  Currently my script does this:

MIRROR=/path/to/local/mirror
NEW=ssh://gerrit-server
git clone ${MIRROR}/project && cd project

#-- Init/update submodules from our local mirror if possible
git submodule update --recursive --init

#-- Switch to the remote server URL
git config remote.origin.url $(git config remote.origin.url|sed -e
"s|^${MIRROR}|${NEW}|")
git submodule sync #--recursive ; recursive not supported :-[

#-- Checkout remote updates
git pull --ff-only --recurse-submodules origin ${BRANCH}
git submodule update --recursive --init


Is that about the same as you are aiming for?


> 2)  Teach `git fetch` to read 'repocache.path' (or a better-named 
> configuration), and use it to automatically activate borrowing.

Seems like this could be trouble if a local repo is coincidentally
named the same as some unrelated repo you want to clone.  But I can
see the value.

What about something similar to url.insteadOf?   Maybe
'url.${SERVER}.autoBorrow = ${MIRROR}', with replacement semantics
similar to insteadOf.

> 3)  For consistency, `git clone`, `git pull`, and `git submodule update` 
> should probably all learn '--borrow', and forward it to `git fetch`.
>
> 4)  In some scenarios, it may be necessary to temporarily not automatically 
> borrow, so `git fetch`, and everything that calls it may need an argument to 
> do that.

--no-borrow

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

Re: Git Config pushInsteadOf is not working

[Phil Hord]

I thought you had the URLs backwards, but that doesn't seem to be the
problem, assuming I am reading your transcription correctly. Maybe the
'insteadOf' is being applied in addition to (and cancelling out) the
pushInsteadOf.  Does it work as expected if you remove one or the
other?

In any case, it sounds like a Git issue, not a Gerrit one. You should
ask on git@vger.kernel.org, which I have cc'ed here.

Phil

On Tue, Mar 11, 2014 at 4:44 AM, Raf  wrote:
> Hi All,
>
> I have been searching high and low for this issue, but somehow I do not see
> anyone encountering the same issue as me.
>
> Here is the scenario:
>
> I have created a local mirror for my group of developers to download the
> AOSP code from an external gerrit server.
> So the developers will download the code from the mirror but push to the
> external gerrit server.
>
> Hence, I have edited my /home/user/.gitconfig file to add the following:
> #To download from
> [url "ssh://localMirror"]
> insteadOf=ssh://gerritServer
> #to push
> [url "ssh://gerritServer"]
> pushInsteadOf = ssh://localMirror
>
>
> Some how, the pushInsteadOf does not work, when i tried to push the changes
> to the external gerrit server, it still pushes to the local mirror server.
>
> Also, when I tried to manually add the remote to the repository: git remote
> add gerrit_origin ssh://gerritServer
> I tried to push to the gerrit_origin, it still pushes to the local mirror
> server. Which is strange..
>
> Please help. I have spent whole day looking for this solution to no avail.
>
> Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] Introduce git submodule add|update --attach

[Phil Hord]

On Sun, Dec 29, 2013 at 8:49 PM, Francesco Pretto  wrote:
>
> by default "git submodule" performs its add or update operations on a detached
> HEAD. This works well when using an existing full-fledged/indipendent project 
> as
> the submodule, as there's less frequent need to update it or commit back
> changes. When the submodule is actually a large portion of shareable code
> between  different projects, and the superproject needs to track very closely
> the evolution of the submodule (or the other way around), I feel more 
> confortable
> to reattach the HEAD of the submodule with an existing branch. This can be as
> simple as having a superproject "project1" in branch "master" with a submodule
> "common" attached to the branch "master-project1" or, in a more development
> workflow, "project1" in branch "featureA" with the same submodule "common"
> attached to a similarly named branch "featureA". Doing this in git requires me
> the following:
>
> # Maintainer
> $ git submodule add --branch "master-project1"  common
> $ git commit -m "Added submodule"
> $ git config -f .gitmodules submodule.common.ignore all
> $ git push
> $ cd 
> $ git checkout "master-project1"
>
> # Developer
> $ git pull
> $ git submodule init
> $ git submodule update --remote
> $ cd 
> $ branch="$(git config -f ..\.gitmodules submodule.common.branch)"; git 
> checkout $branch
>
> While the burden for the repository maitainer/administrator is acceptable, in
> the developer point of view there are two problems:
> 1) Checking out an attached HEAD of a specified branch as when using 
> "--remote"
> is not really simple as it could be and could require lauching of scrips or
> reading some repository specific documentation. Also in Windows platform the
> syntax for inline shell evaluation of commands is less known between users;
> 2) There's no way to store a similar default behaviour in the repository 
> except
> by using scripts. Also recently submodule..update custom !commands
> in no more supported when stored in .gitmodules [1].
>
> The attached patch tries to solve these problems by introducing an "--attach"
> switch to the "add" and "update" submodule commands and a "--detach" switch 
> just
> for the "update" command. It also add the support for an 
> 'submodule..attach'
> property when updating. Using the "--attach" switch when adding a submodule 
> does:
> - create the submodule checking out an attached HEAD;
> - set the 'submodule..attach' property to 'true';
> - set the 'submodule..ignore' property to 'all' (this is useful as
> attaching to the branch doesn't require tracking of revision sha1).
>
> The rationale of setting 'attach' and 'ignore' properties when adding a
> submodule with the "--attach" switch is to give a convenient default 
> behaviour.
> No other properties are set: the repository responsible will still be required
> to configure a different 'submodule..update' behaviour separetely, if he
> wants that.
>
> When updating, using the '--attach' switch or operating in a repository with
> 'submodule..attach' set to 'true' will:
> - checkout a branch with an attached HEAD if the repository was just cloned;
> - perform a fast-forward only merge of changes if it's a 'checkout' update,
> keeping the HEAD attached;
> - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update
> operation if the HEAD was found detached.

I need to understand this "reattach the HEAD" case better. Can you
give some examples of the expected behavior when merge, rebase or
!command is encountered?


> '--attach' or 'submodule..attach' set to true also implies '--remote', 
> as
> it's needed the origin HEAD sha1 to verify the current HEAD state.
>
> A '--detach' switch is also available. Using the '--detach' switch or
> operating in a repository with 'submodule..attach' set to 'false' during
> update will:
> - checkout a detached HEAD if the repository was just cloned (same behaviour 
> as
> before);
> - detach the HEAD prior performing a 'merge', 'rebase' or '!command' update
> operation if the HEAD was found attached.
>
> 'submodule..attach' works the same way as 'submodule..update'
> property: git copies the values found in ".gitmodules" in ".git/config" when
> performing an "init" command. "update" looks for the values in ".git/config"
> only.
>
> '--attach' and '--detach' switches override an opposite behaviour of 
> 'submodule..attach'
> properties.
>
> The patch is small (touches only git-submodule.sh) and 100% additive with
> respect to currently documented behaviour: when using "add" and "update"
> commands without the introduced switches and properties, git shall operate as
> before. As a bonus (but this was done to ease conditionals and keep the code
> clean) it also clarifies and validates the content of 
> 'submodule..update'
> during 'update' command, warning the user if it's not one of the supported
> values 'checkout', 'merge', 'rebase' and 'none'. Please note that 'checkout'
> update command was documented in 

Re: Finding the repository

[Phil Hord]

On Thu, Oct 24, 2013 at 9:46 AM, Duy Nguyen  wrote:
> On Thu, Oct 24, 2013 at 2:49 PM, Perry Hutchison  
> wrote:
>> Duy Nguyen  wrote:
>>
>>> ... it's not easy to determine ambiguity here, especially when the
>>> repo finding code does not know anything about "bar/barz.c" (is it
>>> a pathname or an argument to an option?).
...
>>> There are more cases to consider, like what if you do
>>> "git rm bar/baz.c and rab/zab.c" where bar and rab are
>>> two different repositories..
>>
>> So we remove baz.c from bar and zab.c from rab.  It's not clear
>> to me that there's anything wrong with that -- it's exactly what
>> I would expect to have happen (and also what the hackish script
>> I posted will do).
>
> For "git rm", maybe. Many other commands need repo information and it
> would not make sense to have paths from two different repositories.
> For example, commit, rev-list or log. And it may break more things as
> most of current commands are designed to work on one repo from a to z.
> Some may support multi-repo operations if they're part of submodule
> support.

I've done some preliminary work on extending this sort of behavior to
submodule commands.  For example,

  git grep --recurse-submodules foo

which would look in the current project path and also any submodules
encountered.  This usage also begs for this extension:

  git grep --recurse-submodules foo path/to/sub/bar.c

Where 'path/to/sub' is a submodule, and therefore a foreign git repo
to this one.  Solving this is a little bit easier than your case
because git is already running inside a repo. Extending the reach to
submodules only requires more odb's than our first one to be
considered.  Along the way, I have considered your case, but I haven't
focused on it. Lately I haven't had time to focus on my case either,
though.  :-\

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

Re: Bug: [hostname:port]:repo.git notation no longer works (for ssh)

[Phil Hord]

On Fri, Sep 27, 2013 at 4:07 AM, Morten Stenshorne  wrote:
> If I don't go via the ssh tunnel (I finally have some VPN stuff these
> days, so I don't really need the tunnel thing anymore, but that's going
> to be a lot of remotes to update, so I'd prefer it just worked like it
> used to):
>
> -url = [localhost:2223]:blink.git
> +url = git:blink.git
>
> ... it works fine.


Until you get a proper fix, I wonder if this will help:

  git config --global --add url."git:".insteadOf  "[localhost:2223]:"

See "git help config" for details on the insteadOf config setting.

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

Re: [PATCH 3/3] for-each-ref: introduce %(upstream:track[short])

[Phil Hord]

On Fri, Sep 27, 2013 at 8:10 AM, Ramkumar Ramachandra
 wrote:
> Introduce %(upstream:track) to display "[ahead M, behind N]" and
> %(upstream:trackshort) to display "=", ">", "<", or "<>"
> appropriately (inspired by contrib/completion/git-prompt.sh).
>
> Now you can use the following format in for-each-ref:
>
>   %C(green)%(refname:short)%C(reset)%(upstream:trackshort)
>
> to display refs with terse tracking information.

Thanks.  I like this.

>
> Note that :track and :trackshort only work with "upstream", and error
> out when used with anything else.

I think I would like to use %(refname:track) myself, but this does not
detract from this change.

>
> Signed-off-by: Ramkumar Ramachandra 
> ---
>  Documentation/git-for-each-ref.txt |  6 +-
>  builtin/for-each-ref.c | 44 
> --
>  2 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index f1d4e9e..682eaa8 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -93,7 +93,11 @@ objectname::
>  upstream::
> The name of a local ref which can be considered ``upstream''
> from the displayed ref. Respects `:short` in the same way as
> -   `refname` above.
> +   `refname` above.  Additionally respects `:track` to show
> +   "[ahead N, behind M]" and `:trackshort` to show the terse
> +   version (like the prompt) ">", "<", "<>", or "=".  Has no
> +   effect if the ref does not have tracking information
> +   associated with it.
>
>  HEAD::
> Used to indicate the currently checked out branch.  Is '*' if
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index e54b5d8..10843bb 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -631,6 +631,7 @@ static void populate_value(struct refinfo *ref)
> int eaten, i;
> unsigned long size;
> const unsigned char *tagged;
> +   int upstream_present = 0;

This flag is out of place.  It should be in the same scope as 'branch'
since the code which depends on this flag also depends on '!!branch'.

However, I don't think it is even necessary.  The only way to reach
the places where this flag is tested is when (name="upstream") and
(upstream exists).  In all other cases, the parser loops before
reaching the track/trackshort code or else it doesn't enter it.

>
> ref->value = xcalloc(sizeof(struct atom_value), used_atom_cnt);
>
> @@ -648,6 +649,7 @@ static void populate_value(struct refinfo *ref)
> int deref = 0;
> const char *refname;
> const char *formatp;
> +   struct branch *branch;
>
> if (*name == '*') {
> deref = 1;
> @@ -659,7 +661,6 @@ static void populate_value(struct refinfo *ref)
> else if (!prefixcmp(name, "symref"))
> refname = ref->symref ? ref->symref : "";
> else if (!prefixcmp(name, "upstream")) {
> -   struct branch *branch;
> /* only local branches may have an upstream */
> if (prefixcmp(ref->refname, "refs/heads/"))
> continue;
> @@ -669,6 +670,7 @@ static void populate_value(struct refinfo *ref)
> !branch->merge[0]->dst)
> continue;
> refname = branch->merge[0]->dst;
> +   upstream_present = 1;
> }
> else if (!strcmp(name, "flag")) {
> char buf[256], *cp = buf;
> @@ -686,6 +688,7 @@ static void populate_value(struct refinfo *ref)
> } else if (!strcmp(name, "HEAD")) {
> const char *head;
> unsigned char sha1[20];
> +
> head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
> if (!strcmp(ref->refname, head))
> v->s = "*";
> @@ -698,11 +701,48 @@ static void populate_value(struct refinfo *ref)
> formatp = strchr(name, ':');
> /* look for "short" refname format */
> if (formatp) {
> +   int num_ours, num_theirs;
> +
> formatp++;
> if (!strcmp(formatp, "short"))
> refname = shorten_unambiguous_ref(refname,
>   warn_ambiguous_refs);
> -   else
> +   else if (!strcmp(formatp, "track") &&
> +   !prefixcmp(name, "upstream")) {
> +   char buf[40];
> +
> +   if (!upstream_present)
> +   continue;
> +  

Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color

[Phil Hord]

On Fri, Sep 27, 2013 at 8:10 AM, Ramkumar Ramachandra
 wrote:
> Enhance 'git for-each-ref' with color formatting options.  You can now
> use the following format in for-each-ref:
>
>   %C(green)%(refname:short)%C(reset)
>
> Signed-off-by: Ramkumar Ramachandra 
> ---
>  Documentation/git-for-each-ref.txt |  4 +++-
>  builtin/for-each-ref.c | 23 +++
>  2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index f2e08d1..078a116 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -45,7 +45,9 @@ OPTIONS
> It also interpolates `%%` to `%`, and `%xx` where `xx`
> are hex digits interpolates to character with hex code
> `xx`; for example `%00` interpolates to `\0` (NUL),
> -   `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
> +   `%09` to `\t` (TAB) and `%0a` to `\n` (LF). Additionally,
> +   colors can be specified using `%C(colorname)`. Use
> +   `%C(reset)` to reset the color.

Reduce the color explanation here and refer to the config page.
Something like pretty-formats does:

'%C(...)': color specification, as described in color.branch.*
config option;

>
>  ...::
> If one or more patterns are given, only refs are shown that
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 1d4083c..a1ca186 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -9,6 +9,7 @@
>  #include "quote.h"
>  #include "parse-options.h"
>  #include "remote.h"
> +#include "color.h"
>
>  /* Quoting styles */
>  #define QUOTE_NONE 0
> @@ -155,10 +156,13 @@ static const char *find_next(const char *cp)
> while (*cp) {
> if (*cp == '%') {
> /*
> +* %C( is the start of a color;
>  * %( is the start of an atom;
>  * %% is a quoted per-cent.
>  */
> -   if (cp[1] == '(')
> +   if (cp[1] == 'C' && cp[2] == '(')
> +   return cp;
> +   else if (cp[1] == '(')
> return cp;
> else if (cp[1] == '%')
> cp++; /* skip over two % */
> @@ -180,8 +184,11 @@ static int verify_format(const char *format)
> const char *ep = strchr(sp, ')');
> if (!ep)
> return error("malformed format string %s", sp);
> -   /* sp points at "%(" and ep points at the closing ")" */
> -   parse_atom(sp + 2, ep);
> +   /* Ignore color specifications: %C(
> +* sp points at "%(" and ep points at the closing ")"
> +*/
> +   if (prefixcmp(sp, "%C("))
> +   parse_atom(sp + 2, ep);
> cp = ep + 1;
> }
> return 0;
> @@ -933,12 +940,20 @@ static void emit(const char *cp, const char *ep)
>  static void show_ref(struct refinfo *info, const char *format, int 
> quote_style)
>  {
> const char *cp, *sp, *ep;
> +   char color[COLOR_MAXLEN];
>
> for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
> ep = strchr(sp, ')');
> if (cp < sp)
> emit(cp, sp);
> -   print_value(info, parse_atom(sp + 2, ep), quote_style);
> +
> +   /* Do we have a color specification? */
> +   if (!prefixcmp(sp, "%C("))
> +   color_parse_mem(sp + 3, ep - sp - 3, "--format", 
> color);
> +   else {
> +   printf("%s", color);

'color' used uninitialized here?

> +   print_value(info, parse_atom(sp + 2, ep), 
> quote_style);
> +   }
> }
> if (*cp) {
> sp = cp + strlen(cp);
> --
> 1.8.4.478.g55109e3
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git tag output order is incorrect (IMHO)

[Phil Hord]

Someone at $work asked me this week how to find the current and
previous tags on his branch so he could generate release notes.  I
just need "last two tags on head in topo-order". I was surprised by
how complicated this turned out to be. I ended up with this:

  git log --decorate=full --pretty=format:'%d' HEAD |
sed -n -e 's-^.* refs/tags/\(.*\)[ )].*$-\1-p' |
head -2

Surely there's a cleaner way, right?

Phil



On Sun, Sep 8, 2013 at 6:49 PM, Felipe Contreras
 wrote:
> On Thu, Jul 18, 2013 at 10:27 AM, Rahul Bansal  
> wrote:
>> I am posting here first time, so please excuse me if this is not right place 
>> to send something like this.
>>
>> Please check - 
>> http://stackoverflow.com/questions/6091306/can-i-make-git-print-x-y-z-style-tag-names-in-a-sensible-order
>>
>> And also - https://github.com/gitlabhq/gitlabhq/issues/4565
>>
>> IMHO "git tag" is expected to show tag-list ordered by versions.
>>
>> It may be case, that people do not follow same version numbering convention. 
>> Most people after x.9.x increment major version (that is why they may not be 
>> affected because of this)
>>
>> Another option like "git tag --date-asc" can be added which will print tags 
>> by creation date. (As long as people do not create backdated tag, this will 
>> work).
>
> I completely agree, and there was a proposal to an option like this a
> long time ago:
>
> http://article.gmane.org/gmane.comp.version-control.git/111032
>
> --
> Felipe Contreras
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-gui: Modify push dialog to support Gerrit review

[Phil Hord]

On Fri, Sep 6, 2013 at 6:30 AM, Joergen Edelbo  wrote:
> Problem: It is not possible to push for Gerrit review as you will
> always try to push to "refs/heads/..." on the remote.
>
> Changes done:
>
> Add an option to select "Gerrit review" and a corresponding entry
> for a branch name. If this option is selected, push the changes to
> "refs/for//". In this way the local branch
> names will be used as topic branches on Gerrit.


In my gerrit repos, I have this configuration

  $  git config remote.origin.push HEAD:refs/for/master

And so I can simply 'git push' and git does what I mean.

My main complaint with git-gui's push is that it ignores my
configuration. Can you teach git-gui to honor this setting, instead?

I would like for this remote."name".push option to be smarter and figure
out which $branch I mean when I am not on master, but that is a different
discussion. Having said that, I see that your change to git-gui attempts to
make that automatic.  Kudos for that, but I don't think this will work for
me as I am often on a detached branch and so "refs/heads/$b" is meaningless.

Can you think of a sane way to separate the "from" and the "to" branches in
the GUI?  I mean, I would like to push "HEAD" and I would like it to
go to "refs/for/frotz-2.0".

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

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Tue, Aug 27, 2013 at 12:07 AM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Not necessarily.  If the user is asking the question in a more
>> natural way (I want to see where in 'next' branch's tip commit hits
>> appear, by the way, I know I am only interested in builtin/ so I'd
>> give pathspec as well when I am asking this question), the output
>> does give   , so it is more than coincidence.
>
> This part needs to be qualified.  "Natural" is of course in the eyes
> of beholder.  If we assume that your #1 holds true (i.e. the tuple
>  is
> important and merging them into one  lose
> information),

"My #1" is really "what I inferred from your statements."  I did not
think the tuple was important, but I agree that may be my
shortsightedness.  If the tuple is important, then it seems to me that
the --null behavior is a bug (the colon is left intact); but changing
it now seems senseless or harmful.

> then it is natural to ask "inside origin/master tree,
> where do I have hits?  By the way, I am only interested in builtin/"
> and get "origin/master:builtin/pack-objects.c" as an answer (this is
> from your earlier example), than asking "inside origin/master:builtin
> tree, where do I have hits?"
>
> If we do not consider #1 is false and the tree information can be
> discarded, then it does not matter if we converted the colon after
> origin/master to slash when we answer the latter question, and the
> latter question stops being unnatural.
>
>> ...
>> but it might be a good change to allow A:B:C to be parsed as a
>> proper extended SHA-1 expression and make it yield
>>
>>   git rev-parse $(git rev-parse $(git rev-parse A):B):C
>>
>> Right now, "B:C" is used as a path inside tree-ish "A", but I think
>> we can safely fall back, when path B:C does not appear in tree-ish
>> A, to see if path B appears in it and is a tree, and then turn it
>> into a look-up of path C in that tree A:B.
>
> And if we want to keep the  tuple, but still want to
> make it easier to work with the output, allowing A:B:C to be parsed
> as an extended SHA-1 expression would be a reasonable solution, not
> a work-around. The answer is given to the question asked in either
> way (either "in origin/master, but limited to these pathspecs" or
> "in the tree origin/master:builtin/") without losing information,
> but the issue you had is that the answer to the latter form of
> question is not understood by the object name parser, which I
> personally think is a bug.

I am beginning to agree, though we discovered some other weird output
from grep which also does not parse. (origin/master:../relative/path).

It seems that fixing this bug could be very confusing for
get_sha1_with_context unless the context was rewritten to match the
traditional syntax.

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

Re: [PATCH 2/2] grep: use slash for path delimiter, not colon

[Phil Hord]

On Mon, Aug 26, 2013 at 4:52 PM, Jeff King  wrote:
> On Mon, Aug 26, 2013 at 10:13:14PM +0200, Johannes Sixt wrote:
>
>> Am 26.08.2013 21:56, schrieb Jeff King:
>> > Also, prevent the delimiter being added twice, as happens now in these
>> > examples:
>> >
>> >   git grep -l foo HEAD:
>> >   HEAD::some/path/to/foo.txt
>> >^
>>
>> Which one of these two does it print then?
>>
>> HEAD:/some/path/to/foo.txt
>> HEAD:some/path/to/foo.txt
>
> It should (and does) print the latter.
>
> But I do note that our pathspec handling for subdirectories seems buggy.
> If you do:
>
>   $ cd Documentation
>   $ git grep -l foo | head -1
>   RelNotes/1.5.1.5.txt
>
> that's fine; we limit to the current directory. But then if you do:
>
>   $ git grep -l foo HEAD | head -1
>   HEAD:RelNotes/1.5.1.5.txt
>
> we still limit to the current directory, but the output does not note
> this (it should be "HEAD:./RelNotes/1.5.1.5.txt"). I think this bug is
> orthogonal to Phil's patch, though.

Maybe not.  My path completes the assumption that the L:R value
returned by grep is an object ref; but Junio still thought it wasn't.
I think this is another case where his view was correct.

There's more bad news on this front.

$ cd Documentation
$ git grep -l foo HEAD .. | head -1
HEAD:../.gitignore

That's not a valid ref, either (though maybe it could be).

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

Re: [PATCH 2/2] grep: use slash for path delimiter, not colon

[Phil Hord]

On Mon, Aug 26, 2013 at 4:13 PM, Johannes Sixt  wrote:
> Am 26.08.2013 21:56, schrieb Jeff King:
>> Also, prevent the delimiter being added twice, as happens now in these
>> examples:
>>
>>   git grep -l foo HEAD:
>>   HEAD::some/path/to/foo.txt
>>^
>
> Which one of these two does it print then?
>
> HEAD:/some/path/to/foo.txt
> HEAD:some/path/to/foo.txt


With my patch it prints the latter.

This is because get_sha1_with_context("HEAD:"...) returns an empty
'path' string.  The code decides to use ':' as the delimiter in that
case, but it sees there already is one at the end of "HEAD:".

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

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 1:26 PM, Junio C Hamano  wrote:
> Phil Hord  writes:
>
>>> If your justification were "above says 'there may be a readon why
>>> the user wanted to ask it in that way', i.e. 'find in this tree
>>> object HEAD:some/path and report where hits appear', but the reason
>>> can only be from laziness and/or broken script and the user always
>>> wants the answer from within the top-level tree-ish", then that
>>> argument may make some sense. You need to justify why it is OK to
>>> lose information in the answer by rewriting the colon that separates
>>> the question ("in this tree object") and the answer ("at this path
>>> relative to the tree object given").
>>>
>>> Whether you rewrite the input or the output is not important; you
>>> are trying to give an answer to a different question, which is what
>>> I find questionable.
>>
>> Ok, so if I can summarize what I am inferring from your objection:
>>
>>  1. The (tree-path, found-path) pair is useful information to get back
>> from git-grep.
>
> At least that was the intent. I can be persuaded that your change
> will not break anybody if you successfully argue that it is not a
> useful information, though.

Anyone who is interested in the matched trees from the original
command-line only needs to compare the matched paths' prefixes with
the original args to see which one is responsible for each hit.  But
this is not convincing to me because they may not know the original
args to the grep command.

I do not know if this a good reason to keep supporting this mode,
though.  I can just as easily see some script being confused by four
colons in

origin/master:some/:path/file.c:1:int main()

instead of only three that he is used to because someone passed in a
longer tree-path than expected.

I don't know if I can make that argument.

I think I _can_ argue that the colon separator here is historically
just a part of the filename because it is not affected by "--null".

>>  2. A colon is used to delimit these pieces of information, just as a
>> colon is used to delimit the filename from the matched-line results.
>>
>>  3. The fact that the colon is also the separator used in object refs
>> is mere coincidence; the colon was _not_ chosen because it
>> conveniently turns the results list into valid object references.  A
>> comma could have been instead, or even a \t.
>
> Not necessarily.  If the user is asking the question in a more
> natural way (I want to see where in 'next' branch's tip commit hits
> appear, by the way, I know I am only interested in builtin/ so I'd
> give pathspec as well when I am asking this question), the output
> does give   , so it is more than coincidence.
>
> I do not think it is worth doing only for this particular use case,
> but it might be a good change to allow A:B:C to be parsed as a
> proper extended SHA-1 expression and make it yield
>
> git rev-parse $(git rev-parse $(git rev-parse A):B):C
>
> Right now, "B:C" is used as a path inside tree-ish "A", but I think
> we can safely fall back, when path B:C does not appear in tree-ish
> A, to see if path B appears in it and is a tree, and then turn it
> into a look-up of path C in that tree A:B.

That would also solve this problem, usually.  But I don't like it
nearly as much as my patch, and I agree it seems extreme for this one
corner-case.

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

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 1:03 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> If your justification were "above says 'there may be a readon why
>> the user wanted to ask it in that way', i.e. 'find in this tree
>> object HEAD:some/path and report where hits appear', but the reason
>> can only be from laziness and/or broken script and the user always
>> wants the answer from within the top-level tree-ish", then that
>> argument may make some sense. You need to justify why it is OK to
>> lose information in the answer by rewriting the colon that separates
>> the question ("in this tree object") and the answer ("at this path
>> relative to the tree object given").
>>
>> Whether you rewrite the input or the output is not important; you
>> are trying to give an answer to a different question, which is what
>> I find questionable.
>
> For example, one of the cases the proposed change will break that I
> am worried about is a script that wants to take N trees and a
> pattern, and report where in the given trees hits appear, something
> like:
>
> git grep -c -e $pattern "$@" |
> perl -e '
> my @trees = @ARGV;
> my %found = ();
> while (<>) {
> my $line = $_;
> for (@trees) {
> my $tree_prefix = $_ . ":";
> my $len = len($tree_prefix);
> if (substr($line, 0, $len) eq $tree_prefix) {
> my ($path_count) = substr($line, $len);
> my ($path, $count) = $path_count =~ 
> /^(.*):(\d+)$/
> $found{$tree} = [$path, $count];
> }
> }
> }
> # Do stats on %found
> ' "$@"
>

I do understand there is potential breakage when a script is parsing
the output.  I did not consider that this was a feature someone may
want; it really only looks like a breakage to me, for the reasons I've
already given.

It's surprising just how broken it looks to me (given that I now know
it is not) since all the other filenames output from 'git-grep -l' are
valid filenames or object references.  There is only this one
tree-path instance which does not.  I suppose I will learn to live
with it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 12:49 PM, Phil Hord  wrote:
> If so, then I would like to point out to you the convenience I
> accidentally encountered using this tool.  Perhaps you didn't realize
> how helpful it was when you chose to use a colon there.

My itch comes from a case where I am looking for references in some
other branch and I want to narrow my search.

  $ git grep object origin/master
  origin/master:.gitignore:/git-count-objects
  origin/master:.gitignore:/git-fsck-objects
  origin/master:.gitignore:/git-hash-object
  <8600 lines more deleted>

I find hits in the region that interests me and then I try to zoom in
there.  Conveniently, the path I want to search is right there on my
screen.  So I copy the part of the object reference I want to limit my
search to, and I try again:

   $ git grep object origin/master:builtin/

Now, I find the file that has the code I wanted.  But I want to do
something fancier to it than grep, so this time I copy-and-paste the
filename into my command line after typing 'git show':

   $ git show origin/master:builtin/pack-objects.c | vim -

But this doesn't work if the delimiter used was a colon.

   $ git show origin/master:builtin:pack-objects.c | vim -

I have to edit my copy-and-pasted text, which means I have think about
it a bit, and it interrupts all the other things I was thinking about
already.  Eventually I might learn to do this edit automatically,
except it is not needed most of the time.  It is only needed when I
provide a tree-path instead of a "branch  path". In the latter
case, the full path is spelled out for me correctly.  And in all other
cases the filenames provided by git-grep are valid filenames or object
names.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 12:23 PM, Junio C Hamano  wrote:
> Phil Hord  writes:
>
>> On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano  wrote:
>>> Junio C Hamano  writes:
>>>
>>>> Jonathan Nieder  writes:
>>>>
>>>>> I think Phil meant that when "git grep" is asked to search within
>>>>> "HEAD:some/path", filenames tacked on at the end should be appended
>>>>> with a '/' separator instead of the usual ':' (e.g.,
>>>>> "HEAD:some/path/inner/path.c", not "HEAD:some/path:inner/path.c").
>>>>
>>>> Ah, OK.
>>>>
>>>> I am not sure if we have a proper hint in the revision parser
>>>> machinery, but it may not be too hard to teach rev-cmdline interface
>>>> to keep that kind of information (i.e. "This tree object name is a
>>>> result of parsing ':path' syntax").
>>>
>>> Actually, having thought about this a bit more, I am no longer sure
>>> if this is even a good idea to begin with.
>>>
>>> An output line that begins with HEAD:some/path:inner/path.c from
>>> "git grep" is an answer to this question: find the given pattern in
>>> a tree-ish specified with "HEAD:some/path".
>>>
>>> On the other hand, HEAD:some/path/inner/path.c is an answer to a
>>> different question: find the pattern in a tree-ish specified with
>>> "HEAD".  The question may optionally be further qualified with "but
>>> limit the search to some/path".  Both the question and its answer
>>> are more intuitive than the former one.
>>
>> I disagree.  The man page says that git grep lists the filenames of
>> matching files.
>
> But you need to realize that the manual page gives a white lie in
> order to stay short enough to be readable.  It does give filenames
> when reading from the working tree, the index or a top-level tree
> object.  When given arbitrary tree object that is not top-level, it
> does give filenames relative to the given tree object, which is the
> answer HEAD:some/path:inner/path.c to the question "where in the
> tree HEAD:some/path do we have hits?".
>
>>> If the user asked the question of the former form, i.e.
>>>
>>> $ git grep -e pattern HEAD:some/path
>>>
>>> there may be a reason why the user wanted to ask it in that
>>> (somewhat strange) way.  I am not 100% sure if it is a good idea to
>>> give an answer to a question different from what the user asked by
>>> internally rewriting the question to
>>>
>>> $ git grep -e pattern HEAD -- some/path
>>
>> We are not rewriting the question at all.
>
> That statement is trapped in a narrow "implementor" mind. I know you
> are not rewriting the question in your implementation, but what do
> the end users see? It gives an answer to a question different from
> they asked; the observed behaviour is as if the question was
> rewritten before the machinery went to work.
>
> If your justification were "above says 'there may be a readon why
> the user wanted to ask it in that way', i.e. 'find in this tree
> object HEAD:some/path and report where hits appear', but the reason
> can only be from laziness and/or broken script and the user always
> wants the answer from within the top-level tree-ish", then that
> argument may make some sense. You need to justify why it is OK to
> lose information in the answer by rewriting the colon that separates
> the question ("in this tree object") and the answer ("at this path
> relative to the tree object given").
>
> Whether you rewrite the input or the output is not important; you
> are trying to give an answer to a different question, which is what
> I find questionable.

Ok, so if I can summarize what I am inferring from your objection:

 1. The (tree-path, found-path) pair is useful information to get back
from git-grep.

 2. A colon is used to delimit these pieces of information, just as a
colon is used to delimit the filename from the matched-line results.

 3. The fact that the colon is also the separator used in object refs
is mere coincidence; the colon was _not_ chosen because it
conveniently turns the results list into valid object references.  A
comma could have been instead, or even a \t.

Have I got that right?

If so, then I would like to point out to you the convenience I
accidentally encountered using this tool.  Perhaps you didn't realize
how helpful it was when you chose to use a colon there.

On the other hand, perhaps a colon is an unfortunate syntactic
collision which should be corrected in the future.

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

[PATCHv2] grep: use slash for path delimiter, not colon

[Phil Hord]

When a commit is grepped and matching filenames are printed, grep-objects
creates the filename by prefixing the original cmdline argument to the
matched path separated by a colon.  Normally this forms a valid blob
reference to the filename, like this:

  git grep -l foo HEAD
  HEAD:some/path/to/foo.txt
  ^

But a tree path may be given to grep instead; in this case the colon is
not a valid delimiter to use since it is placed inside a path.

  git grep -l foo HEAD:some
  HEAD:some:path/to/foo.txt
   ^

The slash path delimiter should be used instead.  Fix git grep to
discern the correct delimiter so it can report valid object names.

  git grep -l foo HEAD:some
  HEAD:some/path/to/foo.txt
   ^

Also, prevent the delimiter being added twice, as happens now in these
examples:

  git grep -l foo HEAD:
  HEAD::some/path/to/foo.txt
   ^
  git grep -l foo HEAD:some/
  HEAD:some/:path/to/foo.txt
^

Add a test to confirm correct path forming.
---
This version is a bit more deterministic and also adds a test.

It accepts the expense of examining the path argument again to 
determine if it is a tree-ish + path rather than just a tree (commit).
The get_sha1 call occurs one extra time for each tree-ish argument,
so it's not expensive. We avoid mucking with the object_array API this
way, and also do not rely on the object-type to tell us anything about
the way the object name was spelled.

This one also adds a check to avoid duplicating an extant delimiter.

 builtin/grep.c  |  9 -
 t/t7810-grep.sh | 15 +++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 03bc442..6fc418f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -480,8 +480,15 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
len = name ? strlen(name) : 0;
strbuf_init(&base, PATH_MAX + len + 1);
if (len) {
+   struct object_context ctx;
+   unsigned char sha1[20];
+   char delimiter = ':';
+   if (!get_sha1_with_context(name, 0, sha1, &ctx) &&
+   ctx.path[0]!=0)
+   delimiter='/';
strbuf_add(&base, name, len);
-   strbuf_addch(&base, ':');
+   if (name[len-1] != delimiter)
+   strbuf_addch(&base, delimiter);
}
init_tree_desc(&tree, data, size);
hit = grep_tree(opt, pathspec, &tree, &base, base.len,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f698001..2494bfc 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -886,6 +886,21 @@ test_expect_success 'grep -e -- -- path' '
 '
 
 cat >expected actual &&
+   test_cmp expected actual
+'
+
+cat >expected 

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Jonathan Nieder  writes:
>>
>>> I think Phil meant that when "git grep" is asked to search within
>>> "HEAD:some/path", filenames tacked on at the end should be appended
>>> with a '/' separator instead of the usual ':' (e.g.,
>>> "HEAD:some/path/inner/path.c", not "HEAD:some/path:inner/path.c").
>>
>> Ah, OK.
>>
>> I am not sure if we have a proper hint in the revision parser
>> machinery, but it may not be too hard to teach rev-cmdline interface
>> to keep that kind of information (i.e. "This tree object name is a
>> result of parsing ':path' syntax").
>
> Actually, having thought about this a bit more, I am no longer sure
> if this is even a good idea to begin with.
>
> An output line that begins with HEAD:some/path:inner/path.c from
> "git grep" is an answer to this question: find the given pattern in
> a tree-ish specified with "HEAD:some/path".
>
> On the other hand, HEAD:some/path/inner/path.c is an answer to a
> different question: find the pattern in a tree-ish specified with
> "HEAD".  The question may optionally be further qualified with "but
> limit the search to some/path".  Both the question and its answer
> are more intuitive than the former one.

I disagree.  The man page says that git grep lists the filenames of
matching files.  And it usually does.  When told to search in a
different branch, the filename is helpfully shown in a form that other
git commands recognize, namely $branch:$path. This is useful for
scripts that want to do something with the resulting file names.

But when a path included in the query with the branch, the output is
useless to my scripts or finger memory without some cleanup first.
The aim of this patch is to fix that so the cleanup is not necessary.

   $ git grep -l setup_check HEAD Documentation
   HEAD:Documentation/technical/api-gitattributes.txt

   $ git grep -l setup_check HEAD:Documentation
   HEAD:Documentation:technical/api-gitattributes.txt

The path in the first example is meaningful.  The path in the second
example is erroneous.


> And we have a nice way to ask that question directly, i.e.
>
> $ git grep -e pattern HEAD some/path
>
> which can be extended naturally to more than one path, e.g.
>
> $ git grep -e pattern HEAD some/path another/hierarchy
>
> without having to repeat HEAD: part again for each path.

Yes, but that's not always what I want. Sometimes I want to search on
different trees. When doing so, why should I be crippled with broken
output?

$ git grep -e pattern origin/master:some/path origin/next:another/hierarchy
origin/master:some/path:sub/dir/foo.txt
origin/next:another/hierarchy:path/frotz.c

I would prefer to have real paths I can pass to 'git show', ones with
just one meaningful colon rather than two vague ones:

origin/master:some/path/sub/dir/foo.txt
origin/next:another/hierarchy/path/frotz.c

> If the user asked the question of the former form, i.e.
>
> $ git grep -e pattern HEAD:some/path
>
> there may be a reason why the user wanted to ask it in that
> (somewhat strange) way.  I am not 100% sure if it is a good idea to
> give an answer to a question different from what the user asked by
> internally rewriting the question to
>
> $ git grep -e pattern HEAD -- some/path

We are not rewriting the question at all.

The current code assumes the user gave only an object name and is
trying to help by prefixing that name on the matched path using the
colon as a separator, as would be the norm.  But that is the wrong
separator in some cases, specifically when the tree reference includes
a path.

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

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Sat, Aug 24, 2013 at 9:35 PM, Phil Hord  wrote:
>
> When the pathspec given to grep includes a tree name, the full
> name of matched files is assembled using colon as a separator.
> If the pathspec includes a tree name, it should use a slash
> instead.
>
> Check if the pathspec already names a tree and ref (including
> a colon) and use a slash if so.

I think I used lots of wrong terminology there.  What do I call these
things?

HEAD:path is a tree.

HEAD is a commit name.

Maybe like this?

  When a tree is given to grep, the full name of matched files
  is assembled using colon as a separator.

  If the tree name includes an object name, as in
  HEAD:some/path, it should use a slash instead.

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

[RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

When the pathspec given to grep includes a tree name, the full
name of matched files is assembled using colon as a separator.
If the pathspec includes a tree name, it should use a slash
instead.

Check if the pathspec already names a tree and ref (including
a colon) and use a slash if so.
---

I'm not sure about the detection I used here.  It works, but it is
not terribly robust.  Is there a better way to handle this?  Maybe
something like 'prefix_pathspec(name,"");'.

 builtin/grep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 03bc442..d0deae4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -480,8 +480,9 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
len = name ? strlen(name) : 0;
strbuf_init(&base, PATH_MAX + len + 1);
if (len) {
+   int has_colon = !!strchr(name,':');
strbuf_add(&base, name, len);
-   strbuf_addch(&base, ':');
+   strbuf_addch(&base, has_colon?'/':':');
}
init_tree_desc(&tree, data, size);
hit = grep_tree(opt, pathspec, &tree, &base, base.len,
-- 
1.8.4.557.g34b3a2e

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

Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

[Phil Hord]

On Tue, Aug 13, 2013 at 1:31 AM, Luke San Antonio
 wrote:
>
> So I found an isolated case, it's very strange...
>
> Here's a script!
>
>


Thanks for that.  It was hard to read, but it demonstrates the problem well.


> ... Copy and paste that into a terminal and you should have a recreated
> version of my repository there! Now that the file is partly stashed and
> partly in the index, check out the difference in diffs:
>
> try:
>   git diff --staged
> then try:
>   git stash show -p

This one is pretty sneaky.  It is not limited to git-stash.  I can
demonstrate the problem now using 'git merge-file'.

But I can only make this problem show itself when:
   1. The collisions are separated by just one line of common code, and
   2. One of the lines of common code is duplicated in one of the
collisions, and
   3. The first two lines of the file are duplicated, and
   4. One of the first two lines is deleted on one side but not the other.

I have managed to boil the test down to this script:

#-
cat >base >> right
  8 changed


But it should look like this instead:

  1 duplicate
  3 unchanged
  4 changed
  7 duplicated
  6 new line
  7 duplicated
  8 changed


A similar (but different) stupidity shows up if you remove line "3"
from all three files.

I tested this in 1.6.5 and the same thing occurs there, so this is NOT
recent regression.

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

[PATCH] t/t7407: fix two typos in submodule tests

[Phil Hord]

In t/t7407-submodule-foreach.sh there is a typo in one of the
path names given for a test step.  The correct path is
nested1/nested2/.git, but nested1/nested1/nested2/.git is
given instead.  The typo is hidden because this line also
accidentally omits the && chain operator.  The omitted chain
also means the return values of all the previous commands in
this test are also being ignored.

Fix the path and add the chain operator so the entire test
sequence can be properly validated.

Signed-off-by: Phil Hord 
---
 t/t7407-submodule-foreach.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 91d4fd1..be93f10 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -145,7 +145,7 @@ test_expect_success 'use "submodule foreach" to checkout 
2nd level submodule' '
git rev-parse --resolve-git-dir nested1/.git &&
test_must_fail git rev-parse --resolve-git-dir 
nested1/nested2/.git &&
git submodule foreach "git submodule update --init" &&
-   git rev-parse --resolve-git-dir nested1/nested1/nested2/.git
+   git rev-parse --resolve-git-dir nested1/nested2/.git &&
test_must_fail git rev-parse --resolve-git-dir 
nested1/nested2/nested3/.git
)
 '
-- 
1.8.4.rc1.433.g415b943

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

Re: Repo with only one file

[Phil Hord]

On Fri, Aug 9, 2013 at 6:03 AM, shawn wilson  wrote:
> On Fri, Aug 9, 2013 at 2:50 AM, Johannes Sixt  wrote:
>> Let's check: After running your command above to remove other files, does
>> the command
>>
>>git filter-branch -f HEAD webban.pl
>>
>
> Ahha, no but:
> git filter-branch -f HEAD -- webban.pl
> did
>
> Thanks.
>
> The question still stands though - why is that unassociated commit left there?

Maybe you need  --prune-empty.

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

Re: Repo with only one file

[Phil Hord]

On Wed, Aug 7, 2013 at 5:07 PM, shawn wilson  wrote:
> On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt  wrote:
>> Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of
>>> these scripts and I'd like to keep the commit history.
>>>
>>> Ok, so:
>>> % find -type f ! -iname "webban.pl" | while read f; do git
>>> filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f"
>>> HEAD ; done
>>>
>>> Which basically did it. But, I've got this one commit that seems to be
>>> orphaned - it doesn't change any files.
>>
>> Try this:
>>
>>   git filter-branch HEAD -- webban.pl
>>
>
>  % git filter-branch HEAD -- webban.pl
> Cannot create a new backup.
> A previous backup already exists in refs/original/
> Force overwriting the backup with -f
>  % git filter-branch -f HEAD -- webban.pl
> Rewrite 1e04b18c256c996312f167be808733bcc755f1e3 (9/9)
> WARNING: Ref 'refs/heads/master' is unchanged

I think you can ignore the warning.  Maybe you want to create a new
branch which only has this file in it now.

   $ git checkout -b webban

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

Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

[Phil Hord]

On Thu, Aug 8, 2013 at 3:07 AM, Luke San Antonio
 wrote:
>
> Hi, my name's Luke!
>
> Today, I had a problem merging a stash after immediately creating it.
> This is exactly what I did!
>
> git stash save --keep-index
> git stash pop
>
> And BAM! Merge conflict! This was especially weird because my file had
> this in it (taken directly from my code!)
>

Luke,

I think the issue is that your working directory receives your cached
file when you say 'git stash --keep-index'.  When you restore the
stash, your previous working directory now conflicts with your new
working directory, but neither is the same as HEAD.

Here's a test script to demonstrate the issue, I think.  Did I get
this right, Luke?

 # cd /tmp && rm -rf foo
 git init foo && cd foo
 echo "foo" > bar &&  git add bar && git commit -mfoo
 echo "bar" > bar &&  git add bar
 echo "baz" > bar
 echo "Before stash  bar: $(cat bar)"
 git stash --keep-index
 echo "After stash  bar: $(cat bar)"
 git stash apply


The output looks like this:

$  git init foo && cd foo
Initialized empty Git repository in /tmp/foo/.git/
$ git commit --allow-empty -mInitialCommit
[master (root-commit) b5ecc7e] InitialCommit
$ echo "Bar" > bar &&  git add bar && git commit -mBar
[master 16d708b] Bar
 1 file changed, 1 insertion(+)
 create mode 100644 bar
$ echo "bar" > bar &&  git add bar
$  echo "baz" > bar
$  echo "Before stash  bar: $(cat bar)"
Before stash  bar: baz
$  git stash --keep-index
Saved working directory and index state WIP on master: 16d708b Bar
HEAD is now at 16d708b Bar
$  echo "After stash  bar: $(cat bar)"
After stash  bar: bar
$  git stash apply
Auto-merging bar
CONFLICT (content): Merge conflict in bar
Recorded preimage for 'bar'
$ cat bar
<<< Updated upstream
bar
===
baz
>>> Stashed changes



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

Re: git clone -b

[Phil Hord]

It would be nice to support more generic specs for the --branch
switch. But it is complicated because the refs have not been fetched
yet during the clone, and so normal refs operations -- which expect to
work on a local repository -- do not work.  So, the ref is looked up
locally from a list in expected locations after fetching the remote
refs but before the clone occurs.  The remote refs which are fetched
is not configurable during clone, and so only 'refs/heads/*' is
fetched for non-mirrors.

I was able to tweak git-clone to fetch the remote ref when I hacked
builtin/clone.c to check in 'refs' and also to extend the refspec to
something more broad ("+refs/*:refs/remotes/origin/*"), but this is
not a workable solution.  But there probably is a more correct way
than the hack I tried.

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

Re: Splitting a rev list into 2 sets

[Phil Hord]

On Thu, Jun 20, 2013 at 6:14 AM, Francis Moreau  wrote:
> I'd like to write a script that would parse commits in one of my repo.
> Ideally this script should accept any revision ranges that
> git-rev-list would accept.
>
> This script should consider commits in master differently than the
> ones in others branches.
>
> To get the commit set which can't be reached by master (ie commits
> which are specific to branches other than master) I would do:
>
>   # "$@" is the range spec passed to the script
>   git rev-list "$@" ^master | check_other_commit
>
> But I don't know if it's possible to use a different git-rev-list
> command to get the rest of the commits, ie the ones that are reachable
> by the specified range and master.
>
> One way to do that is to record the first commit set got by the first
> rev-list command and check that the ones returned by "git rev-list $@"
> are not in the record.
>
> But I'm wondering if someone can see another solution more elegant ?

I do not know if I would call this elegant, but I think this
codification of your "One way to do that" is at least small and mostly
readable:

   git rev-list "$@" |grep -v -f <(git rev-list "$@" ^master)

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

[PATCHv2] fix "builtin-*" references to be "builtin/*"

[Phil Hord]

Documentation and some comments still refer to files in builtin/
as 'builtin-*.[cho]'.  Update these to show the correct location.

Signed-off-by: Phil Hord 
Reviewed-by: Jonathan Nieder 
Assisted-by: Junio C Hamano 
---
 Documentation/git-log.txt |  4 ++--
 Documentation/technical/api-builtin.txt   |  2 +-
 Documentation/technical/api-parse-options.txt | 12 ++--
 Documentation/user-manual.txt | 13 +++--
 builtin/help.c|  2 --
 builtin/notes.c   |  2 +-
 builtin/replace.c |  2 +-
 transport.c   |  2 +-
 transport.h   |  2 +-
 9 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 4687fe8..2ea79ba 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -128,9 +128,9 @@ Examples
in the "release" branch, along with the list of paths
each commit modifies.
 
-`git log --follow builtin-rev-list.c`::
+`git log --follow builtin/rev-list.c`::
 
-   Shows the commits that changed builtin-rev-list.c, including
+   Shows the commits that changed builtin/rev-list.c, including
those commits that occurred before the file was given its
present name.
 
diff --git a/Documentation/technical/api-builtin.txt 
b/Documentation/technical/api-builtin.txt
index 4a4228b..f3c1357 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -39,7 +39,7 @@ where options is the bitwise-or of:
on bare repositories.
This only makes sense when `RUN_SETUP` is also set.
 
-. Add `builtin-foo.o` to `BUILTIN_OBJS` in `Makefile`.
+. Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.
 
 Additionally, if `foo` is a new command, there are 3 more things to do:
 
diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 1317db4..0be2b51 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -275,10 +275,10 @@ Examples
 
 
 See `test-parse-options.c` and
-`builtin-add.c`,
-`builtin-clone.c`,
-`builtin-commit.c`,
-`builtin-fetch.c`,
-`builtin-fsck.c`,
-`builtin-rm.c`
+`builtin/add.c`,
+`builtin/clone.c`,
+`builtin/commit.c`,
+`builtin/fetch.c`,
+`builtin/fsck.c`,
+`builtin/rm.c`
 for real-world examples.
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..2436124 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -4256,15 +4256,16 @@ no longer need to call `setup_pager()` directly).
 Nowadays, `git log` is a builtin, which means that it is _contained_ in the
 command `git`.  The source side of a builtin is
 
-- a function called `cmd_`, typically defined in `builtin-.c`,
-  and declared in `builtin.h`,
+- a function called `cmd_`, typically defined in `builtin/`
+  (note that older versions of Git used to have it in `builtin-.c`
+  instead), and declared in `builtin.h`.
 
 - an entry in the `commands[]` array in `git.c`, and
 
 - an entry in `BUILTIN_OBJECTS` in the `Makefile`.
 
 Sometimes, more than one builtin is contained in one source file.  For
-example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin-log.c`,
+example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin/log.c`,
 since they share quite a bit of code.  In that case, the commands which are
 _not_ named like the `.c` file in which they live have to be listed in
 `BUILT_INS` in the `Makefile`.
@@ -4287,10 +4288,10 @@ For the sake of clarity, let's stay with `git 
cat-file`, because it
 - is plumbing, and
 
 - was around even in the initial commit (it literally went only through
-  some 20 revisions as `cat-file.c`, was renamed to `builtin-cat-file.c`
+  some 20 revisions as `cat-file.c`, was renamed to `builtin/cat-file.c`
   when made a builtin, and then saw less than 10 versions).
 
-So, look into `builtin-cat-file.c`, search for `cmd_cat_file()` and look what
+So, look into `builtin/cat-file.c`, search for `cmd_cat_file()` and look what
 it does.
 
 --
@@ -4366,7 +4367,7 @@ Another example: Find out what to do in order to make 
some script a
 builtin:
 
 -
-$ git log --no-merges --diff-filter=A builtin-*.c
+$ git log --no-merges --diff-filter=A builtin/*.c
 -
 
 You see, Git is actually the best tool to find out about the source of Git
diff --git a/builtin/help.c b/builtin/help.c
index 062957f..f1e236b 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -1,6 +1,4 @@
 /*
- * builtin-help.c
- *
  * Builtin help command
  */
 #include "cache.h"
diff --git a/builtin/notes.c b/builtin/notes.c

[PATCH] fix "builtin-*" references to be "builtin/*"

[Phil Hord]

Documentation and some comments still refer to files in builtin/
as 'builtin-*.[cho]'.  Update these to show the correct location.

Signed-off-by: Phil Hord 
---
 Documentation/git-log.txt |  4 ++--
 Documentation/technical/api-builtin.txt   |  2 +-
 Documentation/technical/api-parse-options.txt | 12 ++--
 Documentation/user-manual.txt | 10 +-
 builtin/help.c|  2 +-
 builtin/notes.c   |  2 +-
 builtin/replace.c |  2 +-
 transport.c   |  2 +-
 transport.h   |  2 +-
 9 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 4687fe8..2ea79ba 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -128,9 +128,9 @@ Examples
in the "release" branch, along with the list of paths
each commit modifies.
 
-`git log --follow builtin-rev-list.c`::
+`git log --follow builtin/rev-list.c`::
 
-   Shows the commits that changed builtin-rev-list.c, including
+   Shows the commits that changed builtin/rev-list.c, including
those commits that occurred before the file was given its
present name.
 
diff --git a/Documentation/technical/api-builtin.txt 
b/Documentation/technical/api-builtin.txt
index 4a4228b..f3c1357 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -39,7 +39,7 @@ where options is the bitwise-or of:
on bare repositories.
This only makes sense when `RUN_SETUP` is also set.
 
-. Add `builtin-foo.o` to `BUILTIN_OBJS` in `Makefile`.
+. Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.
 
 Additionally, if `foo` is a new command, there are 3 more things to do:
 
diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 1317db4..0be2b51 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -275,10 +275,10 @@ Examples
 
 
 See `test-parse-options.c` and
-`builtin-add.c`,
-`builtin-clone.c`,
-`builtin-commit.c`,
-`builtin-fetch.c`,
-`builtin-fsck.c`,
-`builtin-rm.c`
+`builtin/add.c`,
+`builtin/clone.c`,
+`builtin/commit.c`,
+`builtin/fetch.c`,
+`builtin/fsck.c`,
+`builtin/rm.c`
 for real-world examples.
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..2483700 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -4256,7 +4256,7 @@ no longer need to call `setup_pager()` directly).
 Nowadays, `git log` is a builtin, which means that it is _contained_ in the
 command `git`.  The source side of a builtin is
 
-- a function called `cmd_`, typically defined in `builtin-.c`,
+- a function called `cmd_`, typically defined in `builtin/.c`,
   and declared in `builtin.h`,
 
 - an entry in the `commands[]` array in `git.c`, and
@@ -4264,7 +4264,7 @@ command `git`.  The source side of a builtin is
 - an entry in `BUILTIN_OBJECTS` in the `Makefile`.
 
 Sometimes, more than one builtin is contained in one source file.  For
-example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin-log.c`,
+example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin/log.c`,
 since they share quite a bit of code.  In that case, the commands which are
 _not_ named like the `.c` file in which they live have to be listed in
 `BUILT_INS` in the `Makefile`.
@@ -4287,10 +4287,10 @@ For the sake of clarity, let's stay with `git 
cat-file`, because it
 - is plumbing, and
 
 - was around even in the initial commit (it literally went only through
-  some 20 revisions as `cat-file.c`, was renamed to `builtin-cat-file.c`
+  some 20 revisions as `cat-file.c`, was renamed to `builtin/cat-file.c`
   when made a builtin, and then saw less than 10 versions).
 
-So, look into `builtin-cat-file.c`, search for `cmd_cat_file()` and look what
+So, look into `builtin/cat-file.c`, search for `cmd_cat_file()` and look what
 it does.
 
 --
@@ -4366,7 +4366,7 @@ Another example: Find out what to do in order to make 
some script a
 builtin:
 
 -
-$ git log --no-merges --diff-filter=A builtin-*.c
+$ git log --no-merges --diff-filter=A builtin/*.c
 -
 
 You see, Git is actually the best tool to find out about the source of Git
diff --git a/builtin/help.c b/builtin/help.c
index 062957f..ce7b889 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -1,5 +1,5 @@
 /*
- * builtin-help.c
+ * builtin/help.c
  *
  * Builtin help command
  */
diff --git a/builtin/notes.c b/builtin/notes.c
index 57748a6..d9a67d9 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -4,7 +4,7 @@
  * Copyright (c) 2010 Johan 

Re: [PATCH 1/2] sha1_name: stop hard-coding 40-character hex checks

[Phil Hord]

On Sat, Jun 15, 2013 at 1:38 PM, Ramkumar Ramachandra
 wrote:
> In two places, get_sha1_basic() assumes that strings are possibly sha1
> hexes if they are 40 characters long, and calls get_sha1_hex() in these
> two cases.  This 40-character check is ugly and wrong: there is nothing
> preventing a revision or branch name from being exactly 40 characters.
> Replace it with a call to the more robust get_short_sha1().

I share your disdain for the bare '40's in the code.  But I think this
code is less clear than the previous version with the magic number.

>
> Signed-off-by: Ramkumar Ramachandra 
> ---
>  sha1_name.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 90419ef..d862af3 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -451,7 +451,7 @@ static int get_sha1_basic(const char *str, int len, 
> unsigned char *sha1)
> int refs_found = 0;
> int at, reflog_len, nth_prior = 0;
>
> -   if (len == 40 && !get_sha1_hex(str, sha1)) {
> +   if (!get_short_sha1(str, strlen(str), sha1, GET_SHA1_QUIETLY)) {

Use len instead of strlen(str) here.  It's faster and more correct.

But also get_short_sha1 is much heavier than get_sha1_hex and does not
seem appropriate here.

> refs_found = dwim_ref(str, len, tmp_sha1, &real_ref);
> if (refs_found > 0 && warn_ambiguous_refs) {
> warning(warn_msg, len, str);
> @@ -492,9 +492,9 @@ static int get_sha1_basic(const char *str, int len, 
> unsigned char *sha1)
> int detached;
>
> if (interpret_nth_prior_checkout(str, &buf) > 0) {
> -   detached = (buf.len == 40 && !get_sha1_hex(buf.buf, 
> sha1));
> +   detached = get_short_sha1(buf.buf, buf.len, sha1, 
> GET_SHA1_QUIETLY);
> strbuf_release(&buf);
> -   if (detached)
> +   if (detached != SHORT_NAME_NOT_FOUND)

The semantic meaning of 'detached' seems less clear now if you have to
compare against an enumerated constant to determine the result.  But
also, I do not see why you have to test '!= SHORT_NAME_NOT_FOUND' here
but you did not have to in the other instance.

I think it would be improved if you did this comparison in the
assignment of detached so 'detached' could keep its original boolean
meaning.

But anyway, having looked inside get_short_sha1, it really does seem
to do much more than you want here.

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

Re: [PATCH 5/5] rebase: use 'git stash store' to simplify logic

[Phil Hord]

On Fri, Jun 14, 2013 at 6:32 AM, Ramkumar Ramachandra
 wrote:
> rebase has no reason to know about the implementation of the stash.  In
> the case when applying the autostash results in conflicts, replace the
> relevant code in finish_rebase () to simply call 'git stash store'.
>
> Signed-off-by: Ramkumar Ramachandra 
> ---
>  git-rebase.sh | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index d0c11a9..bf37259 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -153,11 +153,7 @@ finish_rebase () {
> then
> echo "$(gettext 'Applied autostash.')"
> else
> -   ref_stash=refs/stash &&
> -   >>"$GIT_DIR/logs/$ref_stash" &&
> -   git update-ref -m "autostash" $ref_stash $stash_sha1 
> ||
> -   die "$(eval_gettext 'Cannot store $stash_sha1')"
> -
> +   git stash store -m "autostash" -e "Cannot store 
> $stash_sha1." $stash_sha1

nit: adds a period where there was not one previously.

Maybe this doesn't matter so much since this code is new anyway.  But
showing a period after sha1 seems wrong, too. Or maybe I am confused
again.  Does eval_gettext routinely add a period to the end of
translated strings?

> gettext 'Applying autostash resulted in conflicts.
>  Your changes are safe in the stash.
>  You can run "git stash pop" or "git stash drop" it at any time.
> --
> 1.8.3.1.383.g0d5ad6b
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] pull: respect rebase.autostash

[Phil Hord]

On Fri, Jun 14, 2013 at 8:29 AM, John Keeping  wrote:
> On Fri, Jun 14, 2013 at 08:12:56AM -0400, Phil Hord wrote:
>> On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra
>>  wrote:
>> > If a rebasing pull is requested, pull unconditionally runs
>> > require_clean_worktree() resulting in:
>> >
>> >   # dirty worktree or index
>> >   $ git pull
>> >   Cannot pull with rebase: Your index contains uncommitted changes.
>> >   Please commit or stash them.
>> >
>> > It does this to inform the user early on that a rebase cannot be run on
>> > a dirty worktree, and that a stash is required.  However,
>> > rr/rebase-autostash lifts this limitation on rebase by providing a way
>> > to automatically stash using the rebase.autostash configuration
>> > variable.  Read this variable in pull, and take advantage of this
>> > feature.
>>
>> This commit message does not tell me what this commit does.  It mostly
>> describes the current situation.  Then it refers to something called
>> "rr/rebase-autostash" which will lose meaning in the future when this
>> commit is no longer current on the list.  A better way to refer to
>> this commit is to say "this commit".  However, even this is not the
>> norm for this project.  The norm here is to avoid such noise by
>> speaking in the imperative mood.  That is, do not tell me what this
>> commit does; instead, tell the code what to do.  See
>> Documentation/SubmittingPatches:
>
> It seems to me that Ram's message is already in the imperative.  The
> only (slight) issue is that rr/rebase-autostash will become hard to find
> once Junio cleans up feature branches that have graduated.  Since that
> branch has graduated to master, it would be clearer to refer to commit
> 5879477 (rebase: implement --[no-]autostash and rebase.autostash,
> 2013-05-12).  Is something like this clearer?
>
> "git pull" currently cannot be used with the "autostash" feature
> added to "git rebase" by commit 5879477 (rebase: implement
> --[no-]autostash and rebase.autostash, 2013-05-12) because it
> unconditionally calls requre_clean_worktree early on, which results
> in:
>
> # dirty worktree or index
> $ git pull
> Cannot pull with rebase: Your index contains uncommitted changes.
> Please commit or stash them.
>
> Remove this restriction by skipping the call to
> require_clean_worktree if the "rebase.autostash" configuration
> variable is set.


Yes, thanks.   I was mislead by my poor understanding all the players
involved.  This disambiguates things nicely.

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