date:20170507

Jonathan Tan  writes:

> We noticed this when trying to use Git to make a signed push with push
> options to a server using JGit (which rejects such pushes because the
> Git client makes requests that are, strictly speaking, incompatible with
> the documented protocol).
>
> There have been several commits (see the commits linked in the commit
> messages of the patches sent in reply to this e-mail) to support push
> options (that are read by receive hooks) when using "git push", but the
> interaction between push options and signed pushes are not very clear.
> Here are some patches (containing both code and documentation) that
> clarify this interaction.
>
> I would appreciate a review of this - if we could make the protocol
> clear, we could then update JGit to use the updated protocol and be no
> longer incompatible with existing Git clients.

I've seen the exchanges on list on these three patches, and I agree
with what Jonathan said.

These are going in the right direction in general but a few nits
need to be picked before becoming ready for 'next'.

Thanks.

>
> Jonathan Tan (3):
>   docs: correct receive.advertisePushOptions default
>   receive-pack: verify push options in cert
>   protocol docs: explain receive-pack push options
>
>  Documentation/config.txt  |  5 ++--
>  Documentation/git-receive-pack.txt| 10 +++
>  Documentation/technical/pack-protocol.txt | 32 
>  builtin/receive-pack.c| 49 
> ---
>  t/t5534-push-signed.sh| 15 ++
>  5 files changed, 98 insertions(+), 13 deletions(-)

Re: [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings

Johannes Schindelin  writes:

> The test t4051-diff-function-context.sh passes on Linux when
> core.autocrlf=true even without marking its support files as LF-only,
> but they fail when core.autocrlf=true in Git for Windows' SDK.
>
> The reason is that `grep ... >file.c.new` will keep CR/LF line endings
> on Linux (obviously treating CRs as if they were regular characters),
> but will be converted to LF-only line endings with MSYS2's grep that is
> used in Git for Windows.

Ahem.  

I thought that according to your claim a UNIX tool like "grep" would
alway use LF endings?  ;-)

> As we do not want to validate the way the available `grep` works, let's
> just mark the input as LF-only and move on.

I agree with this conclusion; just like we do not want to worry
about how `grep` works when given CRLF files in this patch, we do
not want to worry about how other commands like `sh` works when
given CRLF files.  And that is consistent with the overall theme of
this series that marked *.sh, *.perl and other files with eol=lf
attribute.

The only question I still have with this series is about the
README/COPYING thing.  I _think_ it was my ancient mistake to use
the toplevel README and COPYING as test files, and that mistake was
corrected by somebody else earlier by having a frozen copy in
t/diff-lib/ and making tests use these files from that directory.
If we broke our tests to again use these files from outside
t/diff-lib/, then we would need to fix the tests not to do so.  And
if we are only looking at t/diff-lib/ copy, then I think it is more
consistent with the rest of this series to mark them with eol=lf
rather than passing them through "tr -d '\015'".

Thanks.

>
> Reviewed-by: Jonathan Nieder 
> Signed-off-by: Johannes Schindelin 
> ---
>  t/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/.gitattributes b/t/.gitattributes
> index 3525ca43f30..bdd82cf31f7 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -5,6 +5,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
>  /t4034/*/* eol=lf
>  /t4013/* eol=lf
>  /t4018/* eol=lf
> +/t4051/* eol=lf
>  /t4100/* eol=lf
>  /t4101/* eol=lf
>  /t4109/* eol=lf

Re: [PATCH v2 6/7] Fix the remaining tests that failed with core.autocrlf=true

Johannes Schindelin  writes:

> Note: the test suite also uses the t/README file as well as the COPYING
> file in t/diff-lib/, expecting LF-only line endings explicitly and
> failing if that assumption does not hold true. That is why we mark them
> as LF-only in the .gitattributes, too.

I said the previous step that used COPYING was good because it
didn't force the file to be checked out with lf (and instead fixed
the test to strip CR if/as necessary), but come to think of it,
these COPYING/README files are in t/diff-lib/ that are shipped as
test vector, and meant to stay constant even when the end-user
facing COPYING and README at the top level changed.

I do not see t/diff-lib/* marked as eol=lf in this patch, but
shouldn't it be done here, just like all these test vector files?

I also wonder if that makes the previous step unnecessary.

>
> This patch can be validated even on Linux by using this cadence:
>
>   git config core.autocrlf true
>   rm .git/index && git stash
>   make -j15 DEVELOPER=1 test
>
> Reviewed-by: Jonathan Nieder 
> Signed-off-by: Johannes Schindelin 
> ---
>  t/.gitattributes | 20 +++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/t/.gitattributes b/t/.gitattributes
> index 2d44088f56e..3525ca43f30 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -1,2 +1,20 @@
>  t[0-9][0-9][0-9][0-9]/* -whitespace
> -t0110/url-* binary
> +/t0110/url-* binary
> +/t3900/*.txt eol=lf
> +/t3901/*.txt eol=lf
> +/t4034/*/* eol=lf
> +/t4013/* eol=lf
> +/t4018/* eol=lf
> +/t4100/* eol=lf
> +/t4101/* eol=lf
> +/t4109/* eol=lf
> +/t4110/* eol=lf
> +/t4135/* eol=lf
> +/t4211/* eol=lf
> +/t4252/* eol=lf
> +/t5100/* eol=lf
> +/t5515/* eol=lf
> +/t556x_common eol=lf
> +/t7500/* eol=lf
> +/t8005/*.txt eol=lf
> +/t9*/*.dump eol=lf

Re: [PATCH v2 5/7] t4003, t4005, t4007 & t4008: handle CR/LF in t/README & t/diff-lib/README

Johannes Schindelin  writes:

> diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
> index df2accb6555..c3e0a3c3fc9 100755
> --- a/t/t4003-diff-rename-1.sh
> +++ b/t/t4003-diff-rename-1.sh
> @@ -11,7 +11,7 @@ test_description='More rename detection
>  
>  test_expect_success \
>  'prepare reference tree' \
> -'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&

Good ;-).  Thanks.

Re: [PATCH v2 4/7] t3901: move supporting files into t/t3901/

Johannes Schindelin  writes:

> The current convention is to either generate files on the fly in tests,
> or to use supporting files taken from a t/t/ directory (where 
> matches the test's number, or the number of the test from which we
> borrow supporting files).
>
> The test t3901-i18n-patch.sh was obviously introduced before that
> convention was in full swing, hence its supporting files still lived in
> t/t3901-8859-1.txt and t/t3901-utf8.txt, respectively.
>
> Let's adjust to the current convention.

Thanks for a clean-up.  This obviously is a good change ;-)

Re: [PATCH v2 2/7] git-new-workdir: mark script as LF-only

Johannes Schindelin  writes:

> Bash does not handle scripts with CR/LF line endings correctly, therefore
> they *have* to be forced to LF-only line endings.
>
> Funnily enough, this fixes t3000-ls-files-others and
> t1021-rerere-in-workdir when git.git was checked out with
> core.autocrlf=true, as these test still use git-new-workdir (once `git
> worktree` is no longer marked as experimental, both scripts probably
> want to be ported to using that command instead).
>
> Reviewed-by: Jonathan Nieder 
> Signed-off-by: Johannes Schindelin 
> ---

I wouldn't bother fixing these myself, but the above two credit
lines are swapped.  You wrote, then Jonathan reviewed (to which I'll
append my own as the 'editor' of the history when I commit).

>  contrib/workdir/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 contrib/workdir/.gitattributes
>
> diff --git a/contrib/workdir/.gitattributes b/contrib/workdir/.gitattributes
> new file mode 100644
> index 000..1f78c5d1bd3
> --- /dev/null
> +++ b/contrib/workdir/.gitattributes
> @@ -0,0 +1 @@
> +/git-new-workdir eol=lf

Re: [PATCH v2 1/7] Fix build with core.autocrlf=true

Johannes Schindelin  writes:

> When generating common-cmds.h, the Unix tools we use generally operate on
> the assumption that input and output deliminate their lines using LF-only
> line endings. Consequently, they would happily copy the CR byte verbatim
> into the strings in common-cmds.h, which in turn makes the C preprocessor
> barf (that interprets them as MacOS-style line endings). Therefore, we
> have to mark the input files as LF-only: command-list.txt and
> Documentation/git-*.txt.

I guess nobody in the Windows land opens the doc source with
Notepad, like experienced Git developers runs "less" on them as a
faster way than accessing HTMLized (or manified) versions, and
marking Documentation/git-*.txt explicitly as eol=lf is OK.  That
was the only thing I had brief worry about this patch.

Thanks, will queue.

> diff --git a/.gitattributes b/.gitattributes
> index 320e33c327c..8ce9c6b 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -1,3 +1,9 @@
>  * whitespace=!indent,trail,space
>  *.[ch] whitespace=indent,trail,space diff=cpp
> -*.sh whitespace=indent,trail,space
> +*.sh whitespace=indent,trail,space eol=lf
> +*.perl eol=lf
> +*.pm eol=lf
> +/Documentation/git-*.txt eol=lf
> +/command-list.txt eol=lf
> +/GIT-VERSION-GEN eol=lf
> +/mergetools/* eol=lf
> diff --git a/git-gui/.gitattributes b/git-gui/.gitattributes
> index 33d07c06bd9..59cd41dbff7 100644
> --- a/git-gui/.gitattributes
> +++ b/git-gui/.gitattributes
> @@ -2,3 +2,4 @@
>  *   encoding=US-ASCII
>  git-gui.sh  encoding=UTF-8
>  /po/*.poencoding=UTF-8
> +/GIT-VERSION-GEN eol=lf

Re: [PATCH] pack-objects: disable pack reuse for object-selection options

Jeff King  writes:

> If certain options like --honor-pack-keep, --local, or
> --incremental are used with pack-objects, then we need to
> feed each potential object to want_object_in_pack() to see
> if it should be filtered out.  This is totally contrary to
> the purpose of the pack-reuse optimization, which tries hard
> to avoid doing any per-object work.  Therefore we need to
> disable this optimization when these options are in use.

I read this five times, as I wanted to understand what you are
saying, but I am not sure if I got it right.  One of the reasons why
I was confused was that I originally thought this "reuse" was about
delta reuse, but it is not.  It is "sending a slice of the original
packfile straight to the output".  But even after getting myself out
of that confusion, I still do not see why we "need to disable".
Surely, even if we need to exclude some objects from an existing
packfile due to these selection options, we should be able to reuse
the non-excluded part, no?  The end result may involve having to
pick and reuse more and smaller slices from existing packfiles,
which may be much less efficient, but it is no immediately obvious
to me if it leads to "need to disable".  I would understand it if it
were "it becomes much less efficient and we are better off not using
the bitmap code at all", though.

Is the real reason why we want to disable the "reuse" because it is
not easy to update the reuse_partial_packfile_from_bitmap() logic to
take these selection options into account?  If so, I would also
understand why this is a good change.

Puzzled.

> This bug has been present since the inception of the
> pack-reuse code, but was unlikely to come up in practice.

> +test_expect_success 'pack reuse respects --honor-pack-keep' '
> + test_when_finished "rm -f .git/objects/pack/*.keep" &&
> + for i in .git/objects/pack/*.pack; do
> + >${i%.pack}.keep
> + done &&

Micronit: style.

Re: [PATCH v2 6/9] dir: change linkage of cmp_name() and check_contains()

Samuel Lijin  writes:

> diff --git a/dir.h b/dir.h
> index bf23a470a..1ddd8b611 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -326,6 +326,9 @@ static inline int dir_path_match(const struct dir_entry 
> *ent,
> has_trailing_dir);
>  }
>  
> +int cmp_name(const void *p1, const void *p2);
> +int check_contains(const struct dir_entry *out, const struct dir_entry *in);
> +

Both of these would have been perfectly sensible names when they
were private to dir.c, but are way too generic to live in the global
namespace.  When a person who works on Git internal hears a name
"check_contains()", I am sure that the first guess of what the
function does is to see if a commit is a descendant of another
commit.

Re: [PATCH v2 2/9] t7061: expect failure where expected behavior will change

Samuel Lijin  writes:

> This changes tests for `status --ignored` from test_expect_success to
> test_expect_failure in preparation for a change in its expected behavior
> (namely, that ignored files in untracked dirs will be reported).
>
> Signed-off-by: Samuel Lijin 
> ---

This is an odd way to do this.  If we stop applying your patches at
this step, these tests will still see output from "status --ignored"
that is expected by them, so there is no expect_failure here.

If we decide that the current output from "status --ignored" is
WRONG, and your series to fix "clean -d" FIXES "status --ignored" as
a side effect, then having a step to describe a desired behaviour in
the new world order in the test like this patch does makes sense,
but if that is what is going on, then not just flipping "success" to
"failure", the patch would be changing the expected output as well,
i.e. by adding the ignored files in untracked directories in the
expected output.  Obviously the code at this point after applying
only patches 1 & 2 will not produce such an output, so marking the
test that expects output based on the new world order as "expect
failure" would make sense.  Then your future commit that FIXES
"status --ignored" output would flip _failure to _success.

It is unclear to me if the new behaviour of "status --ignored" is a
bugfix, or a new bug, though.

>  t/t7061-wtstatus-ignore.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
> index cdc0747bf..dc3be92a2 100755
> --- a/t/t7061-wtstatus-ignore.sh
> +++ b/t/t7061-wtstatus-ignore.sh
> @@ -11,7 +11,7 @@ cat >expected <<\EOF
>  ?? untracked/
>  EOF
>  
> -test_expect_success 'status untracked directory with --ignored' '
> +test_expect_failure 'status untracked directory with --ignored' '
>   echo "ignored" >.gitignore &&
>   mkdir untracked &&
>   : >untracked/ignored &&
> @@ -20,7 +20,7 @@ test_expect_success 'status untracked directory with 
> --ignored' '
>   test_cmp expected actual
>  '
>  
> -test_expect_success 'same with gitignore starting with BOM' '
> +test_expect_failure 'same with gitignore starting with BOM' '
>   printf "\357\273\277ignored\n" >.gitignore &&
>   mkdir -p untracked &&
>   : >untracked/ignored &&

Re: [RFC] Add warning when git discard changes on another branch?

On Mon, May 08, 2017 at 01:19:58PM +0900, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > But to help "some users are not aware of this" situation, an opt-in
> > "feature" would not help all that much.  The same number of lines in
> > the documentation to tell end-users how to toggle on such a "safety"
> > feature can be spent to teach them that their local changes in the
> > working tree do *not* belong to any particular branch, and as soon
> > as it is understood, the user would be OK.
> >
> > So...
> 
> It might help if we treat this similarly to how we treat the
> "detached HEAD" state.  By default when you do "git checkout HEAD^0"
> (not "git checkout --detach HEAD"), you would get a large warning,
> which you can silence by the advice.detachedhead configuration.  In
> addition to the list of "these paths have local modifications" that
> we show as a reminder, perhaps you want to show a warning that tells
> the user that the local modifications in these paths are not
> recorded anywhere else, or somesuch, and silence it with a new
> advice.* variable?

That would be helpful. But, frankly, if a user would be aware of that `advice.*'
variable, he would have enough knowledge of Git to be aware of that situation.
So, I think that 'M lala.txt' in transitions from branch to branch would be
sufficient.

---
Yubin

Re: [PATCH v2 0/9] Keep git clean -d from inadvertently removing ignored files

Samuel Lijin  writes:

> Addresses the issues raised by Stefan and Junio (thanks for your
> feedback) about not using C99-style comments and keeping tests
> working on every commit to prevent breaking git bisect. (About the
> latter one: is it necessary to prevent compiler warnings, in
> addition to compiler errors? Because if so I should probably
> squash some of the commits together.)

Some of us build with -Werror, so yes.  If by "squashing" you mean
"instead of piling a fix on top of a broken patch, I need to do
things right from the beginning", then yes, please do so, not just
for compiler warnings but for all forms of changes.

> Note that this introduces a breaking change in the behavior of git
> status: when invoked with --ignored, git status will now return
> ignored files in an untracked directory, whereas previously it
> would not.

What do you mean by a "breaking change"?  Is it just "a new bug"?
Or "the current behaviour is logically broken, but people and
scripts might have relied on that odd behaviour and fixing it this
late in the game would break their expectations"? 

> It's possible that there are standard practices that I might have
> missed, so if there is anything along those lines, I'd appreciate
> you letting me know. (As an aside, about the git bisect thing: is
> there a script somewhere that people use to test patch series
> before sending them out?)

I hear that people use variations of

git rebase -x "make test"

on their topic.

Re: [RFC] Add warning when git discard changes on another branch?

Junio C Hamano  writes:

> But to help "some users are not aware of this" situation, an opt-in
> "feature" would not help all that much.  The same number of lines in
> the documentation to tell end-users how to toggle on such a "safety"
> feature can be spent to teach them that their local changes in the
> working tree do *not* belong to any particular branch, and as soon
> as it is understood, the user would be OK.
>
> So...

It might help if we treat this similarly to how we treat the
"detached HEAD" state.  By default when you do "git checkout HEAD^0"
(not "git checkout --detach HEAD"), you would get a large warning,
which you can silence by the advice.detachedhead configuration.  In
addition to the list of "these paths have local modifications" that
we show as a reminder, perhaps you want to show a warning that tells
the user that the local modifications in these paths are not
recorded anywhere else, or somesuch, and silence it with a new
advice.* variable?

Re: [PATCH v3] send-email: --batch-size to work around some SMTP server limit

xiaoqiang zhao  writes:

> @@ -1664,6 +1674,14 @@ foreach my $t (@files) {
>   }
>   }
>   $message_id = undef;
> + $num_sent++;
> + if ($num_sent == $batch_size) {
> + $num_sent = 0;
> + $smtp->quit;
> + $smtp = undef;
> + $auth = 0;

Two suggestions.

 (1) I do not think $smtp is always valid when we come here; it is
 unsafe to unconditionally say $smtp->quit like this patch does.

$smtp->quit if defined $smtp;

 may help codepaths like $dry_run and also the case where
 $smtp_server is the absolute path to a local program.

 (2) You are setting $auth to zero to force re-authentication to
 happen.  It would be more consistent to the state of $auth that
 is not-yet-used to "undef $auth;" here instead.  After all, the
 variable starts its life in an undefined state.

So all in all

$smtp->quit if defined $smtp;
undef $smtp;
undef $auth;

perhaps?

This change of course forces re-authentication every N messages,
which may not hurt those who use some form of credential helper, but
that may be something we want to mention in the log message.

> + sleep($relogin_delay);
> + }
>  }

Thanks.

Re: [RFC] Add warning when git discard changes on another branch?

On Mon, May 08, 2017 at 12:41:03PM +0900, Junio C Hamano wrote:
> Yubin Ruan  writes:
> 
> > I understand this. I just suggest that git add some warning in case some 
> > users
> > are not aware of this, as it does when , on branch 'issue', changes to 
> > 'lala.txt'
> > are based on a commit different from where the checkout happened, i.e.
> >   
> >  on branch 'master'
> >  |
> >  |  <-- git checkout -b issue
> >   \
> >\  <-- modification to git happened on a commit different from 
> > where
> >   the checkout happened
> >  
> > in this situation, git would warn us something like this:
> >  
> >  error: Your local changes to the following files would be overwritten 
> > by checkout:
> > lala.txt
> >  Please, commit your changes or stash them before you can switch 
> > branches.
> >  Aborting
> 
> That does not have much to do with "are commits the same?".  If
> 'master' and 'issue' branches are pointing at different commit, as
> long as they record the same content at the path "lala.txt", you can
> check out between these branches freely, as we can do so without
> having to resort to merging the edit the user made to the working
> tree to the different contents of "lala.txt".
> 
> There already is an indication that you have local modification in
> the working tree when we check out another branch (you would have
> seen "M lala.txt" when you did a "checkout" of another branch while
> you have local changes to the path).  

That is convincing :)
 
> Because it is a quite commonly used feature that you can checkout
> another branch and carry local changes with you, making it error out
> like we do when the branches record different contents for the
> locally changed paths when we do not have to would be a bad change
> that hurts productivity of users who do use Git correctly, which
> would mean that we need to make it an opt-in feature. 

I agree.

> But to help "some users are not aware of this" situation, an opt-in
> "feature" would not help all that much.  The same number of lines in
> the documentation to tell end-users how to toggle on such a "safety"
> feature can be spent to teach them that their local changes in the
> working tree do *not* belong to any particular branch, and as soon
> as it is understood, the user would be OK.
> 
> So...
> 

---
Yubin

Re: [RFC] Add warning when git discard changes on another branch?

Yubin Ruan  writes:

> I understand this. I just suggest that git add some warning in case some users
> are not aware of this, as it does when , on branch 'issue', changes to 
> 'lala.txt'
> are based on a commit different from where the checkout happened, i.e.
>   
>  on branch 'master'
>  |
>  |  <-- git checkout -b issue
>   \
>\  <-- modification to git happened on a commit different from 
> where
>   the checkout happened
>  
> in this situation, git would warn us something like this:
>  
>  error: Your local changes to the following files would be overwritten by 
> checkout:
>   lala.txt
>  Please, commit your changes or stash them before you can switch branches.
>  Aborting

That does not have much to do with "are commits the same?".  If
'master' and 'issue' branches are pointing at different commit, as
long as they record the same content at the path "lala.txt", you can
check out between these branches freely, as we can do so without
having to resort to merging the edit the user made to the working
tree to the different contents of "lala.txt".

There already is an indication that you have local modification in
the working tree when we check out another branch (you would have
seen "M lala.txt" when you did a "checkout" of another branch while
you have local changes to the path).  

Because it is a quite commonly used feature that you can checkout
another branch and carry local changes with you, making it error out
like we do when the branches record different contents for the
locally changed paths when we do not have to would be a bad change
that hurts productivity of users who do use Git correctly, which
would mean that we need to make it an opt-in feature. 

But to help "some users are not aware of this" situation, an opt-in
"feature" would not help all that much.  The same number of lines in
the documentation to tell end-users how to toggle on such a "safety"
feature can be spent to teach them that their local changes in the
working tree do *not* belong to any particular branch, and as soon
as it is understood, the user would be OK.

So...

Re: [RFC] Add warning when git discard changes on another branch?

On Mon, May 08, 2017 at 12:05:31PM +0900, Junio C Hamano wrote:
> Yubin Ruan  writes:
> 
> > I think it would be better if git can warn use if we switch to another 
> > branch
> > without committing the modification. Git will warn if the modification is 
> > based
> > on a commit different from where the checkout happened.
> >
> > For example, say I am now on branch 'master' and all files *clean*. Now if 
> > I do:
> > $ git checkout -b issue
> > and make some changes to a file:
> > $ echo "modification on branch issue" >> lala.txt
> > and then switch back to branch 'master':
> > $ git checkout master
> > and git can see the changes:
> > $ git status
> >   On branch master
> >   Changes not staged for commit:
> > (use "git add ..." to update what will be committed)
> > (use "git checkout -- ..." to discard changes in working 
> > directory)
> >   
> > modified:   lala.txt
> >   
> >   no changes added to commit (use "git add" and/or "git commit -a")
> >   
> > Now, if I do "git checkout -- lala.txt", then I will lose that change on 
> > branch
> > 'issue' too!!! 
> 
> There may be a fundamental misunderstanding here.  In Git, changes
> you make in the working tree do *not* belong to any branch.  The
> request "git checkout -- lala.txt" you made in this step does *not*
> say "Hey, Git, these changes to lala.txt are not necessary in the
> 'master' branch".  It says "I started editing lala.txt, but it turns
> out that I do not need that change at all, anywhere, please remove
> it."

I understand this. I just suggest that git add some warning in case some users
are not aware of this, as it does when , on branch 'issue', changes to 
'lala.txt'
are based on a commit different from where the checkout happened, i.e.
  
 on branch 'master'
 |
 |  <-- git checkout -b issue
  \
   \  <-- modification to git happened on a commit different from where
  the checkout happened
 
in this situation, git would warn us something like this:
 
 error: Your local changes to the following files would be overwritten by 
checkout:
lala.txt
 Please, commit your changes or stash them before you can switch branches.
 Aborting

> If you meant the changes while you were on "issues" branch were not
> yet ready to be committed, but now you want to work on "master"
> branch without having to worry about these changes, "git stash" may
> be a useful tool.  Alternatively, you can just create a temporary
> commit while on "issues" branch before checking out "master" branch
> to work on something else, and when you are ready to continue
> working on the "issues" branch, check out "issues" branch and either
> (1) start with "reset HEAD^" or (2) just continue working on it and
> conclude with "commit --amend".
 
Nice suggestion though.

---
Yubin

Re: [RFC] Add warning when git discard changes on another branch?

Yubin Ruan  writes:

> I think it would be better if git can warn use if we switch to another branch
> without committing the modification. Git will warn if the modification is 
> based
> on a commit different from where the checkout happened.
>
> For example, say I am now on branch 'master' and all files *clean*. Now if I 
> do:
> $ git checkout -b issue
> and make some changes to a file:
> $ echo "modification on branch issue" >> lala.txt
> and then switch back to branch 'master':
> $ git checkout master
> and git can see the changes:
> $ git status
>   On branch master
>   Changes not staged for commit:
> (use "git add ..." to update what will be committed)
> (use "git checkout -- ..." to discard changes in working 
> directory)
>   
>   modified:   lala.txt
>   
>   no changes added to commit (use "git add" and/or "git commit -a")
>   
> Now, if I do "git checkout -- lala.txt", then I will lose that change on 
> branch
> 'issue' too!!! 

There may be a fundamental misunderstanding here.  In Git, changes
you make in the working tree do *not* belong to any branch.  The
request "git checkout -- lala.txt" you made in this step does *not*
say "Hey, Git, these changes to lala.txt are not necessary in the
'master' branch".  It says "I started editing lala.txt, but it turns
out that I do not need that change at all, anywhere, please remove
it."

If you meant the changes while you were on "issues" branch were not
yet ready to be committed, but now you want to work on "master"
branch without having to worry about these changes, "git stash" may
be a useful tool.  Alternatively, you can just create a temporary
commit while on "issues" branch before checking out "master" branch
to work on something else, and when you are ready to continue
working on the "issues" branch, check out "issues" branch and either
(1) start with "reset HEAD^" or (2) just continue working on it and
conclude with "commit --amend".

Re: Git smart http: parsing commit messages in git-receive-pack

Bryan Turner  writes:

> That means if you pair a pre-receive hook with Git 2.11 or newer on
> the server, you should be able to achieve what you're looking for.

It probably is worth mentioning that even without the "quarantine"
changes of 2.11, the new objects brought in by a rejected push
cannot be used because they are not reachable from any ref, and will
be discarded as garbage when the next GC run.  So pre-receive should
be sufficient for the purpose of "not accepting" an undesirable
push, with older versions of Git.  You can view the "quarantine"
feature mostly as "quality of implementation" issue.

Re: [PATCH v2] tests: fix tests broken under GETTEXT_POISON=YesPlease

Ævar Arnfjörð Bjarmason   writes:

> Junio: I think between Travis now testing for this & the scary i18n
> reflog regression (not that poison caught that, but that was lack of
> testing, poisoining catches that class of issue) it makes sense to
> discard my patch for removing GETTEXT_POISON & queue this up instead.

Let's queue this and then the travis patches on top.  Thanks.

Re: [PATCH v7 00/10] refactor the filter process code into a reusable module

Ben Peart  writes:

> Changes from V6 include:
>
> convert: remove erroneous tests for errno == EPIPE
>  - split into separate patch to fix a preexisting bug discovered in the 
> review process

Thanks.

> pkt-line: Update packet_read_line() to test for len > 0
>  - split into separate patch to deal with errors that return negative lengths
>
> pkt-line: add packet_read_line_gently()
>  - update documentation to clarify return values
>  - update white space in function definition
>

I also see some style fixes applied to a few patches.  Thanks for
paying attention to details.

Will queue; during the pre-release freeze, new things would move
slowly, but let's see if we have more comments from others and then
merge it to 'next' soon after the 2.13 final.

Thanks.

Re: [PATCH 0/2] split index extra bits

Junio C Hamano  writes:

> Christian Couder  writes:
>
>> This patch series contains 2 patches that have already been sent to
>> the list but have felt through the cracks. It would be nice if they
>> could be considered for v2.13.0.
>
> There is no way for anything new to go to 2.13 without getting
> reviewed and discussed at this point---it simply is way too late.

I found that this is <20170419093314.4454-1-pclo...@gmail.com> in
mid April (please do not make readers dig the list archive when you
can).

I didn't see anybody (not even you, to whom the patch was apparently
addressed) commenting back then on the patch and that was why I
didn't touch it.  I've skimmed the change in 1/2 now, and although I
didn't see anything glaringly wrong, it would be good to have a test
if this fixes a reproducible crash.

Thanks.

Re: [PATCH 0/2] split index extra bits

Christian Couder  writes:

> This patch series contains 2 patches that have already been sent to
> the list but have felt through the cracks. It would be nice if they
> could be considered for v2.13.0.

There is no way for anything new to go to 2.13 without getting
reviewed and discussed at this point---it simply is way too late.

Re: [PATCH] doc: replace a couple of broken gmane links

Junio C Hamano  writes:

>> Perhaps someone else will have better luck with the other ones, which
>> are:

Here is my attempt to fill the remainder.

 Documentation/git-bisect-lk2009.txt | 2 +-
 git-rebase--interactive.sh  | 2 +-
 t/t4038-diff-combined.sh| 2 +-
 tree-walk.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-bisect-lk2009.txt 
b/Documentation/git-bisect-lk2009.txt
index b5d5e8b544..78479b003e 100644
--- a/Documentation/git-bisect-lk2009.txt
+++ b/Documentation/git-bisect-lk2009.txt
@@ -1353,6 +1353,6 @@ References
 - [[[4]]] 
https://public-inbox.org/git/7vps5xsbwp.fsf...@assigned-by-dhcp.cox.net/[Junio 
C Hamano. 'Automated bisect success story'.]
 - [[[5]]] https://lwn.net/Articles/317154/[Christian Couder. 'Fully automated 
bisecting with "git bisect run"'. LWN.net.]
 - [[[6]]] https://lwn.net/Articles/277872/[Jonathan Corbet. 'Bisection divides 
users and developers'. LWN.net.]
-- [[[7]]] http://article.gmane.org/gmane.linux.scsi/36652/[Ingo Molnar. 'Re: 
BUG 2.6.23-rc3 can't see sd partitions on Alpha'. Gmane.]
+- [[[7]]] http://marc.info/?l=linux-kernel=119702753411680=2[Ingo Molnar. 
'Re: BUG 2.6.23-rc3 can't see sd partitions on Alpha'. Linux-kernel mailing 
list.]
 - [[[8]]] 
https://www.kernel.org/pub/software/scm/git/docs/git-bisect.html[Junio C Hamano 
and the git-list. 'git-bisect(1) Manual Page'. Linux Kernel Archives.]
 - [[[9]]] https://github.com/Ealdwulf/bbchop[Ealdwulf. 'bbchop'. GitHub.]
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c9c0165b5..90b1fbe9cf 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2006 Johannes E. Schindelin
 #
 # The original idea comes from Eric W. Biederman, in
-# http://article.gmane.org/gmane.comp.version-control.git/22407
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
 #
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 0b4f7dfdc6..e2824d3437 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -354,7 +354,7 @@ test_expect_failure 'combine diff coalesce three parents' '
 '
 
 # Test for a bug reported at
-# http://thread.gmane.org/gmane.comp.version-control.git/224410
+# 
https://public-inbox.org/git/20130515143508.go25...@login.drsnuggles.stderr.nl/
 # where a delete lines were missing from combined diff output when they
 # occurred exactly before the context lines of a later change.
 test_expect_success 'combine diff missing delete bug' '
diff --git a/tree-walk.c b/tree-walk.c
index ff77605680..f25a08fddf 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1075,7 +1075,7 @@ static enum interesting do_match(const struct name_entry 
*entry,
 * later on.
 * max_depth is ignored but we may consider support it
 * in future, see
-* 
http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
+* 
https://public-inbox.org/git/7vmxo5l2g4@alter.siamese.dyndns.org/
 */
if (ps->recursive && S_ISDIR(entry->mode))
return entry_interesting;

Re: [PATCH] doc: replace a couple of broken gmane links

Ævar Arnfjörð Bjarmason   writes:

> Replace a couple of broken links to gmane with links to other
> archives. See commit 54471fdcc3 ("README: replace gmane link with
> public-inbox", 2016-12-15) for prior art.
>
> With this change there's still 4 references left in the code:
>
> $ git grep -E '(article|thread)\.gmane.org' -- |grep -v RelNotes|wc -l
> 4
>
> I couldn't find alternative links for those.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>
> Perhaps someone else will have better luck with the other ones, which
> are:
>
> Documentation/git-bisect-lk2009.txt-1355-- [[[6]]] 
> https://lwn.net/Articles/277872/[Jonathan Corbet. 'Bisection divides users 
> and developers'. LWN.net.]

This link seems to be alive.

> Documentation/git-bisect-lk2009.txt:1356:- [[[7]]] 
> http://article.gmane.org/gmane.linux.scsi/36652/[Ingo Molnar. 'Re: BUG 
> 2.6.23-rc3 can't see sd partitions on Alpha'. Gmane.]

Message-id for this message is <20071207113734.ga14...@elte.hu>
Xref: news.gmane.org gmane.linux.scsi:36652 gmane.linux.kernel:611204

> Documentation/git-bisect-lk2009.txt-1357-- [[[8]]] 
> https://www.kernel.org/pub/software/scm/git/docs/git-bisect.html[Junio C 
> Hamano and the git-list. 'git-bisect(1) Manual Page'. Linux Kernel Archives.]

There would be a corresponding git-scm.org manual page, no?

> git-rebase--interactive.sh-7-# The original idea comes from Eric W. 
> Biederman, in
> git-rebase--interactive.sh:8:# 
> http://article.gmane.org/gmane.comp.version-control.git/22407

https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/

FYI, https://public-inbox.org/git/ has a "search" interface that
lets you type "gmane:22047" to the box and click.


> git-rebase--interactive.sh-9-#
> --
> t/t4038-diff-combined.sh-356-# Test for a bug reported at
> t/t4038-diff-combined.sh:357:# 
> http://thread.gmane.org/gmane.comp.version-control.git/224410

https://public-inbox.org/git/20130515143508.go25...@login.drsnuggles.stderr.nl/

> t/t4038-diff-combined.sh-358-# where a delete lines were missing from 
> combined diff output when they
> --
> tree-walk.c-1077-* in future, see
> tree-walk.c:1078:* 
> http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840

https://public-inbox.org/git/7vmxo5l2g4@alter.siamese.dyndns.org/

Re: [RFC PATCH 02/10] Move textconv_object to be with other textconv methods

Stefan Beller  writes:

> I guess it is ok for now and in this series, but we may want
> to split up diff.[ch] in the future into multiple finer grained files.

For what end?  Such a split would require more symbols internal to
diff.[ch] to become external, which is a big downside, so we need to
have a large reward to compensate it if we were to go there.

Re: [PATCH 0/7] Update the compat/regex engine from upstream

Ævar Arnfjörð Bjarmason   writes:

> See the first patch for motivation & why.

I do not think it quite explains the motivation, though.  "Doing
this way, we can keep in sync with the upstream more easily"?  Or is
there anything more to it, e.g. "and we need to update from the
upstream now to fix this and that issues"?

Having these "fixup!" as separate patches on the list makes them
smaller and easier to understand.  What do we want to do with them
once they are applied?  Squash them all up, because these do not
have their own explanations in their log message and it is not worth
keeping them separate?

> The only reason this has a cover letter is to explain the !fixup
> commits. IIRC the mailing list has a 100K limit, which this series
> would violate, so I split up the second commit.
>
> Consider all these !fixup commits to have by Signed-off-by, easier to
> say that here than to modify them all.
>
> Ævar Arnfjörð Bjarmason (7):
>   compat/regex: add a README with a maintenance guide
>   compat/regex: update the gawk regex engine from upstream
>   fixup! compat/regex: update the gawk regex engine from upstream
>   fixup! compat/regex: update the gawk regex engine from upstream
>   fixup! compat/regex: update the gawk regex engine from upstream
>   fixup! compat/regex: update the gawk regex engine from upstream
>   fixup! compat/regex: update the gawk regex engine from upstream
>
>  Makefile   |   8 +-
>  compat/regex/README|  21 +
>  compat/regex/intprops.h| 448 
> +
>  .../0001-Add-notice-at-top-of-copied-files.patch   | 120 ++
>  .../0002-Remove-verify.h-use-from-intprops.h.patch |  41 ++
>  compat/regex/regcomp.c | 356 +---
>  compat/regex/regex.c   |  32 +-
>  compat/regex/regex.h   | 120 +++---
>  compat/regex/regex_internal.c  | 118 +++---
>  compat/regex/regex_internal.h  | 118 +++---
>  compat/regex/regexec.c | 242 +--
>  compat/regex/verify.h  | 286 +
>  12 files changed, 1487 insertions(+), 423 deletions(-)
>  create mode 100644 compat/regex/README
>  create mode 100644 compat/regex/intprops.h
>  create mode 100644 
> compat/regex/patches/0001-Add-notice-at-top-of-copied-files.patch
>  create mode 100644 
> compat/regex/patches/0002-Remove-verify.h-use-from-intprops.h.patch
>  create mode 100644 compat/regex/verify.h

Re: [RESEND PATCH] diff: recurse into nested submodules for inline diff

Jacob Keller  writes:

> On Thu, May 4, 2017 at 2:43 PM, Stefan Beller  wrote:
>> When fd47ae6a5b (diff: teach diff to display submodule difference with an
>> inline diff, 2016-08-31) was introduced, we did not think of recursing
>> into nested submodules.
>>
>> When showing the inline diff for submodules, automatically recurse
>> into nested submodules as well with inline submodule diffs.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>>
>> This is a resend of
>> https://public-inbox.org/git/20170331231733.11123-3-sbel...@google.com/
>>
>> In the cover letter of that patch, I said I would want to redo the tests with
>> scrubbed output. However given the widespread use of unscrubbed output,
>> I think this is fine as-is to include.
>>
>> Thanks,
>> Stefan
>
> Fix looks obviously correct to me. I would like to change the tests to
> not use the unscrubbed output, but that's a problem for a separate
> patch.

OK.

Thanks, both.

Re: [PATCH v4 11/25] checkout: fix memory leak

René Scharfe  writes:

>>  /*
>>   * NEEDSWORK:
>>   * There is absolutely no reason to write this as a blob object
>> - * and create a phony cache entry just to leak.  This hack is
>> - * primarily to get to the write_entry() machinery that massages
>> - * the contents to work-tree format and writes out which only
>> - * allows it for a cache entry.  The code in write_entry() needs
>> - * to be refactored to allow us to feed a 
>> - * instead of a cache entry.  Such a refactoring would help
>> - * merge_recursive as well (it also writes the merge result to the
>> - * object database even when it may contain conflicts).
>> + * and create a phony cache entry.  This hack is primarily to get
>> + * to the write_entry() machinery that massages the contents to
>> + * work-tree format and writes out which only allows it for a
>> + * cache entry.  The code in write_entry() needs to be refactored
>> + * to allow us to feed a  instead of a cache
>> + * entry.  Such a refactoring would help merge_recursive as well
>> + * (it also writes the merge result to the object database even
>> + * when it may contain conflicts).
>>   */
>>  if (write_sha1_file(result_buf.ptr, result_buf.size,
>>  blob_type, oid.hash))
>
> Random observation: Using pretend_sha1_file here would at least avoid
> writing the blob.

Yup, you should have told that to me back in Aug 2008 ;-) when I did
0cf8581e ("checkout -m: recreate merge when checking out of unmerged
index", 2008-08-30); pretend_sha1_file() was available since early
2007, and there is no excuse that this codepath did not use it.
>
>> @@ -251,6 +251,7 @@ static int checkout_merged(int pos, const struct 
>> checkout *state)
>>  if (!ce)
>>  die(_("make_cache_entry failed for path '%s'"), path);
>>  status = checkout_entry(ce, state, NULL);
>> +free(ce);
>>  return status;
>>   }
>
> I wonder if that's safe.  Why document a leak when it could have been
> plugged this easily instead?
>
> A leak is better than a use after free, so
> let's be extra careful here.  Would it leave the index inconsistent?  Or
> perhaps freeing it has become safe in the meantime?
>
> @Junio: Do you remember the reason for the leaks in 0cf8581e330
> (checkout -m: recreate merge when checking out of unmerged index).

Yes.

In the very old days it was not allowed to free(3) contents of
active_cache[] and this was an old brain fart that came out of
inertia.  We are manufacturing a brand new ce, only to feed it to
checkout_entry() without even registering it to the active_cache[]
array, and the ancient rule doesn't even apply to such a case.

So I think it was safe to free(3) even back then.

> And result_buf is still leaked here, right?

Good spotting.  It typically would make a larger leak than a single
ce, I would suppose ;-)

Re: [PATCH v2 00/21]

On Thu, May 4, 2017 at 2:45 PM, Junio C Hamano  wrote:
>
> Nguyễn Thái Ngọc Duy   writes:
>
> > Changes since v1:
> >
> >  - fopen_or_warn() and warn_on_fopen_errors() are introduced. The
> >latter's name is probably not great...
> >  - A new patch (first one) to convert a bunch to using xfopen()
> >  - Fix test failure on Windows, found by Johannes Sixt
> >  - Fix the memory leak in log.c, found by Jeff
> >
> > There are still a couple of fopen() remained, but I'm getting slow
> > again so let's get these in first and worry about the rest when
> > somebody has time.

Hmm, is this topic responsible for recent breakage Travis claims on MacOS X?

https://travis-ci.org/git/git/jobs/229585098#L3091

seems to expect an error when test-config is fed a-directory but we are
not getting the expected warning and error?

Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

Liam Beguin  writes:

> Sorry for the delay, I don't mind switching to C but it would probably
> be easier to see if the scripted version gets approved first.
> If it does, I could then get started on the C implementation.
> If you prefer I could also forget about the scripted version, make a C
> implementation work and see if that gets approved.

I am not sure what "approved" would mean in the context of this
project, though ;-) Your patch to the scripted version would
certainly not be in the upcoming release.  If you define the
"approval" as "it is queued to my tree somewhere", the patch would
start its life like everybody else by getting merged to the 'pu'
branch, where there already is a topic that removes the code you
patch your enhancement into.

The list _can_ agree that it is a good idea to have an option to
populate the todo list with shortened insn words from the beginning
(instead of merely accepting a short-hand while executing), which is
what your patch wants to do, without actually having the updated
scripted "rebase -i" merged in any of the integration branches in my
tree.  If you meant by "approval" to have such a list concensus, I
think you may already have one.  I personally do not think it is a
great idea but I do not think it is a horrible one, either.  As long
as it is an opt-in feature that many people find useful (which may
be the case already, judging from the list traffic), I do not mind
;-)

Re: [PATCH] archive-tar: fix a sparse 'constant too large' warning

Ramsay Jones  writes:

> Sure, I can add this. (Although, I don't think it actually
> matters).
>
> Hmm, unless somebody objects in the meantime, I will re-roll
> the patch (tomorrow, it's late!).

Thanks.

Re: [PATCH 00/10] RFC Partial Clone and Fetch

Jonathan Nieder  writes:

> - there shouldn't be any need for the blobs to even be mentioned in
>   the .pack stored locally.  The .idx file maps from sha1 to offset
>   within the packfile --- a special offset could mean "this is a
>   missing blob".

Clever.

> - However, the list of missing blobs can be inferred from the existing
>   pack format, without a change to the pack format used in git
>   protocol.  As part of constructing the idx, "git index-pack"
>   inflates every object in the pack file sent by the server.  This
>   means we know what blobs they reference, so we can easily produce a
>   list for the idx file without changing the pack file format.

A minor wrinkle to keep in mind if you were to go this route is that
you'd need a way to tell the reason why a blob that is referenced by
a tree in the pack stream is not in the same pack stream.  

If the resulting repository on the receiving side has that blob
after the transfer, it is likely that the reason why the blob does
not appear in the pack is because the want/have/ack exchange told
the sending side that the receiving side has a commit that contains
the blob.  But when the blob does not exist in the receiving side
after the transfer, we cannot tell between two possible cases.  The
server may have actively wanted to omit it (i.e. the case we are
interested in in this discussion thread).  Or the receiving end said
that it has a commit that contains the blob, but due to preexisting
corruption, the receiving repository was missing the blob in
reality.

Re: vger not relaying some of Junio's messages today?

Eric Wong  writes:

> Samuel Lijin  wrote:
>> Sorry, should've been clearer - I did check my spambox in my original
>> message. Some old patches from Brandon were in there, but the ones I
>> mentioned in my original message just seem to have been dropped.
>
> Apparently, vger also throttles mail to gmail aggressively, so
> maybe some show up eventually:
>
> https://marc.info/?i=20170501.105057.824365162373797902.da...@davemloft.net
>

Thanks for a pointer. That does explain the symptom I saw number of
times, seeing list traffic that directly came to me hours before I
see a copy on the public list mirrors like gmane and public-inbox.

As it is a pain to access gmail inbox via imap (I was told that
something called "offline imap" may alleviate the pain, but I
haven't tried it yet), I tend to use NNTP interface to either gmane
or public-inbox almost exclusively, so like everybody else, it often
takes a message hours before I see it due to this throttling.  I do
not see the message I am responding to on public-inbox, and I am
responding to the copy I got direct from you, for example.

[Announce] delaying 2.13 final by a few days

As -rc2 was delayed for a few days, I'd like to delay the release of
the final 2.13 (which was originally scheduled on 8th) by a few days.
For one thing that would give translaors a bit more time to update
the message strings.

Thanks.

Re: vger not relaying some of Junio's messages today?

Samuel Lijin  wrote:
> > Samuel Lijin  wrote:
> >> Yep, I see these on public-inbox.org/git/ but not in my gmail inbox:
> >
> > Hi Samuel, check your Spam box (and move it to a normal inbox so
> > they can train it).  Gmail filters are known to trigger happy
> > and incorrectly flag messages.  It's been a problem on LKML,
> > too.
> 
> Sorry, should've been clearer - I did check my spambox in my original
> message. Some old patches from Brandon were in there, but the ones I
> mentioned in my original message just seem to have been dropped.

Yikes.  I wonder if Gmail automatically nukes messages if
they end up in enough Spam boxes...  Really hoping Googlers
can do something about this (or better, more people start
running their own SMTP servers, again).

Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-07 Thread Liam Beguin

Hi Junio,

On Wed, 2017-05-03 at 22:04 -0700, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
> > > If 'git-rebase--interactive.sh' is bound to be replaced, I could
> > > just shrink this to the Documentation cleanup (patches 4 and 5)
> > > and rework the rest on top of your new implementation.
> > 
> > I kind of hoped that Junio would chime in with his verdict. That would be
> > the ultimate deciding factor, I think.
> 
> What I can predict is that within two or three release cycles
> (unless you completely lose interest) the todo-list generation will
> be all in C and that I anticipate that the C version may even be
> capable of generating different kind of todo command (e.g. to
> support tools like your Garden Shears more natively), so the
> mid-term direction definitely is that any enhancement would in the
> end needs to happen on top of or in coordination with the C rewrite
> we've been discussing recently.
> 
> I didn't know what the comfort levels of Liam working with scripted
> vs C code, and the "vertict" depends on that, I would think.  We may
> want to discuss the enhancement in the original scripted form Liam
> did with new tests while the C rewrite is still cooking, and either
> Liam, you or somebody else can make it work with your C rewrite when
> both are ready.  If Liam feels comfortable working with you and the
> code after the C rewrite that is still in-flight, it is also fine if
> the next iteration from Liam were on top of your series.
> 
> 

Sorry for the delay, I don't mind switching to C but it would probably
be easier to see if the scripted version gets approved first.
If it does, I could then get started on the C implementation.
If you prefer I could also forget about the scripted version, make a C
implementation work and see if that gets approved.

Thanks,
Liam

[RFC] Add warning when git discard changes on another branch?

Hi,
I think it would be better if git can warn use if we switch to another branch
without committing the modification. Git will warn if the modification is based
on a commit different from where the checkout happened.

For example, say I am now on branch 'master' and all files *clean*. Now if I do:
$ git checkout -b issue
and make some changes to a file:
$ echo "modification on branch issue" >> lala.txt
and then switch back to branch 'master':
$ git checkout master
and git can see the changes:
$ git status
  On branch master
  Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git checkout -- ..." to discard changes in working 
directory)
  
modified:   lala.txt
  
  no changes added to commit (use "git add" and/or "git commit -a")
  
Now, if I do "git checkout -- lala.txt", then I will lose that change on branch
'issue' too!!! 

If, on branch 'issue', the changes to 'lala.txt' are based on a commit different
from where the checkout happened, i.e.
 
on branch 'master'
|
|  <-- git checkout -b issue
 \
  \  <-- modification to git happened on a commit different from where
 the checkout happened

then git would warn us something like this:

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

So, I think it is better to add some similar warinings if the modification
happened on a commit the *same* as where the checkout happened, i.e.

on branch 'master'
|
|  <-- git checkout -b issue
   <-- modification to git happened on a commit different from where
   the checkout happened

I already lost some of my work by this ...

(p.s please add me to the Cc because I am not in this list)
---
Yubin

Re: [PATCH 0/5] Abide by our own rules regarding line endings

Johannes Schindelin  writes:

> Let me repeat myself:

Don't. Instead, read through what you are responding to the end
before start typing a byte.  In case you didn't do that, in the end
I agree with the direction of the series ;-).

[GSoC] Project Selected: Incremental rewrite of git-submodules

2017-05-07 Thread Prathamesh Chavan

Hey everyone,

I am Prathamesh Chavan, an undergraduate student from the department
of Computer Science and Engineering, at Indian Institue of Technology,
Kharagpur. I applied for Google Summer of Code 2017 and my project
"Incremental rewrite of git-submodules" has been selected.
This project will be done under the guidance of the mentors:
Stefan Beller and Christian Couder.

Brief introduction of the Project:

Git submodule subcommands are currently implemented by using shell script
'git-submodule.sh'. There are several reasons why we'll prefer not to
use the shell script. My project intends to convert the subcommands into
C code, thus making them builtins. This will increase Git's portability
and hence the efficiency of working with the git-submodule commands.
Link to the complete proposal: [1]

Before I discuss my future plan, first I would like to thank everyone
who was involved in helping me apply to GSoC and reviewing my patches.
I would especially like to thank Stefan Beller, Christian Couder and
Junio C Hamano for reviewing my patches and helping me get my first
patch(microproject) merged.

Current status of my patches:

1. t2027: avoid using pipes:
This patch has been merged to the master branch and also has
been included in the Git v2.13.0

2. Disallow git commands from within unpopulated submodules:
This patch was reviewed by Stefan Bellar and since to disallow
the git commands in unpopulated submodules required git commands
to identify whether the user has entered the command in
an unpopulated or uninitialized submodule individually at
low level than catching them for a complete group of
builtin commands. I found the review useful and
understood what needs to be done. Also, I wish to work on
it, but currently I would like to work on my project first.
I have included this issue in my wishlist and will
work on this after completing my project.

3. submodule: port subcommand foreach from shell to C
I have started implementing the suggestion in the previous
reviews and currently I'm adding more test-cases which my
patch still fails. Also, still this patch is failing tests
6, 7, 8 and 9 from t7407-submodule-foreach and test 64 from
t3600-rm.
Link to the patch: [2]

Plan for this week:

According to my proposal, time till 15th May is allocated as
community bonding period and hence I have
resumed taking a brief overview of the files:
1. strbuf.h
2. Various API files from Documentation/technical
since some functions from these may be used in every future porting.

I'm also aiming to read carefully:
1. submodule.h and submodule.c
2. git-submodule.sh
3. builtin/submodule--helper.c
since in every submodule subcommand's porting I'll be using the
functions from these files and it will also help me avoid code
duplication in future.

Along with this, I'm also working on porting of the subcommand
foreach and wish to complete it before the end of the community
bonding period.

I'll officially start coding from 16th May, as per my proposed
schedule and will be pushing my work on GitHub [3] regularly so
that the mentors can monitor the progress of the project. Also, I'll
be posting a weekly update on changes made throughout the week
and my plan for the next week.

Thanks,
Prathamesh Chavan

[1]:
https://public-inbox.org/git/CAME+mvXBuLbbRJu1DAA8o-u6DeZATKypH=W=hpeks3kl5wm...@mail.gmail.com/
[2]: https://public-inbox.org/git/20170422195804.18477-1-pc44...@gmail.com/T/#u
[3]: https://github.com/pratham-pc/git/

Re: vger not relaying some of Junio's messages today?

Samuel Lijin  wrote:
> Sorry, should've been clearer - I did check my spambox in my original
> message. Some old patches from Brandon were in there, but the ones I
> mentioned in my original message just seem to have been dropped.

Apparently, vger also throttles mail to gmail aggressively, so
maybe some show up eventually:

https://marc.info/?i=20170501.105057.824365162373797902.da...@davemloft.net

On a side note, I just started trying the "mutt-patched"
package in Debian stable.  It looks like "mutt" package will
absorb it's functionality in future releases, but maybe more
mutt users can have NNTP support in their favorite MUA :)

[PATCH v3] send-email: --batch-size to work around some SMTP server limit

2017-05-07 Thread xiaoqiang zhao

Some email servers (e.g. smtp.163.com) limit the number emails to be
sent per session(connection) and this will lead to a faliure when
sending many messages.

Teach send-email to disconnect after sending a number of messages
(configurable via the --batch-size= option), wait for a few
seconds (configurable via the --relogin-delay= option) and
reconnect, to work around such a limit.

Also add this two configure option for git config command.

Signed-off-by: xiaoqiang zhao 
---
 contrib/completion/git-completion.bash |  2 ++
 git-send-email.perl| 18 ++
 2 files changed, 20 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index af658995d..29496353a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2608,6 +2608,8 @@ _git_config ()
sendemail.thread
sendemail.to
sendemail.validate
+   sendemail.smtpbatchsize
+   sendemail.smtprelogindelay
showbranch.default
status.relativePaths
status.showUntrackedFiles
diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..5cbe97898 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -81,6 +81,10 @@ git send-email --dump-aliases
  This setting forces to use one of the 
listed mechanisms.
 --smtp-debug<0|1>  * Disable, enable Net::SMTP debug.
 
+--batch-size  * send max  message per connection.
+--relogin-delay   * delay  seconds between two 
successive login, default to 1,
+ This option can only be used with 
--batch-size
+
   Automating:
 --identity* Use the sendemail. options.
 --to-cmd  * Email To: via ` \$patch_path`
@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
 my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
+my $num_sent = 0;
 
 # Regexes for RFC 2047 productions.
 my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
@@ -216,6 +221,7 @@ my ($cover_cc, $cover_to);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
+my ($batch_size, $relogin_delay) = (0, 0);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -247,6 +253,8 @@ my %config_settings = (
 "smtppass" => \$smtp_authpass,
 "smtpdomain" => \$smtp_domain,
 "smtpauth" => \$smtp_auth,
+"smtpbatchsize" => \$batch_size,
+"smtprelogindelay" => \$relogin_delay,
 "to" => \@initial_to,
 "tocmd" => \$to_cmd,
 "cc" => \@initial_cc,
@@ -358,6 +366,8 @@ $rc = GetOptions(
"force" => \$force,
"xmailer!" => \$use_xmailer,
"no-xmailer" => sub {$use_xmailer = 0},
+   "batch-size=i" => \$batch_size,
+   "relogin-delay=i" => \$relogin_delay,
 );
 
 usage() if $help;
@@ -1664,6 +1674,14 @@ foreach my $t (@files) {
}
}
$message_id = undef;
+   $num_sent++;
+   if ($num_sent == $batch_size) {
+   $num_sent = 0;
+   $smtp->quit;
+   $smtp = undef;
+   $auth = 0;
+   sleep($relogin_delay);
+   }
 }
 
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
-- 
2.13.0.rc2.1.gd46ef338b.dirty

Re: vger not relaying some of Junio's messages today?

Eric Wong  wrote:
> (OTOH, I noticed a thread/mbox download bug in public-inbox,
> https://public-inbox.org/git/xmqqmvaq702u@gitster.mtv.corp.google.com/t.mbox.gz
> only shows two messages out of many.   Will need to fix that...)

I think I fixed that bug, at least:
https://public-inbox.org/meta/20170507110328.GA20102@whir/

And vger is still down...

Re: vger not relaying some of Junio's messages today?