Re: js/mingw-http-ssl, was Re: What's cooking in git.git (Oct 2018, #05; Fri, 26)

[Kaartic Sivaraam]

Hi Junio,


On 30 October 2018 11:07:39 GMT+05:30, Junio C Hamano  wrote:
>Eric Sunshine  writes:
>>> On platforms with recent cURL library, http.sslBackend
>configuration
>>> variable can be used to choose different SSL backend at runtime.
>>
>> s/choose/& a/
>>
>>> The Windows port uses this mechanism to switch between OpenSSL
>and
>>> Secure Channel while talking over the HTTPS protocol.
>
>Thanks.
>
>By the way, I am beginning to like this phrasing quite a lot.  It
>encourages those with other ports like Linux and Mac to see if they
>want a similar runtime switching by singling out Windows port, which
>was what I wanted to achive with the original one, but I think the
>updated text does so more effectively.

The new phrasing seems to be reading quite better indeed.

-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: git fetch behaves weirdely when run in a worktree

[Kaartic Sivaraam]

On 3 October 2018 00:13:06 GMT+05:30, Kaartic Sivaraam 
 wrote:
>Hi,
>
>Sorry for the delay. Got a little busy over the weekend. I seem to have
>found the reason behind the issue in the mean time :-)
>

Oops, I forgot to mention there's more comments inline!

BTW, is there an issue if .git/HEAD and .git/index are owned by root? The 
owners seem to have changed since I created the worktree possibly due to the 
cron job. Just wondering if it might cause some issues.


>On Wed, 2018-09-26 at 10:05 -0700, Junio C Hamano wrote:
>> Duy Nguyen  writes:
>> 
>> > On Wed, Sep 26, 2018 at 05:24:14PM +0200, Duy Nguyen wrote:
>> > > On Wed, Sep 26, 2018 at 6:46 AM Kaartic Sivaraam
>> > >  wrote:
>> > > > This is the most interesting part of the issue. I **did not**
>run
>> > > > 'git fetch ...' in between those cat commands. I was surprised
>by
>> > > > how the contents of FETCH_HEAD are changing without me spawning
>any
>> > > > git processes that might change it. Am I missing something
>here? As
>> > > > far as i could remember, there wasn't any IDE running that
>might
>> > > > automatically spawn git commands especially in that work
>> > > > tree. Weird.
>> > 
>> > Maybe something like this could help track down that rogue "git
>fetch"
>> > process (it's definitely _some_ process writing to the wrong file;
>or
>> > some file synchronization thingy is going on). You can log more of
>> > course, but this is where FETCH_HEAD is updated.
>> 
>
>Thanks for the patch! It really helped me identify the issue.
>
>The actual culprit here doesn't seem to be Git itself. It was the "git-
>prompt" bash plug-in[1] I was using. It seems to be spawning "git
>fetch" for every command I type on shell. That answers why the
>FETCH_HEAD was being updated even though I wasn't explicitly running
>it. The git bash plug-in seems to be fetching changes for *all* the
>upstream branches. That answers why there FETCH_HEAD was populated with
>info about all the branches when I explicitly requested for the next
>branch. I've also verified that `git fetch origin next` works as
>intended (FETCH_HEAD has info only about orgin/next) when I turn-off
>the plug-in which confirms that it's the culprit. A cursory search
>found me a related issue[2] but I'm not sure if it's the exact same
>one.
>
>I could identify the culprit only with the help of Duy's patch. Thanks
>Duy! Sorry for not realising this earlier. I almost forgot I'm using it
>as I've been accustomed to it a lot.
>
>
>> Well, a background-ish thing could be some vendor-provided copy of
>> Git that has nothing to do with what Kaartic would be compiling with
>> this patch X-<.
>
>Fortunately, it wasn't the case here as the plug-in was using my
>manually-built version of Git :-)
>
>Thanks for the help!
>
>Tag-line: Sometimes tools become part of our workflow so much that we
>really don't realise their presence. It's an indication of a good tool
>but we should be aware of suspecting them when an issue arises!
>Something which I should have done to realise the issue ealier x-<
>
>
>References:
>[1]: https://github.com/magicmonty/bash-git-prompt
>[2]: https://github.com/magicmonty/bash-git-prompt/issues/125
>
>Thanks,
>Sivaraam

-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: git fetch behaves weirdely when run in a worktree

[Kaartic Sivaraam]

Hi,

Sorry for the delay. Got a little busy over the weekend. I seem to have
found the reason behind the issue in the mean time :-)


On Wed, 2018-09-26 at 10:05 -0700, Junio C Hamano wrote:
> Duy Nguyen  writes:
> 
> > On Wed, Sep 26, 2018 at 05:24:14PM +0200, Duy Nguyen wrote:
> > > On Wed, Sep 26, 2018 at 6:46 AM Kaartic Sivaraam
> > >  wrote:
> > > > This is the most interesting part of the issue. I **did not** run
> > > > 'git fetch ...' in between those cat commands. I was surprised by
> > > > how the contents of FETCH_HEAD are changing without me spawning any
> > > > git processes that might change it. Am I missing something here? As
> > > > far as i could remember, there wasn't any IDE running that might
> > > > automatically spawn git commands especially in that work
> > > > tree. Weird.
> > 
> > Maybe something like this could help track down that rogue "git fetch"
> > process (it's definitely _some_ process writing to the wrong file; or
> > some file synchronization thingy is going on). You can log more of
> > course, but this is where FETCH_HEAD is updated.
> 

Thanks for the patch! It really helped me identify the issue.

The actual culprit here doesn't seem to be Git itself. It was the "git-
prompt" bash plug-in[1] I was using. It seems to be spawning "git
fetch" for every command I type on shell. That answers why the
FETCH_HEAD was being updated even though I wasn't explicitly running
it. The git bash plug-in seems to be fetching changes for *all* the
upstream branches. That answers why there FETCH_HEAD was populated with
info about all the branches when I explicitly requested for the next
branch. I've also verified that `git fetch origin next` works as
intended (FETCH_HEAD has info only about orgin/next) when I turn-off
the plug-in which confirms that it's the culprit. A cursory search
found me a related issue[2] but I'm not sure if it's the exact same
one.

I could identify the culprit only with the help of Duy's patch. Thanks
Duy! Sorry for not realising this earlier. I almost forgot I'm using it
as I've been accustomed to it a lot.


> Well, a background-ish thing could be some vendor-provided copy of
> Git that has nothing to do with what Kaartic would be compiling with
> this patch X-<.

Fortunately, it wasn't the case here as the plug-in was using my
manually-built version of Git :-)

Thanks for the help!

Tag-line: Sometimes tools become part of our workflow so much that we
really don't realise their presence. It's an indication of a good tool
but we should be aware of suspecting them when an issue arises!
Something which I should have done to realise the issue ealier x-<


References:
[1]: https://github.com/magicmonty/bash-git-prompt
[2]: https://github.com/magicmonty/bash-git-prompt/issues/125

Thanks,
Sivaraam

Re: git fetch behaves weirdely when run in a worktree

[Kaartic Sivaraam]

Hi,

I just wanted make a point a little more clear. See comment inline.


On 24 September 2018 01:39:26 GMT+05:30, Kaartic Sivaraam 
 wrote:
> To add to that
>confusion when I run
>
>   $ cat $MAIN_WORKTREE/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD
>
>it seems to be printing the info about the remote refs once and then if
>I run it again immediately nothing is printed. If I repeat it again,
>info about remote refs is printed but this time the info about the
>remote refs is printed thrice. This happens randomly. Sample output:
>
>   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
>53f9a3e157dbbc901a02ac2c73346d375e24978c   not-for-merge   branch 'maint'
>of https://github.com/git/git
>150f307afc13961b0322ad0e7205a7b193368586   not-for-merge   branch 'master'
>of https://github.com/git/git
>01d371f741e6f0b0076d9ed942d99bdb23757eac   not-for-merge   branch 'next' of
>https://github.com/git/git
>7a81cbc028be113058e0b55062706ec6de62ed94   not-for-merge   branch 'pu' of
>https://github.com/git/git
>722746685bce03f223ed75febe312495e6c139da   not-for-merge   branch 'todo' of
>https://github.com/git/git
>   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
>   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
>   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
>53f9a3e157dbbc901a02ac2c73346d375e24978c   not-for-merge   branch 'maint'
>of https://github.com/git/git
>150f307afc13961b0322ad0e7205a7b193368586   not-for-merge   branch 'master'
>of https://github.com/git/git
>01d371f741e6f0b0076d9ed942d99bdb23757eac   not-for-merge   branch 'next' of
>https://github.com/git/git
>7a81cbc028be113058e0b55062706ec6de62ed94   not-for-merge   branch 'pu' of
>https://github.com/git/git
>722746685bce03f223ed75febe312495e6c139da   not-for-merge   branch 'todo' of
>https://github.com/git/git
>53f9a3e157dbbc901a02ac2c73346d375e24978c   not-for-merge   branch 'maint'
>of https://github.com/git/git
>150f307afc13961b0322ad0e7205a7b193368586   not-for-merge   branch 'master'
>of https://github.com/git/git
>01d371f741e6f0b0076d9ed942d99bdb23757eac   not-for-merge   branch 'next' of
>https://github.com/git/git
>7a81cbc028be113058e0b55062706ec6de62ed94   not-for-merge   branch 'pu' of
>https://github.com/git/git
>722746685bce03f223ed75febe312495e6c139da   not-for-merge   branch 'todo' of
>https://github.com/git/git
>53f9a3e157dbbc901a02ac2c73346d375e24978c   not-for-merge   branch 'maint'
>of https://github.com/git/git
>150f307afc13961b0322ad0e7205a7b193368586   not-for-merge   branch 'master'
>of https://github.com/git/git
>01d371f741e6f0b0076d9ed942d99bdb23757eac   not-for-merge   branch 'next' of
>https://github.com/git/git
>7a81cbc028be113058e0b55062706ec6de62ed94   not-for-merge   branch 'pu' of
>https://github.com/git/git
>722746685bce03f223ed75febe312495e6c139da   not-for-merge   branch 'todo' of
>https://github.com/git/git
>   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
>53f9a3e157dbbc901a02ac2c73346d375e24978c   not-for-merge   branch 'maint'
>of https://github.com/git/git
>150f307afc13961b0322ad0e7205a7b193368586   not-for-merge   branch 'master'
>of https://github.com/git/git
>01d371f741e6f0b0076d9ed942d99bdb23757eac   not-for-merge   branch 'next' of
>https://github.com/git/git
>7a81cbc028be113058e0b55062706ec6de62ed94   not-for-merge   branch 'pu' of
>https://github.com/git/git
>722746685bce03f223ed75febe312495e6c139da   not-for-merge   branch 'todo' of
>https://github.com/git/git
>

This is the most interesting part of the issue. I **did not** run 'git fetch 
...' in between those cat commands. I was surprised by how the contents of 
FETCH_HEAD are changing without me spawning any git processes that might change 
it. Am I missing something here? As far as i could remember, there wasn't any 
IDE running that might automatically spawn git commands especially in that work 
tree. Weird.

-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: git fetch behaves weirdely when run in a worktree

[Kaartic Sivaraam]

On 26 September 2018 03:10:17 GMT+05:30, Junio C Hamano  
wrote:
> 
> That looks like fetching only the 'next' branch and nothing else to
> me.
> 

Interesting.


> Perhaps your script is referring to a variable whose assignment is
> misspelled and invoking
> 
>   git fetch $origin $branch
> 
> and you are expecting the $branch variable to have string 'next' but
> due to some bugs it is empty somehow?  That explains why sometimes
> (i.e. when $branch does not get the value you expect it to have) the
> script fetches everything and some other times (i.e. when $branch
> does get the right value) the script fetches only the named branch.

Noce guees but I should have made this clear. The weirdness I described
in my initial mail happens when I run "git fetch origin next" manually
in the terminal.

The script only helped me identity that there was an issue as it was
automatically building the wrong version of Git. It was building and
installing the current 'origin/maint' when I wrote it for building
origin/next. I could easily identify the difference as 'origin/maint'
was at v2.18 while 'origin/next' was at v2.19 when I notived this
issue. Also the script doesn't depend on any variables for the branch
name. I've hardcoded the 'next' branch into it. For the sake of
reference, I've attached the script inline at the end of this mail.
Note that I've sanitized the path in which the worktree exists as
$COMMON_ROOT.

I did not notice this weirdness in another PC. But, this happens in all
the worktrees other than the main worktree in my laptop. I'm not sure
what I'm doing weirdly that might have caused this issue.

I'm not sure whether I mentioned this before I'm currently using a
manually built version of Git. It was built from origin/next pointing
at 01d371f741. But this also happens in Git v2.18.0 that comes via the
pacakge manager of my operating system (Debian testing).

--
Sivaraam

#!/bin/sh
#
# A script for the cron job that automatically fetches
# updates for the 'next' branch of Git and builds and
# installs the binary from source.
#
# The build succeeds only if the config.mak is present
# in the directory with appropriate configuration.
#
# The binary is installed into the default location if
# a prefix is not specified in the config.mak file.
# Bringing the binary into use is in the hands of the user.
# Hint: A bash alias might help.

GIT_NEXT_BUILD_AUTOMATE_DIR='$COMMON_ROOT/git-next-build-automate'
LOG_FILE="$GIT_NEXT_BUILD_AUTOMATE_DIR/GIT-NEXT-INSTALLATION-LOG.txt"
LOG_MSG_COMMON="installation of git 'next' build on $(date)"

if cd "$GIT_NEXT_BUILD_AUTOMATE_DIR"
then
# Fetch the remote changes
if git fetch origin next && git checkout -f FETCH_HEAD
then
if make install
then
GIT_NEXT_BUILD_STATUS=0
else
GIT_NEXT_BUILD_STATUS=1
fi
else
GIT_NEXT_BUILD_STATUS=1
fi
else
GIT_NEXT_BUILD_STATUS=1
fi

if test $GIT_NEXT_BUILD_STATUS -eq 0
then
echo "SUCCESS: $LOG_MSG_COMMON" >>"$LOG_FILE"
else
echo "FAILURE: $LOG_MSG_COMMON" >>"$LOG_FILE"
fi

Re: git fetch behaves weirdely when run in a worktree

[Kaartic Sivaraam]

On Mon, 2018-09-24 at 17:17 +0200, Duy Nguyen wrote:
> On Sun, Sep 23, 2018 at 10:19 PM Kaartic Sivaraam
>  wrote:
> 
> Yes, some bugs. It behaves correctly for me. There must be something
> strange that triggers this. What's your "git worktree list" (iow
> anything strange there, duplicate worktrees perhaps)?

Nothing seems strange in the list.

$ git worktree list
$COMMON_ROOT/git  01d371f741 (detached HEAD)
$COMMON_ROOT/git-next cfa73bbfcb (detached HEAD)
$COMMON_ROOT/git-next-build-automate  01d371f741 (detached HEAD)
$COMMON_ROOT/git-pu   363c5c06bb (detached HEAD)

Note: I sanitized the path in which the git worktrees (including the
main worktree) is present as $COMMON_ROOT.


>  Also please try
> "git fetch" again with GIT_TRACE=1 and GIT_TRACE_SETUP=1. Hopefully we
> could catch something with that.

$ GIT_TRACE_SETUP=1 GIT_TRACE=1 git fetch origin next
23:10:26.049785 trace.c:377 setup: git_dir: 
$COMMON_ROOT/git/.git/worktrees/git-next-build-automate
23:10:26.049868 trace.c:378 setup: git_common_dir: 
$COMMON_ROOT/git/.git
23:10:26.049901 trace.c:379 setup: worktree: 
$COMMON_ROOT/git-next-build-automate
23:10:26.049922 trace.c:380 setup: cwd: 
$COMMON_ROOT/git-next-build-automate
23:10:26.049941 trace.c:381 setup: prefix: (null)
23:10:26.049955 git.c:415   trace: built-in: git fetch origin next
23:10:26.051033 run-command.c:637   trace: run_command: git-remote-https 
origin https://github.com/git/git.git
23:10:28.366526 run-command.c:637   trace: run_command: git rev-list 
--objects --stdin --not --all --quiet
23:10:28.400979 run-command.c:637   trace: run_command: git rev-list 
--objects --stdin --not --all --quiet
23:10:28.402745 trace.c:377 setup: git_dir: 
$COMMON_ROOT/git/.git/worktrees/git-next-build-automate
23:10:28.402787 trace.c:378 setup: git_common_dir: 
$COMMON_ROOT/git/.git
23:10:28.402793 trace.c:379 setup: worktree: 
$COMMON_ROOT/git-next-build-automate
23:10:28.402798 trace.c:380 setup: cwd: 
$COMMON_ROOT/git-next-build-automate
23:10:28.402802 trace.c:381 setup: prefix: (null)
23:10:28.402815 git.c:415   trace: built-in: git rev-list --objects 
--stdin --not --all --quiet
>From https://github.com/git/git
 * branch  next   -> FETCH_HEAD
23:10:28.437350 run-command.c:1553  run_processes_parallel: preparing to 
run up to 1 tasks
23:10:28.437481 run-command.c:1585  run_processes_parallel: done
23:10:28.437763 run-command.c:637   trace: run_command: git gc --auto
23:10:28.439608 trace.c:377 setup: git_dir: 
$COMMON_ROOT/git/.git/worktrees/git-next-build-automate
23:10:28.439655 trace.c:378 setup: git_common_dir: 
$COMMON_ROOT/git/.git
23:10:28.439667 trace.c:379 setup: worktree: 
$COMMON_ROOT/git-next-build-automate
23:10:28.439677 trace.c:380 setup: cwd: 
$COMMON_ROOT/git-next-build-automate
23:10:28.439687 trace.c:381 setup: prefix: (null)
23:10:28.439699 git.c:415   trace: built-in: git gc --auto

HTH,
Sivaraam

git fetch behaves weirdely when run in a worktree

[Kaartic Sivaraam]

Hi,

I was actually trying to automae the building and installation of Git
source code to reduce my burden. I tried to automate it with the help
of a script that runs daily via cron and a separate worktree used only
by the build script.y run

The script typically fetches new changes for the next branch by running
the following in the build worktree (which is not the main worktree):

   $ git fetch origin next

I thought that would result in FETCH_HEAD pointing to the latest
changes for origin/next if the command succeeded.

Unfortunately, it seems to be behaving weirdely when run in a worktree.
It sems to be behaving as if I ran 'git fetch origin'. To add to that
confusion when I run

   $ cat $MAIN_WORKTREE/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD

it seems to be printing the info about the remote refs once and then if
I run it again immediately nothing is printed. If I repeat it again,
info about remote refs is printed but this time the info about the
remote refs is printed thrice. This happens randomly. Sample output:

   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
   53f9a3e157dbbc901a02ac2c73346d375e24978c not-for-merge   branch 'maint' 
of https://github.com/git/git
150f307afc13961b0322ad0e7205a7b193368586not-for-merge   branch 'master' 
of https://github.com/git/git
01d371f741e6f0b0076d9ed942d99bdb23757eacnot-for-merge   branch 'next' 
of https://github.com/git/git
7a81cbc028be113058e0b55062706ec6de62ed94not-for-merge   branch 'pu' of 
https://github.com/git/git
722746685bce03f223ed75febe312495e6c139danot-for-merge   branch 'todo' 
of https://github.com/git/git
   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
   53f9a3e157dbbc901a02ac2c73346d375e24978c not-for-merge   branch 'maint' 
of https://github.com/git/git
150f307afc13961b0322ad0e7205a7b193368586not-for-merge   branch 'master' 
of https://github.com/git/git
01d371f741e6f0b0076d9ed942d99bdb23757eacnot-for-merge   branch 'next' 
of https://github.com/git/git
7a81cbc028be113058e0b55062706ec6de62ed94not-for-merge   branch 'pu' of 
https://github.com/git/git
722746685bce03f223ed75febe312495e6c139danot-for-merge   branch 'todo' 
of https://github.com/git/git
53f9a3e157dbbc901a02ac2c73346d375e24978cnot-for-merge   branch 'maint' 
of https://github.com/git/git
150f307afc13961b0322ad0e7205a7b193368586not-for-merge   branch 'master' 
of https://github.com/git/git
01d371f741e6f0b0076d9ed942d99bdb23757eacnot-for-merge   branch 'next' 
of https://github.com/git/git
7a81cbc028be113058e0b55062706ec6de62ed94not-for-merge   branch 'pu' of 
https://github.com/git/git
722746685bce03f223ed75febe312495e6c139danot-for-merge   branch 'todo' 
of https://github.com/git/git
53f9a3e157dbbc901a02ac2c73346d375e24978cnot-for-merge   branch 'maint' 
of https://github.com/git/git
150f307afc13961b0322ad0e7205a7b193368586not-for-merge   branch 'master' 
of https://github.com/git/git
01d371f741e6f0b0076d9ed942d99bdb23757eacnot-for-merge   branch 'next' 
of https://github.com/git/git
7a81cbc028be113058e0b55062706ec6de62ed94not-for-merge   branch 'pu' of 
https://github.com/git/git
722746685bce03f223ed75febe312495e6c139danot-for-merge   branch 'todo' 
of https://github.com/git/git
   01:23 $ cat ../git/.git/worktrees/$BUILD_WORKTREE/FETCH_HEAD 
   53f9a3e157dbbc901a02ac2c73346d375e24978c not-for-merge   branch 'maint' 
of https://github.com/git/git
150f307afc13961b0322ad0e7205a7b193368586not-for-merge   branch 'master' 
of https://github.com/git/git
01d371f741e6f0b0076d9ed942d99bdb23757eacnot-for-merge   branch 'next' 
of https://github.com/git/git
7a81cbc028be113058e0b55062706ec6de62ed94not-for-merge   branch 'pu' of 
https://github.com/git/git
722746685bce03f223ed75febe312495e6c139danot-for-merge   branch 'todo' 
of https://github.com/git/git

   'git fetch  ' behaves correctly in the main worktree.

   Why is this weirdness happening when run in other worktrees?

   Why isn't 'git fetch   not fetching the changes for
   just the specified branch?

   Am I missing something?

   --
   Sivaraam

Re: [ANNOUNCE] Git v2.19.0-rc0

[Kaartic Sivaraam]

On Thu, 2018-08-23 at 06:26 -0400, Derrick Stolee wrote:
> 
> Around the time that my proposed approaches were getting vetoed for 
> alignment issues, I figured I was out of my depth here. I reached out to 
> Daniel Lemire (of EWAH bitmap fame) on Twitter [1]. His blog is full of 
> posts of word-based approaches to different problems, so I thought he 
> might know something off the top of his head that would be applicable. 
> His conclusion (after looking only a short time) was to take a 'hasheq' 
> approach [2] like Peff suggested [3]. Since that requires auditing all 
> callers of hashcmp to see if hasheq is appropriate, it is not a good 
> solution for 2.19 but (in my opinion) should be evaluated as part of the 
> 2.20 cycle.
> 

That was an interesting blog post, indeed. It had an interesting
comments section. One comment especially caught my eyes was [a]:

"So the way gcc (and maybe clang) handles this is specifically by
recognizing memcmp and checking whether a only a 2-way result is needed
and then essentially replacing it with a memcmp_eq call.

..."

I find this to be an interesting note. It seems GCC does optimize when
we clearly indicate that we use the result of the memcmp as a boolean.
So would that help in anyway? Maybe it would help in writing a `hasheq`
method easily? I'm not sure.

> [1] https://twitter.com/stolee/status/1032312965754748930
> 
> [2] 
> https://lemire.me/blog/2018/08/22/avoid-lexicographical-comparisons-when-testing-for-string-equality/
> 
> [3] 
> https://public-inbox.org/git/20180822030344.ga14...@sigill.intra.peff.net/
> 
> [4] 
> https://public-inbox.org/git/7ea416cf-b043-1274-e161-85a8780b8...@gmail.com/


[a]: 
https://lemire.me/blog/2018/08/22/avoid-lexicographical-comparisons-when-testing-for-string-equality/#comment-344073

--
Sivaraam

Re: [PATCH] Makefile: enable DEVELOPER by default

[Kaartic Sivaraam]

On Mon, 2018-08-06 at 13:02 -0400, Randall S. Becker wrote:
> 
> Any idea when this is going to be in an official release, and exactly
> what the settings will be for "Not Developer". I assume DEVELOPER=0
> and DEVOPTS=error, which is the current behaviour, correct? I am the
> platform maintainer for HPE NonStop and need to make sure I'm not
> packaging DEV builds to anyone, since I'm the only one doing this for
> the platform. It's another hoop, but hopefully not a bad one. The
> question is the best place to set this, assuming we are using Jenkins
> for our builds, and I'd rather keep the existing config.mak.uname the
> same, since at least it seems stable.

Just a FYI and in case you aren't aware, you could create a
"config.mak" to store your custom configurations. You can be sure it's
used due to the following Makefile part:

...

include config.mak.uname
-include config.mak.autogen
-include config.mak
...

It's just not a hard dependency.

Hope that helps,
Sivaraam

Re: What's cooking in git.git (Aug 2018, #06; Wed, 29)

[Kaartic Sivaraam]

On 30 August 2018 21:41:35 GMT+05:30, Elijah Newren  wrote:
>That makes sense.  I'm not sure I can concisely convey all the right
>points, but here's a stab at rewording:
>
>Recent addition of "directory rename" heuristics to the
>merge-recursive backend makes the command susceptible to false
>positives and false negatives.  In the context of "git am -3", which
>does not know about surrounding unmodified paths and thus cannot
>inform the merge machinery about the full trees involved, this risk is
>particularly severe.  As such, the heuristic is disabled for "git am
>-3" to keep the machinery "more stupid but predictable".

FWIW, this sounds better to me than the original.
-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Doc/SubmittingPatches: re-phrashing a sentence about alternate solutions (was Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv")

[Kaartic Sivaraam]

On Sun, 2018-06-17 at 14:00 -0400, Eric Sunshine wrote:
> Whether or not to talk about alternate solutions in the commit message
> is a judgment call. Same for deciding what belongs in the commit
> message proper and what belongs in the "commentary" section of a
> patch. A patch author should strive to convey the problem succinctly
> in the commit message, to not overload the reader with unnecessary (or
> confusing) information, while, at the same time, not be sparing with
> information which is genuinely needed to understand the problem and
> solution.
> 
> Often, this can be done without talking about alternatives; often even
> without spelling out the solution in detail or at all since the
> solution may be "obvious", given a well-written problem description.
> Complex cases, or cases in which multiple solutions may be or seem
> valid, on the other hand, might warrant talking about those alternate
> solutions, so we probably don't want to drop that bullet point.

Well explained, thanks. (Thinking out loud, it might be even nice to
including the above paragraphs into Documentation/SubmittingPatches as
I find it to be more "humane" than the terse bullets. But I refrained
from doing so as the document is already a bit too-long ;-)

> Perhaps, instead, it can be re-worded a bit to make it sound something
> other than mandatory (but I can't think of a good way to phrase it;
> maybe you can?).

How about the following patch? (warning: patch only for discussion
purposes, might be white-space broken). It might be superfluous,
though.

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index a1d0feca3..565bc4397 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -128,7 +128,7 @@ The body should provide a meaningful commit message, which:
 . justifies the way the change solves the problem, i.e. why the
   result with the change is better.
 
-. alternate solutions considered but discarded, if any.
+. alternate solutions considered but discarded, where necessary.
 
 [[imperative-mood]]
 Describe your changes in imperative mood, e.g. "make xyzzy do frotz"


Regards,
Sivaraam

Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv"

[Kaartic Sivaraam]

On Friday 15 June 2018 01:13 PM, Eric Sunshine wrote:
> On Fri, Jun 15, 2018 at 2:58 AM Simon Ruderich  wrote:
>> On Thu, Jun 14, 2018 at 10:25:03PM -0400, Eric Sunshine wrote:
>>> This patch is extra noisy due to the indentation change. Viewing it with
>>> "git diff -w" helps. An alternative to re-indenting would have been to
>>> "undefine NEEDS_LIBICONV", however, 'undefine' was added to GNU make in
>>> 3.82 but MacOS is stuck on 3.81 (from 2006) so 'undefine' was avoided.
>>
>> Should we put the part about MacOS's make into the commit
>> message? Seems like relevant information for future readers.
> 
> No. The bit of commentary mentioning MacOS's very old 'make' was just
> talking about a possible alternate way of implementing the change.
> That alternative was not chosen, so talking about old 'make' in the
> commit message would be confusing for readers.

Interesting. Documentation/SubmittinPatches reads:

The body should provide a meaningful commit message, which:
...

...
. alternate solutions considered but discarded, if any.

The consensus has changed, maybe? In which case, should we remove that
statement from there?


-- 
Sivaraam

QUOTE:

“The three principal virtues of a programmer are Laziness, Impatience,
and Hubris.”

- Camel book

Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] submodule: fix NULL correctness in renamed broken submodules

[Kaartic Sivaraam]

On Thursday 14 June 2018 11:01 PM, Stefan Beller wrote:
> While at it, make sure we only attempt to load the submodule if a git
> directory of the submodule is found as default_name_or_path will return
> NULL in case the git directory cannot be found.

I found this a little hard to read. Maybe it could be sentence could be
shrinked a little. Possibly,

While at it, make sure we only attempt to load the submodule if
a git directory of the submodule is found.

I guess the other part of the sentence doesn't make much sense in the
log message. Maybe it could be an in-code comment. Speaking of in-code
comment, the following would also fit there, wouldn't it?

> Note that passing NULL
> to submodule_from_name is just a semantic error, as submodule_from_name
> accepts NULL as a value, but then the return value is not the submodule
> that was asked for, but some arbitrary other submodule. (Cf. 'config_from'
> in submodule-config.c: "If any parameter except the cache is a NULL
> pointer just return the first submodule. Can be used to check whether
> there are any submodules parsed.")
> 


-- 
Sivaraam

QUOTE:

“The three principal virtues of a programmer are Laziness, Impatience,
and Hubris.”

- Camel book

Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

[PATCH v2] t3200: clarify description of --set-upstream test

[Kaartic Sivaraam]

Support for the --set-upstream option was removed in 52668846ea
(builtin/branch: stop supporting the "--set-upstream" option,
2017-08-17). The change did not completely remove the command
due to an issue noted in the commit's log message.

So, a test was added to ensure that a command which uses the
'--set-upstream' option fails instead of silently acting as an alias
for the '--set-upstream-to' option due to option parsing features.

To avoid confusion, clarify that the option is disabled intentionally
in the corresponding test description.

The test is expected to be around as long as we intentionally fail
on seeing the '--set-upstream' option which in turn we expect to
do for a period of time after which we can be sure that existing
users of '--set-upstream' are aware that the option is no
longer supported.

Signed-off-by: Kaartic Sivaraam 
---
Changes since v1:

- update the test description as suggested by Junio

- updated the commit message to be clear about period upto
  which we should be testing this.

 t/t3200-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6c0b7ea4a..e98dbfc1a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -884,7 +884,7 @@ test_expect_success 'test --unset-upstream on a particular 
branch' '
test_must_fail git config branch.my14.merge
 '
 
-test_expect_success '--set-upstream fails' '
+test_expect_success 'disabled option --set-upstream fails' '
 test_must_fail git branch --set-upstream origin/master
 '
 
-- 
2.18.0.rc1.242.g61856ae69

Re: [PATCH] t3200: clarify description of --set-upstream test

[Kaartic Sivaraam]

On Thursday 14 June 2018 11:13 PM, Junio C Hamano wrote:
> It is technically correct to call --set-upstream "unsupported", but
> the reason why we want to see it fail is not because it is
> unsupported, but because we actively interfere with the usual
> "unique prefix" logic parse-options API gives its users and make it
> not to trigger the longer-and-unique --set-upstream-to logic.
> 

That sounds right.


>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>> index 6c0b7ea4a..d14de82ba 100755
>> --- a/t/t3200-branch.sh
>> +++ b/t/t3200-branch.sh
>> @@ -884,7 +884,7 @@ test_expect_success 'test --unset-upstream on a 
>> particular branch' '
>>  test_must_fail git config branch.my14.merge
>>  '
>>  
>> -test_expect_success '--set-upstream fails' '
>> +test_expect_success 'unsupported option --set-upstream fails' '
> 
> In other words, I am wondering if s/unsupported/disabled/ makes it
> even more clear what is going on here.
>

I guess it would :-) Thanks for the better wording. I actually thought
of asking for a better wording for the test message while sending the
patch but somehow forgot to mention it. It seems I've got better
wordings, regardless.

I'll send a v2 with the correction. I'm still open to an alternative
test description in case that make things even more clearer :-)


Thanks,
Sivaraam

QUOTE:

“The three principal virtues of a programmer are Laziness, Impatience,
and Hubris.”

- Camel book

Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

[PATCH] t3200: clarify description of --set-upstream test

[Kaartic Sivaraam]

Support for the --set-upstream option was removed in 52668846ea
(builtin/branch: stop supporting the "--set-upstream" option,
2017-08-17). The change did not completely remove the command
due to an issue noted in the commit's log message.

So, a test was added to ensure that a command which uses the
'--set-upstream' option fails and doesn't silently act as an alias
for the '--set-upstream-to' option due to option parsing features.

To avoid confusion, clarify that the option is an unsupported one
in the corresponding test description.

Signed-off-by: Kaartic Sivaraam 
---
 t/t3200-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6c0b7ea4a..d14de82ba 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -884,7 +884,7 @@ test_expect_success 'test --unset-upstream on a particular 
branch' '
test_must_fail git config branch.my14.merge
 '
 
-test_expect_success '--set-upstream fails' '
+test_expect_success 'unsupported option --set-upstream fails' '
 test_must_fail git branch --set-upstream origin/master
 '
 
-- 
2.18.0.rc1.242.g61856ae69

Re: BUG: submodule code prints '(null)'

[Kaartic Sivaraam]

On Tuesday 05 June 2018 09:01 PM, Duy Nguyen wrote:
> I'll leave it to submodule people to fix this :)
> 

I'm Ccing the only one I know to gain attention.


-- 
Sivaraam

QUOTE:

“The three principal virtues of a programmer are Laziness, Impatience,
and Hubris.”

- Camel book

Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] t3200-branch.sh: use "--set-upstream-to" in test

[Kaartic Sivaraam]

On Tuesday 05 June 2018 04:54 PM, SZEDER Gábor wrote:
> 
> Though arguably the test name could be more descriptive and tell why
> it should fail.
> 

That's arguable, indeed. I was about to send a patch that gives a better
description for the test. I didn't do it as I started wondering, Is it
even worth testing whether a removed option fails? Is this done for
other options that have been removed in the past? Should we just remove
the test completely?

-- 
Sivaraam

QUOTE:

“The three principal virtues of a programmer are Laziness, Impatience,
and Hubris.”

- Camel book

Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] branch: issue "-l" deprecation warning after pager starts

[Kaartic Sivaraam]

On Friday 25 May 2018 08:10 AM, Jeff King wrote:
> Subject: [PATCH] branch: customize "-l" warning in list mode
> 
> People mistakenly use "git branch -l", thinking that it
> triggers list mode. It doesn't, but the lack of non-option
> arguments in that command does (and the "-l" becomes a
> silent noop).
> > Since afc968e579 (branch: deprecate "-l" option, 2018-03-26)
> 
> we've warned that "-l" is going away. But the warning text
> is primarily aimed at people who _meant_ to use "-l", as in
> "git branch -l foo". People who mistakenly said "git branch
> -l" may be left puzzled.
> 

So, this patch is to improve the user experience of people who use "git
branch -l" for listing and not for the people who forget to give a new
branch name argument for "-l". In that case, this makes sense.

BTW, I hope people don't start wondering why "git branch -d" doesn't
trigger list mode ;-)


> + warning("the '-l' option is an alias for 
> '--create-reflog' and");
> + warning("has no effect in list mode. This option will 
> soon be");
> + warning("removed and you should omit it (or use 
> '--list' instead).");

I suppose s/alias/deprecated alias/ makes the point that '-l' will be
removed more stronger.


-- 
Sivaraam

signature.asc
Description: This is a digitally signed message part

Re: [PATCH] branch: issue "-l" deprecation warning after pager starts

[Kaartic Sivaraam]

On Friday 25 May 2018 01:01 AM, Jeff King wrote:
> On Thu, May 24, 2018 at 03:22:14PM -0400, Jeff King wrote:
> 
> Hmm, actually, I suppose the true value of the warning is to help people
> doing "git branch -l foo", and it would still work there. The "more
> extreme" from your suggested patch would only affect "branch -l".
> 
> Still, I think I prefer the gentler version that we get by keeping it as
> a warning even in the latter case.
>

I never wanted to suppress the warning message in the latter case. I
just wanted to avoid listing the branches. Actually the patch I sent in
one the previous threads[1] that avoids listing the branches has the
following behaviour,

$ git branch -l
warning: the '-l' alias for '--create-reflog' is deprecated;
warning: it will be removed in a future version of Git
usage: git branch [] [-r | -a] [--merged | --no-merged]
   or: git branch [] [-l] [-f]  []
   or: git branch [] [-r] (-d | -D) ...
   or: git branch [] (-m | -M) [] 
   or: git branch [] (-c | -C) [] 
   or: git branch [] [-r | -a] [--points-at]
   or: git branch [] [-r | -a] [--format]


So, the warning message isn't lost. It just prevents the listing of
branches.

Wait, maybe I'm misunderstanding what you mean by "warning". You're
probably meaning something related to the way Git exits in both cases.
With your patch "git branch -l" prints a warning, lists the branches and
has an exit status of 0. With my patch it prints the warning, the usage
specifications with exit status 128. In that case, I still don't think
it's bad to turn "git branch -l" into an error now as it's been
incorrect for a long time now and it's not wrong if we correct it now.

Anyways, if you think it mustn't turn into an error now and only in the
next stage, a suggestion follows in another thread.


[1]: https://public-inbox.org/git/1527174618.10589.4.ca...@gmail.com/


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Kaartic Sivaraam]

On Thursday 17 May 2018 07:06 PM, Jeff King wrote:
> But because git-branch does not kick in the pager until later
> (because it only wants to do it for list-mode), that happens _after_
> we've emitted the message.
> 

I observe exactly the consequence of this behaviour. First, the error
is emitted and then the pager kicks in to list the branches.


> On the other hand, I'm not sure this is that big a deal. The point of
> the deprecation warning is to catch people who are actually trying to
> use "-l" as "--create-reflog", and that case does not page. The people
> doing "git branch -l" are actually getting what they want eventually,
> which is to turn it into "--list". In the interim step where it becomes
> an unknown option, they'll get a hard error. 

I just thought we wouldn't want to surprise/confuse users who try to
use "git branch -l" with the warning message about "create reflog"
along-side the list of branches. That would just add to the confusion.
So, I thought we should error out when users do "git branch -l"
instead. Something like the following should help us prevent "git
branch -l" from listing branch names and might also prevent the
confusion.


diff --git a/builtin/branch.c b/builtin/branch.c
index 452742fec..f3c5181bb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -672,7 +672,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 0);
 
-   if (!delete && !rename && !copy && !edit_description && !new_upstream 
&& !unset_upstream && argc == 0)
+   if (!delete && !rename && !copy && !edit_description && !new_upstream 
&& !unset_upstream && !reflog && argc == 0)
list = 1;
 
if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || 
filter.points_at.nr ||


-- 
Kaartic

signature.asc
Description: This is a digitally signed message part

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

On 21 May 2018 01:03:22 GMT+05:30, Paul-Sebastian Ungureanu 
 wrote:
>
> I actually wrote a 
>short paragraph about one of them (the one regarding -p option) on the 
>blog (the first post).
>

That's interesting. I didn't realise that you wrote about one of the bugs in 
your blog. I might have missed it, somehow.

Anyhow, it happens to me all the time ;-)
-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Kaartic Sivaraam]

Hi Ævar,

On Thursday 17 May 2018 03:18 PM, Ævar Arnfjörð Bjarmason wrote:
> I've ended up with that $LESS setting via hackery over the years, so
> maybe I'm doing something retarded, minimal test case:
> 
> PAGER=less  LESS="--no-init --quit-if-one-screen" git branch -l
>
> ...
> 
> So I think this is probably OK for most users, if the have very few
> branches they'll see it, and then if they use default pager settings
> they'll see the stderr output once they quit the pager. I don't know how
> common my (mis)configuration is.
>
I'm not sure this is ok, because I still see the stderr output with your
minimal test case even when I have enough branches to not fit in one
screen. The stderr output is of course above the pager output (after I
quit the pager) and gets hidden out-of display as I stated before. I
also get weird 'ESC[m' characters with you minimal test case. I'm not
sure what I'm missing.

What version of 'less' do you use? Is any other configuration that you
didn't mention affecting what your observation?


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

On Thursday 17 May 2018 02:39 PM, Johannes Schindelin wrote:
> I have great empathy for the desire to see these bugs fixed. The
> conversion must come first, though, and in the interest of making it
> easier on me and other reviewers, I must insist on keeping the conversion
> free of any changes, much in the way as we try to avoid evil merges (i.e.
> merge commits that introduce changes that were not present in any of their
> parents).
> 

Of course, the conversion should be separate from the bug fixes :-)

When I mentioned "while porting it to C", I actually meant the "thought
process of creating a foundation for `git-stash` in C". I thought
hinting at some of the existing and unsolved `git-stash` bugs would
allow the person who would be doing the port of `git-stash` to C to
consider how to avoid this while implementing the basic foundation. I
should have been more explicit about this in my previous mails.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

On Thursday 17 May 2018 12:28 PM, Kaartic Sivaraam wrote:
> Hi Sebi,
> 
> I thought of pointing you to one of the issues with the current
> implementation of 'git stash' which you could probably fix while porting
> it to C.
>
> ...
> 

Forgot to mention about another issue, which I consider to be a bug. I
have elaborated about it in the following mailing list email.


https://public-inbox.org/git/aa43f1ff-9095-fb4d-43bc-bf8283b7d...@gmail.com/

Unfortunately, it didn't receive any replies. See, if you could do
something about it.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

Hi Sebi,

I thought of pointing you to one of the issues with the current
implementation of 'git stash' which you could probably fix while porting
it to C. It's about stashing untracked files.

You could find more information about it in the following mailing list
thread:
https://public-inbox.org/git/1505626069.9625.6.ca...@gmail.com/


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Kaartic Sivaraam]

On Thursday 17 May 2018 11:31 AM, Junio C Hamano wrote:
> * jk/branch-l-0-deprecation (2018-03-26) 3 commits
> 
> ...
>
>  The "-l" option in "git branch -l" is an unfortunate short-hand for
>  "--create-reflog", but many users, both old and new, somehow expect
>  it to be something else, perhaps "--list".  This step deprecates
>  the short-hand and warns about the future removal of the it when it
>  is used.
> 
>  Will cook in 'next'.
>  Perhaps merge to 'master' immediately after 2.18 release?

I still have a slight feeling that we shouldn't list the branches for
"git branch -l" during the deprecation period. If feel this because

i) It would avoid confusions for the users during the
   deprecation period

ii) The warning message seems to add to the confusion:

$ git branch -l
warning: the '-l' alias for '--create-reflog' is deprecated;
warning: it will be removed in a future version of Git
* master
...


If there are ample branches, the warning message might be
hidden out of screen but we shouldn't rely on that, I
suppose.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] mailmap: update brian m. carlson's email address

[Kaartic Sivaraam]

On Wednesday 09 May 2018 05:49 AM, brian m. carlson wrote:
> Correct, it doesn't.  In my case, I was using --pretty='%aN <%aE>',
> which is how I noticed it in the first place.

So, how about updating the commit message to avoid confusions to the
incidental future reader? (Or is it just not worth it? ;-])


-- 
Sivaraam



signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v4 1/3] Teach remote add the --remote-tags option

[Kaartic Sivaraam]

On Tuesday 01 May 2018 10:29 PM, Wink Saville wrote:
> When --remote-tags is passed to `git remote add` the tagopt is set to
> --remote-tags and a second fetch line is added so tags are placed in
> a separate hierarchy per remote.
> 

I find '--remote' in the option name to be redundant given that it is an
option to `git remote add`. I guess '--namespace-tags' would be a better
alternative as it seems to convey the meaning more directly to the user.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/8] push tests: add more testing for forced tag pushing

[Kaartic Sivaraam]

On Monday 30 April 2018 01:50 AM, Ævar Arnfjörð Bjarmason wrote:
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 15c8d5a734..c9a2011915 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -981,7 +981,17 @@ test_expect_success 'push requires --force to update 
> lightweight tag' '
>   git push --force ../child2 Tag &&
>   git tag -f Tag HEAD~ &&
>   test_must_fail git push ../child2 Tag &&
> - git push --force ../child2 Tag
> + git push --force ../child2 Tag &&
> + git tag -f Tag &&
> + test_must_fail git push ../child2 "refs/tags/*:refs/tags/*" &&
> + git push --force ../child2 "refs/tags/*:refs/tags/*" &&
> + git tag -f Tag HEAD~ &&
> + git push ../child2 "+refs/tags/*:refs/tags/*" &&
> + git tag -f Tag &&
> + git push --no-force ../child2 "+refs/tags/*:refs/tags/*" &&
> + git tag -f Tag HEAD~ &&
> + test_must_fail git push ../child2 tag Tag &&
> + git push --force ../child2 tag Tag

As a person who came to know about the "tag " refspec for the
first time while seeing this patch, I found it a little hard to parse
the following two lines of the test:

test_must_fail git push ../child2 tag Tag &&
git push --force ../child2 tag Tag

Maybe some other name than "Tag" for the example would have made it
easier for the person reading it. Something like "foo"/"bar" etc.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/8] push tests: add more testing for forced tag pushing

[Kaartic Sivaraam]

On Tuesday 08 May 2018 08:49 AM, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> I couldn't quite get what you meant by "(but not the other way
>> around)".  Did you mean
>>
>>  $ git push --force ../child2 refs/tags/*:refs/tags/*
>>
>> should not become non-forcing version because of the (lack of)
>> prefix on the refspec does not trump the --force command line
>> option?

When I was reading the commit message, I had the same doubt about what
"(but not the other way around)" actually meant but I assumed it meant
that "the `--no-force` in the command line does not override the '+' in
the refspec". Maybe the commit message could be updated to clarify this?


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: Weak option parsing in `git submodule`

[Kaartic Sivaraam]

On Tuesday 08 May 2018 12:35 AM, Stefan Beller wrote:
>> The lack of checking for the reason behind why `git add` fails seems to
>> be the reason behind that weird message.
> 
> (from the man page)
> git submodule [--quiet] add [] [--]  []
> 
> When options are given after  or  we can count
> the arguments and know something is up. (The number of arguments
> must be 1 or 2. If it is 3 or above, something fishy is going on), which
> I would suggest as a first step.
> 
>> Ways to fix this:
>>
>> 1. Fix this particular issue by adding a '--' after the '--no-warn-
>> embedded-repo' option in the above check. But that would also
>> require that we allow other parts of the script to accept weird
>> paths such as '--path'. Not so useful/helpful.
>>
>> 2. Check for the actual return value of `git add` in the check and
>> act accordingly. Also, check if there are unnecessary arguments for
>> `submodule add`.
> 
> The second part of this suggestion seems to me as the way to go.
> Do you want to write a patch?
> 

Incidentally, I was writing a patch to check for the return value of
`git add` to fix the particular issue I noted in my initial message.
Then I was in a dilemma as to whether this was the right way to do it.
So, I thought it would be better to ask before continuing with the patch
and hence started this thread. I wasn't counting the arguments to `git
submodule add` at that time.

Now that I'm ensured that my initial approach is not the worst way to do
things and as I'm about to write a patch for this, I'll sum up what I'm
about to achieve in the short-term fix patch, for the sake of clarity.

1. Check the return value of `git add ...` and throw an error
   appropriately.

2. Check the no. of arguments to `submodule add` and throw an
   error if there are more arguments than there should be.

   I require a little clarification with this. How should this
   be done. Does checking whether the number of arguments after
is not more than one do the job? Or am I missing
   something?


>> 3. Tighten option parsing in the `git-submodule` script to ensure
>> this doesn't happen in the long term and helps users to get more
>> relevant error messages.
>>
>> I find 3 to be a long term solution but not sure if it's worth trying
>> as there are efforts towards porting `git submodule` to C. So, I guess
>> we should at least do 2 as a short-term solution.
> 
> Yeah, bringing it to C, would be nice as a long term solution instead
> of piling up more and more shell features. :)
> 

Hope the day it is brought into C comes soon.


> Thanks for such a well written bug report with suggested bug fixes. :)

You're welcome :-)


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: main url for linking to git source?

[Kaartic Sivaraam]

On Monday 07 May 2018 11:45 PM, Stefan Beller wrote:
>> I could see arguments both ways, so I thought I'd take a straw poll of
>> what people on the list think.
>  
> Junios reply below focuses on the URL passed to git-clone, which
> is only found at https://git-scm.com/downloads (?)
> 
> There I would try to mirror Junios list of "public repositories"
> https://git-blame.blogspot.com/p/git-public-repositories.html
> without officially endorsing one over another.
>
FWIW, I also seem to support this suggestion as it's not opinionated.


> For those links that link to web pages, I am ok with any of the
> hosting providers, maybe even taking the one with the prettiest
> web page. Maybe we want to reword those sections to rely
> more on indirection, e.g. "get the source[link to the source page],
> checkout the next branch", without the quick web link to a web page
> showing the 'next' tree.

Seems to be a nice suggestion to avoid the "main/official" url issue.

To add a little more, it might be better replace the "Source code" link
with a link to Junio's list of public repositories stated above. Also,
it might be better to rename the link to "Public repositories containing
the source".


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] mailmap: update brian m. carlson's email address

[Kaartic Sivaraam]

On Tuesday 08 May 2018 07:28 AM, brian m. carlson wrote:
> An earlier change, cdb6b5ac (".mailmap: Combine more (name, email) to
> individual persons", 2013-08-12), noted that there were two name
> spellings and two email addresses and mapped the crustytoothpaste.net
> address to the crustytoothpaste.ath.cx address.  The latter is an older,
> obsolete address, while the former is current, so switch the order of
> the addresses so that git log displays the correct address.
> 
Just to be sure, you're meaning the use of `git log --use-mailmap` when
you mean `git log` in the log message. Am I right? Or did you mean `git
shortlog` ?

I'm asking this because I think the `git log` output doesn't consider
the mailmap file by default.

-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/8] push tests: add more testing for forced tag pushing

[Kaartic Sivaraam]

Hi,

On Monday 30 April 2018 01:50 AM, Ævar Arnfjörð Bjarmason wrote:

> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 15c8d5a734..c9a2011915 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -981,7 +981,17 @@ test_expect_success 'push requires --force to update 
> lightweight tag' '

I think the test description has become misleading now that it's testing
for 'force pushing' in general and not just the '--force' option. So, a
better description is needed. Probably, "force pushing required to
update lightweight tag".


>   git push --force ../child2 Tag &&
>   git tag -f Tag HEAD~ &&
>   test_must_fail git push ../child2 Tag &&
> - git push --force ../child2 Tag
> + git push --force ../child2 Tag &&
> + git tag -f Tag &&
> + test_must_fail git push ../child2 "refs/tags/*:refs/tags/*" &&
> + git push --force ../child2 "refs/tags/*:refs/tags/*" &&
> + git tag -f Tag HEAD~ &&
> + git push ../child2 "+refs/tags/*:refs/tags/*" &&

> + git tag -f Tag &&
> + git push --no-force ../child2 "+refs/tags/*:refs/tags/*" &&

This test shouldn't hiding within the tests about force pushing. It
seems to warrant a separate test case to clearly note the behavior that
the "+" in refspec overrides "--no-force". This would help in easily
identifying if this particular behavior is broken or not.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Weak option parsing in `git submodule`

[Kaartic Sivaraam]

I recently faced the consequence of the weak option parsing done by in
`git submodule`. Initially tried to add a new submodule using the `git
submodule add` sub-command in the following way,

$ git submodule add ./foo/bar

This cloned the submodule into a new directory named 'bar' in the
present directory. This was initially confusing as I expected `git
submodule` to use the actual directory itself as it resides inside a
sub-directory the main project and thus avoiding the creation of a new
clone. Then I came to know that `git submodule` wasn't smart enough yet
to identify this, by reading the documentation. Then, after realizing
that I would have to specify the path in the command to avoid the
redundant clone, I corrected the command without consulting the
documentation (thoroughly) to,

$ git submodule add ./foo/bar --path ./foo/bar

expecting that the path needs to be specified after an argument. This
is what triggered the weird consequence of weak option parsing. The
output I got for the above command was:

The following path is ignored by one of your .gitignore files:
--path
Use -f if you really want to add it.

That really confused me pretty much (being a person who doesn't know
much about how `git submodule` works). Instead of complaining about an
inexistent option '--path', it considers '--path' to be the 
argument which seems to be bad. It doesn't even complain about the
extra argument. I also dug to find the reason behind this totally weird
message. It seems to be a consequence of the following lousy check
($sm_path is the normalized  argument):

if test -z "$force" &&
! git add --dry-run --ignore-missing --no-warn-embedded-repo 
"$sm_path" > /dev/null 2>&1
then
eval_gettextln "The following path is ignored by one of your 
.gitignore files:
\$sm_path
Use -f if you really want to add it." >&2
exit 1
fi

The lack of checking for the reason behind why `git add` fails seems to
be the reason behind that weird message.

Ways to fix this:

1. Fix this particular issue by adding a '--' after the '--no-warn-
embedded-repo' option in the above check. But that would also
require that we allow other parts of the script to accept weird
paths such as '--path'. Not so useful/helpful.

2. Check for the actual return value of `git add` in the check and
act accordingly. Also, check if there are unnecessary arguments for
`submodule add`.

3. Tighten option parsing in the `git-submodule` script to ensure
this doesn't happen in the long term and helps users to get more
relevant error messages.

I find 3 to be a long term solution but not sure if it's worth trying
as there are efforts towards porting `git submodule` to C. So, I guess
we should at least do 2 as a short-term solution.

Thoughts??

-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities as it is a common name (NOTE:
it's not a common spelling, just a common name). So, I switched
signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

Hi Sebi,

On Friday 04 May 2018 03:18 AM, Paul-Sebastian Ungureanu wrote:
> Hello everybody,
> 
> The community bonding period started. It is well known that for a
> greater rate of success, it is recommended to send weekly reports
> regarding project status. But, what would be a good form for such a
> report? I, for one, think that starting a blog is one of the best
> options because all the content will be stored in one place. Without
> further ado, I would like you to present my blog [1].
> 
> Any feedback is greatly appreciated! Thank you!
> 

The blog looks pretty well written. I also read your proposal. It also
seems to be pretty much well written. I like the way you explain things.
Particularly, you seem to be explaining the problem and the way you're
about to approach it well. The plan seems pretty good.

I just thought of suggesting one thing which might possibly be
redundant. I think you're aware of the fact that the Git project has
Travis-CI builds enabled[1] which you could take advantage of to ensure
your changes pass in various text environments.

If you're interested in testing your changes (which I suspect you are),
you might also be interested in 'git-test'[2], a tool built by Michael
Haggerty. Unlike the Travis-CI tests which test only the tip of the
change, 'git-test' would help you ensure that every single commit you
make doesn't break the test suite (which is both a nice thing and is
expected here).

Sorry for the off-topic info about the tests in this mail :-)

Hope you're able to achieve your goal as planned and have a great time
during this summer of code!


References:
[1]:
https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L70

[2]: https://github.com/mhagger/git-test


Regards,
Kaartic

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky

KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] git: add -N as a short option for --no-pager

[Kaartic Sivaraam]

On Wednesday 25 April 2018 11:55 AM, Johannes Sixt wrote:
> 
> I considered -P, but thought that it would better be reserved for
> something related to "paths". We have --{html,man,info}-path, which
> would be served better with -[HMI]. That leaves --exec-path, which we
> would probably abbreviate as -x or -X. So, -P is perhaps not that bad
> after all.
> 

It might be a good choice but I don't think it's not the best choice
anyway because '-P' doesn't seem to communicate 'no pager' well.


> Perhaps --no-pager means also "unpaginated" (-u, -U), "linear" (-l, -L),
> "streamed" (-s, -S). Other ideas, anyone?
>

Considering the other alternatives you suggested (as I couldn't come up
with a better "single letter" option ;-) ), I think '-s' might be a nice
choice but would possibly need some explanation in the documentation to
the wondering users.

In the end, lacking a better one letter option, I think '-P' would be a
better choice after all. It would, possibly, be easily understood as a
negation of pagination (-p). So, I guess it would be better to go with
'-P' than '-N'.

-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky

KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] git: add -N as a short option for --no-pager

[Kaartic Sivaraam]

On Thursday 26 April 2018 01:01 AM, Johannes Sixt wrote:
> 
> Right. But I have LESS=RS because I do *not* want it to remain on screen
> by default.
> 

In that case how about updating the commit message to be more clear
about it. How about,

It is possible to configure 'less', the pager, to use an
alternate screen to show the content. When it is closed in this
case, it switches back to the original screen, and all content
is gone.

-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky

KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: Draft of Git Rev News edition 38

[Kaartic Sivaraam]

Hi,

On Tuesday 17 April 2018 03:56 AM, Christian Couder wrote:
> Hi,
> 
> On Mon, Apr 16, 2018 at 5:07 PM, Kaartic Sivaraam
> <kaartic.sivar...@gmail.com> wrote:
>>
>> That said, I read the draft and found it good except for two minor issues,
> 
> Thanks for your comments!
> 

You're welcome!

I'm sorry to say that I read only part of the draft  when I sent my
previous email though I accidentally didn't mention it explicitly.

Now that I have read the draft completely I find a few typos in the
"Developer Spotlight: Jiang Xin" section:

1.
"... because I feel it is hard to track changes of GitHub UI and the
book will become obsolte very quickly."

obsolte -> obsolete


2.
"We also developped ..."

developped -> developed


On seeing the section "Light reading" to be empty, I thought I could
suggest something. I'm not sure whether you take Stack Overflow answers
for a light reading but I found the following answer to be interesting,

https://stackoverflow.com/a/6521223/5614968

That's all.

-- 
Kaartic

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

Re: Draft of Git Rev News edition 38

[Kaartic Sivaraam]

Hi,

On Monday 16 April 2018 08:33 PM, Sergey Organov wrote:
> Christian Couder  writes:
>> Here "the above article" means the Jake's "branch -l: print useful
>> info whilst rebasing a non-local branch" article above the current
>> article.

Just a little correction. I suppose Chris actually meant the "rebase -i:
offer to recreate merge commits" article written by Jake and not the
"branch -l: print useful info whilst rebasing a non-local branch" article.

That said, I read the draft and found it good except for two minor issues,

1. I see the following sentence in the "Rebasing merges: a jorney to the
ultimate solution (Road Clear) (written by Jacob Keller)" article

"A few examples were tried, but it was proven that the original
concept did not work, as dropped commits could end up being
replaid into the merge commits, turning them into "evil"
merges."

I'm not sure if 'replaid' is proper English assuming the past tense of
replay was intended there (which I think is 'replayed').


2. I see a minor Markdown syntax issue in the "branch -l: print useful
info whilst rebasing a non-local branch" article.

... reworked his original patch to improve `git branch
--list̀

Specifically, in the '--list̀' part. I guess it should be "--list`".

-- 
Kaartic

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

[PATCH v3 2/2] t3200: verify "branch --list" sanity when rebasing from detached HEAD

[Kaartic Sivaraam]

From: Eric Sunshine <sunsh...@sunshineco.com>

"git branch --list" shows an in-progress rebase as:

  * (no branch, rebasing )
master
...

However, if the rebase is started from a detached HEAD, then there is no
, and it would attempt to print a NULL pointer. The previous
commit fixed this problem, so add a test to verify that the output is
sane in this situation.

Signed-off-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 t/t3200-branch.sh | 24 
 1 file changed, 24 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 503a88d02..89fff3fa9 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -6,6 +6,7 @@
 test_description='git branch assorted tests'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
 
 test_expect_success 'prepare a trivial repository' '
echo Hello >A &&
@@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
--no-merged' '
test_must_fail git branch --merged HEAD --no-merged HEAD
 '
 
+test_expect_success '--list during rebase' '
+   test_when_finished "reset_rebase" &&
+   git checkout master &&
+   FAKE_LINES="1 edit 2" &&
+   export FAKE_LINES &&
+   set_fake_editor &&
+   git rebase -i HEAD~2 &&
+   git branch --list >actual &&
+   test_i18ngrep "rebasing master" actual
+'
+
+test_expect_success '--list during rebase from detached HEAD' '
+   test_when_finished "reset_rebase && git checkout master" &&
+   git checkout master^0 &&
+   oid=$(git rev-parse --short HEAD) &&
+   FAKE_LINES="1 edit 2" &&
+   export FAKE_LINES &&
+   set_fake_editor &&
+   git rebase -i HEAD~2 &&
+   git branch --list >actual &&
+   test_i18ngrep "rebasing detached HEAD $oid" actual
+'
+
 test_expect_success 'tracking with unexpected .fetch refspec' '
rm -rf a b c d &&
git init a &&
-- 
2.17.0.484.g0c8726318

Re: [PATCH v2 2/2] t3200: verify "branch --list" sanity when rebasing from detached HEAD

[Kaartic Sivaraam]

On Tuesday 03 April 2018 01:30 PM, Eric Sunshine wrote:
>> Note that the "detached HEAD" test case might actually fail in some cases
>> as the actual output of "git branch --list" might contain remote branch
>> names which is not considered by the test case as it is rare to happen
>> in the test environment.
> 
> This paragraph was not in the original patch[1]. I _think_ what you
> are saying (which took a while to decipher) is that if a command such
> as "git checkout origin/next" ever gets inserted into the script
> before the test, the test will be fooled since "git branch --list"
> will show "detached HEAD origin/next" rather than "detached HEAD
> d3adb33f", the latter of which is what the test is expecting.
> 

Yeah, you're right. To know the reason for the unclear paragraph, see below.


> Unfortunately, this paragraph makes it sound as if the test can fail
> randomly (which, I believe, is not the case), and nobody would want a
> test added which is unreliable, thus this paragraph is not helping to
> sell this patch (in fact, it's actively hurting it). Ideally, the test
> should be entirely deterministic so that it can't be fooled like this.
> Rather than including this (harmful) paragraph in the commit message,
> let's ensure that the test is deterministic (see below).
> 

Sorry for the harmful and not so clear paragraph! I actually kept that
paragraph there to **remind me** that I have to fix the issue which it
describes before sending out the patch but I somehow forgot about it
after I added it initially :-(


>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
>> --no-merged' '
>> +test_expect_success '--list during rebase from detached HEAD' '
>> +   test_when_finished "reset_rebase && git checkout master" &&
>> +   git checkout HEAD^0 &&
> 
> This is the line which I think is causing concern for you. If someone
> inserted a new test before this one which invoked "git checkout
> origin/next" (or something), then even after "git checkout HEAD^0",
> "git branch --list" would still report the unexpected "detached HEAD
> origin/next". Let's fix this, and make the test deterministic, by
> doing this instead:
> 
> git checkout master^0 &&
>

Nice idea, will re-send a v3 with this fix and the harmful paragraph
removed.


Thanks,
Kaartic



signature.asc
Description: OpenPGP digital signature

[PATCH v2 0/2] branch --list: print useful info whilst interactive rebasing a detached HEAD

[Kaartic Sivaraam]

Ok, I seem to have forgotten the cover letter. So, here's one.

The changes from v1 are as follows:

* Changes to the commit message of 1/2 to fix some errors

* Code changes to 1/2 to address the comments from v1

* Patch 2/2 is new. It's adds tests for the issue that 1/2 tries to fix.
  It's written by Eric with a little fix by me to make it work with
  GETTEXT_POISON.

An interdiff for 1/2:

diff --git a/ref-filter.c b/ref-filter.c
index a4c917c96..db2baedfe 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1309,18 +1309,14 @@ char *get_head_description(void)
memset(, 0, sizeof(state));
wt_status_get_state(, 1);
if (state.rebase_in_progress ||
-   state.rebase_interactive_in_progress)
-   {
-   const char *rebasing = NULL;
-   if (state.branch != NULL)
-   rebasing = state.branch;
+   state.rebase_interactive_in_progress) {
+   if (state.branch)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
else
-   rebasing = state.detached_from;
-
-   strbuf_addf(, _("(no branch, rebasing %s)"),
-   rebasing);
-   }
-   else if (state.bisect_in_progress)
+   strbuf_addf(, _("(no branch, rebasing
detached HEAD %s)"),
+   state.detached_from);
+   } else if (state.bisect_in_progress)
strbuf_addf(, _("(no branch, bisect started on %s)"),
state.branch);
else if (state.detached_from) {



Eric Sunshine (1):
  t3200: verify "branch --list" sanity when rebasing from detached HEAD

Kaartic Sivaraam (1):
  branch --list: print useful info whilst interactive rebasing a
detached HEAD

 ref-filter.c  | 12 
 t/t3200-branch.sh | 24 
 2 files changed, 32 insertions(+), 4 deletions(-)



signature.asc
Description: OpenPGP digital signature

[PATCH v2 1/2] branch --list: print useful info whilst interactive rebasing a detached HEAD

[Kaartic Sivaraam]

When rebasing interactively (rebase -i), "git branch --list" prints
a line indicating the current branch being rebased. This works well
when the interactive rebase is initiated when a local branch is
checked out.

This doesn't play well when the rebase is initiated on a detached
HEAD. When "git branch --list" tries to print information related
to the interactive rebase in this case it tries to print the name
of a branch using an uninitialized variable and thus tries to
print a "null pointer string". As a consequence, it does not provide
useful information while also inducing undefined behaviour.

So, print the point from which the rebase was started when interactive
rebasing a detached HEAD.

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 ref-filter.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index f9e25aea7..db2baedfe 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1309,10 +1309,14 @@ char *get_head_description(void)
memset(, 0, sizeof(state));
wt_status_get_state(, 1);
if (state.rebase_in_progress ||
-   state.rebase_interactive_in_progress)
-   strbuf_addf(, _("(no branch, rebasing %s)"),
-   state.branch);
-   else if (state.bisect_in_progress)
+   state.rebase_interactive_in_progress) {
+   if (state.branch)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
+   else
+   strbuf_addf(, _("(no branch, rebasing detached 
HEAD %s)"),
+   state.detached_from);
+   } else if (state.bisect_in_progress)
strbuf_addf(, _("(no branch, bisect started on %s)"),
state.branch);
else if (state.detached_from) {
-- 
2.17.0.rc0.231.g781580f06

[PATCH v2 2/2] t3200: verify "branch --list" sanity when rebasing from detached HEAD

[Kaartic Sivaraam]

From: Eric Sunshine <sunsh...@sunshineco.com>

"git branch --list" shows an in-progress rebase as:

  * (no branch, rebasing )
master
...

However, if the rebase is started from a detached HEAD, then there is no
, and it would attempt to print a NULL pointer. The previous
commit fixed this problem, so add a test to verify that the output is
sane in this situation.

Note that the "detached HEAD" test case might actually fail in some cases
as the actual output of "git branch --list" might contain remote branch
names which is not considered by the test case as it is rare to happen
in the test environment.

Signed-off-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 t/t3200-branch.sh | 24 
 1 file changed, 24 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 503a88d02..738b5eb22 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -6,6 +6,7 @@
 test_description='git branch assorted tests'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
 
 test_expect_success 'prepare a trivial repository' '
echo Hello >A &&
@@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
--no-merged' '
test_must_fail git branch --merged HEAD --no-merged HEAD
 '
 
+test_expect_success '--list during rebase' '
+   test_when_finished "reset_rebase" &&
+   git checkout master &&
+   FAKE_LINES="1 edit 2" &&
+   export FAKE_LINES &&
+   set_fake_editor &&
+   git rebase -i HEAD~2 &&
+   git branch --list >actual &&
+   test_i18ngrep "rebasing master" actual
+'
+
+test_expect_success '--list during rebase from detached HEAD' '
+   test_when_finished "reset_rebase && git checkout master" &&
+   git checkout HEAD^0 &&
+   oid=$(git rev-parse --short HEAD) &&
+   FAKE_LINES="1 edit 2" &&
+   export FAKE_LINES &&
+   set_fake_editor &&
+   git rebase -i HEAD~2 &&
+   git branch --list >actual &&
+   test_i18ngrep "rebasing detached HEAD $oid" actual
+'
+
 test_expect_success 'tracking with unexpected .fetch refspec' '
rm -rf a b c d &&
git init a &&
-- 
2.17.0.rc0.231.g781580f06

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

[Kaartic Sivaraam]

On Sunday 25 March 2018 11:18 AM, Eric Sunshine wrote:
> On Sun, Mar 25, 2018 at 09:11:34AM +0530, Kaartic Sivaraam wrote:
>> On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:
>>> Can we have a couple new tests: one checking "git branch --list" for
>>> the typical case (when rebasing off a named branch) and one checking
>>> when rebasing from a detached HEAD?
>>
>> Sure, but I guess it would take some time for me to add the tests. I'll
>> send a v2 with the suggested changes.
> 
> A couple more comments:
> 
> * Please run the commit message through a spell checker; it contains
>   several typographical errors.
> 

Thanks for motivating me to search for a spell checker. I have now
discovered the spell check feature (:set spell) in Vim!


> * I wonder if it makes sense to give slightly different output in the
>   detached HEAD case. Normal output is:
> 
>   (no branch, rebasing )
> 
>   and, with your change, detached HEAD output is:
> 
>   (no branch, rebasing d3adb33f)
> 
>   which is okay, but perhaps it could be better; for instance:
> 
>   (no branch, rebasing detached HEAD d3adb33f)
> 

I just recently discovered that the variable used to print information
related to detached HEAD (state.detached_from) might also contain remote
branch names (origin/master, etc.) other than commit hashes. So, it
might make sense to distinguish detached HEAD.


> Anyhow, I wrote the tests for you. When you re-roll, you can make the
> following patch 2/2 and your fix 1/2.
Thanks a lot!


> (If you go with the above idea
> of using a slightly different wording for the detached HEAD case, then
> you'll need to adjust the 'grep' slightly in the second test.)
> 
> --- >8 ---
> From: Eric Sunshine <sunsh...@sunshineco.com>
> Date: Sun, 25 Mar 2018 01:29:58 -0400
> Subject: [PATCH] t3200: verify "branch --list" sanity when rebasing from
>  detached HEAD
> 
> "git branch --list" shows an in-progress rebase as:
> 
>   * (no branch, rebasing )
> master
> ...
> 
> However, if the rebase is started from a detached HEAD, then there is no
> , and it would attempt to print a NULL pointer. The previous
> commit fixed this problem, so add a test to verify that the output is
> sane in this situation.
> 
> Signed-off-by: Eric Sunshine <sunsh...@sunshineco.com>
> ---
>  t/t3200-branch.sh | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 6c0b7ea4ad..d1f80c80ab 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -6,6 +6,7 @@
>  test_description='git branch assorted tests'
>  
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
>  
>  test_expect_success 'prepare a trivial repository' '
>   echo Hello >A &&
> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
> --no-merged' '
>   test_must_fail git branch --merged HEAD --no-merged HEAD
>  '
>  
> +test_expect_success '--list during rebase' '
> + test_when_finished "reset_rebase" &&
> + git checkout master &&
> + FAKE_LINES="1 edit 2" &&
> + export FAKE_LINES &&
> + set_fake_editor &&
> + git rebase -i HEAD~2 &&
> + git branch --list >actual &&
> + grep "rebasing master" actual
> +'
> +
> +test_expect_success '--list during rebase from detached HEAD' '
> + test_when_finished "reset_rebase && git checkout master" &&
> + git checkout HEAD^0 &&
> + oid=$(git rev-parse --short HEAD) &&
> + FAKE_LINES="1 edit 2" &&
> + export FAKE_LINES &&
> + set_fake_editor &&
> + git rebase -i HEAD~2 &&
> + git branch --list >actual &&
> + grep "rebasing $oid" actual
> +'
> +
>  test_expect_success 'tracking with unexpected .fetch refspec' '
>   rm -rf a b c d &&
>   git init a &&
> 


-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

[Kaartic Sivaraam]

On Sunday 25 March 2018 10:03 AM, Jeff King wrote:
> ...
> but I'd prefer to avoid those kinds of magic rules if we can. They're
> very hard to explain to the user, and can be quite baffling when they go
> wrong.
>

I fell the same too.

> IMHO we should do one of:
> 
>   1. Nothing. ;)
> 
>   2. Complain about "-l" in list mode to help educate users about the
>  current craziness.
> 
>   3. Drop "-l" (probably with a deprecation period); it seems unlikely
>  to me that anybody uses it for branch creation, and this would at
>  least reduce the confusion (then it would just be "so why don't we
>  have -l" instead of "why is -l not what I expect").
> 
>   4. Repurpose "-l" as a shortcut for --list (also after a deprecation
>  period). This is slightly more dangerous in that it may confuse
>  people using multiple versions of Git that cross the deprecation
>  line. But that's kind of what the deprecation period is for...
> 

I think we should do 2 as a short term fix for sure. For the long term,
I would prefer 4 as I think most users would expect "-l" to be a
shortcut for "--list" particularly given the current situation that "git
branch -l" lists all the branch names.

That said, I would not mind considering 3 if 4 has more bad consequences
than the good it does (but I heavily doubt it ;-) ).

I don't consider 1 to be an option ;-)


-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

[Kaartic Sivaraam]

On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:
> On Sat, Mar 24, 2018 at 2:38 PM, Kaartic Sivaraam
> <kaartic.sivar...@gmail.com> wrote:
>> When rebasing interacitvely (rebase -i), "git branch -l" prints a line
> 
> The "git branch -l" threw me since "-l" is short for --create-reflog.
> I'm guessing you meant "git branch --list".
> 

That's surprising, I just tried "git branch -l" on a repository and I
did get a list of branch names. Is this a consequence of some option
parsing weirdness ?!

To be honest, I actually assumed "-l" to be a shorthand for "--list" and
didn't check with it in the documentation; which I should have. Sorry,
for that. I still wonder why "git branch -l" prints a list of branch
names when it is not a shorthand for "--list" ? (BTW, I'm also surprised
by the fact that "-l" is not act shorthand for "--list"!)

Regardless, I'll update the commit message to use "--list" in place of "-l".


>> indicating the current branch being rebased. This works well when the
>> interactive rebase was intiated when a local branch is checked out.
>>
>> This doesn't play well when the rebase was initiated on a remote
>> branch or an arbitrary commit that is not pointed to by a local
>> branch.
> 
> A shorter way of saying "arbitrary commit ... not pointed at by local
> branch" would be "detached HEAD".
> 

Thanks. I was actually searching for this word. It didn't strike when I
wrote the commit message, yesterday.


> You could collapse the whole thing back down to:
> 
> strbuf_addf(, _("(no branch, rebasing %s)"),
> state.branch ? state.branch : state.detached_from);
> 
> which means you don't need the 'rebasing' variable or the braces.
> 

Nice point.


> Can we have a couple new tests: one checking "git branch --list" for
> the typical case (when rebasing off a named branch) and one checking
> when rebasing from a detached HEAD?
> 

Sure, but I guess it would take some time for me to add the tests. I'll
send a v2 with the suggested changes.


-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

[PATCH] branch -l: print useful info whilst rebasing a non-local branch

[Kaartic Sivaraam]

When rebasing interacitvely (rebase -i), "git branch -l" prints a line
indicating the current branch being rebased. This works well when the
interactive rebase was intiated when a local branch is checked out.

This doesn't play well when the rebase was initiated on a remote
branch or an arbitrary commit that is not pointed to by a local
branch. In this case "git branch -l" tries to print the name of a
branch using an unintialized variable and thus tries to print a "null
pointer string". As a consequence, it does not provide useful
information while also inducing undefined behaviour.

So, print the commit from which the rebase started when interactive
rebasing a non-local branch.

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 ref-filter.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index f9e25aea7..a4c917c96 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1310,8 +1310,16 @@ char *get_head_description(void)
wt_status_get_state(, 1);
if (state.rebase_in_progress ||
state.rebase_interactive_in_progress)
+   {
+   const char *rebasing = NULL;
+   if (state.branch != NULL)
+   rebasing = state.branch;
+   else
+   rebasing = state.detached_from;
+
strbuf_addf(, _("(no branch, rebasing %s)"),
-   state.branch);
+   rebasing);
+   }
else if (state.bisect_in_progress)
strbuf_addf(, _("(no branch, bisect started on %s)"),
state.branch);
-- 
2.17.0.rc0.231.g781580f06

Re: [PATCH v4 2/3] builtin/branch: give more useful error messages when renaming

[Kaartic Sivaraam]

On Friday 16 March 2018 02:03 AM, Junio C Hamano wrote:
> Quite honestly, I am not sure if this amount of new code that
> results in sentence lego is really worth it.

Speaking specifically about the new code for the sentence lego: I
currently lack knowledge of a better way to achieve the same outcome the
new code does. Let me know if there is a better way.


> Is it so wrong for
> "branch -m tset master" to complain that master already exists so no
> branch can be renamed to it?
>

Speaking in general about the patch itself: though I still find the fact
that "the error about an inexistent source branch seconds the error
about an existing destination branch" to be a little unintuitive, I
actually went on to reboot this after a long time as this also seems to
bring consistency in the error messages related to moving a branch. It
seems that the commit message requires an update as it currently seems
to be misleading as it currently doesn't specify the motivation completely.

That said, I won't be against dropping the patch if it seems to be
adding less value at the cost of more code.

-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 1/3] branch: introduce dont_fail parameter for branchname validation

[Kaartic Sivaraam]

On Friday 16 March 2018 01:57 AM, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivar...@gmail.com> writes:
> 
> So... I am not finding dont_fail that was mentioned on the title
> anywhere else in the patch.  Such lack of attention to detail is
> a bit off-putting.
> 

I'm absolutely sorry x-<

I should have forgotten to update the commit subject during one of the
old rebases in which I renamed the parameter from 'dont_fail' to 'gently'.

I shouldn't have assumed that the commit messages of the old series held
in the reboot too. I should have re-read them "completely" and double
checked it. :-(


>> +enum branch_validation_result {
>> +/* Flags that convey there are fatal errors */
>> +VALIDATION_FATAL_BRANCH_EXISTS_NO_FORCE = -3,
>> +VALIDATION_FATAL_CANNOT_FORCE_UPDATE_CURRENT_BRANCH,
>> +VALIDATION_FATAL_INVALID_BRANCH_NAME,
>> +/* Flags that convey there are no fatal errors */
>> +VALIDATION_PASS_BRANCH_DOESNT_EXIST = 0,
>> +VALIDATION_PASS_BRANCH_EXISTS = 1,
>> +VALIDATION_WARN_BRANCH_EXISTS = 2
>> +};
> 
> where adding new error types will force us to touch _two_ lines
> (i.e. either you add a new error before NO_FORCE with value -4 and
> then remove the "= -3" from NO_FORCE, or you add a new error after
> INVALID, and update NO_FORCE to -4), which can easily be screwed up
> by a careless developer.  The current code is not wrong per-se, but
> I wonder if it can be made less error prone.
> 

At the top of my head I could think of 2 ways to get around this,

- Assigning the actual value to every single entry in the enum.

  This should solve the issue as a any new entry that would be
  added is expected to go "with a value". The compiler would
  warn you in the case of duplicate values. The drawback is: it
  might be a little restrictive and a little ugly. It would also
  likely cause maintenance issues if the number of values in the
  enum get bigger.

  (Of course this doesn't hold if, the careless programmer
   shatters "consistency" and adds an entry without a value to
   the enum and that change gets merged into the codebase ;-) )

- Using non-negative values for both errors and non-errors.

  This might make it hard to distinguish errors from non-errors
  but this would avoid errors completely without much issues,
  otherwise.

I might prefer the former as I find the possibility of the requirement
to distinguish the errors from non-errors to be high when compared with
the possibility of the requirement to add more new entries to the enum.

Any other ideas/suggestions ?

-- 

Kaartic

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

Re: Bug, git rebase -i does not abort correctly

[Kaartic Sivaraam]

On Friday 26 January 2018 02:54 PM, Duy Nguyen wrote:
> 
> Sort of. It smells bad design to me when a mistake can easily become a
> feature. But with your help, I think I should be able to disable this
> feature on my local build. Thanks.
> 

In case you're still facing this issue, it just struck me recently that
you could have a different alias for 'git rebase -i --onto'. In which
case you could possibly avoid falling prey to the syntax issue ;-)
Something like,

$ git config alias.rio 'rebase -i --onto'



-- 
Kaartic

QUOTE
---

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

[PATCH v4 0/3] give more useful error messages while renaming branch (reboot)

[Kaartic Sivaraam]

It's been a long time since the v3 of the patch. So, it's worth restating
the reason behind this patch.

>From v1 of this patch,

 In builtin/branch, the error messages weren't handled directly by the 
branch
 renaming function and was left to the other function. Though this avoids
 redundancy this gave unclear error messages in some cases. So, make
 builtin/branch give more useful error messages.

Changes since v3:

 - Handled more error related to old branch name.

 - Incorporated changes suggested in v3 which include using ';' as a sentence
   connector instead 'and'.

 - Error messages use the interpreted branch names (without the (refs/heads/ 
part).

The unrelated cleanup patches which were in the previous versions have
since been submitted as a separate series and have been merged into
the codebase.

The first two patches are related to the topic of this patch. The 3rd one
is a little typo fix that I noticed on the way.

This patch was based off 'master' and has been rebased to incorporate
the new changes to 'master'. So, it generally should apply cleanly on
'master'. Let me know if it doesn't.

The sample input/output cases for this patch are as follows,

$ git branch
* master
  foo
  bar

Before patch,

# Case 1: Trying to rename non-existent branch
$ git branch -m hypothet no_such_branch
error: refname refs/heads/hypothet not found
fatal: Branch rename failed

# Case 2: Trying to rename non-existent branch to an existing one
$ git branch -m hypothet master
fatal: A branch named 'master' already exists.

# Case 3: Trying to force update current branch
$ git branch -M foo master
fatal: Cannot force update the current branch.

# Case 4: Trying to force rename an in-existent branch with an invalid 
name
$ git branch -M hypothet ?123
fatal: '?123' is not a valid branch name.

After patch,

# Case 1: Trying to rename non-existent branch
$ git branch -m hypothet no_such_branch
fatal: branch 'hypothet' doesn't exist

# Case 2: Trying to rename non-existent branch to an existing one
$ git branch -m hypothet master
fatal: branch 'hypothet' doesn't exist; branch 'master' already exists

# Case 3: Trying to force update current branch
$ git branch -M foo master
fatal: cannot force update the current branch

# Case 4: Trying to force rename an in-existent branch with an invalid 
name
$ git branch -M hypothet ?123
fatal: branch 'hypothet' doesn't exist; new branch name '?123' is 
invalid


Note: Thanks to the strbuf API that made it possible to easily
construct the composite error message strings!

Kaartic Sivaraam (3):
  branch: introduce dont_fail parameter for branchname validation
  builtin/branch: give more useful error messages when renaming
  t/t3200: fix a typo in a test description

 branch.c   |  59 +---
 branch.h   |  61 -
 builtin/branch.c   | 111 ++---
 builtin/checkout.c |   5 +-
 t/t3200-branch.sh  |   2 +-
 5 files changed, 181 insertions(+), 57 deletions(-)

-- 
2.16.1.291.g4437f3f13

[PATCH v4 1/3] branch: introduce dont_fail parameter for branchname validation

[Kaartic Sivaraam]

This parameter allows the branchname validation functions to
optionally return a flag specifying the reason for failure, when
requested. This allows the caller to know why it was about to die.
This allows more useful error messages to be given to the user when
trying to rename a branch.

The flags are specified in the form of an enum and values for success
flags have been assigned explicitly to clearly express that certain
callers rely on those values and they cannot be arbitrary.

Only the logic has been added but no caller has been made to use
it, yet. So, no functional changes.

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 branch.c   | 59 
 branch.h   | 61 +-
 builtin/branch.c   |  4 +--
 builtin/checkout.c |  5 ++--
 4 files changed, 88 insertions(+), 41 deletions(-)

diff --git a/branch.c b/branch.c
index 2672054f0..0eeb8403e 100644
--- a/branch.c
+++ b/branch.c
@@ -178,41 +178,48 @@ int read_branch_desc(struct strbuf *buf, const char 
*branch_name)
return 0;
 }
 
-/*
- * Check if 'name' can be a valid name for a branch; die otherwise.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-int validate_branchname(const char *name, struct strbuf *ref)
+enum branch_validation_result validate_branchname(const char *name, struct 
strbuf *ref, unsigned gently)
 {
-   if (strbuf_check_branch_ref(ref, name))
-   die(_("'%s' is not a valid branch name."), name);
+   if (strbuf_check_branch_ref(ref, name)) {
+   if (gently)
+   return VALIDATION_FATAL_INVALID_BRANCH_NAME;
+   else
+   die(_("'%s' is not a valid branch name."), name);
+   }
 
-   return ref_exists(ref->buf);
+   return ref_exists(ref->buf) ? VALIDATION_PASS_BRANCH_EXISTS : 
VALIDATION_PASS_BRANCH_DOESNT_EXIST;
 }
 
-/*
- * Check if a branch 'name' can be created as a new branch; die otherwise.
- * 'force' can be used when it is OK for the named branch already exists.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+enum branch_validation_result validate_new_branchname(const char *name, struct 
strbuf *ref, int force, unsigned gently)
 {
const char *head;
 
-   if (!validate_branchname(name, ref))
-   return 0;
+   if (gently) {
+   enum branch_validation_result res = validate_branchname(name, 
ref, 1);
+   if (res == VALIDATION_FATAL_INVALID_BRANCH_NAME || res == 
VALIDATION_PASS_BRANCH_DOESNT_EXIST)
+   return res;
+   } else {
+   if (validate_branchname(name, ref, 0) == 
VALIDATION_PASS_BRANCH_DOESNT_EXIST)
+   return VALIDATION_PASS_BRANCH_DOESNT_EXIST;
+   }
 
-   if (!force)
-   die(_("A branch named '%s' already exists."),
-   ref->buf + strlen("refs/heads/"));
+   if (!force) {
+   if (gently)
+   return VALIDATION_FATAL_BRANCH_EXISTS_NO_FORCE;
+   else
+   die(_("A branch named '%s' already exists."),
+   ref->buf + strlen("refs/heads/"));
+   }
 
head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
-   if (!is_bare_repository() && head && !strcmp(head, ref->buf))
-   die(_("Cannot force update the current branch."));
+   if (!is_bare_repository() && head && !strcmp(head, ref->buf)) {
+   if (gently)
+   return 
VALIDATION_FATAL_CANNOT_FORCE_UPDATE_CURRENT_BRANCH;
+   else
+   die(_("Cannot force update the current branch."));
+   }
 
-   return 1;
+   return VALIDATION_WARN_BRANCH_EXISTS;
 }
 
 static int check_tracking_branch(struct remote *remote, void *cb_data)
@@ -259,8 +266,8 @@ void create_branch(const char *name, const char *start_name,
explicit_tracking = 1;
 
if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
-   ? validate_branchname(name, )
-   : validate_new_branchname(name, , force)) {
+   ? validate_branchname(name, , 0)
+   : validate_new_branchname(name, , force, 0)) {
if (!force)
dont_change_ref = 1;
else
diff --git a/branch.h b/branch.h
index 473d0a93e..ee5f1c0e7 100644
--- a/branch.h
+++ b/branch.h
@@ -28,20 +28,59 @@ void create_branch(const char *name, const char *start_name,
   int force, int clobber_head_ok,
   int reflog, int qu

[PATCH v4 2/3] builtin/branch: give more useful error messages when renaming

[Kaartic Sivaraam]

When trying to rename an "inexistent" branch name to a branch name
that "already exists" the rename failed stating that the new branch
name exists rather than stating that the branch trying to be renamed
doesn't exist.

$ git branch -m tset master
fatal: A branch named 'master' already exists.

It's conventional to report that 'tset' doesn't exist rather than
reporting that 'master' exists, the same way the 'mv' command does.

(hypothetical)
$ git branch -m tset master
fatal: branch 'tset' doesn't exist.

That has the problem that the error about an existing branch is shown
only after the user corrects the error about inexistent branch.

$ git branch -m test master
fatal: A branch named 'master' already exists.

This isn't useful either because the user would have corrected this
error in a single go if he had been told this alongside the first
error. So, give more useful error messages by giving errors about old
branch name and new branch name at the same time. This is possible as
the branch name validation functions now return the reason they were
about to die, when requested.

$ git branch -m tset master
fatal: branch 'tset' doesn't exist; branch 'master' already exists

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 builtin/branch.c | 111 +++
 1 file changed, 94 insertions(+), 17 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 5412aa78f..ab78a7f45 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -55,6 +55,13 @@ enum color_branch {
BRANCH_COLOR_UPSTREAM = 5
 };
 
+enum old_branch_validation_result {
+   VALIDATION_1_FATAL_OLD_BRANCH_DOESNT_EXIST = -2,
+   VALIDATION_1_FATAL_INVALID_OLD_BRANCH_NAME = -1,
+   VALIDATION_1_PASS_OLD_BRANCH_EXISTS = 0,
+   VALIDATION_1_WARN_BAD_OLD_BRANCH_NAME = 1
+};
+
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
@@ -458,13 +465,86 @@ static void reject_rebase_or_bisect_branch(const char 
*target)
free_worktrees(worktrees);
 }
 
+static void get_error_msg(struct strbuf* error_msg,
+ const char* oldname, enum 
old_branch_validation_result old_branch_name_res,
+ const char* newname, enum branch_validation_result 
new_branch_name_res)
+{
+   const char* connector_string = "; ";
+   unsigned append_connector = 0;
+
+   switch (old_branch_name_res) {
+   case VALIDATION_1_FATAL_INVALID_OLD_BRANCH_NAME:
+   strbuf_addf(error_msg,
+   _("old branch name '%s' is invalid"), oldname);
+   append_connector = 1;
+   break;
+   case VALIDATION_1_FATAL_OLD_BRANCH_DOESNT_EXIST:
+   strbuf_addf(error_msg,
+   _("branch '%s' doesn't exist"), oldname);
+   append_connector = 1;
+   break;
+
+   /* not necessary to handle nonfatal cases */
+   case VALIDATION_1_PASS_OLD_BRANCH_EXISTS:
+   case VALIDATION_1_WARN_BAD_OLD_BRANCH_NAME:
+   break;
+   }
+
+   switch (new_branch_name_res) {
+   case VALIDATION_FATAL_BRANCH_EXISTS_NO_FORCE:
+   strbuf_addf(error_msg, "%s",
+   (append_connector) ? connector_string : "");
+   strbuf_addf(error_msg,
+   _("branch '%s' already exists"), newname);
+   break;
+   case VALIDATION_FATAL_CANNOT_FORCE_UPDATE_CURRENT_BRANCH:
+   strbuf_addf(error_msg, "%s",
+   (append_connector) ? connector_string : "");
+   strbuf_addstr(error_msg,
+   _("cannot force update the current branch"));
+   break;
+   case VALIDATION_FATAL_INVALID_BRANCH_NAME:
+   strbuf_addf(error_msg, "%s",
+   (append_connector) ? connector_string : "");
+   strbuf_addf(error_msg,
+   _("new branch name '%s' is invalid"), newname);
+   break;
+
+   /* not necessary to handle nonfatal cases */
+   case VALIDATION_PASS_BRANCH_DOESNT_EXIST:
+   case VALIDATION_PASS_BRANCH_EXISTS:
+   case VALIDATION_WARN_BRANCH_EXISTS:
+   break;
+   }
+}
+
+/* Validate the old branch name and return the result */
+static enum old_branch_validation_result validate_old_branchname (const char* 
name, struct strbuf *oldref) {
+   int bad_ref = strbuf_check_branch_ref(oldref, name);
+   int branch_exists = ref_exists(oldref->buf);
+
+   if (bad_ref) {
+   if(branch_exists)
+   return VALIDATION_1_WARN_BAD_OLD_BRANCH_NAME;
+   else
+   return VALIDATION_1_FATAL_INVALID_OL

[PATCH v4 3/3] t/t3200: fix a typo in a test description

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 t/t3200-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 503a88d02..6c0b7ea4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -528,7 +528,7 @@ test_expect_success 'git branch -c -f o/q o/p should work 
when o/p exists' '
git branch -c -f o/q o/p
 '
 
-test_expect_success 'git branch -c qq rr/qq should fail when r exists' '
+test_expect_success 'git branch -c qq rr/qq should fail when rr exists' '
git branch qq &&
git branch rr &&
test_must_fail git branch -c qq rr/qq
-- 
2.16.1.291.g4437f3f13

Re: [PATCH] t/known-leaky: add list of known-leaky test scripts

[Kaartic Sivaraam]

On Wednesday 21 February 2018 02:14 AM, Martin Ågren wrote:
> To sum up: I probably won't be looking into Travis-ing such a blacklist
> in the near future.
> 

Just thinking out loud, how about having a white-list instead of a
black-list and using it to run only those tests in the white list.
Something like,

t/white_list

t
t0001

To run
--

cd t/
for test in $(cat white_list)
do
white_list_tests="$white_list_tests $test*"
done
make SANITIZE=leak $white_list_tests



-- 
Kaartic

QUOTE
---

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] Fix misconversion of gitsubmodule.txt

[Kaartic Sivaraam]

Hi,

On Wednesday 21 February 2018 12:20 AM, Stefan Beller wrote:
> Kaartic was the last to touch that file,
> (as found via git log origin/master -- Documentation/gitsubmodules.txt),
> sorry I did not find this in the review.
> 

"Non-ASCII characters" made me dig into to this a little deeper as I
generally don't use them particularly for text files.

"git blame" points at d48034551 (submodules: overhaul documentation,
2017-06-22) as the offending comment.


> Thanks for the patch!

Yeah, nice catch!


--
Kaartic

QUOTE

“It is impossible to live without failing at something, unless you live
so cautiously that you might as well not have lived at all – in which
case, you fail by default.”

  - J. K. Rowling



signature.asc
Description: OpenPGP digital signature

Re: Bug/comment

[Kaartic Sivaraam]

On Tuesday 30 January 2018 05:32 AM, Andrew Ardill wrote:
> Hi Ilija,
> 
> On 30 January 2018 at 10:21, Ilija Pecelj  wrote:
>> Though it might not be considered a bug 'per se' it is definitely wired.
>> Namely, when you type 'yes' word and hit enter in git bash for widnows, the
>> process enters infinite loop and just prints 'y' letter in new line.
>
> ...
>
> I agree it's a little weird if you have no idea what it's doing, but
> it is very useful and very old, used by many many different scripts
> etc, and so unlikely to change.
>

Just to make things more clear, 'yes' is an UNIX utility (as hinted in
the Wikipedia article link) that might come as part of Cygwin and is not
a part of Git itself.


-- 
Kaartic

QUOTE

“It is impossible to live without failing at something, unless you live
so cautiously that you might as well not have lived at all – in which
case, you fail by default.”

  - J. K. Rowling


WIKIPEDIA: DID YOU KNOW?

Only 33% of internet users in India have heard of Wikipedia !!

* What do you think could be the reason behind this?

* What are ways in which the awareness about Wikipedia in India and
other countries be increased ?

Reference:

* Give your ideas for increasing the awareness of Wikipedia in India and
in other countries and get a Grant from the Wikimedia Foundation to
bring your idea to life.

  https://meta.wikimedia.org/wiki/Grants:IdeaLab/Inspire

* Know more about the awareness problem of Wikipedia

  https://meta.wikimedia.org/wiki/New_Readers/Awareness

  https://meta.wikimedia.org/wiki/New_Readers/Next_steps/Raising_awareness



signature.asc
Description: OpenPGP digital signature

Re: Bug, git rebase -i does not abort correctly

[Kaartic Sivaraam]

Hi,

On Thursday 25 January 2018 05:43 PM, Duy Nguyen wrote:
> rebase scripts are too much for me, so I'll play the user reporting
> bugs this time :)
> 
> Instead of doing
> 
> $ git rebase -i --onto origin/master @~3
> 
> I sometimes accidentally type
> 
> $ git rebase -i origin/master @~3
> 
> ("rebase -i" is actually an alias, which is why I never forget to
type -i)
> 
> Usually the todo list in $EDITOR shows noop, I realize my mistake and
> try to abort it by clearing the todo list before saving and closing
> $EDITOR. The problem is, HEAD is moved away anyway (to origin/master
I
> think) 

For me it left HEAD at @~3. Reading the synopsis of `man git rebase` I
could guess that the corresponding abstract form would be,

$ git rebase -i  


> even if rebase is supposed to abort the operation and leave
> HEAD untouched.
> 

This might seem to be a bug as the comment in "git-rebase-todo" says,

However, if you remove everything, the rebase will be aborted.

But "man git rebase" clearly says,

If  is specified, git rebase will perform an automatic "git
checkout " before doing anything else. Otherwise it remains
on
the current branch.

Junio has previously confirmed that "git rebase [-i] 
" is just a short hand for "git checkout  && git rebase
[-i] ".[ref 1] So, it's not surprising that it left HEAD at
@~3 when you completely removed the contents of git-rebase-todo and
exited the editor.

Does that help solve your issue?


[ref 1]: 

https://public-inbox.org/git/%3cxmqqpo8387hz@gitster.mtv.corp.google.com%3E

-- 
Kaartic

QUOTE

“It is impossible to live without failing at something, unless you live
so cautiously that you might as well not have lived at all – in which
case, you fail by default.”

  - J. K. Rowling


WIKIPEDIA: DID YOU KNOW?

Only 33% of internet users in India have heard of Wikipedia !!

* What do you think could be the reason behind this?

* What are ways in which the awareness about Wikipedia in India and
other countries be increased ?

Reference:

* Give your ideas for increasing the awareness of Wikipedia in India
and
in other countries and get a Grant from the Wikimedia Foundation to
bring your idea to life.

  https://meta.wikimedia.org/wiki/Grants:IdeaLab/Inspire

* Know more about the awareness problem of Wikipedia

  https://meta.wikimedia.org/wiki/New_Readers/Awareness

  https://meta.wikimedia.org/wiki/New_Readers/Next_steps/Raising_awareness
-- 
KaarticHi,

I seem to able to reproduce your issue. More comments inline.

On Thursday 25 January 2018 05:43 PM, Duy Nguyen wrote:
> rebase scripts are too much for me, so I'll play the user reporting
> bugs this time :)
> 
> Instead of doing
> 
> $ git rebase -i --onto origin/master @~3
> 
> I sometimes accidentally type
> 
> $ git rebase -i origin/master @~3
> 
> ("rebase -i" is actually an alias, which is why I never forget to type -i)
> 
> Usually the todo list in $EDITOR shows noop, I realize my mistake and
> try to abort it by clearing the todo list before saving and closing
> $EDITOR. The problem is, HEAD is moved away anyway (to origin/master I
> think) 

For me it left HEAD at @~3. Reading the synopsis of `man git rebase` I
could guess that the corresponding abstract form would be,

$ git rebase -i  


> even if rebase is supposed to abort the operation and leave
> HEAD untouched.
> 

This might seem to be a bug as the comment in "git-rebase-todo" says,

However, if you remove everything, the rebase will be aborted.

But "man git rebase" clearly says,

If  is specified, git rebase will perform an automatic git
checkout  before doing anything else. Otherwise it remains on
   the current branch.
Surprisingly when git-rebase-todo's content is only a "noop" just
closing the editor without removing the contents also seems to be lying
as it checks
-- 
Kaartic

QUOTE

“It is impossible to live without failing at something, unless you live
so cautiously that you might as well not have lived at all – in which
case, you fail by default.”

  - J. K. Rowling


WIKIPEDIA: DID YOU KNOW?

Only 33% of internet users in India have heard of Wikipedia !!

* What do you think could be the reason behind this?

* What are ways in which the awareness about Wikipedia in India and
other countries be increased ?

Reference:

* Give your ideas for increasing the awareness of Wikipedia in India and
in other countries and get a Grant from the Wikimedia Foundation to
bring your idea to life.

  https://meta.wikimedia.org/wiki/Grants:IdeaLab/Inspire

* Know more about the awareness problem of Wikipedia

  https://meta.wikimedia.org/wiki/New_Readers/Awareness

  https://meta.wikimedia.org/wiki/New_Readers/Next_steps/Raising_awareness

Re: [PATCH v3 0/2] Doc/submodules: a few updates

[Kaartic Sivaraam]

On Wednesday 17 January 2018 01:32 AM, Junio C Hamano wrote:
> I had a few "Huh?"
> moments while reading the resulting text, but nothing show-stopping.
> 

It always happens when there are people around who are trying to be over
careful :)

Anyways, it's only now that I remember that I've missed a change that I
thought of doing. The change is about clarifying what a "de-initialized"
submodule means and what an "inactive" submodule means and how they work
together (IIUC, a submodule has not active flag when its deinitialized).
I foresee people confusing 'init' and 'active'. So, I thought the
documentation should be helpful enough in that aspect.
Documenation/gitsubmodules doesn't seem to be talking much about 'init'.
(It talks about 'active' a lot after these patches :)

Now I think it's better to do that as separate change and move this
forward as I don't want to make this clumsy anymore. Please let me know,
if I'm over thinking things again. :)

-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

[PATCH v3 2/2] Doc/git-submodule: improve readability and grammar of a sentence

[Kaartic Sivaraam]

While at it, correctly quote important words.

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/git-submodule.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ff612001d..71c5618e8 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -70,8 +70,8 @@ status [--cached] [--recursive] [--] [...]::
Show the status of the submodules. This will print the SHA-1 of the
currently checked out commit for each submodule, along with the
submodule path and the output of 'git describe' for the
-   SHA-1. Each SHA-1 will be prefixed with `-` if the submodule is not
-   initialized, `+` if the currently checked out submodule commit
+   SHA-1. Each SHA-1 will possibly be prefixed with `-` if the submodule is
+   not initialized, `+` if the currently checked out submodule commit
does not match the SHA-1 found in the index of the containing
repository and `U` if the submodule has merge conflicts.
 +
@@ -132,15 +132,15 @@ expects by cloning missing submodules and updating the 
working tree of
 the submodules. The "updating" can be done in several ways depending
 on command line options and the value of `submodule..update`
 configuration variable. The command line option takes precedence over
-the configuration variable. if neither is given, a checkout is performed.
-update procedures supported both from the command line as well as setting
-`submodule..update`:
+the configuration variable. If neither is given, a 'checkout' is performed.
+The 'update' procedures supported both from the command line as well as
+through the `submodule..update` configuration are:
 
checkout;; the commit recorded in the superproject will be
checked out in the submodule on a detached HEAD.
 +
 If `--force` is specified, the submodule will be checked out (using
-`git checkout --force` if appropriate), even if the commit specified
+`git checkout --force`), even if the commit specified
 in the index of the containing repository already matches the commit
 checked out in the submodule.
 
@@ -150,8 +150,8 @@ checked out in the submodule.
merge;; the commit recorded in the superproject will be merged
into the current branch in the submodule.
 
-The following procedures are only available via the `submodule..update`
-configuration variable:
+The following 'update' procedures are only available via the
+`submodule..update` configuration variable:
 
custom command;; arbitrary shell command that takes a single
argument (the sha1 of the commit recorded in the
-- 
2.16.0.rc1.281.g969645f98

[PATCH v3 1/2] Doc/gitsubmodules: make some changes to improve readability and syntax

[Kaartic Sivaraam]

* Only mention porcelain commands in examples

* Split a sentence for better readability

* Add missing apostrophes

* Clearly specify the advantages of using submodules

* Avoid abbreviations

* Use "Git" consistently

* Improve readability of certain lines

* Clarify when a submodule is considered active

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Helped-by: Stefan Beller <sbel...@google.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 100 +++-
 1 file changed, 79 insertions(+), 21 deletions(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index 46cf120f6..4d6c17782 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -36,8 +36,8 @@ The `gitlink` entry contains the object name of the commit 
that the
 superproject expects the submodule’s working directory to be at.
 
 The section `submodule.foo.*` in the `.gitmodules` file gives additional
-hints to Gits porcelain layer such as where to obtain the submodule via
-the `submodule.foo.url` setting.
+hints to Git's porcelain layer. For example, the `submodule.foo.url`
+setting specifies where to obtain the submodule.
 
 Submodules can be used for at least two different use cases:
 
@@ -51,18 +51,21 @@ Submodules can be used for at least two different use cases:
 
 2. Splitting a (logically single) project into multiple
repositories and tying them back together. This can be used to
-   overcome current limitations of Gits implementation to have
+   overcome current limitations of Git's implementation to have
finer grained access:
 
-* Size of the git repository:
+* Size of the Git repository:
   In its current form Git scales up poorly for large repositories 
containing
   content that is not compressed by delta computation between trees.
-  However you can also use submodules to e.g. hold large binary assets
-  and these repositories are then shallowly cloned such that you do not
+  For example, you can use submodules to hold large binary assets
+  and these repositories can be shallowly cloned such that you do not
   have a large history locally.
 * Transfer size:
   In its current form Git requires the whole working tree present. It
   does not allow partial trees to be transferred in fetch or clone.
+  If the project you work on consists of multiple repositories tied
+  together as submodules in a superproject, you can avoid fetching the
+  working trees of the repositories you are not interested in.
 * Access control:
   By restricting user access to submodules, this can be used to implement
   read/write policies for different users.
@@ -73,9 +76,10 @@ The configuration of submodules
 Submodule operations can be configured using the following mechanisms
 (from highest to lowest precedence):
 
- * The command line for those commands that support taking submodule specs.
-   Most commands have a boolean flag '--recurse-submodules' whether to
-   recurse into submodules. Examples are `ls-files` or `checkout`.
+ * The command line for those commands that support taking submodules
+   as part of their pathspecs. Most commands have a boolean flag
+   `--recurse-submodules` which specify whether to recurse into submodules.
+   Examples are `grep` and `checkout`.
Some commands take enums, such as `fetch` and `push`, where you can
specify how submodules are affected.
 
@@ -87,8 +91,8 @@ Submodule operations can be configured using the following 
mechanisms
 For example an effect from the submodule's `.gitignore` file
 would be observed when you run `git status --ignore-submodules=none` in
 the superproject. This collects information from the submodule's working
-directory by running `status` in the submodule, which does pay attention
-to its `.gitignore` file.
+directory by running `status` in the submodule while paying attention
+to the `.gitignore` file of the submodule.
 +
 The submodule's `$GIT_DIR/config` file would come into play when running
 `git push --recurse-submodules=check` in the superproject, as this would
@@ -97,20 +101,20 @@ remotes are configured in the submodule as usual in the 
`$GIT_DIR/config`
 file.
 
  * The configuration file `$GIT_DIR/config` in the superproject.
-   Typical configuration at this place is controlling if a submodule
-   is recursed into at all via the `active` flag for example.
+   Git only recurses into active submodules (see "ACTIVE SUBMODULES"
+   section below).
 +
 If the submodule is not yet initialized, then the configuration
-inside the submodule does not exist yet, so configuration where to
+inside the submodule does not exist yet, so where to
 obtain the submodule from is configured here for example.
 
- * the `.gitmodules` file inside the superproject. Additionally to the
-   required mapping between submod

[PATCH v3 0/2] Doc/submodules: a few updates

[Kaartic Sivaraam]

Quoting from v1,

These are just a few improvements that I thought would make the 
documentation
related to submodules a little better in various way such as readability,
consistency etc., These were things I noticed while reading thise documents.

Changes since v2:

   - Made some changes suggested by Stefan.
   - A few more that caught my eyes.

Inter diff between v2 and v3:

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 801d291ca..71c5618e8 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -70,8 +70,8 @@ status [--cached] [--recursive] [--] [...]::
Show the status of the submodules. This will print the SHA-1 of the
currently checked out commit for each submodule, along with the
submodule path and the output of 'git describe' for the
-   SHA-1. Each SHA-1 will be prefixed with `-` if the submodule is not
-   initialized, `+` if the currently checked out submodule commit
+   SHA-1. Each SHA-1 will possibly be prefixed with `-` if the submodule is
+   not initialized, `+` if the currently checked out submodule commit
does not match the SHA-1 found in the index of the containing
repository and `U` if the submodule has merge conflicts.
 +
diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index ce2369c2d..47bbc62e8 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -101,7 +101,7 @@ remotes are configured in the submodule as usual in the 
`$GIT_DIR/config`
 file.
 
  * The configuration file `$GIT_DIR/config` in the superproject.
-   Git only recurses into active submodules (see 'ACTIVE SUBMODULES'
+   Git only recurses into active submodules (see "ACTIVE SUBMODULES"
section below).
 +
 If the submodule is not yet initialized, then the configuration
@@ -164,52 +164,59 @@ from another repository.
 To completely remove a submodule, manually delete
 `$GIT_DIR/modules//`.
 
-Active submodules
+ACTIVE SUBMODULES
 -
 
 A submodule is considered active,
 
-  (a) if `submodule..active` is set
+  (a) if `submodule..active` is set to `true`
  or
-  (b) if the submodules path matches the pathspec in `submodule.active`
+  (b) if the submodule's path matches the pathspec in `submodule.active`
  or
   (c) if `submodule..url` is set.
 
+and these are evaluated in this order.
+
 For example:
 
-[submodule "foo"]
-active = false
-url = https://example.org/foo
-[submodule "bar"]
-active = true
-url = https://example.org/bar
-[submodule "baz"]
-url = https://example.org/baz
+  [submodule "foo"]
+active = false
+url = https://example.org/foo
+  [submodule "bar"]
+active = true
+url = https://example.org/bar
+  [submodule "baz"]
+url = https://example.org/baz
 
-In the above config only the submodule bar and baz are active,
-bar due to (a) and baz due to (c).
+In the above config only the submodule 'bar' and 'baz' are active,
+'bar' due to (a) and 'baz' due to (c). 'foo' is inactive because
+(a) takes precedence over (c).
 
-Note that '(c)' is a historical artefact and will be ignored if the
-pathspec set in (b) excludes the submodule. For example:
+Note that (c) is a historical artefact and will be ignored if the
+(a) and (b) specify that the submodule is not active. In other words,
+if we have an `submodule..active` set to `false` or if the
+submodule's path is excluded in the pathspec in `submodule.active`, the
+url doesn't matter whether it is present or not. This is illustrated in
+the example that follows.
 
-[submodule "foo"]
-active = true
-url = https://example.org/foo
-[submodule "bar"]
-url = https://example.org/bar
-[submodule "baz"]
-url = https://example.org/baz
-[submodule "bob"]
-ignore = true
-[submodule]
-active = b*
-active = (:exclude) baz
+  [submodule "foo"]
+active = true
+url = https://example.org/foo
+  [submodule "bar"]
+url = https://example.org/bar
+  [submodule "baz"]
+url = https://example.org/baz
+  [submodule "bob"]
+ignore = true
+  [submodule]
+active = b*
+active = :(exclude) baz
 
-In here all submodules except baz (foo, bar, bob) are active.
+In here all submodules except 'baz' (foo, bar, bob) are active.
 'foo' due to its own active flag and all the others due to the
 submodule active pathspec, which specifies that any submodule
-starting with 'b' except 'baz' are also active, no matter if
-the .url field is present.
+starting with 'b' except 'baz' are also active, regardless of the
+presence of the .url field.
 
 Workflow for a third party library
 --



Kaartic Sivaraam (2):
  Doc/gitsubmodules: make som

[PATCH v2 0/2] Doc/submodules: a few updates

[Kaartic Sivaraam]

+} directory
may be there as after deinitializing the [-git-]{+Git+} directory is kept 
around.
The directory which is supposed to be the working directory is empty instead.
+
A submodule can be deinitialized by running `git submodule deinit`.
@@ -164,6 +164,53 @@ from another repository.
To completely remove a submodule, manually delete
`$GIT_DIR/modules//`.

{+Active submodules+}
{+-+}

{+A submodule is considered active,+}

{+  (a) if `submodule..active` is set+}
{+ or+}
{+  (b) if the submodules path matches the pathspec in `submodule.active`+}
{+ or+}
{+  (c) if `submodule..url` is set.+}

{+For example:+}

{+[submodule "foo"]+}
{+active = false+}
{+url = https://example.org/foo+}
{+[submodule "bar"]+}
{+active = true+}
{+url = https://example.org/bar+}
{+[submodule "baz"]+}
{+url = https://example.org/baz+}

{+In the above config only the submodule bar and baz are active,+}
{+bar due to (a) and baz due to (c).+}

{+Note that '(c)' is a historical artefact and will be ignored if the+}
{+pathspec set in (b) excludes the submodule. For example:+}

{+[submodule "foo"]+}
{+active = true+}
{+url = https://example.org/foo+}
{+[submodule "bar"]+}
{+url = https://example.org/bar+}
{+[submodule "baz"]+}
{+url = https://example.org/baz+}
{+[submodule "bob"]+}
{+ignore = true+}
{+[submodule]+}
{+active = b*+}
{+active = (:exclude) baz+}

{+In here all submodules except baz (foo, bar, bob) are active.+}
{+'foo' due to its own active flag and all the others due to the+}
{+submodule active pathspec, which specifies that any submodule+}
{+starting with 'b' except 'baz' are also active, no matter if+}
{+the .url field is present.+}

Workflow for a third party library
--



Kaartic Sivaraam (2):
  Doc/gitsubmodules: make some changes to improve readability and syntax
  Doc/git-submodule: improve readability and grammar of a sentence

 Documentation/git-submodule.txt | 12 +++---
 Documentation/gitsubmodules.txt | 93 +++--
 2 files changed, 78 insertions(+), 27 deletions(-)

-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH v2 2/2] Doc/git-submodule: improve readability and grammar of a sentence

[Kaartic Sivaraam]

While at it, correctly quote important words.

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/git-submodule.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ff612001d..801d291ca 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -132,15 +132,15 @@ expects by cloning missing submodules and updating the 
working tree of
 the submodules. The "updating" can be done in several ways depending
 on command line options and the value of `submodule..update`
 configuration variable. The command line option takes precedence over
-the configuration variable. if neither is given, a checkout is performed.
-update procedures supported both from the command line as well as setting
-`submodule..update`:
+the configuration variable. If neither is given, a 'checkout' is performed.
+The 'update' procedures supported both from the command line as well as
+through the `submodule..update` configuration are:
 
checkout;; the commit recorded in the superproject will be
checked out in the submodule on a detached HEAD.
 +
 If `--force` is specified, the submodule will be checked out (using
-`git checkout --force` if appropriate), even if the commit specified
+`git checkout --force`), even if the commit specified
 in the index of the containing repository already matches the commit
 checked out in the submodule.
 
@@ -150,8 +150,8 @@ checked out in the submodule.
merge;; the commit recorded in the superproject will be merged
into the current branch in the submodule.
 
-The following procedures are only available via the `submodule..update`
-configuration variable:
+The following 'update' procedures are only available via the
+`submodule..update` configuration variable:
 
custom command;; arbitrary shell command that takes a single
argument (the sha1 of the commit recorded in the
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH v2 1/2] Doc/gitsubmodules: make some changes to improve readability and syntax

[Kaartic Sivaraam]

* Only mention porcelain commands in examples

* Split a sentence for better readability

* Add missing apostrophes

* Clearly specify the advantages of using submodules

* Avoid abbreviations

* Use "Git" consistently

* Improve readability of certain lines

* Clarify when a submodule is considered active

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Helped-by: Stefan Beller <sbel...@google.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 93 +++--
 1 file changed, 72 insertions(+), 21 deletions(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index 46cf120f6..ce2369c2d 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -36,8 +36,8 @@ The `gitlink` entry contains the object name of the commit 
that the
 superproject expects the submodule’s working directory to be at.
 
 The section `submodule.foo.*` in the `.gitmodules` file gives additional
-hints to Gits porcelain layer such as where to obtain the submodule via
-the `submodule.foo.url` setting.
+hints to Git's porcelain layer. For example, the `submodule.foo.url`
+setting specifies where to obtain the submodule.
 
 Submodules can be used for at least two different use cases:
 
@@ -51,18 +51,21 @@ Submodules can be used for at least two different use cases:
 
 2. Splitting a (logically single) project into multiple
repositories and tying them back together. This can be used to
-   overcome current limitations of Gits implementation to have
+   overcome current limitations of Git's implementation to have
finer grained access:
 
-* Size of the git repository:
+* Size of the Git repository:
   In its current form Git scales up poorly for large repositories 
containing
   content that is not compressed by delta computation between trees.
-  However you can also use submodules to e.g. hold large binary assets
-  and these repositories are then shallowly cloned such that you do not
+  For example, you can use submodules to hold large binary assets
+  and these repositories can be shallowly cloned such that you do not
   have a large history locally.
 * Transfer size:
   In its current form Git requires the whole working tree present. It
   does not allow partial trees to be transferred in fetch or clone.
+  If the project you work on consists of multiple repositories tied
+  together as submodules in a superproject, you can avoid fetching the
+  working trees of the repositories you are not interested in.
 * Access control:
   By restricting user access to submodules, this can be used to implement
   read/write policies for different users.
@@ -73,9 +76,10 @@ The configuration of submodules
 Submodule operations can be configured using the following mechanisms
 (from highest to lowest precedence):
 
- * The command line for those commands that support taking submodule specs.
-   Most commands have a boolean flag '--recurse-submodules' whether to
-   recurse into submodules. Examples are `ls-files` or `checkout`.
+ * The command line for those commands that support taking submodules
+   as part of their pathspecs. Most commands have a boolean flag
+   `--recurse-submodules` which specify whether to recurse into submodules.
+   Examples are `grep` and `checkout`.
Some commands take enums, such as `fetch` and `push`, where you can
specify how submodules are affected.
 
@@ -87,8 +91,8 @@ Submodule operations can be configured using the following 
mechanisms
 For example an effect from the submodule's `.gitignore` file
 would be observed when you run `git status --ignore-submodules=none` in
 the superproject. This collects information from the submodule's working
-directory by running `status` in the submodule, which does pay attention
-to its `.gitignore` file.
+directory by running `status` in the submodule while paying attention
+to the `.gitignore` file of the submodule.
 +
 The submodule's `$GIT_DIR/config` file would come into play when running
 `git push --recurse-submodules=check` in the superproject, as this would
@@ -97,20 +101,20 @@ remotes are configured in the submodule as usual in the 
`$GIT_DIR/config`
 file.
 
  * The configuration file `$GIT_DIR/config` in the superproject.
-   Typical configuration at this place is controlling if a submodule
-   is recursed into at all via the `active` flag for example.
+   Git only recurses into active submodules (see 'ACTIVE SUBMODULES'
+   section below).
 +
 If the submodule is not yet initialized, then the configuration
-inside the submodule does not exist yet, so configuration where to
+inside the submodule does not exist yet, so where to
 obtain the submodule from is configured here for example.
 
- * the `.gitmodules` file inside the superproject. Additionally to the
-   required mapping between submodule's name

Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Kaartic Sivaraam]

On Wednesday 10 January 2018 01:01 AM, Stefan Beller wrote:
  The submodule's `$GIT_DIR/config` file would come into play when running
  `git push --recurse-submodules=check` in the superproject, as this would
 @@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
 configuration
  inside the submodule does not exist yet, so configuration where to
  obtain the submodule from is configured here for example.

>>
>> I caught this in the context while replying. "so configuration where to
>> obtain the submodule from is configured here for example." doesn't seem
>> to read well. Maybe removing configuration from the sentence will make
>> it sound better?
>>

I'm going to make this change.


>>
 - * the `.gitmodules` file inside the superproject. Additionally to the
 -   required mapping between submodule's name and path, a project usually
 + * The `.gitmodules` file inside the superproject. Additionally, if 
 mapping
 +   is required between a submodule's name and its path, a project usually
>>>
>>> This changes meaning, originally it tries to say:
>>>
>>> * it requires mapping path <-> names.
>>
>> I get this ...
>>
>>> * but there can be more.
>>
>> ... but not this. Did the previous version really try to say this?
>> Anyways how does this sound?
> 
> Well that was me being very sloppy trying to say that there might be
> submodule..{url, ignored, shallow} settings which just happen to
> be there.
> 
>>   * The `.gitmodules` file inside the superproject. A project usually
>> uses this file to suggest defaults for the upstream collection
>> of repositories for the mapping that is required between a
>> submodule's name and its path.
>>
>> I think it conveys the "it requires mapping path <-> names." correctly
>> but doesn't convey the "but there can be more." part. I'm not sure how
>> to get that into the sentence, correctly.
> 
> I did not consider that part the important part, hence my sloppiness.
> Sorry for the confusion.
> 
> My main point was to say that the mapping is the important part and
> must be found in the .gitmodules file, otherwise we do not consider
> it a submodule (for whatever "it" is, maybe a gitlink at a path=name).
> 

So, I'm going to use the version that I specified above as I think it
seems to convey that clearly (at least to me),

The `.gitmodules` file inside the superproject. A project usually
uses this file to suggest defaults for the upstream collection
of repositories for the mapping that is required between a
submodule's name and its path.

Let me know, if there are issues.

Thanks,
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/8] Doc/gitsubmodules: avoid abbreviations

[Kaartic Sivaraam]

On Wednesday 10 January 2018 12:56 AM, Stefan Beller wrote:
> On Tue, Jan 9, 2018 at 8:06 AM, Kaartic Sivaraam
> <kaartic.sivar...@gmail.com> wrote:
>> On Tuesday 09 January 2018 12:15 AM, Stefan Beller wrote:
>>>>
>>>> - * The command line for those commands that support taking submodule 
>>>> specs.
>>>
>>> ++ The command line for those commands that support taking submodules
>>> as part of their pathspecs[1].
>>> ++
>>> ++[1] pathspec is an official term according to `man gitglossary`.
>>>
>>> Maybe?
>>>
>>
>> Yeah, I actually did think 'specification' wasn't a the best fit for
>> this (should have mentioned that somewhere) Now, the real term came out :)
>>
>> Just to be sure, that "[1] pathspec ..." part goes to the end of the
>> document doesn't it?
> 
> The [1] part was just to highlight it for the sake of this discussion;
> I would not include it into the man page.
> 

That '++' before it made me think otherwise :)



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/8] Doc/gitsubmodules: specify how submodules help in reduced size

[Kaartic Sivaraam]

On Wednesday 10 January 2018 12:31 AM, Stefan Beller wrote:
> On Tue, Jan 9, 2018 at 8:01 AM, Kaartic Sivaraam
> <kaartic.sivar...@gmail.com> wrote:
>> On Tuesday 09 January 2018 12:08 AM, Stefan Beller wrote:
>>>> diff --git a/Documentation/gitsubmodules.txt 
>>>> b/Documentation/gitsubmodules.txt
>>>> index cb795c6b6..3f73983d5 100644
>>>> --- a/Documentation/gitsubmodules.txt
>>>> +++ b/Documentation/gitsubmodules.txt
>>>> @@ -63,6 +63,9 @@ Submodules can be used for at least two different use 
>>>> cases:
>>>>  * Transfer size:
>>>>In its current form Git requires the whole working tree present. It
>>>>does not allow partial trees to be transferred in fetch or clone.
>>>> +  If you have your project as multiple repositories tied together as
>>>> +  submodules in a superproject, you can avoid fetching the working
>>>> +  trees of the repositories you are not interested in.
>>>
>>> You do not fetch a working tree, but a whole repository?
>>>
>>
>> Maybe I misunderstood submodules when I wrote that example. Could you
>> help out with a better and precise replacement?
> 
> If your project consists of multiple repositories tied together, some 
> submodules
> may not be of interest for all users, who do not need to fetch such submodule
> repositories.
> 

OK, now I get why I couldn't get your point. I actually was thinking of
the version of the message I had tweaked for v2 when reading your
message. It doesn't have the confusing meaning. It actually reads,

   ...
   If the project you work on consists of multiple repositories tied
   together as submodules in a superproject, you can avoid fetching the
   working trees of the repositories you are not interested in.

So, my version takes the perspective of the person who gains the
advantage of having to clone unnecessary repos. And yours, the
perspective of the person who gives the consumer of the repo that
advantage. Both sound nice to me. But if mine doesn't sound nice to you,
let me know so that I could replace it with your version.


>> Just putting in some context as to why I did this change, I thought this
>> was the only thing that lacked an example and wanted to make it consistent.
> 
> Oh, sure I like the example; I was just worried about the wording, as a 
> worktree
> is part of a repository, and the repository is the whole thing. In the
> current situation
> you can only fetch all-or-nothing, specifically you cannot fetch "just
> the worktree"
> (a shallow clone/fetch is the closest to that, but that still has the
> same amount of
> information the .git dir, than in the working tree)
> 

I get that!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 0/8] Doc/submodules: a few updates

[Kaartic Sivaraam]

On Tuesday 09 January 2018 12:38 AM, Stefan Beller wrote:
> On Sat, Jan 6, 2018 at 10:46 AM, Kaartic Sivaraam
> <kaartic.sivar...@gmail.com> wrote:
> 
> While small patches are really appreciated for code (bisect, automated
> testing, and
> the general difficulty to reason about code, as a very small change
> may affect the whole
> code base), I am not sure if they benefit in documentation.
> Documentation is a rather
> local human readable thing, so by changing one sentence we don't
> affect the understanding
> of documentation at a completely unrelated place.
> 
> Also it helps to read more than just sentence fragments, i.e. I tried
> looking at the
> whole paragraph for review. May I suggest to squash them all and
> resend as one patch?
> 

I wouldn't mind that. I thought it might be easy to find to find the
parts I changed when the patches are small. So, I sent them without
squashing them together. In case you feel it's not worth, let me know so
I'll squash them in.

BTW, in case I did squash them in, would it be nice to keep the commit
subjects of the current patch series as bullet points in the unified
commit message?


> 
>>
>> I based these patches on top of 'master'.
> 
> I am not aware of other submodule patches affecting documentation in 
> master..pu,
> so this should be easy to merge.
> 
>>
>> Apart from the changes, I saw a few things that needed 
>> improvement/clarification
>> but wasn't able to do that myself due to my limited knowledge of submodules. 
>> They
>> are listed below. I'll add in patches for them if they are correctly 
>> clarified.
>>
>>
>> 1.
>>
>>  man gitsubmodules
>>
>>·   The configuration file $GIT_DIR/config in the superproject. 
>> Typical configuration at this place is controlling if a submodule is
>>recursed into at all via the active flag for example.
>>
>>If the submodule is not yet initialized, then the configuration 
>> inside the submodule does not exist yet, so configuration where to
>>obtain the submodule from is configured here for example.
>>
>> What's the "active flag" mentioned above? Also I find the phrase "is 
>> recursed into at all"
>> to be a little slippery. How could it be improved?
> 
> There are multiple ways to indicate if a submodule is "active", i.e. if Git is
> supposed to pay attentio. Historically we had to set the
> submodule..url flag in the config, but last year Brandon added
> submodule.active as well as submodule..active which supersede
> the .url flag.
> 
> (See is_submodule_active() in submodule.c to see the definitive answer to
> "should Git pay attention?")
> https://github.com/git/git/blob/master/submodule.c#L224
> 

Thanks for the info!


> I wonder if this indicates a lack of documentation when the active
> flags were introduced.
> They are found in 'man git config', but maybe we need to spell them
> out explicitly
> in the submodule related docs.
> 

Possibly. So, why not in Documentation/gitsubmodules! Here's a replaced
version of that paragraph,

* The configuration file `$GIT_DIR/config` in the superproject.
   Typically this file is used to specify whether the submodule
   is recursed into at all via the `active` flag for example. A
   submodule is considered active if `submodule..url` is set
   or if the submodules path is present in `submodule.active` or
   if `submodule..url` is set.


>> 2.
>>
>>  man git submodule
>>
>>update
>>...
>>
>>checkout
>>
>>
>>If --force is specified, the submodule will be checked out 
>> (using git checkout --force if appropriate), even if the commit
>>specified in the index of the containing repository already 
>> matches the commit checked out in the submodule.
>>
>> I'm not sure this is conveying all the information it should be conveying.
>> It seems to making the user wonder, "How at all does 'git submodule update 
>> --force'
>> differs from 'git submodule update'?" also "using git checkout --force if 
>> appropriate"
>> seems to be invoking all sorts confusion as "appropriate" is superfluous.
> 
> When "submodule update" is invoked with the `--force` flag, that flag is 
> passed
> on to the 'checkout' operation. If you do not give the --force, then
> the checkout
> will also be done without --force.
> 

If that's the case then shouldn't the "if appropriate" part of "(using
git checkout --force if appropriate)" be dropped? That seems to make it
clear, at least for me. Or is intended as '--force' will not be passed
to git checkout all the time?

>>
>> How could these confusions be clarified?
> 
> I tried giving an alternative snippet above, not sure how else to tell.
> 



-- 
Kaartic

Quote: "Be creative. Be adventurous. Be original. And above all else, be
young." - Wonder Woman



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Kaartic Sivaraam]

On Tuesday 09 January 2018 12:19 AM, Stefan Beller wrote:
> On Sat, Jan 6, 2018 at 10:46 AM, Kaartic Sivaraam
> <kaartic.sivar...@gmail.com> wrote:
>>
>> - * The command line for those commands that support taking submodule
>> -   specifications. Most commands have a boolean flag '--recurse-submodules
>> -   whether to recurse into submodules. Examples are `ls-files` or 
>> `checkout`.
>> + * The command line arguments of those commands that support taking 
>> submodule
>> +   specifications. Most commands have a boolean flag '--recurse-submodules'
>> +   which specify whether they should recurse into submodules. Examples are
>> +   `ls-files` or `checkout`.
>> Some commands take enums, such as `fetch` and `push`, where you can
>> specify how submodules are affected.
>>
>> @@ -90,8 +91,8 @@ Submodule operations can be configured using the following 
>> mechanisms
>>  For example an effect from the submodule's `.gitignore` file
>>  would be observed when you run `git status --ignore-submodules=none` in
>>  the superproject. This collects information from the submodule's working
>> -directory by running `status` in the submodule, which does pay attention
>> -to its `.gitignore` file.
>> +directory by running `status` in the submodule while paying attention
>> +to the `.gitignore` file of the submodule.
> 
> Both are grammatically correct and expressive, thanks!
>

You're welcome!


>>  +
> 
> Extra spurious line?
>

No. That's a "real" plus in the document that's usually present between
paragraphs :) I think I now get why Junio suggests people to review
patches in context (possibly, by applying them) ;)


>>  The submodule's `$GIT_DIR/config` file would come into play when running
>>  `git push --recurse-submodules=check` in the superproject, as this would
>> @@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
>> configuration
>>  inside the submodule does not exist yet, so configuration where to
>>  obtain the submodule from is configured here for example.
>>

I caught this in the context while replying. "so configuration where to
obtain the submodule from is configured here for example." doesn't seem
to read well. Maybe removing configuration from the sentence will make
it sound better?


>> - * the `.gitmodules` file inside the superproject. Additionally to the
>> -   required mapping between submodule's name and path, a project usually
>> + * The `.gitmodules` file inside the superproject. Additionally, if mapping
>> +   is required between a submodule's name and its path, a project usually
> 
> This changes meaning, originally it tries to say:
> 
> * it requires mapping path <-> names.

I get this ...

> * but there can be more.
> 

... but not this. Did the previous version really try to say this?
Anyways how does this sound?

  * The `.gitmodules` file inside the superproject. A project usually
uses this file to suggest defaults for the upstream collection
of repositories for the mapping that is required between a
submodule's name and its path.

I think it conveys the "it requires mapping path <-> names." correctly
but doesn't convey the "but there can be more." part. I'm not sure how
to get that into the sentence, correctly.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/8] Doc/gitsubmodules: avoid abbreviations

[Kaartic Sivaraam]

On Tuesday 09 January 2018 12:15 AM, Stefan Beller wrote:
>>
>> - * The command line for those commands that support taking submodule specs.
> 
> ++ The command line for those commands that support taking submodules
> as part of their pathspecs[1].
> ++
> ++[1] pathspec is an official term according to `man gitglossary`.
> 
> Maybe?
> 

Yeah, I actually did think 'specification' wasn't a the best fit for
this (should have mentioned that somewhere) Now, the real term came out :)

Just to be sure, that "[1] pathspec ..." part goes to the end of the
document doesn't it?


>> -   Most commands have a boolean flag '--recurse-submodules' whether to
>> -   recurse into submodules. Examples are `ls-files` or `checkout`.
>> + * The command line for those commands that support taking submodule
>> +   specifications. Most commands have a boolean flag '--recurse-submodules
>> +   whether to recurse into submodules. Examples are `ls-files` or 
>> `checkout`.
>> Some commands take enums, such as `fetch` and `push`, where you can
>> specify how submodules are affected.
>>
>> --
>> 2.16.0.rc0.223.g4a4ac8367
>>




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/8] Doc/gitsubmodules: specify how submodules help in reduced size

[Kaartic Sivaraam]

On Tuesday 09 January 2018 12:08 AM, Stefan Beller wrote:
>> diff --git a/Documentation/gitsubmodules.txt 
>> b/Documentation/gitsubmodules.txt
>> index cb795c6b6..3f73983d5 100644
>> --- a/Documentation/gitsubmodules.txt
>> +++ b/Documentation/gitsubmodules.txt
>> @@ -63,6 +63,9 @@ Submodules can be used for at least two different use 
>> cases:
>>  * Transfer size:
>>In its current form Git requires the whole working tree present. It
>>does not allow partial trees to be transferred in fetch or clone.
>> +  If you have your project as multiple repositories tied together as
>> +  submodules in a superproject, you can avoid fetching the working
>> +  trees of the repositories you are not interested in.
> 
> You do not fetch a working tree, but a whole repository?
> 

Maybe I misunderstood submodules when I wrote that example. Could you
help out with a better and precise replacement?

Just putting in some context as to why I did this change, I thought this
was the only thing that lacked an example and wanted to make it consistent.


-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/8] Doc/gitsubmodules: clearly specify advantage of submodule

[Kaartic Sivaraam]

>>content that is not compressed by delta computation between trees.
>> -  However you can also use submodules to e.g. hold large binary assets
>> +  Therefore you can use submodules to hold large binary assets
> 
> If this improves readability by a lot, I'd be all for it. But this use
> case is just
> exemplary. There are also cases of submodules that do not contain big files,
> but e.g. have a lengthy history with lots of small files.
> So I don't know, as I would want to keep emphasized that this is just
> an example.
> 

You're right I actually screwed up by emphasizing that was the only use
case it was envisioned for. Fixed it by replacing "Therefore" with " For
example, ..."

Thanks,
Kaartic



signature.asc
Description: OpenPGP digital signature

[BUG] "git stash -p" doesn't work well when splitting nearby changes

[Kaartic Sivaraam]

I recently made two changes to nearby lines (approx. 3 lines b/w them)
in a file and wanted to stash one part of it and not the other. I tried
to use "git stash -p" for this. I split the hunk of concern (of course,
they came out as a single hunk) and gave 'y' for one and 'n' for the
other. The stash was created successfully but the discarding that change
from the work tree failed with the following error,

error: patch failed: Documentation/gitsubmodules.txt:57
error: Documentation/gitsubmodules.txt: patch does not apply
Cannot remove worktree changes

I guess this a bug that has been for a very long time now (I generally
use the one built from 'next' but this error occurs even if I use 2.11)?
Or is this intentional? I couldn't glean any reason for that, though.

In case you need a reproduction recipe,

git init stash-patch-test &&
cd stash-patch-test &&
echo 'Let us put some lines to ensure
Git would allow us to split
a change we make
to this text.
It should be a little longer, I guess.
A little more.
Almost there.
And DONE.' >test-files &&
git add . &&
git commit -m "Initial commit"


Apply this patch or do something similar to what it does,

diff --git a/test-files b/test-files
index daa67cf..d3142e7 100644
--- a/test-files
+++ b/test-files
@@ -1,7 +1,7 @@
-Let us put some lines to ensure
+Let's put some lines to ensure
 Git would allow us to split
 a change we make
-to this text.
+to this text
 It should be a little longer, I guess.
 A little more.
 Almost there.

Then,

$ git stash -p
diff --git a/test-files b/test-files
index daa67cf..d3142e7 100644
--- a/test-files
+++ b/test-files
@@ -1,7 +1,7 @@
-Let us put some lines to ensure
+Let's put some lines to ensure
 Git would allow us to split
 a change we make
-to this text.
+to this text
 It should be a little longer, I guess.
 A little more.
 Almost there.
Stash this hunk [y,n,q,a,d,/,s,e,?]? s
Split into 2 hunks.
@@ -1,3 +1,3 @@
-Let us put some lines to ensure
+Let's put some lines to ensure
 Git would allow us to split
 a change we make
Stash this hunk [y,n,q,a,d,/,j,J,g,e,?]? n
@@ -2,6 +2,6 @@
 Git would allow us to split
 a change we make
-to this text.
+to this text
 It should be a little longer, I guess.
 A little more.
 Almost there.
Stash this hunk [y,n,q,a,d,/,K,g,e,?]? y

Saved working directory and index state WIP on master: f0dff20 Initial
commit
error: patch failed: test-files:1
error: test-files: patch does not apply
Cannot remove worktree changes



-- 
Kaartic

Quote: "Be creative. Be adventurous. Be original. And above all else, be
young." - Wonder Woman



signature.asc
Description: OpenPGP digital signature

[PATCH 8/8] Doc/git-submodule: correctly quote important words

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/git-submodule.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index befbccde6..5c4d941cc 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -132,8 +132,8 @@ expects by cloning missing submodules and updating the 
working tree of
 the submodules. The "updating" can be done in several ways depending
 on command line options and the value of `submodule..update`
 configuration variable. The command line option takes precedence over
-the configuration variable. If neither is given, a checkout is performed.
-The update procedures supported both from the command line as well as
+the configuration variable. If neither is given, a 'checkout' is performed.
+The 'update' procedures supported both from the command line as well as
 through the `submodule..update` configuration are:
 
checkout;; the commit recorded in the superproject will be
@@ -150,8 +150,8 @@ checked out in the submodule.
merge;; the commit recorded in the superproject will be merged
into the current branch in the submodule.
 
-The following procedures are only available via the `submodule..update`
-configuration variable:
+The following 'update' procedures are only available via the
+`submodule..update` configuration variable:
 
custom command;; arbitrary shell command that takes a single
argument (the sha1 of the commit recorded in the
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 6/8] Doc/gitsubmodules: improve readability of certain lines

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index 745a3838e..339fb73db 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -76,9 +76,10 @@ The configuration of submodules
 Submodule operations can be configured using the following mechanisms
 (from highest to lowest precedence):
 
- * The command line for those commands that support taking submodule
-   specifications. Most commands have a boolean flag '--recurse-submodules
-   whether to recurse into submodules. Examples are `ls-files` or `checkout`.
+ * The command line arguments of those commands that support taking submodule
+   specifications. Most commands have a boolean flag '--recurse-submodules'
+   which specify whether they should recurse into submodules. Examples are
+   `ls-files` or `checkout`.
Some commands take enums, such as `fetch` and `push`, where you can
specify how submodules are affected.
 
@@ -90,8 +91,8 @@ Submodule operations can be configured using the following 
mechanisms
 For example an effect from the submodule's `.gitignore` file
 would be observed when you run `git status --ignore-submodules=none` in
 the superproject. This collects information from the submodule's working
-directory by running `status` in the submodule, which does pay attention
-to its `.gitignore` file.
+directory by running `status` in the submodule while paying attention
+to the `.gitignore` file of the submodule.
 +
 The submodule's `$GIT_DIR/config` file would come into play when running
 `git push --recurse-submodules=check` in the superproject, as this would
@@ -107,13 +108,13 @@ If the submodule is not yet initialized, then the 
configuration
 inside the submodule does not exist yet, so configuration where to
 obtain the submodule from is configured here for example.
 
- * the `.gitmodules` file inside the superproject. Additionally to the
-   required mapping between submodule's name and path, a project usually
+ * The `.gitmodules` file inside the superproject. Additionally, if mapping
+   is required between a submodule's name and its path, a project usually
uses this file to suggest defaults for the upstream collection
of repositories.
 +
-This file mainly serves as the mapping between name and path in
-the superproject, such that the submodule's Git directory can be
+This file mainly serves as the mapping between the name and path of submodules
+in the superproject, such that the submodule's Git directory can be
 located.
 +
 If the submodule has never been initialized, this is the only place
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 7/8] Doc/git-submodule: improve readability and grammar of a sentence

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/git-submodule.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ff612001d..befbccde6 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -132,9 +132,9 @@ expects by cloning missing submodules and updating the 
working tree of
 the submodules. The "updating" can be done in several ways depending
 on command line options and the value of `submodule..update`
 configuration variable. The command line option takes precedence over
-the configuration variable. if neither is given, a checkout is performed.
-update procedures supported both from the command line as well as setting
-`submodule..update`:
+the configuration variable. If neither is given, a checkout is performed.
+The update procedures supported both from the command line as well as
+through the `submodule..update` configuration are:
 
checkout;; the commit recorded in the superproject will be
checked out in the submodule on a detached HEAD.
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 4/8] Doc/gitsubmodules: avoid abbreviations

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index 3f73983d5..e3c798d2a 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -76,9 +76,9 @@ The configuration of submodules
 Submodule operations can be configured using the following mechanisms
 (from highest to lowest precedence):
 
- * The command line for those commands that support taking submodule specs.
-   Most commands have a boolean flag '--recurse-submodules' whether to
-   recurse into submodules. Examples are `ls-files` or `checkout`.
+ * The command line for those commands that support taking submodule
+   specifications. Most commands have a boolean flag '--recurse-submodules
+   whether to recurse into submodules. Examples are `ls-files` or `checkout`.
Some commands take enums, such as `fetch` and `push`, where you can
specify how submodules are affected.
 
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 5/8] Doc/gitsubmodules: use "Git directory" consistently

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index e3c798d2a..745a3838e 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -113,7 +113,7 @@ obtain the submodule from is configured here for example.
of repositories.
 +
 This file mainly serves as the mapping between name and path in
-the superproject, such that the submodule's git directory can be
+the superproject, such that the submodule's Git directory can be
 located.
 +
 If the submodule has never been initialized, this is the only place
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 3/8] Doc/gitsubmodules: specify how submodules help in reduced size

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index cb795c6b6..3f73983d5 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -63,6 +63,9 @@ Submodules can be used for at least two different use cases:
 * Transfer size:
   In its current form Git requires the whole working tree present. It
   does not allow partial trees to be transferred in fetch or clone.
+  If you have your project as multiple repositories tied together as
+  submodules in a superproject, you can avoid fetching the working
+  trees of the repositories you are not interested in.
 * Access control:
   By restricting user access to submodules, this can be used to implement
   read/write policies for different users.
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 2/8] Doc/gitsubmodules: clearly specify advantage of submodule

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index bf46b0fb5..cb795c6b6 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -57,7 +57,7 @@ Submodules can be used for at least two different use cases:
 * Size of the git repository:
   In its current form Git scales up poorly for large repositories 
containing
   content that is not compressed by delta computation between trees.
-  However you can also use submodules to e.g. hold large binary assets
+  Therefore you can use submodules to hold large binary assets
   and these repositories are then shallowly cloned such that you do not
   have a large history locally.
 * Transfer size:
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 1/8] Doc/gitsubmodules: split a sentence for better readability

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Documentation/gitsubmodules.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index 46cf120f6..bf46b0fb5 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -36,8 +36,8 @@ The `gitlink` entry contains the object name of the commit 
that the
 superproject expects the submodule’s working directory to be at.
 
 The section `submodule.foo.*` in the `.gitmodules` file gives additional
-hints to Gits porcelain layer such as where to obtain the submodule via
-the `submodule.foo.url` setting.
+hints to Gits porcelain layer. For example, the `submodule.foo.url`
+setting specifies where to obtain the submodule.
 
 Submodules can be used for at least two different use cases:
 
-- 
2.16.0.rc0.223.g4a4ac8367

[PATCH 0/8] Doc/submodules: a few updates

[Kaartic Sivaraam]

These are just a few improvements that I thought would make the documentation
related to submodules a little better in various way such as readability,
consistency etc., These were things I noticed while reading thise documents.

Sorry, for the highly granular patches. I did the commits as and when I was
reading them and tried to keep them focused to one particular change by rebasing
them as needed. In case they need some change, let me know. 

I based these patches on top of 'master'.

Apart from the changes, I saw a few things that needed improvement/clarification
but wasn't able to do that myself due to my limited knowledge of submodules. 
They
are listed below. I'll add in patches for them if they are correctly clarified.


1.

 man gitsubmodules

   ·   The configuration file $GIT_DIR/config in the superproject. Typical 
configuration at this place is controlling if a submodule is
   recursed into at all via the active flag for example.

   If the submodule is not yet initialized, then the configuration 
inside the submodule does not exist yet, so configuration where to
   obtain the submodule from is configured here for example.

What's the "active flag" mentioned above? Also I find the phrase "is recursed 
into at all"
to be a little slippery. How could it be improved?


2.

 man git submodule

   update
   ...

   checkout
   

   If --force is specified, the submodule will be checked out 
(using git checkout --force if appropriate), even if the commit
   specified in the index of the containing repository already 
matches the commit checked out in the submodule.

I'm not sure this is conveying all the information it should be conveying.
It seems to making the user wonder, "How at all does 'git submodule update 
--force'
differs from 'git submodule update'?" also "using git checkout --force if 
appropriate"
seems to be invoking all sorts confusion as "appropriate" is superfluous.

How could these confusions be clarified?


---
Kaartic


Kaartic Sivaraam (8):
  Doc/gitsubmodules: split a sentence for better readability
  Doc/gitsubmodules: clearly specify advantage of submodule
  Doc/gitsubmodules: specify how submodules help in reduced size
  Doc/gitsubmodules: avoid abbreviations
  Doc/gitsubmodules: use "Git directory" consistently
  Doc/gitsubmodules: improve readability of certain lines
  Doc/git-submodule: improve readability and grammar of a sentence
  Doc/git-submodule: correctly quote important words

 Documentation/git-submodule.txt | 10 +-
 Documentation/gitsubmodules.txt | 28 
 2 files changed, 21 insertions(+), 17 deletions(-)

-- 
2.16.0.rc0.223.g4a4ac8367

Re: What's cooking in git.git (Dec 2017, #05; Wed, 27)

[Kaartic Sivaraam]

On Wednesday 03 January 2018 02:56 PM, Daniel Knittl-Frank wrote:

On Wed, Dec 27, 2017 at 10:34 PM, Junio C Hamano  wrote:

* dk/describe-all-output-fix (2017-12-27) 1 commit
  - describe: prepend "tags/" when describing tags with embedded name

  An old regression in "git describe --all $annotated_tag^0" has been
  fixed.

  Will merge to 'next'.


Shouldn't this be merged to 'maint' since it is a bugfix (for a long
standing bug)? Or am I misinterpreting the meaning of the 'maint'
branch?



IIUC, the pipe line here is something like,

  [PATCH]

 |

 (after the patch gets some consensus
  when it's not trivial)

 |
 V

   Merge to 'pu'


 |

 (after waiting for some time to see if someone
  shouts about a build failing or complaining
   a regression about the PATCH in 'pu')

 |
 V

   Merge to 'next'

 |

 (after waiting for some time to see if someone
  shouts about a build failing or complaining
   a regression about the PATCH in 'next')

 |
/ \
   /   \
  / \
 /   \
| |

  (if it's a bugfix for  (if it's a new feature or
   an already releasedan enhancement)
   version of Git)

| |
V V
  Merge to 'maint'Merge to 'master'



Of course 'maint' and 'master' are not diverged completely. They are 
'synced' at times.


Disclaimer: I won't say I'm 100% correct with the pipeline. This is just 
what I've understood in observing the mailing list, the "What's cooking" 
emails and the history of 'git' in the short time that I've been here. 
So, there are possibilities that I've said something incorrectly. I 
guess the "Documentation/howto/maintain-git.txt" document covers it more 
comprehensively especially "The Policy" section describes the branches 
more clearly.


--
Kaartic

Quote: "Be creative. Be adventurous. Be original. And above all else, be 
young." - Wonder Woman

Re: [ANNOUNCE] Git v2.16.0-rc0

[Kaartic Sivaraam]

On Friday 29 December 2017 10:00 AM, Junio C Hamano wrote:

  * "git branch" and "git checkout -b" are now forbidden from creating
a branch whose name is "HEAD".


"git branch" already forbid a branch named "HEAD", didn't it? I thought 
we just made "git checkout -b" to reject "HEAD" as a valid branch name, 
recently.


--
Kaartic

Re: Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam]

On Friday 22 December 2017 05:19 PM, Johannes Schindelin wrote:

Hi Kaartic,



I think I didn't mention I've set `commit.gpgsign` to `true` for that
repo, did I?


Hah! I had troubles to associate the correct line in my versions of Git's
source code (the line numbers alone are only reliable if you also have a
commit from which the Git binaries were built), 


Should have mentioned that, sorry.



but I did this free() at
(https://github.com/git/git/blob/cd54ea2b18/sequencer.c#L1975:

if (read_oneliner(, rebase_path_gpg_sign_opt(), 1)) {
if (!starts_with(buf.buf, "-S"))
strbuf_reset();
else {
free(opts->gpg_sign);
^
opts->gpg_sign = xstrdup(buf.buf + 2);
}
strbuf_reset();
}



Seems you got the right one, regardless. :)



The culprit is that we have these really unclear ownership rules, where it
is not at all clear who is responsible for releasing allocated memory:
caller or callee? (Hannes would not rightfully point out that this would
be a non-issue if we would switch to C++). In C, the common pattern is to
use `const char *` for users that are *not* supposed to take ownership,
and `char *` for users that are supposed to take custody. And in this
instance, `gpg_sign` should be owned by `struct replay_opts` because of
this (https://github.com/git/git/blob/cd54ea2b18/sequencer.h#L38):

char *gpg_sign;

Technically, using `char *` (instead of `const char *`) does not say
"you're now responsible for de-allocating this memory", of course, but in
practice it is often used like this (and the signature of `free(void *)`
certainly encourages that type of interpreting the `const` qualifier).



Nice explanation.



I spent a little quality time with the code and came up with a patch that
I will send out in a moment.



That relieves Philip of the burden, I guess. :)



By the way, Kaartic, thank you so much for focusing on correctness before
focusing on style issues. I know it is harder to review patches for
correctness than it is to concentrate on style issues, but in my mind it
is not only much more work, but also a lot more valuable.



Though it's good to hear these words and I do like to focus on 
correctness, there wasn't much I did to focus on correctness in this 
case ;-) It was you actually, after seeing such a clear explanation!.


I just used Git built from 'next' for my personal use and encountered 
the issue I stated in the start of this sub-thread.



--
Kaartic

Re: Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam]

On Thu, 2017-12-21 at 16:53 +, phillip.w...@talktalk.net wrote:
> Hm, There is a problem with sequencer_remove_state() which does 
> 
> free(opts->gpg_sign)
> 
> however unless a gpg key was given on the commandline, opts->gpg is 
> initialized to "" which is statically allocated. 
> 
> This untested diff should fix that,

It did seem to. I don't get that error any more.


>  but I'm not sure if you're problem 
> is related to it

As the fix you suggested did work, I think the problem is related. Did
you have anything else in mind when you said you're not sure whether or
not it's related?


> also I'm on webmail so apologies if the patch is 
> mangled)
> 

No issues. The patch seems quite small :)


Thanks,
Kaartic

Re: Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam]

On Thu, 2017-12-21 at 15:06 +0100, Johannes Schindelin wrote:
> Hi Kaartic,
> I fear that the strace does not cover the `git-rebase` process nor the
> `git-rebase--helper` process (which must have been part of your run, as
> the commit affected only that part of the rebase operation).
> 

Yep. It striked me only later that the entry point of "git rebase" is
essentially a script.


> If you have valgrind installed, you may want to give it a try. Since
> git-rebase is (still, much to my dislike) a Unix shell script, you will
> have to work quite hard to get it to valgrind correctly.
> 

You're right about that it's quite hard to get to the right point of
the issue when scripts are involved.


> This is how I typically do it: 

> Then, I look for that call (I imagine it is the `exec git rebase--helper
> ${force_rebase:+--no-ff} --continue` line), then copy-edit it and guard it
> by an environment variable:
> 
>   test -z "$" || {
>   valgrind git rebase--helper ${force_rebase:+--no-ff} \
>   --continue
>   exit
>   }
>   exec git rebase--helper ${force_rebase:+--no-ff} --continue
> 
> After that, you can run the same command, and trigger the new code path by
> that environment variable:
> 
>   =1 /mnt/Source/Git/git rebase -i HEAD~10
> 
> That way, you can keep the rest of your Git calls be unaffected by the
> patch.
> 

Thanks for detailing the way to get the right information for this
issue. That was intriguing.


> BTW from your invocation, I imagine that you wanted to actually run your
> Git just-built-from-source, in-place. But for that, you would need to pass
> the --exec-path option, too, like so:
> 
>   =1 /mnt/Source/Git/git --exec-path=/mnt/Source/Git \
>   rebase -i HEAD~10
> 
> That way, you could edit the git-rebase--interactive file that is *not*
> installed, and avoid affecting other Git operations (you would also not
> need to guard the new code path behind a conditional).
> 

Makes sense. (I had actually used incorrect directories in my previous
mail as I did part of it manually (big mistake!) so the trace itself
might not have much sense in some parts, sorry)

Speaking of trace, (thanks to Dscho!) the one I got using 'valgrind'
(with `--leak-check=full` option) can be found below. I've kept only
relevant parts of it to avoid unwanted noise of `set -x`. Let me know
if that's needed too.

...

+ git diff-files --quiet --ignore-submodules
+ git diff-index --cached --quiet --ignore-submodules HEAD --
+ test 0 = 1
+ test -z 1
+ valgrind --leak-check=full git rebase--helper --continue
==10384== Memcheck, a memory error detector
==10384== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==10384== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==10384== Command: git rebase--helper --continue
==10384== 
==10384== Invalid free() / delete / delete[] / realloc()
==10384==at 0x4C2CDDB: free (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10384==by 0x24E3F8: read_populate_opts (sequencer.c:1964)
==10384==by 0x24E3F8: sequencer_continue (sequencer.c:2753)
==10384==by 0x1735DC: cmd_rebase__helper (rebase--helper.c:52)
==10384==by 0x11B847: run_builtin (git.c:346)
==10384==by 0x11B847: handle_builtin (git.c:554)
==10384==by 0x11BB05: run_argv (git.c:606)
==10384==by 0x11BB05: cmd_main (git.c:683)
==10384==by 0x11AC0B: main (common-main.c:43)
==10384==  Address 0x2a898a is in a r-x mapped file 
/mnt/Source/Git/git-next/git segment
==10384== 
Successfully rebased and updated refs/heads/public.
==10384== 
==10384== HEAP SUMMARY:
==10384== in use at exit: 680,882 bytes in 332 blocks
==10384==   total heap usage: 717 allocs, 386 frees, 1,709,293 bytes allocated
==10384== 
==10384== 2,053 (2,048 direct, 5 indirect) bytes in 1 blocks are definitely 
lost in loss record 75 of 83
==10384==at 0x4C2BADF: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10384==by 0x4C2DE5F: realloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10384==by 0x27E0FE: xrealloc (wrapper.c:138)
==10384==by 0x2072A3: add_object_array_with_path (object.c:319)
==10384==by 0x23D745: add_pending_object_with_path (revision.c:163)
==10384==by 0x24030E: add_pending_object_with_mode (revision.c:170)
==10384==by 0x24030E: add_pending_object (revision.c:176)
==10384==by 0x24030E: add_head_to_pending (revision.c:188)
==10384==by 0x280EFA: has_uncommitted_changes.part.17 (wt-status.c:2288)
==10384==by 0x24DF88: commit_staged_changes (sequencer.c:2705)
==10384==by 0x24DF88: sequencer_continue (sequencer.c:2749)
==10384==by 0x1735DC: cmd_rebase__helper (rebase--helper.c:52)
==10384==by 0x11B847: run_builtin (git.c:346)
==10384==by 0x11B847: handle_builtin (git.c:554)
==10384==by 0x11BB05: run_argv (git.c:606)
==10384==by 0x11BB05: cmd_main (git.c:683)
==10384==by 0x11AC0B: main 

Re: Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam]

On Thursday 21 December 2017 02:09 AM, phillip.w...@talktalk.net wrote:
> 
> 
> Hi Kaartic
> 
> I'm replying off list as I've only got access to webmail at the
> moment.

No issues brought the CCs including the list to ensure we don't miss things.


> Are you able to do a backtrace with debugging symbols
> please, I'm finding it hard to glean anything from the backtrace
> here. 

That's confusing, I already had the following in my `config.mak` while 
compiling,


DEVELOPER=1
DEVELOPERS=1
CFLAGS += -g -O2
CFLAGS += -DFLEX_ARRAY=2048

NO_GETTEXT=1
prefix=/home/$(USER)/.local
#CFLAGS += -Wno-unused-value


As you can see the `-g` flag is there in CFLAGS.

If this output doesn't give much sense could you be specific about what 
output you want i.e., by mentioning the command, showing a sample etc., 
I'm not sure how to go about debugging with "git rebase" as it's a script.


> Also does it definitely bisect to this patch or the merge of
> this branch?
> 

"git bisect" speaking,

28d6daed4f119940ace31e523b3b272d3d153d04 is the first bad commit
commit 28d6daed4f119940ace31e523b3b272d3d153d04
Author: Phillip Wood 
Date:   Wed Dec 13 11:46:21 2017 +

sequencer: improve config handling

The previous config handling relied on global variables, called
git_default_config() even when the key had already been handled by
git_sequencer_config() and did not initialize the diff configuration
variables. Improve this by: i) loading the default values for message
cleanup and gpg signing of commits into struct replay_opts;
ii) restructuring the code to return immediately once a key is
handled; and iii) calling git_diff_basic_config(). Note that
unfortunately it is not possible to return early if the key is handled
by git_gpg_config() as it does not indicate to the caller if the key
has been handled or not.

The sequencer should probably have been calling
git_diff_basic_config() before as it creates a patch when there are
conflicts. The shell version uses 'diff-tree' to create the patch so
calling git_diff_basic_config() should match that. Although 'git
commit' calls git_diff_ui_config() I don't think the output of
print_commit_summary() is affected by anything that is loaded by that
as print_commit_summary() always turns on rename detection so would
ignore the value in the user's configuration anyway. The other values
loaded by git_diff_ui_config() are about the formatting of patches so
are not relevant to print_commit_summary().

Signed-off-by: Phillip Wood 
Signed-off-by: Junio C Hamano 

:04 04 bbd487cc285ae12cefb97a9661de4ff27d182727 
9460ac528bf68f331088412f62e5d6a39305854b M  builtin
:100644 100644 99452a0e89777999418c116386f924be396ed04a 
7051b20b76292e3b539875f0dc38108a734d2164 M  sequencer.c
:100644 100644 77cb174b2aaf3972ebb9e6ec379252be96dedd3d 
3a5072c2ab9088c237b83d92deae3c801289e543 M  sequencer.h


FWIW, I started the bisection with `v2.11.0`  as good (because "git
rebase -i HEAD~10 worked there) and the tip of `next` (cfbfd45ee (Sync
with master, 2017-12-19)) as bad.

Here's the bisect log for a shorter range of bisection (which was
possible after I discovered the bad commit in the previous bisect)

git bisect start
# bad: [cfbfd45ee6e49007fdeb8904064ba98f65e0] Sync with master
git bisect bad cfbfd45ee6e49007fdeb8904064ba98f65e0
# good: [a4212f7ebd7158a1e237d776fb4bbd7a295d0bda] Merge branch 
'pw/sequencer-in-process-commit' into next
git bisect good a4212f7ebd7158a1e237d776fb4bbd7a295d0bda
# good: [bdae4af87053490adad2dc9fb184d6d050d46a4c] Merge branch 
'sg/setup-doc-update'
git bisect good bdae4af87053490adad2dc9fb184d6d050d46a4c
# bad: [6d12e08217360cfc042ec484129771f73ad7b97f] Merge branch 
'rs/strbuf-read-once-reset-length' into next
git bisect bad 6d12e08217360cfc042ec484129771f73ad7b97f
# bad: [1b791d2503742b060f91b8159b85c06335634bd8] Merge branch 
'ab/simplify-perl-makefile' into next
git bisect bad 1b791d2503742b060f91b8159b85c06335634bd8
# bad: [abfed699db9f0f0e6f03f5ee3e9d137944ba4216] Merge branch 
'sb/clone-recursive-submodule-doc' into next
git bisect bad abfed699db9f0f0e6f03f5ee3e9d137944ba4216
# bad: [ec4d2b9c843b8b338ffa8906eea70095b3098c1e] Merge branch 
'pw/sequencer-in-process-commit' into next
git bisect bad ec4d2b9c843b8b338ffa8906eea70095b3098c1e
# good: [5279b80103bde3ec5d6108fa8f43de2017668039] Merge branch 
'tb/check-crlf-for-safe-crlf' into next
git bisect good 5279b80103bde3ec5d6108fa8f43de2017668039
# bad: [28d6daed4f119940ace31e523b3b272d3d153d04] sequencer: improve config 
handling
git bisect bad 28d6daed4f119940ace31e523b3b272d3d153d04
# first bad commit: [28d6daed4f119940ace31e523b3b272d3d153d04] sequencer: 
improve config handling

-- 
Kaartic

Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam]

I recently encountered that error when trying to do an interactive
rebase after using filter-branch to remove a file completely in a
repository. I bisected this issue which pointed at this patch. I'm not
sure how it is related as I'm not too familiar with the sequencer code.
I could help in case any specific information is needed. As a first
step, I've posted the output of "strace /mnt/Source//Git/git rebase -i
HEAD~10" below.


-- 8< --
execve("/mnt/Source//Git/git-next/git", ["/mnt/Source//Git/git-next/git", 
"rebase", "-i", "HEAD~10"], [/* 62 vars */]) = 0
brk(NULL)   = 0x55a494d3
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7f25bd94
access("/etc/ld.so.preload", R_OK)  = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=114620, ...}) = 0
mmap(NULL, 114620, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f25bd924000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libz.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\300!\0\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=105088, ...}) = 0
mmap(NULL, 2200072, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bd506000
mprotect(0x7f25bd51f000, 2093056, PROT_NONE) = 0
mmap(0x7f25bd71e000, 8192, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x18000) = 0x7f25bd71e000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libpthread.so.0", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0Pa\0\0\0\0\0\0"..., 832) 
= 832
fstat(3, {st_mode=S_IFREG|0755, st_size=135440, ...}) = 0
mmap(NULL, 2212936, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bd2e9000
mprotect(0x7f25bd301000, 2093056, PROT_NONE) = 0
mmap(0x7f25bd50, 8192, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17000) = 0x7f25bd50
mmap(0x7f25bd502000, 13384, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f25bd502000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/librt.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\340 \0\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=31744, ...}) = 0
mmap(NULL, 2128832, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bd0e1000
mprotect(0x7f25bd0e8000, 2093056, PROT_NONE) = 0
mmap(0x7f25bd2e7000, 8192, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x6000) = 0x7f25bd2e7000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0\4\2\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1689360, ...}) = 0
mmap(NULL, 3795296, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bcd42000
mprotect(0x7f25bced7000, 2097152, PROT_NONE) = 0
mmap(0x7f25bd0d7000, 24576, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x195000) = 0x7f25bd0d7000
mmap(0x7f25bd0dd000, 14688, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f25bd0dd000
close(3)= 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7f25bd922000
arch_prctl(ARCH_SET_FS, 0x7f25bd922e80) = 0
mprotect(0x7f25bd0d7000, 16384, PROT_READ) = 0
mprotect(0x7f25bd50, 4096, PROT_READ) = 0
mprotect(0x7f25bd2e7000, 4096, PROT_READ) = 0
mprotect(0x7f25bd71e000, 4096, PROT_READ) = 0
mprotect(0x55a49424b000, 12288, PROT_READ) = 0
mprotect(0x7f25bd943000, 4096, PROT_READ) = 0
munmap(0x7f25bd924000, 114620)  = 0
set_tid_address(0x7f25bd923150) = 9667
set_robust_list(0x7f25bd923160, 24) = 0
rt_sigaction(SIGRTMIN, {sa_handler=0x7f25bd2eebd0, sa_mask=[], 
sa_flags=SA_RESTORER|SA_SIGINFO, sa_restorer=0x7f25bd2fa0c0}, NULL, 8) = 0
rt_sigaction(SIGRT_1, {sa_handler=0x7f25bd2eec60, sa_mask=[], 
sa_flags=SA_RESTORER|SA_RESTART|SA_SIGINFO, sa_restorer=0x7f25bd2fa0c0}, NULL, 
8) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM64_INFINITY}) = 0
open("/dev/null", O_RDWR)   = 3
close(3)= 0
rt_sigprocmask(SIG_UNBLOCK, [PIPE], NULL, 8) = 0
rt_sigaction(SIGPIPE, {sa_handler=SIG_DFL, sa_mask=[PIPE], 
sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f25bcd75060}, 
{sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
brk(NULL) 

Re: What's cooking in git.git (Dec 2017, #04; Tue, 19)

[Kaartic Sivaraam]

On Wednesday 20 December 2017 03:30 AM, Junio C Hamano wrote:

* pw/sequencer-in-process-commit (2017-12-13) 10 commits
   (merged to 'next' on 2017-12-13 at ec4d2b9c84)
   ...
  The sequencer infrastructure is shared across "git cherry-pick",
  "git rebase -i", etc., and has always spawned "git commit" when it
  needs to create a commit.  It has been taught to do so internally,
  when able, by reusing the codepath "git commit" itself uses, which
  gives performance boost for a few tens of percents in some sample
  scenarios.

  Will merge to 'master'.


I just encountered a small issue with 'rebase -i'. My bisection pointed at,

28d6daed4 (sequencer: improve config handling, 2017-12-13)

So, I guess it would be better if you could delay merging it to 
'master'. I'm not sure what how the commit and the patch are related but 
lets discuss that in the patch thread.


--
Kaartic

[PATCH v3] imap-send: URI encode server folder

[Kaartic Sivaraam]

From: Nicolas Morey-Chaisemartin <nmoreychaisemar...@suse.com>

When trying to send a patch using 'imap-send' with 'curl' and the
following configuration:

[imap]
folder = "[Gmail]/Drafts"
host = imaps://imap.gmail.com
port = 993
sslverify = false

results in the following error,

curl_easy_perform() failed: URL using bad/illegal format or missing URL

This is a consequence of not URI-encoding the folder portion of
the URL which contains characters such as '[' which are not
allowed in a URI. According to RFC3986, these characters should be
URI-encoded.

So, URI-encode the folder before adding it to the URI to ensure it doesn't
contain characters that aren't allowed in a URI.

Reported-by: Doron Behar <doron.be...@gmail.com>
Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemar...@suse.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
Changes in v3:
- updated commit message as suggested by Eric (convert past tense
  to present tense) and added a few tweaks to it that striked me

Eric Sunshine <sunsh...@sunshineco.com> writes:
> Thanks for taking up the slack.

You're welcome. It was easier than waiting for this patch to be
updated so it could get into 'pu' ;-)


 imap-send.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index 54e6a80fd..36c7c1b4f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, 
struct credential *cred)
 {
CURL *curl;
struct strbuf path = STRBUF_INIT;
+   char *uri_encoded_folder;
 
if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
die("curl_global_init failed");
@@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, 
struct credential *cred)
strbuf_addstr(, server.host);
if (!path.len || path.buf[path.len - 1] != '/')
strbuf_addch(, '/');
-   strbuf_addstr(, server.folder);
+
+   uri_encoded_folder = curl_easy_escape(curl, server.folder, 0);
+   if (!uri_encoded_folder)
+   die("failed to encode server folder");
+   strbuf_addstr(, uri_encoded_folder);
+   curl_free(uri_encoded_folder);
 
curl_easy_setopt(curl, CURLOPT_URL, path.buf);
strbuf_release();
-- 
2.15.1.620.gb9897f467

Re: 'git format-patch --summary' seems to be turning off the stat

[Kaartic Sivaraam]

On Tuesday 19 December 2017 12:07 AM, Junio C Hamano wrote:

Kaartic Sivaraam <kaartic.sivar...@gmail.com> writes:


Note: I do see that "--summary" is a diff-option but does that mean we
should't be printing stat information in the patch when the user
didn't mention "--no-stat"? Yeah, "git format-patch  --summary
--stat" does bring back the stat. --- Kaartic


The old design decision shared across "log" family of commands [*1*]
wrt to these "how and what are shown" options is that they started
without any default set of options and the user gave all options (if
they want both summary and stat, they gave them from the command
line); over time they gained a set of built-in default options that
take effect only when the user gives no option.

So, yes, giving "--summary" will turn off the built-in ones that are
"--summary --stat" (IIRC).



Is it worth documenting that built-in defaults are turned off when user 
gives other options? Not sure where, though?



--
Kaartic

[PATCH v2] imap-send: URI encode server folder

[Kaartic Sivaraam]

From: Nicolas Morey-Chaisemartin 

When trying to send a patch using 'imap-send' with 'curl' and the
following configuration:

[imap]
folder = "[Gmail]/Drafts"
host = imaps://imap.gmail.com
port = 993
sslverify = false

resulted in the following error,

curl_easy_perform() failed: URL using bad/illegal format or missing URL

That was a consequence of the not URI encoding the folder portion of
the URL which contained characters such as '[' which are not
allowed in a URI. According to RFC3986, these characters should be
"URI encoded".

So, URI encode the folder portion of the URL to ensure it doesn't
contain characters that aren't allowed in a URI.

Reported-by: Doron Behar 
Signed-off-by: Nicolas Morey-Chaisemartin 
---

I came across the same issue that lead to this patch recently and found
that this patch didn't make it in. So, I thought I could help out and
hence this v2.

Eric Sunshine  writes:
> For someone reading this commit message in the future -- someone who
> didn't follow the email thread which led to this patch -- "this fixes"
> doesn't say much about the actual problem being addressed. Can you
> expand the commit message a bit to make it more self-contained? At
> minimum, perhaps show the error message you were experiencing, and
> cite (as Daniel pointed out) RFC 3986 and the bit about a "legal" URL
> not containing brackets.

I guess I covered this part.


> Also, a natural question which pops into the head of someone reading
> this patch is whether other parts of the URL (host, user, etc.) also
> need to be handled similarly. It's possible that you audited the code
> and determined that they are handled fine already, but the reader of
> the commit message is unable to infer that. Consequently, it might be
> nice to have a sentence about that, as well ("other parts of the URL
> are already encoded, thus are fine" or "other parts of the URL are not
> subject to this problem because ...").

I'm not sure about this one. I guess the host and user don't need encoding
as I suspect they wouldn't contain characters that aren't allowed. I might
be wrong, though. Let me know if I'm missing something.


Thanks,
Kaartic

 imap-send.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index 54e6a80fd..36c7c1b4f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, 
struct credential *cred)
 {
CURL *curl;
struct strbuf path = STRBUF_INIT;
+   char *uri_encoded_folder;
 
if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
die("curl_global_init failed");
@@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, 
struct credential *cred)
strbuf_addstr(, server.host);
if (!path.len || path.buf[path.len - 1] != '/')
strbuf_addch(, '/');
-   strbuf_addstr(, server.folder);
+
+   uri_encoded_folder = curl_easy_escape(curl, server.folder, 0);
+   if (!uri_encoded_folder)
+   die("failed to encode server folder");
+   strbuf_addstr(, uri_encoded_folder);
+   curl_free(uri_encoded_folder);
 
curl_easy_setopt(curl, CURLOPT_URL, path.buf);
strbuf_release();
-- 
2.15.1.620.gb9897f467

'git format-patch --summary' seems to be turning off the stat

[Kaartic Sivaraam]

The documentation for "--summary" option in the format-patch Doc states, 
--summary Output a condensed summary of extended header information such 
as creations, renames and mode changes. It doesn't state anything about 
turning of the stat. Why does the stat get turned off when "--summary" 
option is used along with "git format-patch" ? Note: I do see that 
"--summary" is a diff-option but does that mean we should't be printing 
stat information in the patch when the user didn't mention "--no-stat"? 
Yeah, "git format-patch  --summary --stat" does bring back the 
stat. --- Kaartic

[PATCH v5 2/3] rebase: distinguish user input by quoting it

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 git-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index a299bcc52..0f379ba2b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -477,7 +477,7 @@ then
;;
esac
upstream=$(peel_committish "${upstream_name}") ||
-   die "$(eval_gettext "invalid upstream \$upstream_name")"
+   die "$(eval_gettext "invalid upstream '\$upstream_name'")"
upstream_arg="$upstream_name"
 else
if test -z "$onto"
@@ -539,7 +539,7 @@ case "$#" in
head_name="detached HEAD"
 
else
-   die "$(eval_gettext "fatal: no such branch/commit: 
\$branch_name")"
+   die "$(eval_gettext "fatal: no such branch/commit 
'\$branch_name'")"
fi
;;
 0)
-- 
2.15.0.531.g2ccb3012c

[PATCH v5 3/3] rebase: rebasing can also be done when HEAD is detached

[Kaartic Sivaraam]

Attempting to rebase when the HEAD is detached and is already
up to date with upstream (so there's nothing to do), the
following message is shown

Current branch HEAD is up to date.

which is clearly wrong as HEAD is not a branch.

Handle the special case of HEAD correctly to give a more precise
error message.

Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 git-rebase.sh | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 0f379ba2b..fd72a35c6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -601,11 +601,23 @@ then
test -z "$switch_to" ||
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
git checkout -q "$switch_to" --
-   say "$(eval_gettext "Current branch \$branch_name is up to 
date.")"
+   if test "$branch_name" = "HEAD" &&
+! git symbolic-ref -q HEAD
+   then
+   say "$(eval_gettext "HEAD is up to date.")"
+   else
+   say "$(eval_gettext "Current branch \$branch_name is up 
to date.")"
+   fi
finish_rebase
exit 0
else
-   say "$(eval_gettext "Current branch \$branch_name is up to 
date, rebase forced.")"
+   if test "$branch_name" = "HEAD" &&
+! git symbolic-ref -q HEAD
+   then
+   say "$(eval_gettext "HEAD is up to date, rebase 
forced.")"
+   else
+   say "$(eval_gettext "Current branch \$branch_name is up 
to date, rebase forced.")"
+   fi
fi
 fi
 
-- 
2.15.0.531.g2ccb3012c