Re: [PATCH v6 3/3] rev-list: support --no-filter argument

[Christian Couder]

On Tue, Dec 5, 2017 at 5:50 PM, Jeff Hostetler  wrote:
> From: Jeff Hostetler 
>
> Teach rev-list to support --no-filter to override a
> previous --filter= argument.  This is
> to be consistent with commands that use OPT_PARSE
> macros.
>
> Signed-off-by: Jeff Hostetler 
> ---
>  Documentation/rev-list-options.txt | 15 ++-
>  builtin/rev-list.c |  4 
>  2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/rev-list-options.txt 
> b/Documentation/rev-list-options.txt
> index 11bb87f..8d8b7f4 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -715,16 +715,21 @@ ifdef::git-rev-list[]
>  The form '--filter=blob:none' omits all blobs.
>  +
>  The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes
> -or units.  The value may be zero.
> +or units.  n may be zero.  The suffixes k, m, and g can be used to name

"'' may be zero" would be more consistent with other parts of this file.
s/k, m, and g/'k', 'm', and 'g'/ could also help.

> +units in KiB, MiB, or GiB.  For example, 'blob:limit=1k' is the same
> +as 'blob:limit=1024'.
>  +
> -The form '--filter=sparse:oid=' uses a sparse-checkout
> -specification contained in the object (or the object that the expression
> -evaluates to) to omit blobs that would not be not required for a
> -sparse checkout on the requested refs.
> +The form '--filter=sparse:oid=' uses a sparse-checkout
> +specification contained in the blob (or blob-expression) ''

For example here '' is used.

> +to omit blobs that would not be not required for a sparse checkout on
> +the requested refs.

Re: [PATCH v3 1/7] git-compat-util: introduce skip_to_optional_arg()

[Christian Couder]

On Sun, Dec 10, 2017 at 3:39 PM, Jeff King  wrote:
> On Sun, Dec 10, 2017 at 09:31:18AM -0500, Jeff King wrote:
>
>> On Sat, Dec 09, 2017 at 09:40:07PM +0100, Christian Couder wrote:
>>
>> > The changes compared to v2 are:
>> >
>> >   - s/_val/_arg/ in the name of the functions
>> >   - s/val/arg/ in the name of the third argument of the functions
>> >   - works with NULL as third argument of the functions
>>
>> This whole series looks OK to me, but this third point made me wonder:
>> what would be the use of allowing NULL for the "arg" parameter?
>>
>> I didn't see any use of this in the series, and I'm having trouble
>> figuring out how it would be useful. E.g., if I do:
>>
>>   if (skip_to_optional_arg(arg, "--foo", NULL))
>>  ...
>>
>> what can I do in "..."? I know we matched _some_ type of "--foo", but I
>> cannot know whether it was "--foo" or "--foo=bar", nor what "bar" is. It
>> could only be used by some kind of vague validator to say "well, at
>> least this looks like an option that I _could_ parse if I wanted to".
>>
>> So I guess I don't mind it, as it does the most reasonable thing it can
>> when passed NULL, but I would be surprised if we ever actually exercise
>> the code path.
>
> And of course as soon as I sent this, I went back and double-checked.
> And indeed I totally missed this call:
>
>   +   else if (starts_with(arg, "-B") ||
>   +skip_to_optional_arg(arg, "--break-rewrites", NULL)) {
>   if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)

Yeah, calls like this were discussed in:
https://public-inbox.org/git/xmqqh8t6o9me@gitster.mtv.corp.google.com/

> So that's kind-of weird, because we are parsing "-B", etc, and then
> expecting it to be _reparsed_ by diff_scoreopt_parse. So the two
> callsites must always match. IMHO this ought to do either:
>
>   - we should just ask diff_scoreopt_parser to tell us if this was a
> valid option that it understood
>
> or
>
>   - parse up to the "=", and then ask the scoreopt parser to parse the
> remainder. This would require us passing 'B'/'C'/'M' to the
> function ourselves, I think that's a better pattern. It means we
> could reuse the parser for things like config values if we wanted to
> (our current diff.renames is a bool, but it would not be
> unreasonable for it to take a score).
>
> None of that is a mess of your creation, though, so I'm OK punting on it
> for now.

Yeah, this could be part of the #leftoverbits.

[PATCH] git-gui: Make push remote combobox full width

[Peter Urbanec]

When pushing changes, the Remote combobox for the Destination Repository
does not take all available space in the layout. With long remote names,
this causes the combobox to have truncated entries even though there is
room to display them.

Anchor the remote_m combobox to both left and right edges of the grid
cell and use the same padding as url_t for a consistent, dynamic layout.


Signed-off-by: Peter Urbanec 
---
 lib/transport.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/transport.tcl b/lib/transport.tcl
index a1a424a..1046dc5 100644
--- a/lib/transport.tcl
+++ b/lib/transport.tcl
@@ -173,7 +173,7 @@ proc do_push_anywhere {} {
} else {
eval tk_optionMenu $w.dest.remote_m push_remote 
$all_remotes
}
-   grid $w.dest.remote_r $w.dest.remote_m -sticky w
+   grid $w.dest.remote_r $w.dest.remote_m -sticky we -padx {0 5}
if {[lsearch -sorted -exact $all_remotes origin] != -1} {
set push_remote origin
} else {
-- 
2.15.1

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking tree file changes)

[Alexei Lozovsky]

On Dec 11, 2017, at 01:17, Igor Djordjevic wrote:
> On 10/12/2017 13:22, Phillip Wood wrote:
>> I understood Alexei to mean that it was merging the f!A into A that 
>> caused conflicts due to the fact that f!A has conflicting context
>> that was introduced in B. After all B' the rebased B is merge A A' B
>> whether it is created by 'rebase --autosquash' or 'rebase --onto'. A'
>> must be the same in both cases or one is applying a different fix.
> 
> Yes, I understand and agree you might be right, what you are talking 
> about being what he actually _meant_, but because that is not what he 
> _wrote_, I wanted to see an example of it, (still?) hoping that he 
> really did mean what he wrote (commit B being the problematic one), 
> as then there would be a possibility for improvement.

I'm not really good at remembering the exact details, so if you ask
for a testimony then I'm not sure whether it's the conflicts in the
fixups or the later commits that I was annoyed by :) I'm also not
really versed in the technical details of rebasing, so I cannot give
an educated guess on which one is more likely to cause conflicts.

> Still, I hope for that example...! :D

I keep this thread pinned, so I hope to provide a more concrete example
as soon as I encounter the conflicting situation again in the wild. I'm
not sure that I am able to construct a relevant example artificially.

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking tree file changes)

[Alexei Lozovsky]

On Dec 10, 2017, at 14:22, Phillip Wood wrote:
> 
> I've found conflicts arising from moving fixups can be quite common, so
> these days I tend to edit the commit to be fixed up directly. I have a
> script git-amend that does something like
> 
> target=$(git rev-parse --verify "$1") && GIT_SEQUENCE_EDITOR="sed -i
> s/^pick $target/edit $target/" rebase -ik $target^
> 
> so I can just type 'git amend ' to make this easier

Hm... I just realized that using "edit" command during interactive rebase
should probably be the same as the strategy with a temporary branch and
rebase --onto I described earlier. I should fix my habits, I guess.

Bug: "git status --porcelain --show-stash" does not show stash count

[Eric Duncan]

Using:

   $ git version
git version 2.15.1

Running without --porcelain shows the correct stash count:

$ git status --show-stash
On branch feature-Enable-Unmarshaling-Support
Untracked files:
  (use "git add ..." to include in what will be committed)

testdata/

nothing added to commit but untracked files present (use "git add" to track)
Your stash currently has 1 entry

Notice the last line in that output, "Your stash currently has 1 entry."

But the output when adding --porcelain does not output anything for
stash counts:

$ git status --porcelain --show-stash
?? testdata/

Version 2 does not either:

$ git status --porcelain=2 --show-stash
? testdata/

Also, the --short option seems to be a shortcut to --porcelain with
color coding (and still doesn't show the stash count):

$ git status --short --show-stash
?? testdata/

I would expect something like "SS 1" or "S  1" to show stashes with a
count of 1.

Thanks!
Eric

Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

[Eric Sunshine]

On Sun, Dec 10, 2017 at 4:22 PM, Thomas Gummerer  wrote:
> Make sure that split index doesn't get broken, by running it on travis
> CI.
>
> Run the test suite with split index enabled in linux 64 bit mode, and
> leave split index turned off in 32-bit mode.  The laternative would be

s/laternative/alternative/

> to add an extra target in the matrix, enabling split index mode, but
> that would only use additional cycles on travis and would not bring that
> much benefit, as we are still running the test suite in the "vanilla"
> version in the 32-bit mode.
>
> Signed-off-by: Thomas Gummerer 

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking tree file changes)

[Igor Djordjevic]

Hi Philip,

On 10/12/2017 13:22, Phillip Wood wrote:
> 
> Sorry I should have been clearer. The point I was somewhat obliquely 
> making was that 'rebase --onto' succeeds where 'rebase --autosquash' 
> fails not because it is smarter but because it is doing something 
> different. Specifically it avoids the conflicting merge to create A'
> as the user has already created that commit in the temporary branch

No problem, and thanks for clarifying, I understand and agree to all 
that with you. I was just pointing that it wasn`t something I was 
commenting to (nor specially interested in), because of what Alexei 
actually wrote - here`s his quote (emphasis mine):

  "And then I often find that "rebase -i --autosquash" _fails to apply
  the commit B_ because it expects slightly different context around
  the changed lines."

>From there, it seemed pretty clear he perceived the failure not 
coming from creating A', but applying B on top of it, and that is 
what got my attention. But, read below...

> > - but you`re mentioning `git _commit_ --onto` instead, comparing it
> > with `rebase`... and which one of the two ("--autosquash", I
> > assume)?
> 
> Yes because in an earlier message you said
> 
> > If you mind enough to be bothered testing it out, might be even 
> > existing/initial state of originally proposed `git commit 
> > --onto-parent` script would work for you, as it does incorporate
> > some trivial three-way merge resolution.
> >
> > In your starting situation:
> >
> > ---A---B
> >
> >  you would just do something like:
> >
> > git commit --onto-parent A
> >
> >  hopefully ending up in the desired state (hopefully =
> > conflicts automatically resolved):
> >
> > ---A---C---B'
> 
> and I was pointing out that this would involve performing the same
> merge as 'rebase --autosquash' which has conflicts

Yeah, what I assumed (and agreed to), thanks for confirmation. What 
made me a bit uncertain was that you left that part of my earlier 
message quoted _after_ your inline reply to it, thus making overall 
context a bit difficult to be exactly sure in :P

> I understood Alexei to mean that it was merging the f!A into A that 
> caused conflicts due to the fact that f!A has conflicting context
> that was introduced in B. After all B' the rebased B is merge A A' B
> whether it is created by 'rebase --autosquash' or 'rebase --onto'. A'
> must be the same in both cases or one is applying a different fix.

Yes, I understand and agree you might be right, what you are talking 
about being what he actually _meant_, but because that is not what he 
_wrote_, I wanted to see an example of it, (still?) hoping that he 
really did mean what he wrote (commit B being the problematic one), 
as then there would be a possibility for improvement.

And your analysis seems correct, and that`s what I was afraid of as 
well - but wasn`t really sure, especially as I seem to remember 
something similar from my own (humble) experience, thus leaving a 
possibility for an example to prove differently.

But if that is absolutely impossible, as you claim, like not even due 
to some commit squashing, some edge case, or something - and I don`t 
feel like I have enough knowledge/experience to judge that myself at 
the moment - then you have to be right, and what he wrote is really 
not what he meant... nor what I thought I remembered from my own past 
experience, either :/ Nor there is any chance for improvement here, 
unfortunately, I guess.

Still, I hope for that example...! :D

> I've found conflicts arising from moving fixups can be quite common,
> so these days I tend to edit the commit to be fixed up directly. I
> have a script git-amend that does something like
> 
> target=$(git rev-parse --verify "$1") && GIT_SEQUENCE_EDITOR="sed -i
> s/^pick $target/edit $target/" rebase -ik $target^
> 
> so I can just type 'git amend ' to make this easier

This is useful, thanks. I have something like `git commit --amend 
` on my wish list for quite some time :) Still not getting to 
look into it, though.

> > In that (very?) specific case, proposed `git commit
> > --onto-parent`[1] doesn`t suffer from this, as once f!A is
> > successfully applied onto A (either squashed in with --amend, or on
> > top of it), we take original f!A _snapshot_ (not patch!) made on
> > top of B, and just "declare" it B` (being equal to B + f!A, which
> > we already know, and being correct), without a need to (try to)
> > apply B patch on top of fixed-up A to create B', as `rebase` does
> > (and fails).
> 
> Ah I understand, but that only works when you're fixing up HEAD~1.
> If you had A-B-C-f!A you have to recreate B with a merge.

Yes, and thus the notion of what he mentioned as being a "(very?) 
specific case" ;) That initial/draft version of "git commit 
--onto-parent" script I sent to the list[1] operates on the first 
parent commit only, indeed, though its main point/purpose had nothing 
to do with smarter merges, but just not touching the 

Re: [PATCH Outreachy 1/2] format: create pretty.h file

[Junio C Hamano]

Jeff King  writes:

> On Fri, Dec 08, 2017 at 09:40:09AM -0800, Junio C Hamano wrote:
>
>> I see you've "standardized" to drop "extern" from the declarations
>> in the header; I have an impression that our preference however is
>> to go in the other direction.
>
> Can we revisit that?
>
> I haven't see any compelling reason to include the "extern" in a
> declaration. And all things being equal, I'd prefer the thing that makes
> the source code shorter, and is one less thing for authors to remember
> to do.

Surely, but there is no point revisiting.  I simply misremembered
what we did at around 1354c9b2 ("refs: remove unnecessary "extern"
keywords", 2016-03-31).

As long as we know which way we are standardizing, I personally do
not have strong preference either way.  I appreciate shorter-to-type
(i.e. missing "extern") but I also appreciate the more familiar and
logical declaration in a header file that indicates something exists
somewhere (i.e. explicit "extern") ;-).

Thanks.

Re: t9001 failures on 32-bit Linux?

[Junio C Hamano]

Ramsay Jones  writes:

> Hi Junio,
>
> I noticed the revert of the 'ab/simplify-perl-makefile' branch on
> top of 'pu'. So, I fired up my 32-bit Linux and attempted to see
> if I could debug this t9001 test failure.
>
> Unfortunately, I could not get it to fail. :(
>
> Both of the 'pu' (@77e921d77d) and 'pu~1' (@cfef1ebefd) builds pass
> the full test-suite and, likewise, running t9001 in a loop for a
> count of 100 (about 45 minutes run-time each).
>
> [If it makes any difference, I don't have sendmail installed (no
> /usr/sbin/sendmail or /usr/lib/sendmail), or any sendmail in my
> $PATH.]
>
> Sorry I couldn't help with this! :(

I suspect that the difference is not about send(e)mail per-se, but
the differences between the set of perl modules installed in the
linux32 Travis environment and the other one.  Perhaps the use of
deprecated Error.pm is the culprit?

I do appreciate that the environments we use on Travis are not
monoculture and we end up testing more variations (in this case,
the cause likely does not have anything to do with 64- vs 32-bit,
but Perl modules).  It sometimes gets confusing, though ;-)

Another difference I noticed while scanning the failing log is that
the Linux32 environment seem to lack libsvn-perl and all tests are
skipped there.  I do not think that this particular variation is
helping us to diversify our test base, though ;-)

Thanks.

RE: SSH port ignored when ssh:// prefix isn't specified

[Randall S. Becker]

On December 10, 2017 3:24 PM Mahmoud wrote:

>It appears that for non-standard ports to be specified for ssh-based
clones/checkouts, the leading "ssh://" prefix must
>be applied. I am unsure if there's a reason for this or if it is simply an
overlooked idiosyncrasy in the parser.

>Basically, while `git clone ssh://g...@example.com:/path` works, the
same with the `ssh://` prefix doesn't,
>and attempts to establish a connection to port 22 instead: `git clone
g...@example.com:/path` (I'm not
>sure what it will do with the `:` should the connection actually
succeed).

Git is attempting to resolve the repository "/path" as it's default
behaviour since the ssh method is not specified. This is a situation where
the default is ambiguous so while git is choosing the ssh method, it is also
choosing repository resolution after the ':' which is also default
behaviour.

Cheers,
Randall

-- Brief whoami: NonStop developer since approximately
UNIX(421664400)/NonStop(2112884442) 
-- In my real life, I talk too much.

[PATCH 2/3] prune: fix pruning with multiple worktrees and split index

[Thomas Gummerer]

be489d02d2 ("revision.c: --indexed-objects add objects from all
worktrees", 2017-08-23) made sure that pruning takes objects from all
worktrees into account.

It did that by reading the index of every worktree and adding the
necessary index objects to the set of pending objects.  The index is
read by read_index_from.  As mentioned in the previous commit,
read_index_from depends on the CWD for the location of the split index,
and add_index_objects_to_pending doesn't set that before using
read_index_from.

Instead of using read_index_from, use repo_read_index, which is aware of
the proper paths for the worktree.

This fixes t5304-prune when ran with GIT_TEST_SPLIT_INDEX set.

Signed-off-by: Thomas Gummerer 
---

This also fixes t7009 when ran with GIT_TEST_SPLIT_INDEX.  I'm not
quite sure why it is fixed by this.  Either way I tracked the failure
down to f767178a5a ("Merge branch 'jk/no-null-sha1-in-cache-tree'",
2017-05-16).  Maybe Peff has an idea why this fixes that test?

 repository.c | 11 +++
 repository.h |  2 ++
 revision.c   | 13 -
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/repository.c b/repository.c
index 928b1f553d..3c9bfbd1b8 100644
--- a/repository.c
+++ b/repository.c
@@ -2,6 +2,7 @@
 #include "repository.h"
 #include "config.h"
 #include "submodule-config.h"
+#include "worktree.h"
 
 /* The main repository */
 static struct repository the_repo = {
@@ -146,6 +147,16 @@ int repo_init(struct repository *repo, const char *gitdir, 
const char *worktree)
return -1;
 }
 
+/*
+ * Initialize 'repo' based on the provided worktree
+ * Return 0 upon success and a non-zero value upon failure.
+ */
+int repo_worktree_init(struct repository *repo, struct worktree *worktree)
+{
+   return repo_init(repo, get_worktree_git_dir(worktree),
+worktree->path);
+}
+
 /*
  * Initialize 'submodule' as the submodule given by 'path' in parent repository
  * 'superproject'.
diff --git a/repository.h b/repository.h
index 7f5e24a0a2..2adeb05bf4 100644
--- a/repository.h
+++ b/repository.h
@@ -4,6 +4,7 @@
 struct config_set;
 struct index_state;
 struct submodule_cache;
+struct worktree;
 
 struct repository {
/* Environment */
@@ -87,6 +88,7 @@ extern struct repository *the_repository;
 extern void repo_set_gitdir(struct repository *repo, const char *path);
 extern void repo_set_worktree(struct repository *repo, const char *path);
 extern int repo_init(struct repository *repo, const char *gitdir, const char 
*worktree);
+extern int repo_worktree_init(struct repository *repo, struct worktree 
*worktree);
 extern int repo_submodule_init(struct repository *submodule,
   struct repository *superproject,
   const char *path);
diff --git a/revision.c b/revision.c
index e2e691dd5a..9d8d9b96d1 100644
--- a/revision.c
+++ b/revision.c
@@ -22,6 +22,7 @@
 #include "packfile.h"
 #include "worktree.h"
 #include "argv-array.h"
+#include "repository.h"
 
 volatile show_early_output_fn_t show_early_output;
 
@@ -1346,15 +1347,17 @@ void add_index_objects_to_pending(struct rev_info 
*revs, unsigned int flags)
worktrees = get_worktrees(0);
for (p = worktrees; *p; p++) {
struct worktree *wt = *p;
-   struct index_state istate = { NULL };
+   struct repository *repo;
 
+   repo = xmalloc(sizeof(struct repository));
if (wt->is_current)
continue; /* current index already taken care of */
+   if (repo_worktree_init(repo, wt))
+   BUG("couldn't initialize repository object from 
worktree");
 
-   if (read_index_from(,
-   worktree_git_path(wt, "index")) > 0)
-   do_add_index_objects_to_pending(revs, );
-   discard_index();
+   if (repo_read_index(repo) > 0)
+   do_add_index_objects_to_pending(revs, repo->index);
+   discard_index(repo->index);
}
free_worktrees(worktrees);
 }
-- 
2.15.1.504.g5279b80103

[PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

[Thomas Gummerer]

Make sure that split index doesn't get broken, by running it on travis
CI.

Run the test suite with split index enabled in linux 64 bit mode, and
leave split index turned off in 32-bit mode.  The laternative would be
to add an extra target in the matrix, enabling split index mode, but
that would only use additional cycles on travis and would not bring that
much benefit, as we are still running the test suite in the "vanilla"
version in the 32-bit mode.

Signed-off-by: Thomas Gummerer 
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 281f101f31..c83c766dee 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -39,7 +39,7 @@ env:
 
 matrix:
   include:
-- env: GETTEXT_POISON=YesPlease
+- env: GETTEXT_POISON=YesPlease GIT_TEST_SPLIT_INDEX=YesPlease
   os: linux
   compiler:
   addons:
-- 
2.15.1.504.g5279b80103

[PATCH 0/3] fixes for split index mode

[Thomas Gummerer]

On the current master branch, 95ec6b1b33 ("RelNotes: the eighth
batch", 2017-12-06) , the test suite fails a few tests when
GIT_TEST_SPLIT_INDEX is set:

Test Summary Report
---
t3007-ls-files-recurse-submodules.sh (Wstat: 256 Tests: 21 Failed: 
13)
  Failed tests:  2-14
  Non-zero exit status: 1
t7009-filter-branch-null-sha1.sh (Wstat: 256 Tests: 6 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
t5304-prune.sh   (Wstat: 256 Tests: 25 Failed: 
3)
  Failed tests:  23-25
  Non-zero exit status: 1
t7814-grep-recurse-submodules.sh (Wstat: 256 Tests: 22 Failed: 
13)
  Failed tests:  2-3, 5-10, 12-15, 22
  Non-zero exit status: 1

This series fixes these and makes travis run the test suite with
GIT_TEST_SPLIT_INDEX to avoid similar breakages in the future.

Thomas Gummerer (3):
  repository: fix repo_read_index with submodules
  prune: fix pruning with multiple worktrees and split index
  travis: run tests with GIT_TEST_SPLIT_INDEX

 .travis.yml  |  2 +-
 cache.h  |  1 +
 read-cache.c | 19 +--
 repository.c | 13 -
 repository.h |  2 ++
 revision.c   | 13 -
 6 files changed, 41 insertions(+), 9 deletions(-)

-- 
2.15.1.504.g5279b80103

[PATCH 1/3] repository: fix repo_read_index with submodules

[Thomas Gummerer]

repo_read_index calls read_index_from, which takes an path argument for
the location of the index file.  For the split index however it relies
on the current working directory to construct the path using git_path.

repo_read_index calls read_index_from with the full path for the index
file, however it doesn't change the cwd, so when split index mode is
turned on, read_index_from can't find the file for the split index.

For example t3007-ls-files-recurse-submodules.sh was broken with
GIT_TEST_SPLIT_INDEX set in 188dce131f ("ls-files: use repository
object", 2017-06-22), and t7814-grep-recurse-submodules.sh was also
broken in a similar manner, probably by introducing struct repository
there, although I didn't track down the exact commit for that.

Fix this by introducing a new read_index_for_repo function, which knows
about the correct paths for the submodules.

The alternative would have been to make the callers pass in the base
path for the split index, however that ended up being more complicated,
and I think we want to converge towards using struct repository for
things like these anyway.

Signed-off-by: Thomas Gummerer 
---
 cache.h  |  1 +
 read-cache.c | 19 +--
 repository.c |  2 +-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index cb5db7bf83..d42bea1ef7 100644
--- a/cache.h
+++ b/cache.h
@@ -614,6 +614,7 @@ extern int read_index_preload(struct index_state *, const 
struct pathspec *paths
 extern int do_read_index(struct index_state *istate, const char *path,
 int must_exist); /* for testting only! */
 extern int read_index_from(struct index_state *, const char *path);
+extern int read_index_for_repo(const struct repository *);
 extern int is_index_unborn(struct index_state *);
 extern int read_index_unmerged(struct index_state *);
 
diff --git a/read-cache.c b/read-cache.c
index 2eb81a66b9..4d5c4ad79b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -20,6 +20,7 @@
 #include "split-index.h"
 #include "utf8.h"
 #include "fsmonitor.h"
+#include "repository.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -1871,7 +1872,8 @@ static void freshen_shared_index(char *base_sha1_hex, int 
warn)
free(shared_index);
 }
 
-int read_index_from(struct index_state *istate, const char *path)
+static int do_read_index_from(struct index_state *istate, const char *path,
+ const struct repository *repo)
 {
struct split_index *split_index;
int ret;
@@ -1896,7 +1898,10 @@ int read_index_from(struct index_state *istate, const 
char *path)
split_index->base = xcalloc(1, sizeof(*split_index->base));
 
base_sha1_hex = sha1_to_hex(split_index->base_sha1);
-   base_path = git_path("sharedindex.%s", base_sha1_hex);
+   if (repo)
+   base_path = repo_git_path(repo, "sharedindex.%s", 
base_sha1_hex);
+   else
+   base_path = git_path("sharedindex.%s", base_sha1_hex);
ret = do_read_index(split_index->base, base_path, 1);
if (hashcmp(split_index->base_sha1, split_index->base->sha1))
die("broken index, expect %s in %s, got %s",
@@ -1909,6 +1914,16 @@ int read_index_from(struct index_state *istate, const 
char *path)
return ret;
 }
 
+int read_index_for_repo(const struct repository *repo)
+{
+   return do_read_index_from(repo->index, repo->index_file, repo);
+}
+
+int read_index_from(struct index_state *istate, const char *path)
+{
+   return do_read_index_from(istate, path, NULL);
+}
+
 int is_index_unborn(struct index_state *istate)
 {
return (!istate->cache_nr && !istate->timestamp.sec);
diff --git a/repository.c b/repository.c
index bb2fae5446..928b1f553d 100644
--- a/repository.c
+++ b/repository.c
@@ -229,5 +229,5 @@ int repo_read_index(struct repository *repo)
if (!repo->index)
repo->index = xcalloc(1, sizeof(*repo->index));
 
-   return read_index_from(repo->index, repo->index_file);
+   return read_index_for_repo(repo);
 }
-- 
2.15.1.504.g5279b80103

Re: t9001 failures on 32-bit Linux?

[Ævar Arnfjörð Bjarmason]

On Sun, Dec 10 2017, Ramsay Jones jotted:

> On 10/12/17 20:33, Ævar Arnfjörð Bjarmason wrote:
>> On Sun, Dec 10, 2017 at 8:58 PM, Ramsay Jones
>>  wrote:
>>> I noticed the revert of the 'ab/simplify-perl-makefile' branch on
>>> top of 'pu'. So, I fired up my 32-bit Linux and attempted to see
>>> if I could debug this t9001 test failure.
>>>
>>> Unfortunately, I could not get it to fail. :(
>>>
>>> Both of the 'pu' (@77e921d77d) and 'pu~1' (@cfef1ebefd) builds pass
>>> the full test-suite and, likewise, running t9001 in a loop for a
>>> count of 100 (about 45 minutes run-time each).
>>>
>>> [If it makes any difference, I don't have sendmail installed (no
>>> /usr/sbin/sendmail or /usr/lib/sendmail), or any sendmail in my
>>> $PATH.]
>>>
>>> Sorry I couldn't help with this! :(
>>
>> It's a bug in my patch, I'll follow-up with a patch once I figure out
>> what it is, but for now I wanted to say it's my bad.
>>
>> That this is failing has nothing to do with 32bit per say, but that
>> system doesn't have Error.pm installed, so we fall back on our own
>> copy, there's some bug in the new Error.pm fallback logic I
>> introduced.
>
> Ah, OK, that makes sense.

Updated patch sent just now as <20171210211333.9820-1-ava...@gmail.com>:
https://public-inbox.org/git/20171210211333.9820-1-ava...@gmail.com/

[PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

[Ævar Arnfjörð Bjarmason]

Replace the perl/Makefile.PL and the fallback perl/Makefile used under
NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily
inspired by how the i18n infrastructure's build process works[1].

The reason for having the Makefile.PL in the first place is that it
was initially[2] building a perl C binding to interface with libgit,
this functionality, that was removed[3] before Git.pm ever made it to
the master branch.

We've since since started maintaining a fallback perl/Makefile, as
MakeMaker wouldn't work on some platforms[4]. That's just the tip of
the iceberg. We have the PM.stamp hack in the top-level Makefile[5] to
detect whether we need to regenerate the perl/perl.mak, which I fixed
just recently to deal with issues like the perl version changing from
under us[6].

There is absolutely no reason for why this needs to be so complex
anymore. All we're getting out of this elaborate Rube Goldberg machine
was copying perl/* to perl/blib/* as we do a string-replacement on
the *.pm files to hardcode @@LOCALEDIR@@ in the source, as well as
pod2man-ing Git.pm & friends.

So replace the whole thing with something that's pretty much a copy of
how we generate po/build/**.mo from po/*.po, just with a small sed(1)
command instead of msgfmt. As that's being done rename the files
from *.pm to *.pmc just to indicate that they're generated (see
"perldoc -f require").

While I'm at it, change the fallback for Error.pm from being something
where we'll ship our own Error.pm if one doesn't exist at build time
to one where we just use a Git::Error wrapper that'll always prefer
the system-wide Error.pm, only falling back to our own copy if it
really doesn't exist at runtime. It's now shipped as
Git::FromCPAN::Error, making it easy to add other modules to
Git::FromCPAN::* in the future if that's needed.

Functional changes:

 * This will not always install into perl's idea of its global
   "installsitelib". This only potentially matters for packagers that
   need to expose Git.pm for non-git use, and as explained in the
   INSTALL file there's a trivial workaround.

 * The scripts themselves will 'use lib' the target directory, but if
   INSTLIBDIR is set it overrides it. It doesn't have to be this way,
   it could be set in addition to INSTLIBDIR, but my reading of [7] is
   that this is the desired behavior.

 * We don't build man pages for all of the perl modules as we used to,
   only Git(3pm). As discussed on-list[8] that we were building
   installed manpages for purely internal APIs like Git::I18N or
   private-Error.pm was always a bug anyway, and all the Git::SVN::*
   ones say they're internal APIs.

   There are apparently external users of Git.pm, but I don't expect
   there to be any of the others.

   As a side-effect of these general changes the perl documentation
   now only installed by install-{doc,man}, not a mere "install" as
   before.

1. 5e9637c629 ("i18n: add infrastructure for translating Git with
   gettext", 2011-11-18)

2. b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24)

3. 18b0fc1ce1 ("Git.pm: Kill Git.xs for now", 2006-09-23)

4. f848718a69 ("Make perl/ build procedure ActiveState friendly.",
   2006-12-04)

5. ee9be06770 ("perl: detect new files in MakeMaker builds",
   2012-07-27)

6. c59c4939c2 ("perl: regenerate perl.mak if perl -V changes",
   2017-03-29)

7. 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to
   default perl path", 2013-11-15)

8. 87bmjjv1pu@evledraar.booking.com ("Re: [PATCH] Makefile:
   replace perl/Makefile.PL with simple make rules"

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

This fixes the test failure we had on 32 bit Linux due to v2. That
error had nothing to do with 32 bit per-se, it was just a logic error
in the fallback logic if the system didn't have an Error.pm, and
evidently the 32 bit environment doesn't have that installed.

This also incorporates Junio's typo/grammar fixes of my commit
message, tbdiff with v2:

@@ -27,7 +27,7 @@
 So replace the whole thing with something that's pretty much a copy of
 how we generate po/build/**.mo from po/*.po, just with a small sed(1)
 command instead of msgfmt. As that's being done rename the files
-from *.pm to *.pmc just to indicate that they're genreated (see
+from *.pm to *.pmc just to indicate that they're generated (see
 "perldoc -f require").
 
 While I'm at it, change the fallback for Error.pm from being something
@@ -50,9 +50,9 @@
it could be set in addition to INSTLIBDIR, but my reading of [7] is
that this is the desired behavior.
 
- * We don't man pages for all of the perl modules as we used t, only
-   Git(3pm). As discussed on-list[8] that we were building installed
-   manpages for purely internal APIs like Git::I18N or
+ * We don't build man pages for all of the perl modules as we used to,
+   only Git(3pm). As 

Re: t9001 failures on 32-bit Linux?

[Ramsay Jones]

On 10/12/17 20:33, Ævar Arnfjörð Bjarmason wrote:
> On Sun, Dec 10, 2017 at 8:58 PM, Ramsay Jones
>  wrote:
>> I noticed the revert of the 'ab/simplify-perl-makefile' branch on
>> top of 'pu'. So, I fired up my 32-bit Linux and attempted to see
>> if I could debug this t9001 test failure.
>>
>> Unfortunately, I could not get it to fail. :(
>>
>> Both of the 'pu' (@77e921d77d) and 'pu~1' (@cfef1ebefd) builds pass
>> the full test-suite and, likewise, running t9001 in a loop for a
>> count of 100 (about 45 minutes run-time each).
>>
>> [If it makes any difference, I don't have sendmail installed (no
>> /usr/sbin/sendmail or /usr/lib/sendmail), or any sendmail in my
>> $PATH.]
>>
>> Sorry I couldn't help with this! :(
> 
> It's a bug in my patch, I'll follow-up with a patch once I figure out
> what it is, but for now I wanted to say it's my bad.
> 
> That this is failing has nothing to do with 32bit per say, but that
> system doesn't have Error.pm installed, so we fall back on our own
> copy, there's some bug in the new Error.pm fallback logic I
> introduced.

Ah, OK, that makes sense.

Thanks!

ATB,
Ramsay Jones

Re: t9001 failures on 32-bit Linux?

[Ævar Arnfjörð Bjarmason]

On Sun, Dec 10, 2017 at 8:58 PM, Ramsay Jones
 wrote:
> I noticed the revert of the 'ab/simplify-perl-makefile' branch on
> top of 'pu'. So, I fired up my 32-bit Linux and attempted to see
> if I could debug this t9001 test failure.
>
> Unfortunately, I could not get it to fail. :(
>
> Both of the 'pu' (@77e921d77d) and 'pu~1' (@cfef1ebefd) builds pass
> the full test-suite and, likewise, running t9001 in a loop for a
> count of 100 (about 45 minutes run-time each).
>
> [If it makes any difference, I don't have sendmail installed (no
> /usr/sbin/sendmail or /usr/lib/sendmail), or any sendmail in my
> $PATH.]
>
> Sorry I couldn't help with this! :(

It's a bug in my patch, I'll follow-up with a patch once I figure out
what it is, but for now I wanted to say it's my bad.

That this is failing has nothing to do with 32bit per say, but that
system doesn't have Error.pm installed, so we fall back on our own
copy, there's some bug in the new Error.pm fallback logic I
introduced.

Re: SSH port ignored when ssh:// prefix isn't specified

[Matthieu Moy]

"mqu...@neosmart.net"  writes:

> Basically, while `git clone ssh://g...@example.com:/path` works, the same
> with the `ssh://` prefix doesn't, and attempts to establish a connection to
> port 22 instead: `git clone g...@example.com:/path` (I'm not sure what it
> will do with the `:` should the connection actually succeed).


I don't see a simple way to distinguish between "I want to connect to
port 22 and access directory /path" and "I want to connect to port
 and access directory path". So Git chose for you the first option
(if you replace  with abcd, it clearly makes sense).

-- 
Matthieu Moy
https://matthieu-moy.fr/

SSH port ignored when ssh:// prefix isn't specified

[mqu...@neosmart.net]

Greetings,

It appears that for non-standard ports to be specified for ssh-based
clones/checkouts, the leading "ssh://" prefix must be applied. I am unsure if
there's a reason for this or if it is simply an overlooked idiosyncrasy in the
parser.

Basically, while `git clone ssh://g...@example.com:/path` works, the same
with the `ssh://` prefix doesn't, and attempts to establish a connection to
port 22 instead: `git clone g...@example.com:/path` (I'm not sure what it
will do with the `:` should the connection actually succeed).

Mahmoud Al-Qudsi
NeoSmart Technologies

t9001 failures on 32-bit Linux?

[Ramsay Jones]

Hi Junio,

I noticed the revert of the 'ab/simplify-perl-makefile' branch on
top of 'pu'. So, I fired up my 32-bit Linux and attempted to see
if I could debug this t9001 test failure.

Unfortunately, I could not get it to fail. :(

Both of the 'pu' (@77e921d77d) and 'pu~1' (@cfef1ebefd) builds pass
the full test-suite and, likewise, running t9001 in a loop for a
count of 100 (about 45 minutes run-time each).

[If it makes any difference, I don't have sendmail installed (no
/usr/sbin/sendmail or /usr/lib/sendmail), or any sendmail in my
$PATH.]

Sorry I couldn't help with this! :(

ATB,
Ramsay Jones

Please reply, I have a genuine business to discuss with you.

[Hussein Saleh]

I have a business proposal for you, it is an oil exportation
proposition contract for you and this is a highly prospective crude
oil sales venture; it involves the exportation 300,000 barrels of
Light Crude Oil daily, this project is genuine, legal and highly
lucrative. For more details, please reply.


Sincerely
Hussein Saleh

Re: [PATCH Outreachy 1/2] format: create pretty.h file

[Jeff King]

On Fri, Dec 08, 2017 at 09:40:09AM -0800, Junio C Hamano wrote:

> I see you've "standardized" to drop "extern" from the declarations
> in the header; I have an impression that our preference however is
> to go in the other direction.

Can we revisit that?

I haven't see any compelling reason to include the "extern" in a
declaration. And all things being equal, I'd prefer the thing that makes
the source code shorter, and is one less thing for authors to remember
to do.

-Peff

Re: [PATCH v3 1/7] git-compat-util: introduce skip_to_optional_arg()

[Jeff King]

On Sun, Dec 10, 2017 at 09:31:18AM -0500, Jeff King wrote:

> On Sat, Dec 09, 2017 at 09:40:07PM +0100, Christian Couder wrote:
> 
> > The changes compared to v2 are:
> > 
> >   - s/_val/_arg/ in the name of the functions
> >   - s/val/arg/ in the name of the third argument of the functions
> >   - works with NULL as third argument of the functions
> 
> This whole series looks OK to me, but this third point made me wonder:
> what would be the use of allowing NULL for the "arg" parameter?
> 
> I didn't see any use of this in the series, and I'm having trouble
> figuring out how it would be useful. E.g., if I do:
> 
>   if (skip_to_optional_arg(arg, "--foo", NULL))
>  ...
> 
> what can I do in "..."? I know we matched _some_ type of "--foo", but I
> cannot know whether it was "--foo" or "--foo=bar", nor what "bar" is. It
> could only be used by some kind of vague validator to say "well, at
> least this looks like an option that I _could_ parse if I wanted to".
> 
> So I guess I don't mind it, as it does the most reasonable thing it can
> when passed NULL, but I would be surprised if we ever actually exercise
> the code path.

And of course as soon as I sent this, I went back and double-checked.
And indeed I totally missed this call:

  +   else if (starts_with(arg, "-B") ||
  +skip_to_optional_arg(arg, "--break-rewrites", NULL)) {
  if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)

So that's kind-of weird, because we are parsing "-B", etc, and then
expecting it to be _reparsed_ by diff_scoreopt_parse. So the two
callsites must always match. IMHO this ought to do either:

  - we should just ask diff_scoreopt_parser to tell us if this was a
valid option that it understood

or

  - parse up to the "=", and then ask the scoreopt parser to parse the
remainder. This would require us passing 'B'/'C'/'M' to the
function ourselves, I think that's a better pattern. It means we
could reuse the parser for things like config values if we wanted to
(our current diff.renames is a bool, but it would not be
unreasonable for it to take a score).

None of that is a mess of your creation, though, so I'm OK punting on it
for now.

-Peff

Re: [PATCH v3 1/7] git-compat-util: introduce skip_to_optional_arg()

[Jeff King]

On Sat, Dec 09, 2017 at 09:40:07PM +0100, Christian Couder wrote:

> The changes compared to v2 are:
> 
>   - s/_val/_arg/ in the name of the functions
>   - s/val/arg/ in the name of the third argument of the functions
>   - works with NULL as third argument of the functions

This whole series looks OK to me, but this third point made me wonder:
what would be the use of allowing NULL for the "arg" parameter?

I didn't see any use of this in the series, and I'm having trouble
figuring out how it would be useful. E.g., if I do:

  if (skip_to_optional_arg(arg, "--foo", NULL))
 ...

what can I do in "..."? I know we matched _some_ type of "--foo", but I
cannot know whether it was "--foo" or "--foo=bar", nor what "bar" is. It
could only be used by some kind of vague validator to say "well, at
least this looks like an option that I _could_ parse if I wanted to".

So I guess I don't mind it, as it does the most reasonable thing it can
when passed NULL, but I would be surprised if we ever actually exercise
the code path.

-Peff

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

[Jeff King]

On Sat, Dec 09, 2017 at 02:44:44PM +0100, Johannes Schindelin wrote:

> > > ... and we could simply see whether the environment variable
> > > TEST_SHELL_PATH (which we would set in t/Makefile from the passed-in
> > > SHELL_PATH) is set, and override it again.
> > > 
> > > I still think we can do without recording test-phase details in the
> > > build-phase (which may, or may not, know what the test-phase wants to do).
> > > 
> > > In other words, I believe that we can make the invocation you mentioned
> > > above work, by touching only t/Makefile (to pass SHELL_PATH as
> > > TEST_SHELL_PATH) and t/test-lib.sh (to override the SHELL_PATH from
> > > GIT-BUILD-OPTIONS with TEST_SHELL_PATH, if set).
> > 
> > We could do that, but it makes TEST_SHELL_PATH inconsistent with all of
> > the other config.mak variables.
> 
> It is already inconsistent with the other variables because its scope is
> the "test" phase, not the "build" phase.

I'm not sure that's true. Look at what already goes into
GIT-BUILD-OPTIONS: TEST_OUTPUT_DIRECTORY, GIT_TEST_CMP, GIT_PERF_*, etc.

Interestingly, many of those do something like this in the Makefile:

  ifdef GIT_TEST_CMP
@echo GIT_TEST_OPTS=... >>$@+
  endif

which seems utterly confusing to me. Because it means that if you build
with those options set, then they will override anything in the
environment. But if you don't, then you _may_ override them in the
environment. In other words:

  make
  cd t
  GIT_TEST_CMP=foo ./t-*

will respect that variable. But:

  make GIT_TEST_CMP=foo
  cd t
  GIT_TEST_CMP=bar ./t-*

will not. Which seems weird.  But I guess we could follow that pattern
with TEST_SHELL_PATH.

-Peff

Re: [PATCH v1 1/1] check-non-portable-shell.pl: Quoted `wc -l` is not portable

[Johannes Schindelin]

Hi Torsten,

On Sun, 10 Dec 2017, tbo...@web.de wrote:

> From: Torsten Bögershausen 
> 
> wc -l is used to count the number if lines in test scripts.
> $ wc -l Makefile
> gives a line like this:
> 105 Makefile
> while Mac OS has 4 leading spaces:
>  105 Makefile
> 
> And this means that shell expressions like
> test "$(wc -l  
> A portable way to use `wc -l` is to omit the '"':
> test $(wc -l  
> Add a check in check-non-portable-shell.pl to find '"' between
> `wc -l` and '='
> 
> Signed-off-by: Torsten Bögershausen 
> ---
>  t/check-non-portable-shell.pl | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
> index 03dc9d2852..9ebf65c26f 100755
> --- a/t/check-non-portable-shell.pl
> +++ b/t/check-non-portable-shell.pl
> @@ -21,6 +21,7 @@ while (<>) {
>   /^\s*declare\s+/ and err 'arrays/declare not portable';
>   /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
>   /\btest\s+[^=]*==/ and err '"test a == b" is not portable (please use 
> =)';
> + /\bwc -l.*"\s*=/ and err '`"$(wc -l)"` is not portable, please use 
> `$(wc -l)`';
>   /\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable 
> (please use FOO=bar && export FOO)';
>   # this resets our $. for each file
>   close ARGV if eof;

As noted elsewhere, this should suggest `test_line_count` instead. After
all, that function is not only guaranteed to stay portable (even if we
should ever start supporting systems *without* `wc`), but it also has a
semantically-meaningful name worthy of the current century.

Ciao,
Dscho

Re: What's cooking in git.git (Dec 2017, #01; Mon, 4)

[Johannes Schindelin]

Hi Torsten,

On Sat, 9 Dec 2017, Torsten Bögershausen wrote:

> On Thu, Dec 07, 2017 at 04:33:12PM -0500, Todd Zullinger wrote:
> > Jeff Hostetler wrote:
> > >I'm looking at t5616 now on my mac.
> > >Looks like the MAC doesn't like my line counting in the tests.
> > >I'll fix in my next version.
> > 
> []
> > | sort >expect_2.oids &&
> > -   test "$(wc -l  > +   test_line_count = 8 expect_2.oids &&
> > git -C src blame master -- file.1.txt >expect.blame
> > '
> 
> 
> The problem seems to be the '"' around wc, this would work:
> test $(wc -l  
> 
> What do you guys think a about a lint test like this:
> 
> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
> index 03dc9d2852..9ebf65c26f 100755
> --- a/t/check-non-portable-shell.pl
> +++ b/t/check-non-portable-shell.pl
> @@ -21,6 +21,7 @@ while (<>) {
>   /^\s*declare\s+/ and err 'arrays/declare not portable';
>   /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
>   /\btest\s+[^=]*==/ and err '"test a == b" is not portable (please use 
> =)';
> + /\bwc -l.*"\s*=/ and err '`"$(wc -l)"` is not portable, please use 
> `$(wc -l)`';
>   /\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable 
> (please use FOO=bar && expo

Please suggest `test_line_count` instead.

Ciao,
Dscho

Hello My Dear Friend,

[Mrs.Zainab Ahmed]

I have a business proposal in the tune of $10.2 Million USD for you to handle 
with me. I have opportunity to transfer this abandon fund to your bank account 
in your country which belongs to our client.

I am inviting you in this transaction where this money can be shared between us 
at ratio of 60/40% and help the needy around us don’t be afraid of anything I 
am with you I will instruct you what you will do to maintain this fund.

Please kindly contact me with your information's if you are interested in this 
transaction for more details.

Your Name:..
Your Bank Name:.
Your Account Number:...
Your Telephone Number:
Your Country And Address:
Your Age And Sex:...

Thanks
Mrs.Zainab Ahmed,

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking tree file changes)

[Phillip Wood]

On 10/12/17 01:20, Igor Djordjevic wrote:
> 
> Hi Philip,
> 
> On 09/12/2017 20:01, Phillip Wood wrote:
>>
>>> But thanks for clarifying, anyway, it does feel like `git rebase
>>> -i --autosquash` could be smarter in this regards, if `git rebase 
>>> --onto` does it better...?
>>
>> Creating the fixup directly on A rather than on top of B avoids the 
>> conflicting merge B f!A A. Creating the fixup on top of B and then
>> using git commit --onto A would suffer from the same conflicts as
>> rebase does.
> 
> I`m a bit confused here, as you`re replying to the part where we 
> strictly discussed `rebase --autosquash` versus `rebase --onto`, 
> having the latter succeed where the former fails

Sorry I should have been clearer. The point I was somewhat obliquely
making was that 'rebase --onto' succeeds where 'rebase --autosquash'
fails not because it is smarter but because it is doing something
different. Specifically it avoids the conflicting merge to create A' as
the user has already created that commit in the temporary branch


> - but you`re 
> mentioning `git _commit_ --onto` instead, comparing it with `rebase`... 
> and which one of the two ("--autosquash", I assume)?

Yes because in an earlier message you said

> If you mind enough to be bothered testing it out, might be even
> existing/initial state of originally proposed `git commit
> --onto-parent` script would work for you, as it does incorporate some
> trivial three-way merge resolution.
>
> In your starting situation:
>
> ---A---B
>
>  you would just do something like:
>
> git commit --onto-parent A
>
>  hopefully ending up in the desired state (hopefully = conflicts
> automatically resolved):
>
> ---A---C---B'

and I was pointing out that this would involve performing the same merge
as 'rebase --autosquash' which has conflicts

> 
> Even further, while I do seem to understand (and agree with) what 
> you`re talking about with `commit --onto` and `rebase --autosquah` 
> suffering from the same conflicts in attempt to take f!A, originally 
> created on top of B, and apply it on top of A - the thing is that 
> Alexei actually pointed to B being the problematic one, failing to 
> rebase on top of already (successfully) autosquashed A' (where A' = A 
> + f!A, fixup applied through --autosquash), while it doesn`t fail 
> rebasing --onto f!A when f!A is being committed on top of A directly 
> (and not through --autosquash).

I understood Alexei to mean that it was merging the f!A into A that
caused conflicts due to the fact that f!A has conflicting context that
was introduced in B. After all B' the rebased B is merge A A' B whether
it is created by 'rebase --autosquash' or 'rebase --onto'. A' must be
the same in both cases or one is applying a different fix.

I've found conflicts arising from moving fixups can be quite common, so
these days I tend to edit the commit to be fixed up directly. I have a
script git-amend that does something like

target=$(git rev-parse --verify "$1") && GIT_SEQUENCE_EDITOR="sed -i
s/^pick $target/edit $target/" rebase -ik $target^

so I can just type 'git amend ' to make this easier

> 
> In that (very?) specific case, proposed `git commit --onto-parent`[1] 
> doesn`t suffer from this, as once f!A is successfully applied onto A 
> (either squashed in with --amend, or on top of it), we take original 
> f!A _snapshot_ (not patch!) made on top of B, and just "declare" it 
> B` (being equal to B + f!A, which we already know, and being 
> correct), without a need to (try to) apply B patch on top of fixed-up 
> A to create B', as `rebase` does (and fails).

Ah I understand, but that only works when you're fixing up HEAD~1. If
you had A-B-C-f!A you have to recreate B with a merge.

> 
>> I don't think there is any way for 'git rebase --autosquash' to
>> avoid the conflicts unless it used a special fixup merge strategy
>> that somehow took advantage of the DAG to resolve the conflicts by
>> realizing they come from a later commit. However I don't think that
>> could be implemented reliably as sometimes one wants those
>> conflicting lines from the later commit to be moved to the earlier
>> commit with the fixup.
> 
> I think I agree on this part being tricky (if possible at all), but I 
> also think this is not what Alexei was complaining about, nor what we 
> were discussing (as I tried to explain above) - but please do correct 
> me if I misunderstood you.

No, I don't think Alexei was complaining about that directly, but if
such a solution existed he (and everyone else) wouldn't have to bother
with the --onto approach in the case where merging the fixup creates
conflicts.

Best Wishes


Phillip

> 
> That said, and what I mentioned already, we might really benefit from 
> simple test case(s), showing "rebase --autosquash" failing where 
> "rebase --onto" works, as Alexei explained, giving some more (and 
> firm) context to the discussion.
>
> 
> I *think* I`ve experienced this in the past myself, but now I 

RE: [PATCH] doc: reword gitworflows for neutrality

[BENSOUSSAN--BOHM DANIEL p1507430]

>As a native English speaker, I find the new phrasing odd, and think
>this may a step backward. How about trying a different approach? For
>example:

>Occasionally, the maintainer may get merge conflicts when trying
>to pull changes from downstream. In this case, it may make sense
>to ask downstream to do the merge and resolve the conflicts
>instead (since, presumably, downstream will know better how to
>resolve them).

Indeed, this is an other possibility.

Thanks for the review.

Daniel BENSOUSSAN-BOHM

[PATCH v1 1/1] check-non-portable-shell.pl: Quoted `wc -l` is not portable

[tboegi]

From: Torsten Bögershausen 

wc -l is used to count the number if lines in test scripts.
$ wc -l Makefile
gives a line like this:
105 Makefile
while Mac OS has 4 leading spaces:
 105 Makefile

And this means that shell expressions like
test "$(wc -l