Re: Expected behavior of "git check-ignore"...

[John Szakmeister]

On Mon, Jul 24, 2017 at 3:23 PM, Junio C Hamano  wrote:
[snip]
> I am reasonably sure that the command started its life as a pure
> debugging aid.
>
> The treatment of the negation _might_ impose conflicting goals to
> its purpose as a debugging aid---a user who debugs his .gitignore
> file would want to know what causes a thing that wants to be ignored
> is not or vice versa, and use of the exit status to indicate if it
> is ignored may not mesh well with its goal as a debugging aid, but I
> didn't think about the potential issues deeply myself while writing
> this response.  As you mentioned, use of (or not using) "-v" could
> be used as a sign to see which behaviour the end-user expects, I
> guess.

Is there another way of checking to see if a file is ignored?  If so,
maybe we could suggest that instead.  Perhaps using `git status
--porcelain --ignored` and examining the output?  I'm not sure how
well that would work with directories.

Thanks for the insight Junio.  I'm going to let the exit status thing
drop for now.  You don't seem like it's a good thing to do, and I'm
not particularly fond of having it behave two different ways based on
`-v` being present.

-John

Re: Expected behavior of "git check-ignore"...

[John Szakmeister]

On Sun, Jul 23, 2017 at 12:33 PM, Philip Oakley  wrote:
[snip]
>>
>>
>> git init .
>> echo 'foo/*' > .gitignore
>> echo '!foo/bar' > .gitignore
>
>
> Is this missing the >> append to get the full two line .gitignore?
> adding in a `cat .gitignore` would help check.

Yes, sorry about that.

>
>> mkdir foo
>> touch foo/bar
>
> I don't think you need these. It's the given pathnames that are checked, not
> the file system content.

It was there so you could see that `git status` ignores foo/bar
(though that wasn't part of the little script).

>> git check-ignore foo/bar
>
> Does this need the `-q` option to set the exit status?

No, it's always set.

> echo $? # to display the status.

Sure.  So, to recap the update reproduction recipe would be:


git init .
echo 'foo/*' > .gitignore
echo '!foo/bar' >> .gitignore
mkdir foo
touch foo/bar
git status # foo/ shows as untracked because bar is present
git check-ignore foo/bar
echo $? # show the exit status


It seems like it should print "1", but it prints "0".

>> I expect the last command to return 1 (no files are ignored), but it
>> doesn't.  The StackOverflow user had the same expectation, and imagine
>> others do as well.  OTOH, it looks like the command is really meant to
>> be a debugging tool--to show me the line in a .gitignore associated
>> with this file, if there is one.  In which case, the behavior is
>> correct but the return code description is a bit misleading (0 means
>> the file is ignored, which isn't true here).
>
>
> Maybe the logic isn't that clear? Maybe it is simply detecting if any one of
> the ignore lines is active, and doesn't reset the status for a negation?
>
> I appear to get the same response as yourself, but I haven't spent much time
> on it - I'm clearing a backlog of work at the moment.

Correct, it appears that if any line in the ignore matches, then it
exits with 0.  So it's not that it's ignored, but that there is a
matching line in an ignore file somewhere.  I can see the logic in
this if it's meant to be a debugging tools, especially combined with
-v.  Simply changing it does affect quite a few tests, but I'm not
sure that it was intentional for negation to be treated this way.

> I also tried the -v -n options, and if I swap the ignore lines around it
> still says line 2 is the one that ignores.
> It gets more interesting if two paths are given `foo/bar foo/baz`, to see
> which line picks up which pathname (and with the swapped ignore lines).
>
> Is there a test for this in the test suite?

There are several.  But line 427, test_expect_success_multi 'nested
include', is one that I think is pretty direct about testing this.  I
imagine what happened is that gitignores used to contain only things
you wanted to ignore and when the ability to negate came along the
semantics of this was never changed--and possibly for good reason.
I'm just wondering if it should change, or if the documentation should
be updated to reflect how it actually behaves (the file may not be
ignored, but a line is present in a gitignore that affects its
status).  The behavior is definitely a little unexpected as it stands,
given the documentation though.

Thanks for taking a look Philip!

-John

Expected behavior of "git check-ignore"...

[John Szakmeister]

A StackOverflow user posted a question about how to reliably check
whether a file would be ignored by "git add" and expected "git
check-ignore" to return results that matched git add's behavior.  It
turns out that it doesn't.  If there is a negation rule, we end up
returning that exclude and printing it and exiting with 0 (there are
some ignored files) even though the file has been marked to not be
ignored.

Is the expected behavior of "git check-ignore" to return 0 even if the
file is not ignore when a negation is present?


git init .
echo 'foo/*' > .gitignore
echo '!foo/bar' > .gitignore
mkdir foo
touch foo/bar
git check-ignore foo/bar


I expect the last command to return 1 (no files are ignored), but it
doesn't.  The StackOverflow user had the same expectation, and imagine
others do as well.  OTOH, it looks like the command is really meant to
be a debugging tool--to show me the line in a .gitignore associated
with this file, if there is one.  In which case, the behavior is
correct but the return code description is a bit misleading (0 means
the file is ignored, which isn't true here).

Thoughts?  It seems like this question was asked before several years
ago but didn't get a response.

Thanks!

-John

PS The SO question is here:
https://stackoverflow.com/questions/45210790/how-to-reliably-check-whether-a-file-is-ignored-by-git

Re: [PATCH 0/6] loose-object fsck fixes/tightening

[John Szakmeister]

On Fri, Jan 13, 2017 at 12:52 PM, Jeff King <p...@peff.net> wrote:
> On Fri, Jan 13, 2017 at 04:15:42AM -0500, John Szakmeister wrote:
>
>> > I did notice another interesting case when looking at this. Fsck ends up
>> > in fsck_loose(), which has the sha1 and path of the loose object. It
>> > passes the sha1 to fsck_sha1(), and ignores the path entirely!
>> >
>> > So if you have a duplicate copy of the object in a pack, we'd actually
>> > find and check the duplicate. This can happen, e.g., if you had a loose
>> > object and fetched a thin-pack which made a copy of the loose object to
>> > complete the pack).
>> >
>> > Probably fsck_loose() should be more picky about making sure we are
>> > reading the data from the loose version we found.
>>
>> Interesting find!  Thanks for the information Peff!
>
> So I figured I would knock this out as a fun morning exercise. But
> sheesh, it turned out to be a slog, because most of the functions rely
> on map_sha1_file() to convert the sha1 to an object path at the lowest
> level.

Yeah, I discovered the same thing when I took a look at it a week or so ago. :-(

> But I finally got something working, so here it is. I found another bug
> on the way, along with a few cleanups. And then I did the trailing
> garbage detection at the end, because by that point I knew right where
> it needed to go. :)

I don't know if my opinion counts for much, but the changes look good to me.

-John

Re: "git fsck" not detecting garbage at the end of blob object files...

[John Szakmeister]

On Sat, Jan 7, 2017 at 4:47 PM, Dennis Kaarsemaker
<den...@kaarsemaker.net> wrote:
> On Sat, 2017-01-07 at 07:50 -0500, John Szakmeister wrote:
>> I was perusing StackOverflow this morning and ran across this
>> question: 
>> http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/
>>
>> It was a simple question about why "checking objects" was not
>> appearing, but in it was another issue.  The user purposefully
>> corrupted a blob object file to see if `git fsck` would catch it by
>> tacking extra data on at the end.  `git fsck` happily said everything
>> was okay, but when I played with things locally I found out that `git
>> gc` does not like that extra garbage.  I'm not sure what the trade-off
>> needs to be here, but my expectation is that if `git fsck` says
>> everything is okay, then all operations using that object (file)
>> should work too.
>>
>> Is that unreasonable?  What would be the impact of fixing this issue?
>
> If you do this with a commit object or tree object, fsck does complain.
> I think it's sensible to do so for blob objects as well.

Also very good information.  Thanks Dennis!

-John

Re: "git fsck" not detecting garbage at the end of blob object files...

[John Szakmeister]

On Sun, Jan 8, 2017 at 12:26 AM, Jeff King <p...@peff.net> wrote:
> On Sat, Jan 07, 2017 at 10:47:03PM +0100, Dennis Kaarsemaker wrote:
>> On Sat, 2017-01-07 at 07:50 -0500, John Szakmeister wrote:
>> > I was perusing StackOverflow this morning and ran across this
>> > question: 
>> > http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/
>> >
>> > It was a simple question about why "checking objects" was not
>> > appearing, but in it was another issue.  The user purposefully
>> > corrupted a blob object file to see if `git fsck` would catch it by
>> > tacking extra data on at the end.  `git fsck` happily said everything
>> > was okay, but when I played with things locally I found out that `git
>> > gc` does not like that extra garbage.  I'm not sure what the trade-off
>> > needs to be here, but my expectation is that if `git fsck` says
>> > everything is okay, then all operations using that object (file)
>> > should work too.
>> >
>> > Is that unreasonable?  What would be the impact of fixing this issue?
>>
>> If you do this with a commit object or tree object, fsck does complain.
>> I think it's sensible to do so for blob objects as well.
>
> The existing extra-garbage check is in unpack_sha1_rest(), which is
> called as part of read_sha1_file(). And that's what we hit for commits
> and trees. However, we check the sha1 of blobs using the streaming
> interface (in case they're large). I think you'd want to put a similar
> check into read_istream_loose(). But note if you are grepping for it, it
> is hidden behind a macro; look for read_method_decl(loose).

That's for the pointer.

> I'm actually not sure if this should be downgrade to a warning. It's
> true that it's a form of corruption, but it doesn't actually prohibit us
> from getting the data we need to complete the operation. Arguably fsck
> should be more picky, but it is just relying on the same parse_object()
> code path that the rest of git uses.
>
> I doubt anybody cares too much either way, though. It's not like this is
> a common thing.

I kind of wonder about that myself too, and I'm not sure what to
think about it.  On the one hand, I'd like to know about
*anything* that has changed in an adverse way--it could indicate
a failure somewhere else that needs to be handled.  On the other
hand, scaring the user isn't all that advantageous.  I guess I'm
in the former camp.

As to whether this is common, yeah, it's probably not.  However,
I was surprised by the number of results that turned up when I
search for "garbage at end of loose object".

> I did notice another interesting case when looking at this. Fsck ends up
> in fsck_loose(), which has the sha1 and path of the loose object. It
> passes the sha1 to fsck_sha1(), and ignores the path entirely!
>
> So if you have a duplicate copy of the object in a pack, we'd actually
> find and check the duplicate. This can happen, e.g., if you had a loose
> object and fetched a thin-pack which made a copy of the loose object to
> complete the pack).
>
> Probably fsck_loose() should be more picky about making sure we are
> reading the data from the loose version we found.

Interesting find!  Thanks for the information Peff!

-John

"git fsck" not detecting garbage at the end of blob object files...

[John Szakmeister]

I was perusing StackOverflow this morning and ran across this
question: 
http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/

It was a simple question about why "checking objects" was not
appearing, but in it was another issue.  The user purposefully
corrupted a blob object file to see if `git fsck` would catch it by
tacking extra data on at the end.  `git fsck` happily said everything
was okay, but when I played with things locally I found out that `git
gc` does not like that extra garbage.  I'm not sure what the trade-off
needs to be here, but my expectation is that if `git fsck` says
everything is okay, then all operations using that object (file)
should work too.

Is that unreasonable?  What would be the impact of fixing this issue?

-John

Re: Cleaning ignored files

[John Szakmeister]

On Wed, Nov 9, 2016 at 1:23 PM, Roman Terekhov  wrote:
> Hi,
>
> I want to ask about git clean -dXf command behaviour.
>
> I do the following:
>
> $ mkdir gitignore_test
> $ cd gitignore_test/
> $ git init
> Initialized empty Git repository in ~/gitignore_test/.git/
>
> $ echo *.sln > .gitignore
> $ git add .gitignore
> $ git commit -m "add gitignore"
> [master (root-commit) ef78a3c] add gitignore
>  1 file changed, 1 insertion(+)
>  create mode 100644 .gitignore
>
> $ mkdir src
> $ touch test.sln
> $ touch src/test.sln
> $ tree
> .
> ├── src
> │   └── test.sln
> └── test.sln
>
> 1 directory, 2 files
>
> $ git clean -dXf
> Removing test.sln
>
> $ tree
> .
> └── src
> └── test.sln
>
> 1 directory, 1 file
>
>
> Why git clean -dXf does not remove all my test.sln files, but just one of 
> them?

src/ is not under version control, and currently git does not descend
into unknown folders to remove ignored files.  If you had a tracked or
staged file in src/, then git would descend into src/ and remove
test.sln as expected.  In your example, try doing:

$ touch src/foo.c
$ git add src/foo.c
$ git clean -dXf
Removing src/test.sln
Removing test.sln

Hope that helps!

-John

Re: Bug with worktrees...

[John Szakmeister]

On Thu, Aug 27, 2015 at 10:55 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
[snip]
 I can reproduce with 2.5.0 but not 'master'. Bisection reveals that
 this was fixed by d95138e (setup: set env $GIT_WORK_TREE when work
 tree is set, like $GIT_DIR, 2015-06-26), and was reported previously
 here [1].

I had done a quick search but didn't turn up that thread.  Thank you Eric!

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

Bug with worktrees...

[John Szakmeister]

My apologies if this has already been reported, but I ran into an
interesting bug with worktrees.  In particular, I have an alias 'st'
that maps to 'status -sb'.  When running this under a subdirectory of
a worktree created with 'git worktree add', it fails complaining that
the work tree has already been set.

Here's a script to reproduce the problem:
git init test-repo
cd test-repo
git config --local alias.st 'status -sb'
mkdir subdir
echo file  subdir/file.txt
git add subdir/file.txt
git commit -m 'add file'
git branch foo
git worktree add ../new-worktree foo
cd ../new-worktree/subdir
echo new line  file.txt
echo this will work
git status -sb
echo this fails
git st

When I run it, I see this:

Initialized empty Git repository in
/home/jszakmeister/tmp/test-case/test-repo/.git/
[master (root-commit) 1ec5360] add file
 1 file changed, 1 insertion(+)
 create mode 100644 subdir/file.txt
Enter ../new-worktree (identifier new-worktree)
Switched to branch 'foo'
this will work
## foo
this fails
fatal: internal error: work tree has already been set
Current worktree: /home/jszakmeister/tmp/test-case/new-worktree
New worktree: /home/jszakmeister/tmp/test-case/new-worktree/subdir

Hope this helps!

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

Re: Suggestion: make git checkout safer

[John Szakmeister]

On Wed, Jun 3, 2015 at 5:29 PM, Junio C Hamano gits...@pobox.com wrote:
[snip]
 [Footnote]

 *1* In the context of this discussion, after screwing up the change
 in hello.c, instead of expressing the wish to recover and to
 start from scratch in two separate commands, i.e.

 rm hello.c  update-from-scm

 they will learn to use a single command that is designed for
 that purpose, i.e.

 checkout-from-scm hello.c

 without the rm step, which _is_ an artificial workaround for
 their other SCMs that do not update from the repository unless
 they remove the files.

Just to be clear, Subversion doesn't require you to remove the file to
restore it (I'm sure most of you know that, but just in case others
didn't).  There is a one-step way to restore the file:

svn revert hello.c

Unfortunately, revert in the Git sense is about reverting commits, so
there's a bit of friction between Subversion and Git's terminology.
OTOH, once the team was educated how to think about it, git checkout
path has been pretty natural to use.

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

[PATCH] contrib/completion: escape the forward slash in __git_match_ctag

[John Szakmeister]

The current definition results in an incorrect expansion of the term under zsh.
For instance /^${1\\/}/ under zsh with the argument hi results in:
/^/\/h/\/i/

This results in an output similar to this when trying to complete `git grep
chartab` under zsh:

:: git grep chartabawk: cmd. line:1: /^/\/c/\/h/\/a/\/r/\/t/\/a/\/b/ { 
print $1 }
awk: cmd. line:1:^ backslash not last character on line
awk: cmd. line:1: /^/\/c/\/h/\/a/\/r/\/t/\/a/\/b/ { print $1 }
awk: cmd. line:1:^ syntax error

Leaving the prompt in a goofy state until the user hits a key.

Escaping the literal / in the parameter expansion (using /^${1//\//\\/}/)
results in:
/^chartab/

allowing the completion to work correctly.

This formulation also works under bash.

Signed-off-by: John Szakmeister j...@szakmeister.net
---

I've been bit by this bug quite a bit, but didn't have time to track it down
until today.  I hope the proposed solution is acceptable.

-John

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index c21190d..a899234 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1305,7 +1305,7 @@ _git_gitk ()
 }
 
 __git_match_ctag() {
-   awk /^${1\\/}/ { print \$1 } $2
+   awk /^${1//\//\\/}/ { print \$1 } $2
 }
 
 _git_grep ()
-- 
2.3.1

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

Re: Build error with current source release

[John Szakmeister]

On Tue, Feb 24, 2015 at 9:23 AM, J. R. Westmoreland j...@jrw.org wrote:
 Hi

 I hope it is okay to ask such a question here.

 I cloned the current source tree and tried to build it and I get the 
 following error.
 Could someone tell me why and if there is an easy way to fix it?

If you aren't opposed to using Homebrew, then I believe installing the
docbook package will help you here.  I installed xmlto that way, which
automatically brought in the docbook packages for me.

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

Re: A better git log --graph?

[John Szakmeister]

On Thu, Jan 8, 2015 at 6:39 AM, Yuri D'Elia wav...@thregr.org wrote:
[snip]
 I usually never use frontends. The notable exception is tig when I
 want to get a feeling of the status of several branches and/or blame
 some files. It haves a lot of typing. That being said, I tried gitk, but
 assumed it was also parsing --graph layout.

Try gitk --date-order.  I find it gives me the picture I really want
to see.  I've aliased it to git k in my gitconfig because I find it
very valuable.

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

Re: [PATCH v2] allow TTY tests to run under recent Mac OS

[John Szakmeister]

On Thu, Nov 13, 2014 at 5:40 PM, Mike Blume blume.m...@gmail.com wrote:
 listed bug doesn't reproduce on Mac OS Yosemite. For now, just enable
 TTY on Yosemite and higher

I've tried the reproduction recipe on Mavericks (`uname -r` = 13.4.0)
and it works fine--still going after 12,000 iterations.  Trying the
same thing on Snow Leopard shows that it still fails there--it died
after 182 iterations.

So I think the check can be relaxed to `-ge 13`.

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

Re: [PATCH v4] Allow TTY tests to run under recent Mac OS

[John Szakmeister]

On Fri, Nov 14, 2014 at 6:21 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Mike Blume wrote:

 TTY tests were previously skipped on all Mac OS systems because of a
 bug where reading from pty master occasionally hung. This bug has since
 been found not to be reproducible under Mac OS 10.9 and 10.10.1.

 Therefore, run TTY tests under Mac OS 10.9 (Mavericks) and higher.

 *puzzled* Testing on Yosemite with the following script[1]

 perl -MIO::Pty -MFile::Copy -e '
for (my $i = 0;; $i++) {
my $master = new IO::Pty;
my $slave = $master-slave;
if (fork == 0) {
close $master or die close: $!;
open STDOUT, , $slave or die dup2: $!;
close $slave or die close: $!;
exec(echo, hi, $i) or die exec: $!;
}
close $slave or die close: $!;
copy($master, \*STDOUT) or die copy: $!;
close $master or die close: $!;
wait;
}
 '

 still seems to hang eventually (after 61 iterations when my officemate
 tried it), reproducing the bug.

 Do you get a different result?

Interesting.  It took quite a while, but it did finally fail on my
Mavericks box on the 115,140th iteration.

 The bug was originally found in an autobuilder that would run the test
 suite when new versions were pushed to check for regressions.  Even if
 the hang only happened 0.1% of the time, that would get the
 autobuilder stuck after a while, which was how the problem got
 noticed.

Eek... that's nasty.

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

Re: [PATCH] diff-highlight: exit when a pipe is broken

[John Szakmeister]

On Sat, Nov 1, 2014 at 12:04 AM, Jeff King p...@peff.net wrote:
 On Fri, Oct 31, 2014 at 07:04:04AM -0400, John Szakmeister wrote:

 While using diff-highlight with other tools, I have discovered that Python
 ignores SIGPIPE by default.  Unfortunately, this also means that tools
 attempting to launch a pager under Python--and don't realize this is
 happening--means that the subprocess inherits this setting.  In this case, it
 means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
 with those broken scripts by explicitly setting up a SIGPIPE handler and 
 exiting
 the process.

 My first thought was that this should be handled already by 7559a1b
 (unblock and unignore SIGPIPE, 2014-09-18), but after re-reading your
 message, it sounds like you are using diff-highlight with non-git
 programs?

Yes, that's correct.  It's useful, so with a few tools that use diffs,
I like to run the output through diff-highlight.

 +# Some scripts may not realize that SIGPIPE is being ignored when launching 
 the
 +# pager--for instance scripts written in Python.  Setting $SIG{PIPE} = 
 'DEFAULT'
 +# doesn't work in these instances, so we install our own signal handler 
 instead.

 Why doesn't $SIG{PIPE} = 'DEFAULT' work? I did some limited testing and
 it seemed to work fine for me. Though I simulated the condition with:

   (
 trap '' PIPE
 perl -e '$|=1; print foo\n; print STDERR bar\n'
   ) | true

 which should not ever print bar.

Hehe, now that I see you right it out, I realize my mistake: I didn't
capitalize 'default'.  Trying it out again, it does appear that does
the trick.

[snip]
 Can we exit 141 here? If we are part of a pipeline to a pager, it should
 not matter either way, but I'd rather not lose the exit code if we can
 avoid it (in case of running the script standalone).

 +$SIG{PIPE} = \pipe_handler;

 A minor nit, but would:

   $SIG{PIPE} = sub { ... };

 be nicer to avoid polluting the function namespace?

Sorry, my Perl-fu is kind of low these days.  I used to use it all the
time but switched away from it quite a while ago.  Given that
'DEFAULT' does the trick, I'll just re-roll my patch to use that.
Does that sound fair?

-John

PS  Sorry for the late response, I've been traveling.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] diff-highlight: exit when a pipe is broken

[John Szakmeister]

While using diff-highlight with other tools, I have discovered that Python
ignores SIGPIPE by default.  Unfortunately, this also means that tools
attempting to launch a pager under Python--and don't realize this is
happening--means that the subprocess inherits this setting.  In this case, it
means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
with those broken scripts by restoring the default SIGPIPE handler.

Signed-off-by: John Szakmeister j...@szakmeister.net
---
Incorporates feedback from Jeff King and now we just restore the default signal
handler using the correct case of 'DEFAULT'.

 contrib/diff-highlight/diff-highlight | 4 
 1 file changed, 4 insertions(+)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index c4404d4..69a652e 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -14,6 +14,10 @@ my @removed;
 my @added;
 my $in_hunk;
 
+# Some scripts may not realize that SIGPIPE is being ignored when launching the
+# pager--for instance scripts written in Python.
+$SIG{PIPE} = 'DEFAULT';
+
 while () {
if (!$in_hunk) {
print;
-- 
2.0.1

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

[PATCH] diff-highlight: exit when a pipe is broken

[John Szakmeister]

While using diff-highlight with other tools, I have discovered that Python
ignores SIGPIPE by default.  Unfortunately, this also means that tools
attempting to launch a pager under Python--and don't realize this is
happening--means that the subprocess inherits this setting.  In this case, it
means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
with those broken scripts by explicitly setting up a SIGPIPE handler and exiting
the process.

Signed-off-by: John Szakmeister j...@szakmeister.net
---
 contrib/diff-highlight/diff-highlight | 9 +
 1 file changed, 9 insertions(+)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index c4404d4..dfcc35a 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -14,6 +14,15 @@ my @removed;
 my @added;
 my $in_hunk;
 
+# Some scripts may not realize that SIGPIPE is being ignored when launching the
+# pager--for instance scripts written in Python.  Setting $SIG{PIPE} = 
'DEFAULT'
+# doesn't work in these instances, so we install our own signal handler 
instead.
+sub pipe_handler {
+exit(0);
+}
+
+$SIG{PIPE} = \pipe_handler;
+
 while () {
if (!$in_hunk) {
print;
-- 
2.0.1

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

[PATCH] [kernel] completion: silence fatal: Not a git repository error

[John Szakmeister]

It is possible that a user is trying to run a git command and fail to realize
that they are not in a git repository or working tree.  When trying to complete
an operation, __git_refs would fall to a degenerate case and attempt to use
git for-each-ref, which would emit the error.

Let's fix this by shunting the error message coming from git for-each-ref.

Signed-off-by: John Szakmeister j...@szakmeister.net
---
 contrib/completion/git-completion.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5ea5b82..31b4739 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -388,7 +388,8 @@ __git_refs ()
;;
*)
echo HEAD
-   git for-each-ref --format=%(refname:short) -- 
refs/remotes/$dir/ | sed -e s#^$dir/##
+   git for-each-ref --format=%(refname:short) -- \
+   refs/remotes/$dir/ 2/dev/null | sed -e s#^$dir/##
;;
esac
 }
-- 
2.0.1

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

Re: [PATCH] [kernel] completion: silence fatal: Not a git repository error

[John Szakmeister]

On Tue, Oct 14, 2014 at 6:49 AM, John Szakmeister j...@szakmeister.net wrote:
 It is possible that a user is trying to run a git command and fail to realize
 that they are not in a git repository or working tree.  When trying to 
 complete
 an operation, __git_refs would fall to a degenerate case and attempt to use
 git for-each-ref, which would emit the error.

 Let's fix this by shunting the error message coming from git for-each-ref.

 Signed-off-by: John Szakmeister j...@szakmeister.net
 ---

Sorry for the [kernel] in the subject.  I must have forgotten to
remove that off of my format-patch invocation.  If you need me to
resubmit without it, I can do that.

Thanks!

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

Re: [PATCH] [kernel] completion: silence fatal: Not a git repository error

[John Szakmeister]

On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano gits...@pobox.com wrote:
 John Szakmeister j...@szakmeister.net writes:

 It is possible that a user is trying to run a git command and fail to realize
 that they are not in a git repository or working tree.  When trying to 
 complete
 an operation, __git_refs would fall to a degenerate case and attempt to use
 git for-each-ref, which would emit the error.

 Let's fix this by shunting the error message coming from git for-each-ref.

 Hmph, do you mean this one?

 $ cd /var/tmp ;# not a git repository
 $ git checkout TAB

 -

 $ git checkout fatal: Not a git repository (or any of the parent 
 directories): .git
 HEAD

 I agree it is ugly, but would it be an improvement for the end user,
 who did not realize that she was not in a directory where git checkout
 makes sense, not to tell her that she is not in a git repository in
 some way?

I had thought about that too, but I think--for me--it comes down to two things:

1) We're not intentionally trying to inform the user anywhere else
that they are not in a git repo.  We simply fail to complete anything,
which I think is an established behavior.
2) It mingles with the stuff already on the command line, making it
confusing to know what you typed.  Then you end up ctrl-c'ing your way
out of it and starting over--which is the frustrating part.

For me, I thought it better to just be more well-behaved.  I've also
run across this issue when I legitimately wanted to do something--I
wish I could remember what it was--with a remote repo and didn't
happen to be in a git working tree.  It was frustrating to see this
error message then too, for the same reason as above.  I use tab
completion quite extensively, so spitting things like this out making
it difficult to move forward is a problem.

Would it be better to check that $dir is non-empty and then provide
the extra bits of information?  We could then avoid giving the user
anything in that case.

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

Re: A failing attempt to use Git in a centralized environment

[John Szakmeister]

On Wed, Apr 30, 2014 at 1:15 PM, Geert Bosch bos...@mac.com wrote:

 On Apr 28, 2014, at 02:29, Marat Radchenko ma...@slonopotamus.org wrote:

 In short:
 1. Hack, hack, hack
 2. Commit
 3. Push, woops, reject (non-ff)
 4. Pull
 5. Push

 Just do pull --rebase? This is essentially the same as what SVN
 used to do in your setup.

That's not necessarily a good solution either.  For teams that don't
use rebase, it can leave them with their newly committed stuff now
rebased on the work from upstream--duplicating commits without
understanding why and where they came from, especially if other
branches were built on top of that one.

I agree in concept, but in practice it can be quite confusing. :-(

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

Re: Pull is Mostly Evil

[John Szakmeister]

On Fri, May 2, 2014 at 2:13 PM, Junio C Hamano gits...@pobox.com wrote:
[snip]
 Your earlier long-hand, together with the two examples that pulls
 into the same maint branch Brian gave us, may give us a better
 starting points to think about a saner way.

 To me, the problem sounds like:

 Tutorials of Git often says use 'git pull' to catch up your
 branch with your upstream work and then 'git push' back (and
 worse yet, 'git push' that does not fast-forward suggests doing
 so), but 'git pull' that creates a merge in a wrong direction is
 not the right thing for many people.

Yes, that's a good portion of the problem.

 And proposed solutions range from let's write 'pull' off as a
 failed experiment to let's forbid any merge made by use of 'pull'
 by default, because it is likely that merge may be in reverse.

FWIW, at my company, we took another approach.  We introduced a `git
ffwd` command that fetches from all remotes, and fast-forwards all
your local branches that are tracking a remote, and everyone on the
team uses it all the time.  It should be said this team also likes to
use Git bare-metal, because they like knowing how things work
out-of-the-box.  But they all use the command because it's so
convenient.

I had started making a C version a while back, but never completed it.
 I could take a stab at doing so again, if there's interest.

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

Re: git-draw - draws nearly the full content of a tiny git repository as a graph

[John Szakmeister]

On Wed, Jan 29, 2014 at 4:21 PM, Flo sensor...@gmail.com wrote:
 I just want to present a small tool I wrote. I use it at work to have
 a tool visualizing the Git basic concepts and data structures which
 are really really really simple (Linus' words). That helps me
 teaching my colleagues about Git and answering their questions when
 Git did not behave as they expected.

 https://github.com/sensorflo/git-draw/wiki

Very nice!  Thank you!

I tried it on a couple of my test repos that I use when teaching
people and it appears you need to munge branch names that contain
hyphens in the generated dot file, or dot fails to parse the file.

Otherwise, it's a neat tool.  Thanks again!

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

Re: [PATCH 2/2] format-patch: introduce format.defaultTo

[John Szakmeister]

On Mon, Jan 6, 2014 at 3:18 PM, Jeff King p...@peff.net wrote:
 On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote:

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but

  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?

  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

 Yeah, I had similar thoughts. I personally use branch.*.merge as
 forkedFrom, and it seems like we are going that way anyway with things
 like git rebase and git merge defaulting to upstream. But then there
 is git push -u and push.default = upstream, which treats the
 upstream config as something else entirely.

Just for more reference, I rarely use branch.*.merge as
forkedFrom.  I typically want to use master as my target, but like
Ram, I publish my changes elsewhere for safe keeping.  I think in a
typical, feature branch-based workflow @{u} would be nearly useless.

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

Re: [PATCH 2/2] format-patch: introduce format.defaultTo

[John Szakmeister]

On Mon, Jan 6, 2014 at 3:42 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 John Szakmeister wrote:

I think in a
 typical, feature branch-based workflow @{u} would be nearly useless.

 I thought the idea of @{u} was that it represents which ref one
 typically wants to compare the current branch to.  It is used by
 'git branch -v' to show how far ahead or behind a branch is and
 used by 'git pull --rebase' to forward-port a branch, for example.

 So a topic branch with @{u} pointing to 'master' or 'origin/master'
 seems pretty normal and hopefully the shortcuts it allows can make
 life more convenient.

Is there an outline of this git workflow in the documentation
somewhere?  Do you save your work in a forked repo anywhere?  If so,
how do you typically save your work.  I typically have my @{u}
pointing to where I save my work.  Perhaps I'm missing something
important here, but I don't feel like the current command set and
typical workflow (at least those in tutorials) leads you in that
direction.

Here is one example:
   https://www.atlassian.com/git/workflows#!workflow-feature-branch

 It is *not* primarily about where the branch gets pushed.  After all,
 in both the 'matching' and the 'simple' mode, git push does not push
 the current branch to its upstream @{u} unless @{u} happens to have
 the same name.

Then where does it get pushed?  Do you always specify where to save your work?

FWIW, I think the idea of treating @{u} as the eventual recipient of
your changes is good, but then it seems like Git is lacking the
publish my changes to this other branch concept.

Am I missing something?  If there is something other than @{u} to
represent this latter concept, I think `git push` should default to
that instead.  But, at least with my current knowledge, that doesn't
exist--without explicitly saying so--or treating @{u} as that branch.
If there's a better way to do this, I'd love to hear it!

Thanks!

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

Re: [PATCH 2/2] format-patch: introduce format.defaultTo

[John Szakmeister]

On Mon, Jan 6, 2014 at 5:54 PM, Ramkumar Ramachandra artag...@gmail.com wrote:
 John Szakmeister wrote:
 Then where does it get pushed?  Do you always specify where to save your 
 work?

 FWIW, I think the idea of treating @{u} as the eventual recipient of
 your changes is good, but then it seems like Git is lacking the
 publish my changes to this other branch concept.

 Am I missing something?  If there is something other than @{u} to
 represent this latter concept, I think `git push` should default to
 that instead.  But, at least with my current knowledge, that doesn't
 exist--without explicitly saying so--or treating @{u} as that branch.
 If there's a better way to do this, I'd love to hear it!

 That's why we invented remote.pushdefault and branch.*.pushremote. When you 
 say

   $ git push

 it automatically goes to the right remote instead of going to the
 place you fetched from. You can read up on how push.default interacts
 with this setting too, although I always recommend push.default =
 current.

This was the piece that I was missing.  I remember when
remote.pushdefault was added, but questioned its usefulness because it
just changes the remote that a branch is pushed to, not necessarily
the name.  I somehow missed the 'current' option for 'push.default'...
which is surprising since I seem to spend an incredible amount of time
looking at the git-config man page.  I guess it's not a good idea to
set 'push.default' to 'upstream' in this triangle workflow then,
otherwise the branch name being pushed to will be 'branch.*.merge'.
Is that correct?  I just want to make sure I understand what's
happening here.

Given this new found knowledge, I'm not sure it makes sense for `git
status` to show me the status against the upstream vs. the publish
location.  The latter makes a little more sense to me, though I see
the usefulness of either one.

Thanks for taking the time to explain this.  I'm going to have to
spend some more time with this configuration and see what the sticking
points are.

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

[PATCH v3] contrib/git-credential-gnome-keyring.c: small stylistic cleanups

[John Szakmeister]

Signed-off-by: John Szakmeister j...@szakmeister.net
Signed-off-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Felipe Contreras felipe.contre...@gmail.com
---

Thanks for the extra patch Junio.  I incorporated it and fixed a
few other minor violations I found afterwards.  This version
builds on the first version of the patch--without dropping the
gpointer casts.

-John

 .../gnome-keyring/git-credential-gnome-keyring.c   | 85 ++
 1 file changed, 39 insertions(+), 46 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 635c96b..2a317fc 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -60,7 +60,7 @@
 #define gnome_keyring_memory_free gnome_keyring_free_password
 #define gnome_keyring_memory_strdup g_strdup
 
-static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)
+static const char *gnome_keyring_result_to_message(GnomeKeyringResult result)
 {
switch (result) {
case GNOME_KEYRING_RESULT_OK:
@@ -95,9 +95,9 @@ static const char* 
gnome_keyring_result_to_message(GnomeKeyringResult result)
 
 static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer 
user_data)
 {
-   gpointer *data = (gpointer*) user_data;
-   int *done = (int*) data[0];
-   GnomeKeyringResult *r = (GnomeKeyringResult*) data[1];
+   gpointer *data = (gpointer *)user_data;
+   int *done = (int *)data[0];
+   GnomeKeyringResult *r = (GnomeKeyringResult *)data[1];
 
*r = result;
*done = 1;
@@ -130,34 +130,30 @@ static GnomeKeyringResult 
gnome_keyring_item_delete_sync(const char *keyring, gu
 /*
  * This credential struct and API is simplified from git's credential.{h,c}
  */
-struct credential
-{
-   char  *protocol;
-   char  *host;
+struct credential {
+   char *protocol;
+   char *host;
unsigned short port;
-   char  *path;
-   char  *username;
-   char  *password;
+   char *path;
+   char *username;
+   char *password;
 };
 
-#define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
 
-typedef int (*credential_op_cb)(struct credential*);
+typedef int (*credential_op_cb)(struct credential *);
 
-struct credential_operation
-{
-   char *name;
+struct credential_operation {
+   char *name;
credential_op_cb op;
 };
 
-#define CREDENTIAL_OP_END \
-  { NULL,NULL }
+#define CREDENTIAL_OP_END { NULL, NULL }
 
 /* - GNOME Keyring functions - */
 
 /* create a special keyring option string, if path is given */
-static char* keyring_object(struct credential *c)
+static char *keyring_object(struct credential *c)
 {
if (!c-path)
return NULL;
@@ -170,7 +166,7 @@ static char* keyring_object(struct credential *c)
 
 static int keyring_get(struct credential *c)
 {
-   char* object = NULL;
+   char *object = NULL;
GList *entries;
GnomeKeyringNetworkPasswordData *password_data;
GnomeKeyringResult result;
@@ -204,7 +200,7 @@ static int keyring_get(struct credential *c)
}
 
/* pick the first one from the list */
-   password_data = (GnomeKeyringNetworkPasswordData *) entries-data;
+   password_data = (GnomeKeyringNetworkPasswordData *)entries-data;
 
gnome_keyring_memory_free(c-password);
c-password = gnome_keyring_memory_strdup(password_data-password);
@@ -221,7 +217,7 @@ static int keyring_get(struct credential *c)
 static int keyring_store(struct credential *c)
 {
guint32 item_id;
-   char  *object = NULL;
+   char *object = NULL;
GnomeKeyringResult result;
 
/*
@@ -262,7 +258,7 @@ static int keyring_store(struct credential *c)
 
 static int keyring_erase(struct credential *c)
 {
-   char  *object = NULL;
+   char *object = NULL;
GList *entries;
GnomeKeyringNetworkPasswordData *password_data;
GnomeKeyringResult result;
@@ -298,22 +294,20 @@ static int keyring_erase(struct credential *c)
if (result == GNOME_KEYRING_RESULT_CANCELLED)
return EXIT_SUCCESS;
 
-   if (result != GNOME_KEYRING_RESULT_OK)
-   {
+   if (result != GNOME_KEYRING_RESULT_OK) {
g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
 
/* pick the first one from the list (delete all matches?) */
-   password_data = (GnomeKeyringNetworkPasswordData *) entries-data;
+   password_data = (GnomeKeyringNetworkPasswordData *)entries-data;
 
result = gnome_keyring_item_delete_sync(
password_data-keyring, password_data-item_id);
 
gnome_keyring_network_password_list_free

Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups

[John Szakmeister]

On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano gits...@pobox.com wrote:
[snip]

 I thought we cast without SP after the (typename), i.e.

 gpointer *data = (gpointer *)user_data;

I've found a mixture of both in the code base, and the
CodingGuidelines doesn't say either way.  I'm happy to switch the file
to no SP after the typename if that's the project preference.

 It could be argued that a cast that turns a void * to a pointer to
 another type can go, as Felipe noted, but I think that is better
 done in a separate patch, perhaps as a follow-up to this small
 stylistic clean-ups.

 I said it could be argued above, because I am on the fence on that
 change.  If this were not using a type gpointer, whose point is to
 hide what the actual implementation of that type is, but a plain
 vanilla void *, then I would not have any doubt.  But it feels
 wrong to look behind that deliberate gpointer abstraction and take
 advantage of the knowledge that it happens to be implemented as
 void * (and if we do not start from that knowledge, losing the
 cast is a wrong change).

To be honest, I'm on the fence myself.  Let's just leave the original
patch queued, and if the no SP is preferable, I can do that as a
separate patch.

-John

PS  Sorry about the repeat message Junio.  I forgot to CC the list.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] contrib/git-credential-gnome-keyring.c: small stylistic cleanups

[John Szakmeister]

Signed-off-by: John Szakmeister j...@szakmeister.net
Reviewed-by: Felipe Contreras felipe.contre...@gmail.com
---
v2 removes some unnecessary casting noticed by Felipe.  Thanks for the
feedback Felipe!

 .../gnome-keyring/git-credential-gnome-keyring.c   | 55 ++
 1 file changed, 25 insertions(+), 30 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 635c96b..302122c 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -95,9 +95,9 @@ static const char* 
gnome_keyring_result_to_message(GnomeKeyringResult result)
 
 static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer 
user_data)
 {
-   gpointer *data = (gpointer*) user_data;
-   int *done = (int*) data[0];
-   GnomeKeyringResult *r = (GnomeKeyringResult*) data[1];
+   gpointer *data = user_data;
+   int *done = data[0];
+   GnomeKeyringResult *r = data[1];
 
*r = result;
*done = 1;
@@ -132,27 +132,25 @@ static GnomeKeyringResult 
gnome_keyring_item_delete_sync(const char *keyring, gu
  */
 struct credential
 {
-   char  *protocol;
-   char  *host;
+   char *protocol;
+   char *host;
unsigned short port;
-   char  *path;
-   char  *username;
-   char  *password;
+   char *path;
+   char *username;
+   char *password;
 };
 
-#define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
 
-typedef int (*credential_op_cb)(struct credential*);
+typedef int (*credential_op_cb)(struct credential *);
 
 struct credential_operation
 {
-   char *name;
+   char *name;
credential_op_cb op;
 };
 
-#define CREDENTIAL_OP_END \
-  { NULL,NULL }
+#define CREDENTIAL_OP_END { NULL, NULL }
 
 /* - GNOME Keyring functions - */
 
@@ -221,7 +219,7 @@ static int keyring_get(struct credential *c)
 static int keyring_store(struct credential *c)
 {
guint32 item_id;
-   char  *object = NULL;
+   char *object = NULL;
GnomeKeyringResult result;
 
/*
@@ -262,7 +260,7 @@ static int keyring_store(struct credential *c)
 
 static int keyring_erase(struct credential *c)
 {
-   char  *object = NULL;
+   char *object = NULL;
GList *entries;
GnomeKeyringNetworkPasswordData *password_data;
GnomeKeyringResult result;
@@ -298,8 +296,7 @@ static int keyring_erase(struct credential *c)
if (result == GNOME_KEYRING_RESULT_CANCELLED)
return EXIT_SUCCESS;
 
-   if (result != GNOME_KEYRING_RESULT_OK)
-   {
+   if (result != GNOME_KEYRING_RESULT_OK) {
g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
@@ -312,8 +309,7 @@ static int keyring_erase(struct credential *c)
 
gnome_keyring_network_password_list_free(entries);
 
-   if (result != GNOME_KEYRING_RESULT_OK)
-   {
+   if (result != GNOME_KEYRING_RESULT_OK) {
g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
@@ -325,9 +321,8 @@ static int keyring_erase(struct credential *c)
  * Table with helper operation callbacks, used by generic
  * credential helper main function.
  */
-static struct credential_operation const credential_helper_ops[] =
-{
-   { get,   keyring_get   },
+static struct credential_operation const credential_helper_ops[] = {
+   { get,   keyring_get },
{ store, keyring_store },
{ erase, keyring_erase },
CREDENTIAL_OP_END
@@ -370,7 +365,7 @@ static int credential_read(struct credential *c)
if (!line_len)
break;
 
-   value = strchr(buf,'=');
+   value = strchr(buf, '=');
if (!value) {
g_warning(invalid credential line: %s, key);
gnome_keyring_memory_free(buf);
@@ -384,7 +379,7 @@ static int credential_read(struct credential *c)
} else if (!strcmp(key, host)) {
g_free(c-host);
c-host = g_strdup(value);
-   value = strrchr(c-host,':');
+   value = strrchr(c-host, ':');
if (value) {
*value++ = '\0';
c-port = atoi(value);
@@ -429,16 +424,16 @@ static void credential_write(const struct credential *c)
 static void usage(const char *name)
 {
struct credential_operation const *try_op = credential_helper_ops;
-   const char *basename = strrchr(name,'/');
+   const char *basename = strrchr(name, '/');
 
basename = (basename) ? basename + 1

[PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups

[John Szakmeister]

Signed-off-by: John Szakmeister j...@szakmeister.net
---
The gnome-keyring credential backend had a number of coding style
violations.  I believe this fixes all of them.

 .../gnome-keyring/git-credential-gnome-keyring.c   | 55 ++
 1 file changed, 25 insertions(+), 30 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 635c96b..1613404 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -95,9 +95,9 @@ static const char* 
gnome_keyring_result_to_message(GnomeKeyringResult result)
 
 static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer 
user_data)
 {
-   gpointer *data = (gpointer*) user_data;
-   int *done = (int*) data[0];
-   GnomeKeyringResult *r = (GnomeKeyringResult*) data[1];
+   gpointer *data = (gpointer *) user_data;
+   int *done = (int *) data[0];
+   GnomeKeyringResult *r = (GnomeKeyringResult *) data[1];
 
*r = result;
*done = 1;
@@ -132,27 +132,25 @@ static GnomeKeyringResult 
gnome_keyring_item_delete_sync(const char *keyring, gu
  */
 struct credential
 {
-   char  *protocol;
-   char  *host;
+   char *protocol;
+   char *host;
unsigned short port;
-   char  *path;
-   char  *username;
-   char  *password;
+   char *path;
+   char *username;
+   char *password;
 };
 
-#define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
 
-typedef int (*credential_op_cb)(struct credential*);
+typedef int (*credential_op_cb)(struct credential *);
 
 struct credential_operation
 {
-   char *name;
+   char *name;
credential_op_cb op;
 };
 
-#define CREDENTIAL_OP_END \
-  { NULL,NULL }
+#define CREDENTIAL_OP_END { NULL, NULL }
 
 /* - GNOME Keyring functions - */
 
@@ -221,7 +219,7 @@ static int keyring_get(struct credential *c)
 static int keyring_store(struct credential *c)
 {
guint32 item_id;
-   char  *object = NULL;
+   char *object = NULL;
GnomeKeyringResult result;
 
/*
@@ -262,7 +260,7 @@ static int keyring_store(struct credential *c)
 
 static int keyring_erase(struct credential *c)
 {
-   char  *object = NULL;
+   char *object = NULL;
GList *entries;
GnomeKeyringNetworkPasswordData *password_data;
GnomeKeyringResult result;
@@ -298,8 +296,7 @@ static int keyring_erase(struct credential *c)
if (result == GNOME_KEYRING_RESULT_CANCELLED)
return EXIT_SUCCESS;
 
-   if (result != GNOME_KEYRING_RESULT_OK)
-   {
+   if (result != GNOME_KEYRING_RESULT_OK) {
g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
@@ -312,8 +309,7 @@ static int keyring_erase(struct credential *c)
 
gnome_keyring_network_password_list_free(entries);
 
-   if (result != GNOME_KEYRING_RESULT_OK)
-   {
+   if (result != GNOME_KEYRING_RESULT_OK) {
g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
@@ -325,9 +321,8 @@ static int keyring_erase(struct credential *c)
  * Table with helper operation callbacks, used by generic
  * credential helper main function.
  */
-static struct credential_operation const credential_helper_ops[] =
-{
-   { get,   keyring_get   },
+static struct credential_operation const credential_helper_ops[] = {
+   { get,   keyring_get },
{ store, keyring_store },
{ erase, keyring_erase },
CREDENTIAL_OP_END
@@ -370,7 +365,7 @@ static int credential_read(struct credential *c)
if (!line_len)
break;
 
-   value = strchr(buf,'=');
+   value = strchr(buf, '=');
if (!value) {
g_warning(invalid credential line: %s, key);
gnome_keyring_memory_free(buf);
@@ -384,7 +379,7 @@ static int credential_read(struct credential *c)
} else if (!strcmp(key, host)) {
g_free(c-host);
c-host = g_strdup(value);
-   value = strrchr(c-host,':');
+   value = strrchr(c-host, ':');
if (value) {
*value++ = '\0';
c-port = atoi(value);
@@ -429,16 +424,16 @@ static void credential_write(const struct credential *c)
 static void usage(const char *name)
 {
struct credential_operation const *try_op = credential_helper_ops;
-   const char *basename = strrchr(name,'/');
+   const char *basename = strrchr(name, '/');
 
basename = (basename

Re: [PATCH v2 00/14] Officially start moving to the term 'staging area'

[John Szakmeister]

On Fri, Oct 18, 2013 at 5:46 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 I'm lacking time to read and answer in detail, sorry.

 Junio C Hamano gits...@pobox.com writes:

 It must be done is different from any change is good, as long as
 it introduces more instances of word 'stage'.

 I agree. Something must be done, at least to remove the cache Vs index
 confusion. I'm not sure exactly what's best, and we should agree where
 to go before going there. The previous attempts to introduce more
 stage in Git's command-line (e.g. the git stage alias) introduced
 more confusion than anything else.

I definitely agree about removing the cache vs. index confusion.  I'm
curious about the confusions surrounding this git stage alias.  Was
it simply an implementation issue, or was it an issue surrounding the
name?

FWIW, I've trained my employees to think of it as a staging area as
well.  At least in English, it seems to be the best understood analogy
to the index's purpose.

 The phrase staging area is not an everyday phrase or common CS
 lingo, and that unfortunately makes it a suboptimal choice of words
 especially to those of us, to whom a large portion of their exposure
 to the English language is through the command words we use when we
 talk to our computers.

 I do not think being understandable immediately by non-native is so
 important actually. To me as a french, commit makes no sense as an
 english word to describe what git commit does, but it's OK as I never
 really translate it. Even fr.po translates a commit by un commit.

 That said, having something that immediately makes sense to a non-native
 is obviously a good point.

 Another proposal which I liked BTW was to use the word precommit.
 Short, and easily understood as the place where the next commit is
 prepared.

I'm not sure what concept precommit invokes, but it's certainly not
where the next commit is prepared.  Two thoughts come to mind: the
precommit hook, and what is a pre-commit?  How would you talk about
preparing for a commit?  Do you precommit a file?  Add the file to
the precommit?  I'm just curious.

Thanks!

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

Re: [PATCH v3] Add core.mode configuration

[John Szakmeister]

On Tue, Oct 15, 2013 at 11:55 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
[snip]
 We cannot change the behavior of push.default = simple already, so at least
 that option is not in question.

True.

 Presumably you are worried about the other options that can't be enabled in 
 any
 way.

Yes.

 But think about this; you are worried that if we add an *option* to enable 
 this
 new behaviors, then we would be kind of forced to keep these behaviors. That
 seems to imply that you are proposing the current default; we wait until 2.0
 and not make it an *option*, but make it *default*.

 I think waiting until 2.0 to make it a default without evern having an option,
 and thus nobody actuallly testing this, is way worst than what I'm proposing;
 to add an option to start testing.

My concern is that people don't treat it for what it is--a way to
experiment with the new behaviors--and then they get upset if we
discover that some behavior was not well thought out and it disappears
unexpectedly when we correct the matter.  We have a balance to
strike: annoying users and getting some miles on the new behaviors.  I
see the technical end of this--your proposal to have a
'core.mode'--but where is the non-technical end of this argument?
What message are we proposing to send to the users?  What's our
promise to them surrounding core.mode and the new behaviors it offers?
 Perhaps we don't have much today that this affects, but what about
tomorrow?  Are we saying that behaviors enabled by core.mode=next are
concrete (they're going in as-is, and we won't alter their behavior
before 2.0)?

As I said, the only real drawback is that I see this as the latter,
because any other choice means users will get annoyed when something
changes out from under them.

 So, at the end of the day, I'm just not sure it's worthwhile to have.

 This is exactly what happened on 1.6; nobody really tested the 'git foo'
 behavior, so we just switched from one version to the next. If you are not
 familiar with the outcome; it wasn't good.

You're right, I wasn't around for that.  And on the whole, I
absolutely agree: it's nice to get miles on these new
behaviors/features/etc.  I just worry that having an option like this
means we've committed to it, and I'm not sure that we want to give up
the ability to change them without having to go through some sort of
deprecation cycle.  Or worse, we have to wait until 3.0 and 2.0 hasn't
even come out yet.

I hope others chime in here.  And don't mistake me as dissenting; I'm
not.  And, I'm not assenting either.  I just want to know if you've
thought about what this means to users, and what we're prepared to
deal with.  Right now, I feel like half the argument around the option
is missing.

 So I say we shouldn't just provide warnings, but also have an option to allow
 users (probably a minority) to start testing this.

probably a minority -- I guess that's the part I disagree with.  I'm
not sure what a minority means here, but I don't think it'll be a
handful of people.  How big does that number get before we get
concerned about backlash from users if we decide to change course?
Or, is that simply not an issue?  Why or why not?  I have to be
honest, if the option was available, I'd have my developers turn it
on.  I'm sure a great deal of others would do so too.

Is there some other way we can solve this?  Having an experimental
branch with all the 2.0 features merged and those concerned can just
build that version?  I see the downside of that too: it's not as easy
for people to try, and there is nothing preventing folks from posting
binaries with the new behaviors enabled.  It leads me to feeling that
we're stuck in some regard.  But maybe I'm being overly pessimistic
here, and it's really all a non-issue.  As I said earlier, it'd be
nice if others chimed in here.

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

Re: [PATCH v3] Add core.mode configuration

[John Szakmeister]

On Wed, Oct 16, 2013 at 6:54 AM, John Szakmeister j...@szakmeister.net wrote:
[snip]
 probably a minority -- I guess that's the part I disagree with.  I'm
 not sure what a minority means here, but I don't think it'll be a
 handful of people.  How big does that number get before we get
 concerned about backlash from users if we decide to change course?
 Or, is that simply not an issue?  Why or why not?  I have to be
 honest, if the option was available, I'd have my developers turn it
 on.  I'm sure a great deal of others would do so too.

 Is there some other way we can solve this?  Having an experimental
 branch with all the 2.0 features merged and those concerned can just
 build that version?  I see the downside of that too: it's not as easy
 for people to try, and there is nothing preventing folks from posting
 binaries with the new behaviors enabled.  It leads me to feeling that
 we're stuck in some regard.  But maybe I'm being overly pessimistic
 here, and it's really all a non-issue.  As I said earlier, it'd be
 nice if others chimed in here.

Thinking about this a little more, we do have a proving ground.
That's what the whole pu/next/master construct is for.  So maybe this
is a non-issue.  By the time it lands on master, we should have
decided whether the feature is worth keeping or not.

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

Re: [PATCH v3] Add core.mode configuration

[John Szakmeister]

On Tue, Oct 15, 2013 at 10:51 AM, Krzysztof Mazur krzys...@podlesie.net wrote:
 On Tue, Oct 15, 2013 at 08:29:56AM -0500, Felipe Contreras wrote:
 Krzysztof Mazur wrote:
  On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote:
   Krzysztof Mazur wrote:
   
But with core.mode = next after upgrade you may experience incompatible
change without any warning.
  
   Yes, and that is actually what the user wants. I mean, why would the 
   user set
   core.mode=next, if the user doesn't want to experencie incompatible 
   changes? A
   user that sets this mode is expecting incompatible changes, and will be 
   willing
   to test them, and report back if there's any problem with them.
 
  With your patch, because it's the only way to have 'git add' v2.0.

 Yeah, but that's not what I'm suggesting. I suggested to have *both* a
 fined-tunned way to have this behavior, say core.addremove = true, and a way 
 to
 enable *all* v2.0 behaviors (core.mode = next).

 I'm just not sure if a lot of users would use core.mode=next, because
 of possible different behavior without any warning. Maybe we should also
 add core.mode=next-warn that changes defaults like next but keeps warnings
 enabled until the user accepts that change by setting appropriate
 config option? That's safer than next (at least for interactive use) and
 maybe more users would use that, but I don't think that's worth adding.

I like the idea that we could kick git into a mode that applies the
behaviors we're talking about having in 2.0, but I'm concerned about
one aspect of it.  Not having these behaviors until 2.0 hits means
we're free to renege on our decisions in favor of something better, or
to pull out a bad idea.  But once we insert this knob, I don't know
that we have the same ability.  Once people realize it's there and
start using it, it gets harder to back out.  I guess we could maintain
the stance that the features are not concrete yet, or something like
that, but I think people would still get upset if something changes
out from under them.

So, at the end of the day, I'm just not sure it's worthwhile to have.

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

Re: [PATCH v3] build: add default aliases

[John Szakmeister]

On Tue, Sep 24, 2013 at 10:35 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Tue, Sep 24, 2013 at 6:06 AM, John Szakmeister j...@szakmeister.net 
 wrote:
[snip]
 There's nothing objective about Nobody every (sic) agrees changes are
 good.  If it were true, no changes would get in.

 It is true, where by changes I mean changes from common user's
 point of view, actually, a tiny amount of then do sneak in, so let me
 be precise: Virtually nobody ever agrees important changes from the
 common user's point of view are good.

 So now that I'm being clear, do tell, name one important change in Git
 from the user's point of view that happened in the last two years.

Credential helpers.

 Also, you don't know that any of those changes would benefit 99% of
 all users.  It's a guess or an estimate but it's not based on
 anything concrete.  It might be a good guess--and in this case, I
 think it is--but it's not a concrete fact.  Don't make it sound like
 it is.

 Sure, it's not a concrete fact, but the actual probability most likely
 follows a beta distribution with alpha=15 and beta=1. Is that more
 precise for you?

It's not about precision, it's that you don't have any actual facts to
stand on but you speak like you do.  Then when someone questions it,
you accuse them of being against fixes for the user.  I wish you'd
stop it and do something more constructive to help move things along.

 If you don't agree my comment is accurate, that's one thing, but
 labeling it as an attack is another.

 Don't turn it around.  A number of your patches and emails poke at the
 community of the Git project and you know it.  It's simply not helping
 the situation.

 Show me a patch that pokes at the community. All my patches have
 technical descriptions, and help improve Git.

You're filtering what I said again.  Take a look at the first message
is this thread.

I'll agree the patches themselves have been free of this, but the
cover letters--which I consider to be part of the patch--and ensuing
conversation has not.  If you need another example, look at the cover
letter for your transport helper patches.

None of this is news to you.

[snip]
 I would admit I was wrong if an /etc/gitconfig is indeed shipped by
 default, and agree that the Git project is indeed welcome to change,
 but that's not going to happen.

 And there it is again.  Predicting the future now?  Objectively and
 accurately?  Please stop.

 Yes I am. Feel free to go back to this mail and tell me I was wrong
 when they do what I said they won't.

I have no need for that, and I'm done talking to you about any of this.

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

Re: [PATCH v3] build: add default aliases

[John Szakmeister]

On Sat, Sep 21, 2013 at 3:20 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 For now simply add a few common aliases.

   co = checkout
   ci = commit
   rb = rebase
   st = status

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---

 I still think we should ship a default /etc/gitconfig, but the project needs 
 to
 agree it's a good change, and nobody every agrees changes are good. So this is
 the minimal change that achieves the desired result.

I wish you would stop attacking the project every time you send a
patch--it's simply not productive and it's certainly not getting you
any closer to a resolution.

That said, I think the idea of having some default aliases is good,
and these match what several other version control systems have
already.

FWIW, I alias st to status -sb, but I'm not convinced it's a good
default.  I think the safe thing to do is to just alias it to
status--like you did here--and let folks override it, if they want
something more.

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

Re: [PATCH v3] build: add default aliases

[John Szakmeister]

On Tue, Sep 24, 2013 at 6:25 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Tue, Sep 24, 2013 at 4:19 AM, John Szakmeister j...@szakmeister.net 
 wrote:
 On Sat, Sep 21, 2013 at 3:20 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 For now simply add a few common aliases.

   co = checkout
   ci = commit
   rb = rebase
   st = status

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---

 I still think we should ship a default /etc/gitconfig, but the project 
 needs to
 agree it's a good change, and nobody every agrees changes are good. So this 
 is
 the minimal change that achieves the desired result.

 I wish you would stop attacking the project every time you send a
 patch--it's simply not productive and it's certainly not getting you
 any closer to a resolution.

 I'm not attacking the project, I'm making an objective claim, and I
 can back it up with several instances of evidence where 99% of the
 users would benefit from a change, yet it does not move forward.

There's nothing objective about Nobody every (sic) agrees changes are
good.  If it were true, no changes would get in.

Also, you don't know that any of those changes would benefit 99% of
all users.  It's a guess or an estimate but it's not based on
anything concrete.  It might be a good guess--and in this case, I
think it is--but it's not a concrete fact.  Don't make it sound like
it is.

 If you don't agree my comment is accurate, that's one thing, but
 labeling it as an attack is another.

Don't turn it around.  A number of your patches and emails poke at the
community of the Git project and you know it.  It's simply not helping
the situation.

Your clearly a bright and motivated individual--which makes it all the
more frustrating that you don't approach this differently.  I even
agree with your motivations for Git: I'd like to see less shell and
perl involved and to see Git run faster on Windows.  But I wish you'd
stop with the jabs.

 I would admit I was wrong if an /etc/gitconfig is indeed shipped by
 default, and agree that the Git project is indeed welcome to change,
 but that's not going to happen.

And there it is again.  Predicting the future now?  Objectively and
accurately?  Please stop.

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

Re: [PATCH v3] build: add default aliases

[John Szakmeister]

On Tue, Sep 24, 2013 at 2:39 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Jeff King wrote:
 On Sat, Sep 21, 2013 at 02:20:21PM -0500, Felipe Contreras wrote:

 For now simply add a few common aliases.

   co = checkout
   ci = commit
   rb = rebase
   st = status

 Are these the best definitions of those shortcuts? It seems[1] that some
 people define ci as commit -a, and some people define st as
 status -s or even status -sb.

 I feel bad about having waited for 4 rounds of this patch to say this,
 but the basic change (providing co, ci, etc. as aliases by
 default) does not look well thought through.

 It would be a different story if this were a patch to update
 documentation to suggest adding such aliases at the same time as
 telling Git what your name is.  The user manual doesn't explain how to
 set up aliases at all yet, and that should be fixed.

Yes, better documentation around aliases would be a good idea.

 But making 'ci' a synonym of another command by default while still
 keeping its definition configurable would be doing people a
 disservice, I fear.  As long as 'ci' works out of the box, it will
 start showing up in examples and used in suggestions over IRC, etc,
 which is great.  Unfortunately that means that anyone who has 'ci'
 defined to mean something different can no longer use those examples,
 that advice from IRC, and so on.  So in the world where 'ci' is a
 synonym for 'commit' by default, while people still *can* redefine
 'ci' to include whatever options they like (e.g., -a), actually
 carrying out such a personal customization is asking for trouble.

I'm not sure I agree.  Yes, if someone has configured their shortcut
differently, they may not be able to use the example exactly.  OTOH,
shells have aliases, and while there are sometimes problems, I think
most people overcome them.  I don't see the situation being very
different here.

FWIW, I'm not overly convinced one way or another.  What I can say is,
in the circles I run in, I can only think of one person has gone
without setting up aliases to commit (ci), status (st), and checkout
(co).  The full names are simply to long for day-to-day usage.

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

Re: [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros

[John Szakmeister]

On Sun, Sep 22, 2013 at 10:07:56PM -0700, Brandon Casey wrote:
 A few cleanups, followed by improved usage of the glib library (no need
 to reinvent the wheel when glib provides the necessary functionality), and
 then the addition of support for RHEL 4.x and 5.x.
 
 Brandon Casey (15):
   contrib/git-credential-gnome-keyring.c: remove unnecessary
 pre-declarations
   contrib/git-credential-gnome-keyring.c: remove unused die() function
   contrib/git-credential-gnome-keyring.c: add static where applicable
   contrib/git-credential-gnome-keyring.c: exit non-zero when called
 incorrectly
   contrib/git-credential-gnome-keyring.c: set Gnome application name
   contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not
 ssize_t
   contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty
 before accessing
   contrib/git-credential-gnome-keyring.c: use gnome helpers in
 keyring_object()
   contrib/git-credential-gnome-keyring.c: use secure memory functions
 for passwds
   contrib/git-credential-gnome-keyring.c: use secure memory for reading
 passwords
   contrib/git-credential-gnome-keyring.c: use glib memory allocation
 functions
   contrib/git-credential-gnome-keyring.c: use glib messaging functions
   contrib/git-credential-gnome-keyring.c: report failure to store
 password
   contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring
   contrib/git-credential-gnome-keyring.c: support really ancient
 gnome-keyring

I reviewed all of the commits in this series, and most are nice
cleanups.  The only thing I'm a little shaky on is RHEL4
support (patch 15).  In particular, this:

+/*
+ * Just a guess to support RHEL 4.X.
+ * Glib 2.8 was roughly Gnome 2.12 ?
+ * Which was released with gnome-keyring 0.4.3 ??
+ */

Has that code been tested on RHEL4 at all?  I imagine it's hard
to come by--it's pretty darn old--but it feels like a mistake to
commit untested code.

Otherwise, there are a few stylistic nits that I'd like to clean
up.  Only one was introduced by you--Felipe pointed it out--and
I have a patch for the rest that I can submit after your series
has been applied.

Acked-by: John Szakmeister j...@szakmeister.net

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

Re: Local tag killer

[John Szakmeister]

On Sat, Sep 21, 2013 at 2:42 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 09/21/2013 12:51 AM, Junio C Hamano wrote:
 Junio C Hamano gitster-v...@pobox.com writes:

 I also agree that the documentation is misstated; remote-tracking branch
 may have been a convenient and well understood phrase for whoever wrote
 that part, but the --prune is designed to cull extra refs in the
 hierarchies into
 which refs would be fetched if counterparts existed on the other side, so
 culling tags that do not exist on the remote side should also be described.

 (gleaning-leftovers mode)

 Thanks for following up on this with your proposed documentation patch.
  I have been researching and experimenting, and still find the use of
 fetch confusing with respect to tags.  I think the problem is primarily
 that the behavior is awkward, and that it would be better to change the
 behavior than to document the awkward behavior.

I agree with this sentiment.  I've never liked how `--tags` operates.

 I must have read an old version of the documentation, from which it
 seemed that git fetch --tags fetches all tags from the remote *in
 addition to* the references and tags that would otherwise be fetched.
 This seems like a handy and safe feature, and I wish that this were
 indeed the effect of --tags.

Me too.

 But I see that the documentation for --tags has been changed and now
 states explicitly that --tags is equivalent to specifying
 refs/tags/*:refs/tags/* on the command line, overriding any configured
 refspecs.  This doesn't seem like useful behavior; why would I want to
 fetch tags from a remote without also updating the configured refspecs?
  And contrariwise, how can I fetch the configured refspecs *and* all
 tags at the same time in a single fetch?

 OK, one way to do it is to configure an explicit refspec for fetching
 the tags:

 [remote origin]
 url = [...]
 fetch = +refs/heads/*:refs/remotes/origin/*
 fetch = refs/tags/*:refs/tags/*

 [Here is one oddity: even if the tags refspec doesn't have a + prefix,
 git fetch will do non-ff updates to tags, presumably because of the
 implicit tag-fetching behavior.]

 But if I use this configuration and type git fetch --prune, then any
 local tags that are not present on the remote will be killed.

 In short, when local tags are in use, or tags that are in one remote but
 not another [1], then the current Git implementation makes it impossible to

 - Configure fetch.prune or remote.$REMOTE.prune without preventing
 the use of fetch --tags

 - Configure default fetching of all tags (via a refspec or via
 remote.$REMOTE.tagopt) without preventing the use of fetch --prune

 - Configure fetch.prune or remote.$REMOTE.prune and the default
 fetching of all tags (via a refspec or via remote.$REMOTE.tagopt) at the
 same time.

 This is unfortunate.

 I think it would be preferable if --prune would *not* affect tags, and
 if there were an extra option like --prune-tags that would have to be
 used explicitly to cause tags to be pruned.  Would somebody object to
 such a change?

I, personally, think what you outline makes more sense.  Also, I'm
curious if `git remote update -p $REMOTE` suffers from the same
problem, if the remote was added with the `--tags` option.

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

Re: [RFC] Disabling status hints in COMMIT_EDITMSG

[John Szakmeister]

On Wed, Sep 11, 2013 at 3:24 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Junio C Hamano gits...@pobox.com writes:

 But at the same time, I feel that these redundant lines, especially
 the latter one, would give the users a stronger cue than just saying
 that bar is Untracked; do X to include reminds that bar will not
 be included if nothing is done.

 The one which draw my attention was (use git commit to conclude
 merge) which is particularly counter-productive when you are already
 doing a git commit. The advice for untracked files is less
 counter-productive, but while we're removing the non-sensical ones, I
 think it makes sense to remove the essentially-useless ones too.

FWIW, I think it makes sense to remove the extra advice too.

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

Zero padded file modes...

[John Szakmeister]

I went to clone a repository from GitHub today and discovered
something interesting:

:: git clone https://github.com/liebke/incanter.git
Cloning into 'incanter'...
remote: Counting objects: 10457, done.
remote: Compressing objects: 100% (3018/3018), done.
error: object 4946e1ba09ba5655202a7a5d81ae106b08411061:contains
zero-padded file modes
fatal: Error in object
fatal: index-pack failed

At first, it surprised me that no one has seen the issue before,
but then I remembered I have transfer.fsckObjects=true in my
config.  Turning it off, I was able to clone.  Running `git
fsck` I see:

:: git fsck
Checking object directories: 100% (256/256), done.
warning in tree 4946e1ba09ba5655202a7a5d81ae106b08411061: contains
zero-padded file modes
warning in tree 553c5e006e53a8360126f053c3ade3d1d063c2f5: contains
zero-padded file modes
warning in tree 0a2e7f55d7f8e1fa5469e6d83ff20365881eed1a: contains
zero-padded file modes
Checking objects: 100% (10560/10560), done.

So there appears to be several instances of the issue in the
tree.  Looking in the archives, I ran across this thread:

http://comments.gmane.org/gmane.comp.version-control.git/143288

In there, Nicolas Pitre says:

 This is going to screw up pack v4 (yes, someday I'll have the
 time to make it real).

I don't know if this is still true, but given that patches are
being sent out about it, I thought it relevant.

Also, searching on the issue, you'll find that a number of
repositories have suffered from this problem, and it appears the
only fix will result in different commit ids.  Given all of
that, should Git be updated to cope with zero padded modes?  Or
is there no some way of fixing the issue that doesn't involve
changing commit ids?

Thanks!

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

Re: Zero padded file modes...

[John Szakmeister]

On Thu, Sep 5, 2013 at 11:36 AM, Jeff King p...@peff.net wrote:
 On Thu, Sep 05, 2013 at 10:00:39AM -0400, John Szakmeister wrote:

 I went to clone a repository from GitHub today and discovered
 something interesting:

 :: git clone https://github.com/liebke/incanter.git
 Cloning into 'incanter'...
 remote: Counting objects: 10457, done.
 remote: Compressing objects: 100% (3018/3018), done.
 error: object 4946e1ba09ba5655202a7a5d81ae106b08411061:contains
 zero-padded file modes
 fatal: Error in object
 fatal: index-pack failed

 Yep. These were mostly caused by a bug in Grit that is long-fixed.  But
 the objects remain in many histories. It would have painful to rewrite
 them back then, and it would be even more painful now.

I guess there's still the other side of the question though.  Are
these repositories busted in the sense that something no longer works?
 I doesn't appear to be the case, but I've not used it extensively say
I can't say for certain one way or another.  In the sense that the
content is not strictly compliant, transfer.fsckObjects did its job,
but I wonder if fsck needs to be a little more tolerant now (at least
with respect to transfer objects)?

I can certainly cope with the issue--it's not a problem for me to flip
the flag on the command line.  I think it'd be nice to have
transer.fsckObjects be the default at some point, considering how
little people run fsck otherwise and how long these sorts of issues go
undiscovered.  Issues like the above seem to stand in the way of that
happening though.

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

Re: Git in nutshell Inbox

[John Szakmeister]

On Tue, Jul 9, 2013 at 10:39 AM, Matthieu Moy matthieu@imag.fr wrote:
 Muhammad Bashir Al-Noimi mbno...@gmail.com writes:

 I'm bzr user and I want to migrate to git. Generally I use bzr through
 Bazaar Explorer which is very easy  neat GUI utility for managing bzr
 repositories. May you please guide me to most easy way to migrate to
 Git?

 (I think it's a mistake to stick to GUI: the concepts are easier to
 understand with Git commands IMHO)

Ditto.  I think learning the command line makes the process much
easier in the end.

 But there are nice GUIs for Git too. The official one is git gui,
 distributed with Git. I do not like the visual aspect a lot, but it has
 a very good coverage of Git's functionalities. Otherwise, have a look at

   
 https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools#Graphical_Interfaces

 git-cola is a good one.

You might want to consider giggle.  I've not used it, but it's more
BzrExplorer-like, IIRC.

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

Re: [PATCH] prompt: do not double-discriminate detached HEAD

[John Szakmeister]

On Sun, Jul 7, 2013 at 8:52 AM, Ramkumar Ramachandra artag...@gmail.com wrote:
 When GIT_PS1_SHOWCOLORHINTS is turned on, there is no need to put a
 detached HEAD within parenthesis: the color can be used to discriminate
 the detached HEAD.

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  For cuteness :)

Personally, I'd rather see the parens kept.  Not everyone sees red
very well--I know several people who can't see it at all, and it keeps
it consistent with non-colored output.

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

Re: Bug on OS X...

[John Szakmeister]

On Fri, Jun 28, 2013 at 8:44 AM, Max Horn m...@quendi.de wrote:
[snip]
 I am unable to reproduce this on Mac OS X 10.7.5 with git 1.8.3.1 nor with 
 current git maint. Command run inside /tmp, which is on a normal HFS+ volume 
 (using the default settings, i.e. the FS is case insensitive).


 $ git --version
 git version 1.8.3.1.42.ge2652c0
 $ git clone --depth 1 git://nbd.name/packages.git
 Cloning into 'packages'...
 remote: Counting objects: 4711, done.
 remote: Compressing objects: 100% (3998/3998), done.
 remote: Total 4711 (delta 157), reused 3326 (delta 94)
 Receiving objects: 100% (4711/4711), 3.85 MiB | 0 bytes/s, done.
 Resolving deltas: 100% (157/157), done.

OK, so I finally tracked it down.  Commit
6035d6aad8ca11954c0d7821f6f3e7c047039c8f fixes it:

commit 6035d6aad8ca11954c0d7821f6f3e7c047039c8f
Author: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Date:   Sun May 26 08:16:15 2013 +0700

fetch-pack: prepare updated shallow file before fetching the pack

index-pack --strict looks up and follows parent commits. If shallow
information is not ready by the time index-pack is run, index-pack may
be led to non-existent objects. Make fetch-pack save shallow file to
disk before invoking index-pack.

git learns new global option --shallow-file to pass on the alternate
shallow file path. Undocumented (and not even support --shallow-file=
syntax) because it's unlikely to be used again elsewhere.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com

It looks like I was hitting the race condition.  It's fixed on master,
so I assume it will be in 1.8.3.2.

Thanks for taking a look though!

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

Re: Bug on OS X...

[John Szakmeister]

On Fri, Jun 28, 2013 at 1:13 PM, Junio C Hamano gits...@pobox.com wrote:
 John Szakmeister j...@szakmeister.net writes:

 On Fri, Jun 28, 2013 at 8:44 AM, Max Horn m...@quendi.de wrote:
 [snip]

 I am unable to reproduce this on Mac OS X 10.7.5 with git 1.8.3.1
 nor with current git maint. Command run inside /tmp, which is on
 a normal HFS+ volume (using the default settings, i.e. the FS is
 case insensitive).

 $ git --version
 git version 1.8.3.1.42.ge2652c0
 $ git clone --depth 1 git://nbd.name/packages.git
 Cloning into 'packages'...
 remote: Counting objects: 4711, done.
 remote: Compressing objects: 100% (3998/3998), done.
 remote: Total 4711 (delta 157), reused 3326 (delta 94)
 Receiving objects: 100% (4711/4711), 3.85 MiB | 0 bytes/s, done.
 Resolving deltas: 100% (157/157), done.

 OK, so I finally tracked it down.  Commit
 6035d6aad8ca11954c0d7821f6f3e7c047039c8f fixes it:

 commit 6035d6aad8ca11954c0d7821f6f3e7c047039c8f
 Author: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 Date:   Sun May 26 08:16:15 2013 +0700

 fetch-pack: prepare updated shallow file before fetching the pack
 ...
 It looks like I was hitting the race condition.  It's fixed on master,
 so I assume it will be in 1.8.3.2.

 Hmmmph, I do not think the cited change is about any race.

 If it were that we spawn index-pack and write updated shallow
 information at the same time, and depending on the timings,
 index-pack may not read the updated one and fail, then it would have
 been a race, but that is not the above change is about.  If you
 saw the issue on MacOS, you would have seen the same breakage, as
 long as you started from the same repository status, on other
 platforms, and reliably.

Hmmm, that's what the message seemed to say to me (that it was racy).

 An early part of nd/clone-connectivity-shortcut topic contains the
 said commit, and I do not mind merging it to the maintenance track,
 but I suspect that there is something else going on on your OS X
 box, if you saw differences between platforms.

 Perhaps on your OS X box you have {transfer,fetch}.fsckobjects set
 to true while on others it is set to false, or something?

Good suggestion!  I have a gitconfig that I propagate to many of the
machines I use, but I had not updated the Linux machine I was testing
with.  Turning on transfer.fsckObjects did indeed cause the issue to
appear on Linux as well.

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

Bug on OS X...

[John Szakmeister]

I wanted to look at some OpenWRT bits this morning and ran into an
issue cloning the packages repository when setting up the package
feed.  The feeds script executes this under the hood:

   git clone --depth 1 git://nbd.name/packages.git feeds/packages

When trying to run the command directly on OS X, I see:
   :: git clone --depth 1 git://nbd.name/packages.git
   Cloning into 'packages'...
   remote: Counting objects: 4728, done.
   remote: Compressing objects: 100% (4013/4013), done.
   remote: Total 4728 (delta 158), reused 3339 (delta 94)
   Receiving objects: 100% (4728/4728), 3.85 MiB | 1.79 MiB/s, done.
   Resolving deltas: 100% (158/158), done.
   error: unable to find 9f041557a0c81f696280bb934731786e3d009b36
   fatal: object of unexpected type
   fatal: index-pack failed

I tried on Linux, and it succeeded.  I tested with both 1.8.2 and
1.8.3.1.  Unfortunately, I don't have time to dig through what's wrong
at the moment so I thought I'd put it out there for others.

Thanks!

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

Re: [BUG] clone: regression in error messages in master

[John Szakmeister]

On Fri, Jun 21, 2013 at 1:11 PM, Junio C Hamano gits...@pobox.com wrote:
 John Szakmeister j...@szakmeister.net writes:
[snip]
 I can see where this is confusing, but can also see how it's useful
 information to have.  On clone, it's probably not that useful since
 you're looking right at the url, but I could see that information
 being more useful on a pull or push with the default arguments (when
 the source and destination aren't quite as obvious).

 The extra error messages is not the first one, but the last one,
 and the suggested revert is a proposal to remove the latter, not the
 repository $URL not found.

Sorry for the confusion.  I realize they were talking about removing
the remote helper line.  What I meant was that with the url there,
it's a bit easier to determine which part of git failed (http, hg, or
bzr remote helper, for instance).  What I was trying to say was
perhaps there are paths through here where it's really helpful to know
that things failed in the remote helper, so we may not want to
wholesale remove it.  Some of remote helpers, such as ones that talk
to foreign VCSes, do quite a bit more than just transfer data, so it
might be helpful to know that it's not core git that's failing but the
remote helper.  Seeing the URL is a reminder that you're interacting
with a remote helper, and it may not be helpful there.  But other
commands don't necessarily show that to you, and it may be more
helpful to have that reminder that it was a remote helper that failed.

I hope that makes more sense.

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

Re: [BUG] clone: regression in error messages in master

[John Szakmeister]

On Thu, Jun 20, 2013 at 9:16 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Hi,

 So this should explain the problem:

   # using v1.8.3.1
   $ git clone https://google.com
   Cloning into 'google.com'...
   fatal: repository 'https://google.com/' not found

   # using master
   $ git clone https://google.com
   Cloning into 'google.com'...
   fatal: repository 'https://google.com/' not found
   fatal: Reading from helper 'git-remote-https' failed

I can see where this is confusing, but can also see how it's useful
information to have.  On clone, it's probably not that useful since
you're looking right at the url, but I could see that information
being more useful on a pull or push with the default arguments (when
the source and destination aren't quite as obvious).

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

Re: [PATCH] Documentation/CommunityGuidelines

[John Szakmeister]

On Tue, Jun 11, 2013 at 3:46 PM, Philip Oakley philipoak...@iee.org wrote:
 From: Michael Haggerty mhag...@alum.mit.edu
 Sent: Tuesday, June 11, 2013 7:52 PM
 [...]


 That's a very good point (and a good illustration, too).  How do you
 like the new second and third sentences below?

 * When reviewing other peoples' code, be tactful and constructive.
 Remember that submitting patches for public critique can be very
 intimidating


 I found this to be true. The tone on the list could at times feel un-helpful
 (to the new person). It is almost as if it is an initiation - those on the
 list know the protocols, and new folk either arrive like a bull in a china
 shop, or more likely, timidly push the patch under the door and run away
 (and variations in between) - some never push out their (drafted) patch.

Interesting!  I've had the opposite opinion.  I've often been
surprised at how much constructive feedback has been given, and the
thoughtfulness of the reviewers to offer up alternative solutions,
show examples, etc.  Junio, Jeff, and especially Jonathan have been
particularly good on that front--at least those are some of the
regulars that stick out in my mind.  Overall, I've been pretty happy
with the community, and while I haven't contributed much, I generally
enjoy reading the emails.  I feel like I learn something new all the
time. :-)

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

Re: [PATCH] credential-osxkeychain: support more protocols

[John Szakmeister]

On Mon, May 27, 2013 at 3:57 AM, Xidorn Quan quanxunz...@gmail.com wrote:
 Add protocol ftp, smtp, and ssh for credential-osxkeychain.
 ---
  contrib/credential/osxkeychain/git-credential-osxkeychain.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

 diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c 
 b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
 index 3940202..4ddcfb3 100644
 --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
 +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
 @@ -127,10 +127,16 @@ static void read_credential(void)
 *v++ = '\0';

 if (!strcmp(buf, protocol)) {
 -   if (!strcmp(v, https))
 +   if (!strcmp(v, ftp))
 +   protocol = kSecProtocolTypeFTP;
 +   else if (!strcmp(v, https))
 protocol = kSecProtocolTypeHTTPS;
 else if (!strcmp(v, http))
 protocol = kSecProtocolTypeHTTP;
 +   else if (!strcmp(v, smtp))
 +   protocol = kSecProtocolTypeSMTP;
 +   else if (!strcmp(v, ssh))
 +   protocol = kSecProtocolTypeSSH;
 else /* we don't yet handle other protocols */
 exit(0);

This looks pretty good, except the last one raises a question.  I'm
using Mac OS X, and ssh already interacts with keychain to get my SSH
key password.  Is this mainly for password logins via SSH?  Assuming
that's the case:

Signed-off-by: John Szakmeister j...@szakmeister.net

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

Re: [PATCH] credential-osxkeychain: support more protocols

[John Szakmeister]

On Mon, May 27, 2013 at 9:55 AM, Xidorn Quan quanxunz...@gmail.com wrote:
[snip]
 I thought that SSH password logins can benefit from it, but I just
 found that it is wrong because it seems that SSH client is responsible
 for authenticating. Consequently, supporting SSH here is useless.
 I will remove that lines and send this patch again.

 Since it is the first time I submit a patch to git, I am not very
 familiar with the convention here. Should I send the modified patch
 to the maintainer directly? And what information should I append to
 my patch before it can get merged?

You'll need to read Documentation/SubmittingPatches (here's a link to
a version online:
https://github.com/git/git/blob/master/Documentation/SubmittingPatches).

You should resend this patch with the fix and change [PATCH] to
[PATCH v2], so the folks involved know that this is the second
iteration.  You also need to include a Signed-off-by line in your
patch, which means you agree to the agreement set forth in the
Developer's Certificate of Origin (which is in the SubmittingPatches
documentation).  You can easily include this line when you make the
commit by using the `-s` option on `git commit`.

You can also add an Acked-by line for me (since I reviewed and
approve of the change):

Acked-by: John Szakmeister j...@szakmeister.net

HTH!

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

Re: first parent, commit graph layout, and pull merge direction

[John Szakmeister]

On Fri, May 24, 2013 at 4:29 AM, John Keeping j...@keeping.me.uk wrote:
[snip]
 Note that in my email that started this, I tried to be clear that I was
 talking about git pull *without a branch name*.  If this user
 explicitly says git pull remote branch then I consider that a clear
 indication that they really do mean to perform a merge; I would not
 recommend changing the current behaviour in that case.

 If the user just says git pull then it is more likely that they are
 just trying to synchronise with the upstream branch, in which case they
 probably don't actually want a merge.

This makes a lot of sense to me.  I was going to write earlier that I
almost wish there was a separate command for getting your local branch
in sync with the remote one.

BTW, it also doesn't help that `git pull` is suggested as the answer
anytime a push cannot succeed.  I've warned my users about using `git
pull`, and--unfortunately--they sometimes forget because the advice is
right there in front of them.

I agree with John here: it's a bare `git pull` that is often the
culprit.  Of course, the asymmetry between `git pull` and `git pull
remote branch` is a little bothersome too, but the team does that
*far* less often.

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

Re: first parent, commit graph layout, and pull merge direction

[John Szakmeister]

On Thu, May 23, 2013 at 5:06 AM, Andreas Krey a.k...@gmx.de wrote:
[snip]
 ...
 Don't do that, then.

 :-) Problem is, in this case 'I' expands to about
 17 people I need to educate on this.

This is a feature of `git pull` that I really despise.  I really wish
`git pull` treated the remote as the first parent in its merge
operation.

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

Re: Reading commit objects

[John Szakmeister]

On Tue, May 21, 2013 at 5:21 PM, Chico Sokol chico.so...@gmail.com wrote:
 Hello,

 I'm building a library to manipulate git repositories (interacting
 directly with the filesystem).

 Currently, we're trying to parse commit objects. After decompressing
 the contents of a commit object file we got the following output:

 commit 191
 author Francisco Sokol chico.so...@gmail.com 1369140112 -0300
 committer Francisco Sokol chico.so...@gmail.com 1369140112 -0300

 first commit

Does `git cat-file -p sha1` show a tree object?  FWIW, I expected to
see a tree line there, so maybe this object was created without a
tree?  I also don't see a parent listed.

I did this on one of my repos:

 buf = open('.git/objects/cd/da219e4d7beceae55af73c44cb3c9e1ec56802', 
 'rb').read()
 import zlib
 zlib.decompress(buf)
'commit 246\x00tree 2abfe1a7bedb29672a223a5c5f266b7dc70a8d87\nparent
0636e7ff6b79470b0cd53ceacea88e7796f202ce\nauthor John Szakmeister
j...@szakmeister.net 1369168481 -0400\ncommitter John Szakmeister
j...@szakmeister.net 1369168481 -0400\n\nGot a file listing.\n'

So at least creating the commits with Git, I see a tree.  How was the
commit you're referencing created?  Perhaps something is wrong with
that process?

 We hoped to get the same output of a git cat-file -p sha1, but
 that didn't happened. From a commit object, how can I find tree object
 hash of this commit?

I'd expect that too.

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

Re: [PATCH 3/4] {fast-export,transport-helper}: style cleanups

[John Szakmeister]

On Wed, May 8, 2013 at 9:16 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  builtin/fast-export.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

 diff --git a/builtin/fast-export.c b/builtin/fast-export.c
 index d60d675..8091354 100644
 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -135,7 +135,7 @@ static void export_blob(const unsigned char *sha1)
[snip]
 @@ -289,13 +289,13 @@ static void handle_commit(struct commit *commit, struct 
 rev_info *rev)
 parse_commit(commit);
 author = strstr(commit-buffer, \nauthor );
 if (!author)
 -   die (Could not find author in commit %s,
 +   die(Could not find author in commit %s,
  sha1_to_hex(commit-object.sha1));

It looks like your simple replace didn't account for calls with
multiple lines.  Now the remaining lines don't line up.
:-)  There's several more places like this in the patch.

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

Re: [PATCH 4/4] fast-export: trivial cleanup

[John Szakmeister]

On Wed, May 8, 2013 at 9:16 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Cast the object to a commit, only to get the object back?

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  builtin/fast-export.c | 5 +
  1 file changed, 1 insertion(+), 4 deletions(-)

 diff --git a/builtin/fast-export.c b/builtin/fast-export.c
 index 8091354..d24b4d9 100644
 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -550,7 +550,6 @@ static void get_tags_and_duplicates(struct 
 rev_cmdline_info *info,

  static void handle_tags_and_duplicates(struct string_list *extra_refs)
  {
 -   struct commit *commit;
 int i;

 for (i = extra_refs-nr - 1; i = 0; i--) {
 @@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct string_list 
 *extra_refs)
 break;
 case OBJ_COMMIT:
 /* create refs pointing to already seen commits */
 -   commit = (struct commit *)object;
 -   printf(reset %s\nfrom :%d\n\n, name,
 -  get_object_mark(commit-object));
 +   printf(reset %s\nfrom :%d\n\n, name, 
 get_object_mark(object));

FWIW, this line is now too long (exceeds 80 columns).  Good catch on
the casting though.

-John

PS  Sorry for the duplicate Felipe... I still need to get used to
hitting Reply All. :-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] fast-export: trivial cleanup

[John Szakmeister]

On Thu, May 9, 2013 at 4:53 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
[snip]
 @@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct 
 string_list *extra_refs)
 break;
 case OBJ_COMMIT:
 /* create refs pointing to already seen commits */
 -   commit = (struct commit *)object;
 -   printf(reset %s\nfrom :%d\n\n, name,
 -  get_object_mark(commit-object));
 +   printf(reset %s\nfrom :%d\n\n, name, 
 get_object_mark(object));

 FWIW, this line is now too long (exceeds 80 columns).  Good catch on
 the casting though.

 -John

 PS  Sorry for the duplicate Felipe... I still need to get used to
 hitting Reply All. :-)

 The guideline is:

  - We try to keep to at most 80 characters per line.

 The key word being *try*.

I saw that, but you actively joined the lines, and there was no need
to.  It didn't even require trying to keep it within 80 columns. :-)

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

Re: Pitfalls in auto-fast-forwarding heads that are not checked out?

[John Szakmeister]

On Sat, May 4, 2013 at 7:35 AM, Martin Langhoff
martin.langh...@gmail.com wrote:
[snip]
 When I do git pull, git is careful to only update the branch I have
 checked out (if appropriate). It leaves any other branches that track
 branches on the remote that has just been fetched untouched. I always
 thought that at some point git pull would learn to evaluate those
 branches and auto-merge them if the merge is a ff.

 I would find that a natural bit of automation in git pull. Of course
 it would mean a change of semantics, existing scripts could be
 affected.

I agree.  I've been using this script for quite a while now:

https://github.com/jszakmeister/etc/blob/master/git-addons/git-ffwd

I've been pretty happy with it.  It's not of my own design, I picked
up from StackOverflow:

http://stackoverflow.com/a/9076361/683080

And made a couple of minor tweaks to cope with my configuration (I
have merge setup to not fast-forward merge by default).

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

Re: git log -p unexpected behaviour - security risk?

[John Szakmeister]

On Sun, Apr 21, 2013 at 2:42 PM, Junio C Hamano gits...@pobox.com wrote:
 Jonathan Nieder jrnie...@gmail.com writes:

 The thing is, I'm not convinced this is a bad default.  Shows no diff
 at all for merges is easy for a person to understand.  It is much
 easier to understand its limitations than -c and --cc.

 Making log -p -m a default before -c/--cc was introduced would
 have been the stupidest thing to do, as it would make the command
 mostly useless.  Nobody would want to see repetitious output from a
 merge that he would eventually get when the traversal drills down to
 individual commits on the merged side branch.

 When I added -c/--cc, I contemplated making -p imply --cc, but
 decided against it primarily because it is a change in traditional
 behaviour, and it is easy for users to say --cc instead of -p from
 the command line.

FWIW, security aside, I would've like to have seen that.  I find it
confusing that merge commits that introduce code don't have a diff
shown when using -p.  And I find it hard to remember --cc.  BTW,
what's the mnemonic for it?  -p = patch, --cc = ?

 On the other hand, show was a newer command and it was easy to
 turn its default to --cc without having to worry too much about
 existing users.

 For that
 reason, it is a much *better* default for security than --cc or -c
 (even though I believe one of the latter would be a better default for
 convenience).

 Yes.  I do not fundamentally oppose to the idea of log -p to imply
 log --cc when -m is not given (log -p -m is specifically
 declining the combined diff simplification).  It may be a usability
 improvement.

Would you consider such a patch?

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

Re: git log -p unexpected behaviour - security risk?

[John Szakmeister]

On Tue, Apr 30, 2013 at 12:37 PM, Junio C Hamano gits...@pobox.com wrote:
 John Szakmeister j...@szakmeister.net writes:

 When I added -c/--cc, I contemplated making -p imply --cc, but
 decided against it primarily because it is a change in traditional
 behaviour, and it is easy for users to say --cc instead of -p from
 the command line.

 FWIW, security aside, I would've like to have seen that.  I find it
 confusing that merge commits that introduce code don't have a diff
 shown when using -p.  And I find it hard to remember --cc.  BTW,
 what's the mnemonic for it?  -p = patch, --cc = ?

 Compact combined.

Thank you.

 By the way, these options are _not_ about showing merge commits
 that introduce code, and they do not help your kind of security.
 As I repeatedly said, you would need -p -m for that.

I'm sorry, I didn't mean to imply that it's useful for security, just
that it better meets my expectations when -p is turned on.  I realize
there are some edges in the logic, but I'm fine with those edges.

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

Re: git log -p unexpected behaviour - security risk?

[John Szakmeister]

On Tue, Apr 30, 2013 at 1:05 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Junio C Hamano gits...@pobox.com writes:

 By the way, these options are _not_ about showing merge commits
 that introduce code, and they do not help your kind of security.
 As I repeatedly said, you would need -p -m for that.

 Actually, while defaulting to --cc may be convenient, it would indeed
 increase the security risk: currently, git log -p shows nothing for
 merges, so it's rather clear that _everything_ is omitted. With --cc,
 the user would see a diff, and could hardly guess that not everything is
 shown without reading the doc very carefully.

I don't believe it's that clear.  I bet people assume there's nothing
to show, and unless you dig in and discover that `-p` doesn't include
merges.  In git 1.8.2, `git help log` doesn't seem to make any mention
of `-p` not showing a diff for merges.

Just to see, I asked several people around here whether they knew `-p`
didn't show diffs for merges, and they were all surprised that diffs
were being omitted for merge commits.

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

Re: consistency problem on ZFS

[John Szakmeister]

On Sun, Apr 28, 2013 at 3:11 PM, Yann Hodique yann.hodi...@gmail.com wrote:
 Hi,

 I have a weird problem that seems to manifest itself only on ZFS
 (actually the Zevo distribution, on OSX). With git 1.8.2.1 by the way.
 I just switched to ZFS, so I can't blame that particular version of git.

 Sometimes (I'd say something like 10-15% of the time, fairly
 reproducible anyway), git diff-files will see changes that don't exist
 for some time, then will catch up with the actual state of the file:

 $ git checkout next; git diff-files; git checkout next; git diff-files
 Already on 'next'
 :100644 100644 bd774cccaa14e061c3c26996567ee28f4f77ec80 
  M  magit.el
 Already on 'next'
 $

Since you're running with Mac OS X, can I ask what version?  Have you
seen this with the regular file system (HFS) at all?  It might be that
you need to disable core.trustctime.

-John

PS  Sorry for the repeat Yann... I forgot to CC the list.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/9] remote-bzr: store converted URL

[John Szakmeister]

On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Mercurial might convert the URL to something more appropriate, like an
 absolute path. Lets store that instead of the original URL, which won't
 work from a different working directory if it's relative.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  contrib/remote-helpers/git-remote-bzr | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)


FWIW, it feels weird to be talking about Mercurial when you're
patching git-remote-bzr. :-)

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

Re: [PATCH v2 7/9] remote-bzr: tell bazaar to be quiet

[John Szakmeister]

On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Otherwise we get notification, progress bars, and what not.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  contrib/remote-helpers/git-remote-bzr | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/contrib/remote-helpers/git-remote-bzr 
 b/contrib/remote-helpers/git-remote-bzr
 index dda2932..8617e25 100755
 --- a/contrib/remote-helpers/git-remote-bzr
 +++ b/contrib/remote-helpers/git-remote-bzr
 @@ -26,6 +26,7 @@ bzrlib.plugin.load_plugins()
  import bzrlib.generate_ids
  import bzrlib.transport
  import bzrlib.errors
 +import bzrlib.ui

  import sys
  import os
 @@ -755,6 +756,8 @@ def main(args):
  if not os.path.exists(dirname):
  os.makedirs(dirname)

 +bzrlib.ui.ui_factory.be_quiet(True)
 +

Nice change!

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

Re: Is there a way to speed up remote-hg?

[John Szakmeister]

On Sat, Apr 20, 2013 at 7:07 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Sat, Apr 20, 2013 at 6:07 AM, John Szakmeister j...@szakmeister.net 
 wrote:
 I really like the idea of remote-hg, but it appears to be awfully slow
 on the clone step:

 The short answer is no. I do have a couple of patches that improve
 performance, but not by a huge factor.

 I have profiled the code, and there are two significant places where
 performance is wasted:

 1) Fetching the file contents

 Extracting, decompressing, transferring, and then compressing and
 storing the file contents is mostly unavoidable, unless we already
 have the contents of such file, which in Git, it would be easy to
 check by analyzing the checksum (SHA-1). Unfortunately Mercurial
 doesn't have that information. The SHA-1 that is stored is not of the
 contents, but the contents and the parent checksum, which means that
 if you revert a modification you made to a file, or move a file, any
 operation that ends up in the same contents, but from a different
 path, the SHA-1 is different. This means the only way to know if the
 contents are the same, is by extracting, and calculating the SHA-1
 yourself, which defeats the purpose of what you want the calculation
 for.

 I've tried, calculating the SHA-1 and use a previous reference to
 avoid the transfer, or do the transfer, and let Git check for existing
 objects doesn't make a difference.

 This is by Mercurial's stupid design, and there's nothing we, or
 anybody could do about it until they change it.

That's a bummer. :-(

 2) Checking for file changes

 For each commit (or revision), we need to figure out which files were
 modified, and for that, Mercurial has a neat shortcut that stores such
 modifications in the commit context itself, so it's easy to retrieve.
 Unfortunately, it's sometimes wrong.

 Since the Mercurial tools never use this information for any real
 work, simply to show the changes to the users, Mercurial folks never
 noticed the contents they were storing were wrong. Which means if you
 have a repository that started with old versions of mercurial, chances
 are this information would be wrong, and there's no real guarantee
 that future versions won't have this problem, since to this day this
 information continues to be used only display stuff to the user.

 So, since we cannot rely on this, we need to manually check for
 differences the way Mercurial does, which blows performance away,
 because you need to get the contents of the two parent revisions, and
 compare them away. My content I mean the the manifest, or list of
 files, which takes considerable amount of time.

Eek!

 For 1) there's nothing we can do, and for 2) we could trust the files
 Mercurial thinks were modified, and that gives us a very significant
 boost, but the repository will sometimes end up wrong. Most of the
 time is spent on 2).

 So unfortunately there's nothing we can do, that's just Mercurial
 design, and it really has nothing to do with Git. Any other tool would
 have the same problems, even a tool that converts a Mercurial
 repository to Mercurial (without using tricks).
[snip]

That's unfortunate, but thank you for taking the time to explain!

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

Is there a way to speed up remote-hg?

[John Szakmeister]

I really like the idea of remote-hg, but it appears to be awfully slow
on the clone step:

...
progress revision 81499 'master' (81500/81664)
progress revision 81599 'master' (81600/81664)
Checking out files: 100% (3744/3744), done.
git clone hg::https://bitbucket.org/python_mirrors/cpython
4484.61s user 41510.05s system 102% cpu 12:29:45.73 total

That seems like an awfully high price to pay.  It there a way to speed
this up at all?  I realize the Python hg repo has more history than
others, but even a smaller project like Sphinx takes a while:

git clone hg::https://bitbucket.org/birkenfeld/sphinx  56.41s user
90.86s system 98% cpu 2:28.87 total

I was just curious if something more could be done here.  I don't go
around cloning Python all the time, so it's not a big issue, but it'd
be nice if it was more performant.

Thanks!

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

Re: [PATCH] git-web--browse: recognize iTerm as a GUI terminal on OS X

[John Szakmeister]

On Wed, Mar 27, 2013 at 10:43 AM, Junio C Hamano gits...@pobox.com wrote:
[snip]
 If that approach is better than what you originally sent, then yes.

 But I do not use OS X, so you may need to pay attention to possible
 complaints and comments from other Mac users on this list for a
 while---there may be people who run the program in question without
 that environment variable.

Sorry it has taken me so long to get back to this.  I searched around
and tried out a few terminal programs that are available, and I think
what you queued--checking that TERM_PROGRAM is non-empty--is the right
fix.

Thanks!

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

[PATCH] git-web--browse: recognize iTerm as a GUI terminal on OS X

[John Szakmeister]

It turns out that the presence of SECURITYSESSIONID is not sufficient
for detecting the presence of a GUI under Mac OS X.  SECURITYSESSIONID
appears to only be set when the user has Screen Sharing enabled.
Disabling Screen Sharing and relaunching the shell showed that the
variable was missing, at least under Mac OS X 10.6.8.  As a result,
let's check for iTerm directly via TERM_PROGRAM.

Signed-off-by: John Szakmeister j...@szakmeister.net
---

On Sun, Mar 24, 2013 at 10:05:53PM +0100, Christian Couder wrote:
[snip]
 Your patch looks good to me, and I cannot really test it as I don't have a 
 Mac.
 Could you just had some of the explanations you gave above to the
 commit message?

Here's an updated patch.  I also noticed that git-bisect.sh is
also trying to determine if a GUI is present by looking for
SECURITYSESSIONID as well.  I wonder if it would be better to
create a shell function in git-sh-setup.sh that the two scripts
could use?

-John

git-web--browse.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-web--browse.sh b/git-web--browse.sh
index 1e82726..1ff5379 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -120,6 +120,7 @@ if test -z $browser ; then
fi
# SECURITYSESSIONID indicates an OS X GUI login session
if test -n $SECURITYSESSIONID \
+   -o $TERM_PROGRAM = iTerm.app \
-o $TERM_PROGRAM = Apple_Terminal ; then
browser_candidates=open $browser_candidates
fi
-- 
1.8.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rebase: strange failures to apply patc 3-way

[John Szakmeister]

On Mon, Mar 11, 2013 at 8:29 PM, Max Horn m...@quendi.de wrote:
[snip]

 sudo launchctl unload /System/Library/LaunchDaemons/com.apple.revisiond.plist

 - this exits revisiond, and prevents launchd from respawning it. After that, 
 the problem is gone, on several attempts. And once I reactivate revisions, 
 the problem reoccurs.

 So it seems pretty clear what the cause of this is. Now I still need to 
 figure out why it affects that particular repository and not others. But at 
 this point I guess it is safe to say that Apple's auto-crap-magic revisiond 
 is the root of the problem, and git is purely a victim. So I'll stop spamming 
 this list with this issue for now, and see if I can figure out a fix that is 
 less invasive than turning off revisiond completely (which some application 
 rely on, so disabling it may break them badly).

I just wanted to say thank you for spamming the list with this.  I've
not upgraded to a new Mac OS X yet, but now I know I need to watch out
for this issue.  I'm glad you were able to get to the bottom of it,
and I'd love to hear if you find a reasonable solution.

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

Re: Credentials and the Secrets API...

[John Szakmeister]

On Wed, Feb 20, 2013 at 12:01 PM, Ted Zlatanov t...@lifelogs.com wrote:
 On Sat, 9 Feb 2013 05:58:47 -0500 John Szakmeister j...@szakmeister.net 
 wrote:
[snip]

 JS Yes, I think it has.  Several other applications appear to be using
 JS it, including some things considered core in Fedora--which is a good
 JS sign.

 Wonderful.  Do you still have interest in working on this credential?

I do, but I'm a bit short on time right now.  So if you or someone
else wants to pick up and run with it, please feel free.  If I get
some cycles over the next couple of months, I'll give it a go though.

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

Re: Credentials and the Secrets API...

[John Szakmeister]

On Thu, Feb 7, 2013 at 9:46 AM, Ted Zlatanov t...@lifelogs.com wrote:
 On Thu, 27 Oct 2011 12:05:03 -0400 John Szakmeister j...@szakmeister.net 
 wrote:

 JS Just wanted to keep folks in the loop.  It turns out that the Secrets
 JS API is still to young.  I asked about the format to store credentials
 JS in (as far as attributes), and got a response from a KDE developer
 JS that says it's still to young on their front.  They hope to have
 JS support in the next release of KDE.  But there's still the issue of
 JS what attributes to use.

 JS With that information, I went ahead and created a
 JS gnome-credential-keyring that uses Gnome's Keyring facility.  I still
 JS need to do a few more things (mainly run it against Jeff's tests), but
 JS it's generally working.  Just wanted to keep folks in the loop.
 JS Hopefully, I can get patches out this weekend.

 Do you think the Secrets API has matured enough?  KDE has had a new
 release since your post...

Yes, I think it has.  Several other applications appear to be using
it, including some things considered core in Fedora--which is a good
sign.

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

Re: [PATCH 8/8] t9402: Use TABs for indentation

[John Szakmeister]

On Sun, Dec 9, 2012 at 5:01 AM, Torsten Bögershausen tbo...@web.de wrote:
[snip]
 PS: for some reason I don't get any mails to my
 (google) account any more, which I use to read the list.
 Am I the only one having this problem?

I noticed that the kernel.org lists are pretty unaccommodating.  If
something hiccups in the delivery, it'll drop (or disable?) sending
emails to you.  I've got some spam protection on my server that was
causing some issues occasionally when a lookup took to long.  I
wouldn't be surprised if a hiccup occurs now and then with gmail, and
the same thing happens.

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

Rename edge case...

[John Szakmeister]

I've been browsing StackOverflow answering git-related questions, and
ran across this one:
http://stackoverflow.com/questions/13300675/git-merge-rename-conflict

It's a bit of an interesting situation.  The user did a couple of
renames in a branch:
foo.txt = fooOld.txt
fooNew.txt = foo.txt

Meanwhile, master had an update to fooNew.txt.  When the user tried to
merge master to the branch, it gave a merge conflict saying fooNew.txt
was deleted, but master tried to update it.

I was a bit surprised that git didn't follow the rename here, though I
do understand why: git only sees it as a rename if the source
disappears completely.  So I played locally with a few ideas, and was
surprised to find out that even breaking up the two renames into two
separate commits git still didn't follow it.

I'm just curious--I don't run into this often myself--but is there a
good strategy for dealing with this that avoids the conflict?

Thanks!

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

Re: Rename edge case...

[John Szakmeister]

On Fri, Nov 9, 2012 at 4:27 AM, Tomas Carnecky tomas.carne...@gmail.com wrote:
[snip]
 When merging two branches, git only looks at the tips. It doesn't inspect
 their histories to see how the files were moved around. So i doesn't matter
 whether you rename the files in a single commit or multiple commits. The
 resulting tree is always the same.

I guess I figured that when I saw the final result, but didn't know if
there was a way to coax Git into doing a better job here.  I guess not
though.  At least it's a situation that doesn't come up often.

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

Re: Rename edge case...

[John Szakmeister]

On Fri, Nov 9, 2012 at 11:09 AM, Jeff King p...@peff.net wrote:
[snip]
 Right. If the source didn't go away, it would be a copy. We can do copy
 detection, but it is not quite as obvious what a merge should do with a
 copy (apply the change to the original? To the copy? In both places? You
 would really want hunk-level copy detection for it to make any sense).

Yeah, I wasn't advocating that.  More along the lines of what you're
talking about below...

 Usually git deals with this double-rename case through the use of
 break or rewrite detection. We notice that the old foo.txt and the
 new foo.txt do not look very much like each other, and break the
 modification apart into an add and a delete. That makes each side
 eligible for rename detection, and we can end up finding the pairs of
 renames above.

I did try using the -B option, and it did detect that foo.txt was
renamed to fooOld.txt, but it didn't show fooNew.txt being renamed to
foo.txt.  I'm running git 1.7.12.3.  It could be that 1.8.0 does
better, but I haven't tried.

 So in theory it just as simple as a one-liner to turn on break-detection
 in merge-recursive. Sadly, that only reveals more issues with how
 merge-recursive handles renames. See this thread, which has pointers to
 the breakages at the end:

   http://thread.gmane.org/gmane.comp.version-control.git/169944

Thank you.  I'll definitely read up on this.

 I've become convinced that the best way forward with merge-recursive is
 to scrap and rewrite it. It tries to do things in a muddled order, which
 makes it very brittle to changes like this. I think it needs to have an
 internal representation of the tree that can represent all of the
 conflicts, and then follow a few simple phases:

   1. structural 3-way merge handling renames, breaks, typechanges,
  etc. Each path in tree might show things like D/F conflicts, or it
  might show content-level merges that still need to happen, even if
  the content from those merges is not coming from the same paths in
  the source trees.

   2. Resolve content-level 3-way merges at each path.

   3. Compare the proposed tree to the working tree and list any problems
  (e.g., untracked files or local modifications that will be
  overwritten).

 Right now it tries to do these things interleaved as it processes paths,
 and as a result we've had many bugs (e.g., the content-level merge
 conflating the content originally at a path and something that was
 renamed into place, and missing corner cases where we actually overwrite
 untracked files that should be considered precious).

 But that is just off the top of my head. I haven't looked at the topic
 in quite a while (and I haven't even started working on any such
 rewrite).

That certainly sounds like a better approach.

 So I played locally with a few ideas, and was surprised to find out
 that even breaking up the two renames into two separate commits git
 still didn't follow it.

 Right, because the merge only looks at the end points. Try doing a
 diff -M between your endpoints with and without -B. We do not have
 any double-renames in git.git, but you can find -B helping a similar
 case: most of a file's content is moved elsewhere, but some small amount
 remains. For example, try this in git.git, with and without -B:

   git show -M --stat --summary --patch 043a449

 It finds the rename only with -B, which would help a merge (it also
 makes the diff shorter and more readable, as you can see what was
 changed as the content migrated to the new file).

I've played with the -B option before, and it's definitely nice in
certain cases.

Thank you for taking the time to write all this up.  It was very informative!

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

Re: The config include mechanism doesn't allow for overwriting

[John Szakmeister]

On Tue, Oct 23, 2012 at 10:13 AM, Ævar Arnfjörð Bjarmason
ava...@gmail.com wrote:
[snip]
 And git config --get foo.bar will give you:

 $ git config -f /tmp/test --get foo.bar
 one
 error: More than one value for the key foo.bar: two
 error: More than one value for the key foo.bar: three

 I think that it would be better if the config mechanism just silently
 overwrote keys that clobbered earlier keys like your patch does.

 But in addition can we simplify things for the consumers of
 git-{config,var} -l by only printing:

 foo.bar=three

 Or are there too many variables like include.path that can
 legitimately appear more than once.

I frequently use pushurl in my remotes to push my master branch both
to the original repo and my forked version.  I find it very helpful in
my workflow, and would hate to lose that.  That said, I do like the
idea of having a config file and the ability to override some of the
variables.

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