Re: [PATCH/RFC 0/7] Support for Ruby

2013-09-22 Thread Fredrik Gustafsson 
On Sun, Sep 22, 2013 at 12:36:51AM -0500, Felipe Contreras wrote:
  I think it's a bad idea to introduce an entirely new runtime, especially
  one known to occasionally blow up on less-common architectures, without
  some advance notice.
 
 This is just FUD. What do you mean blow up on less-common architectures? Do 
 you
 have actual evidence or can we just dismiss that as a baseless argument?
 
  For example, at work I would not be able to deploy a git using Ruby
  immediately because Git is an RPM and Ruby is compiled from source, if it is
  even present at all.
 
 Again, what do you mean? In all the distributions I've seen, vim is compiled
 with Ruby support by default, so unless you think vim is an essoteric package,
 libruby is almost definetly packaged and available.

It would actually be usefull to know stats on where git is runned. In my
world of embedded computing, ruby support definitely isn't a standard,
nor is glibc.

As for architecture speaking I think it's important that git works on
ARM since that architecture increases on the server market. I've no idea
if this is a problem with ruby or not.

 
  Also, the only Python script that is shipped with Git is git-p4, which
  is essentially optional, since most git users probably do not use
  Perforce. Otherwise, all the scripts in git are shell or Perl.
 
 Neither perl, nor shell, nor python scripts solve the forking problem. My
 proposal does.

It does, and so does Lua, which can be bundled with git and used in the
configuration files as well and is pure ansi C. However bundling
something has it bad sides too. At least this will solve the dependency
problem. So let the language war begin =).

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 0/7] Support for Ruby

2013-09-22 Thread Felipe Contreras 
On Sun, Sep 22, 2013 at 2:31 AM, Fredrik Gustafsson iv...@iveqy.com wrote:
 On Sun, Sep 22, 2013 at 12:36:51AM -0500, Felipe Contreras wrote:
  I think it's a bad idea to introduce an entirely new runtime, especially
  one known to occasionally blow up on less-common architectures, without
  some advance notice.

 This is just FUD. What do you mean blow up on less-common architectures? Do 
 you
 have actual evidence or can we just dismiss that as a baseless argument?

  For example, at work I would not be able to deploy a git using Ruby
  immediately because Git is an RPM and Ruby is compiled from source, if it 
  is
  even present at all.

 Again, what do you mean? In all the distributions I've seen, vim is compiled
 with Ruby support by default, so unless you think vim is an essoteric 
 package,
 libruby is almost definetly packaged and available.

 It would actually be usefull to know stats on where git is runned. In my
 world of embedded computing, ruby support definitely isn't a standard,
 nor is glibc.

I come from the embedded world as well, and I've never seen Git used there.

I'd say Windows support is much more important than embedded, and we
are not supporting that properly.

  Also, the only Python script that is shipped with Git is git-p4, which
  is essentially optional, since most git users probably do not use
  Perforce. Otherwise, all the scripts in git are shell or Perl.

 Neither perl, nor shell, nor python scripts solve the forking problem. My
 proposal does.

 It does,

No, it does not. All the **current** perl/shell/python scripts use
'git foo' commands to achieve everything.

 and so does Lua,

There is no lua in Git.

 which can be bundled with git and used in the
 configuration files as well and is pure ansi C. However bundling
 something has it bad sides too. At least this will solve the dependency
 problem. So let the language war begin =).

Talk is cheap, show me the code.

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

Re: [PATCH/RFC 0/7] Support for Ruby

2013-09-22 Thread Fredrik Gustafsson 
On Sun, Sep 22, 2013 at 02:43:39AM -0500, Felipe Contreras wrote:
  It would actually be usefull to know stats on where git is runned. In my
  world of embedded computing, ruby support definitely isn't a standard,
  nor is glibc.
 
 I come from the embedded world as well, and I've never seen Git used there.
 
 I'd say Windows support is much more important than embedded, and we
 are not supporting that properly.

Me neither, it doesn't mean that it isn't used though... I agree with
the lack of windows support from git.git. However since Microsoft
working with libgit2 on a Visual Studio plugin this it might be that the
need for windows support decreases.

 
   Also, the only Python script that is shipped with Git is git-p4, which
   is essentially optional, since most git users probably do not use
   Perforce. Otherwise, all the scripts in git are shell or Perl.
 
  Neither perl, nor shell, nor python scripts solve the forking problem. My
  proposal does.
 
  It does,
 
 No, it does not. All the **current** perl/shell/python scripts use
 'git foo' commands to achieve everything.

As I said, It does meaning Your solution solves the forking problem.

 
  and so does Lua,
 
 There is no lua in Git.

There's no ruby in git either as far as I know... (and no, I don't think
contrib/ counts).

 
  which can be bundled with git and used in the
  configuration files as well and is pure ansi C. However bundling
  something has it bad sides too. At least this will solve the dependency
  problem. So let the language war begin =).
 
 Talk is cheap, show me the code.

See this thread by Jeff King:
http://thread.gmane.org/gmane.comp.version-control.git/206335/focus=206337

And see my humble test of what the speedup would be for git-submodule
even with a faulty lua integration (still forking... but huge
performance boost anyway):
http://thread.gmane.org/gmane.comp.version-control.git/228031/focus=228051

As you can see a lua integration would increase the git binary with
300kb.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 0/7] Support for Ruby

2013-09-22 Thread Felipe Contreras 
On Sun, Sep 22, 2013 at 3:12 AM, Fredrik Gustafsson iv...@iveqy.com wrote:
 On Sun, Sep 22, 2013 at 02:43:39AM -0500, Felipe Contreras wrote:
  It would actually be usefull to know stats on where git is runned. In my
  world of embedded computing, ruby support definitely isn't a standard,
  nor is glibc.

 I come from the embedded world as well, and I've never seen Git used there.

 I'd say Windows support is much more important than embedded, and we
 are not supporting that properly.

 Me neither, it doesn't mean that it isn't used though... I agree with
 the lack of windows support from git.git.

Sure, it *might* be used, but I don't understand this fascination
about worrying about hypothetical users, possibly non-existent, when
we have real users to worry about, and in big numbers.

 However since Microsoft
 working with libgit2 on a Visual Studio plugin this it might be that the
 need for windows support decreases.

I'll believe it when I see it. And then, when the users like it and
don't report brokenness.

Personally, I don't have much faith the in the libgit2 project, and
it's ability to keep up with git.git.

   Also, the only Python script that is shipped with Git is git-p4, which
   is essentially optional, since most git users probably do not use
   Perforce. Otherwise, all the scripts in git are shell or Perl.
 
  Neither perl, nor shell, nor python scripts solve the forking problem. My
  proposal does.
 
  It does,

 No, it does not. All the **current** perl/shell/python scripts use
 'git foo' commands to achieve everything.

 As I said, It does meaning Your solution solves the forking problem.

Oh.

  and so does Lua,

 There is no lua in Git.

 There's no ruby in git either as far as I know... (and no, I don't think
 contrib/ counts).

There is when you apply this patch.

  which can be bundled with git and used in the
  configuration files as well and is pure ansi C. However bundling
  something has it bad sides too. At least this will solve the dependency
  problem. So let the language war begin =).

 Talk is cheap, show me the code.

 See this thread by Jeff King:
 http://thread.gmane.org/gmane.comp.version-control.git/206335/focus=206337

That is very very far from what I'm doing.

 And see my humble test of what the speedup would be for git-submodule
 even with a faulty lua integration (still forking... but huge
 performance boost anyway):
 http://thread.gmane.org/gmane.comp.version-control.git/228031/focus=228051

I don't see how that is relevant, but I'm certain the same can be done
with Ruby.

 As you can see a lua integration would increase the git binary with
 300kb.

And my patch would increase it 49Kb.

IMO the problem with lua is that it's too simple, it's syntax doesn't
resemble c, perl, python, shell, or ruby, it's just weird. Also, it's
much less popular, it's not as powerful, and there isn't a big
community involved with Git like with Ruby.

Sure, for a couple of simple scripts lua bindings would be great, but
for the future, better maintainability, and to grab future developers,
Ruby is simply a better option.

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

Re: [PATCH v2] build: add default configuration

2013-09-22 Thread David Aguilar 
Felipe Contreras felipe.contre...@gmail.com wrote:
David Aguilar wrote:
 Felipe Contreras felipe.contre...@gmail.com wrote:
 On Sat, Sep 21, 2013 at 1:58 PM, Johannes Sixt j...@kdbg.org wrote:
  Am 21.09.2013 13:47, schrieb Felipe Contreras:
  diff --git a/Makefile b/Makefile
  index 3588ca1..18081bf 100644
  --- a/Makefile
  +++ b/Makefile
  @@ -1010,7 +1010,7 @@ ifndef sysconfdir
   ifeq ($(prefix),/usr)
   sysconfdir = /etc
   else
  -sysconfdir = etc
  +sysconfdir = $(prefix)/etc
 
  Not good: There is a reason why this is a relative path. Please
dig
 the
  history, it's pretty clear.
 
 It's pretty clear it's *not* a relative path.
 
 What is relative about 'sysconfdir = /etc'?
 
 Thanks Johannes. Felipe, the history and this comment from the
makefile should shed some light on it:
 
 # Among the variables below, these:
 #   gitexecdir
 #   template_dir
 #   sysconfdir
 # can be specified as a relative path some/where/else;
 # this is interpreted as relative to $(prefix) and git at
 # runtime figures out where they are based on the path to the
executable.

They *can* be, but not necessarily so, and in particular sysconfig for
probably
99% of the users is not relative, it's /etc.

This makes Git relocatable. MsysGit has many more users then developers, and 
represents a pretty large chunk of users. 

-- 
David
--
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: What's cooking in git.git (Sep 2013, #02; Mon, 9)

2013-09-22 Thread Jens Lehmann 
Am 21.09.2013 00:29, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
 
 Am 10.09.2013 00:53, schrieb Junio C Hamano:
 * bc/submodule-status-ignored (2013-09-04) 2 commits
  - submodule: don't print status output with ignore=all
  - submodule: fix confusing variable name

  Originally merged to 'next' on 2013-08-22

  Will merge to 'next'.

 I propose to cook this some time in next to give submodule
 users who have configured ignore=all the opportunity to test
 and comment on that. And as Matthieu noticed the documentation
 is not terribly clear here, I'll prepare one or two patches to
 fix that which should go in together with these changes.
 
 The patches are still in 'next' but I think with the documentation
 update you and Matthieu did, it should be ready to be in 'master'
 now, no?

No objections from my side as nobody reported any problems during
the month that series cooked in next.
--
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

[L10N] Proposed updates of po/git.pot in pu branch

2013-09-22 Thread Jiang Xin 
Hi, Git l10n teams

I created a new branch pu in the l10 coordinator repository to
hold proposed updates of po/git.pot for translations. See commit
log for hints:

commit e9d227174abf2b7f66a7027a509886f9cba1adea

l10n: git.pot: proposed updates since v1.8.4 (48+)

The 1st round of Git l10n often starts very late duing the release
cycle of Git, usually after the release of rc1. This leads to a
gaint update of po/git.pot and lots of works for translators with
a tight deadline (about one week).

This pu branch holds proposed updates of the file po/git.pot for
Git next release (1.8.5), and will be updated and rewound without
notification when there are remarkable updates for it.

Translators could work on this pu branch before the l10n coordinator
starts the 1st round of l10n for this release cycle of Git, and
rebase their works onto the master branch (with the commit for the
1st round of l10n), then send pull request to the l10n coordinator.

Please note:

 1. Because this branch will be rewound occasionally, translators
SHOULD NOT merge, but SHOULD rebase to this rewound branch
instead.

$ git fetch git://github.com/git-l10n/git-po.git pu
$ git rebase --onto FETCH_HEAD HEAD~1

 2. Update your translated message file po/XX.po.

$ msgmerge --add-location --backup=off -U po/XX.po \
  po/git.pot

 3. Then start to translate po/XX.po and commit.

 4. DO NOT send pull request to the l10n coordinator for
translations on this pu branch.

 5. When the l10n coordinator starts the 1st round l10n for the
current Git release cycle, translators should rebase and squash
their works onto the master branch, and then send pull request.

$ git fetch git://github.com/git-l10n/git-po.git master
$ git rebase --onto FETCH_HEAD HEAD~1

Signed-off-by: Jiang Xin worldhello@gmail.com

Hope this may help, and not bring troubles.

-- 
Jiang Xin
--
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

Urgent Response.

2013-09-22 Thread George Daniels 
Hello,
 I am George Daniels, a Banker and credit system programmer (HSBC bank).
I saw your email address while browsing through the bank D.T.C Screen in
 my office
 yesterday so I decided to use this very chance to know you. I believe
 we should use every opportunity to know each other better.

 However, I am contacting you for obvious reason which you will understand.
 I am sending this mail just to know if this email address is OK,
 reply me back so that I will send more details to you.
 I have a very important thing to discuss with you, I look forward to
 receiving your response at
 georgedaniels...@yahoo.com.hk. Have a pleasant day.

 George Daniels





















--
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: What's cooking in git.git (Sep 2013, #02; Mon, 9)

2013-09-22 Thread Matthieu Moy 
Jens Lehmann jens.lehm...@web.de writes:

 Am 21.09.2013 00:29, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
 
 Am 10.09.2013 00:53, schrieb Junio C Hamano:
 * bc/submodule-status-ignored (2013-09-04) 2 commits
  - submodule: don't print status output with ignore=all
  - submodule: fix confusing variable name

  Originally merged to 'next' on 2013-08-22

  Will merge to 'next'.

 I propose to cook this some time in next to give submodule
 users who have configured ignore=all the opportunity to test
 and comment on that. And as Matthieu noticed the documentation
 is not terribly clear here, I'll prepare one or two patches to
 fix that which should go in together with these changes.
 
 The patches are still in 'next' but I think with the documentation
 update you and Matthieu did, it should be ready to be in 'master'
 now, no?

 No objections from my side as nobody reported any problems during
 the month that series cooked in next.

No objection from me either. I'm not a user of the feature, so I can't
really tell how good it is.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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-remote-mediawiki: bugfix for pages w/ 500 revisions

2013-09-22 Thread Benoit Person 
Mediawiki introduced a new API for queries w/ more than 500 results in
version 1.21. That change triggered an infinite loop while cloning a
mediawiki with such a page.

Fix that while still preserving the old behavior for old APIs.

Signed-off-by: Benoit Person benoit.per...@gmail.fr
Reported-by: Benjamin Cathey
---

Patch tested for all mediawiki versions from 1.19 to 1.21.

For now, if the tests suite is run without the fix, the new test
introduces an infinite loop. I am not sure if this should be handled ?
(a timeout of some kind maybe ?)

 contrib/mw-to-git/git-remote-mediawiki.perl | 14 --
 contrib/mw-to-git/t/t9365-continuing-queries.sh | 24 
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100755 contrib/mw-to-git/t/t9365-continuing-queries.sh

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
b/contrib/mw-to-git/git-remote-mediawiki.perl
index c9a4805..2d7af57 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -625,6 +625,9 @@ sub fetch_mw_revisions_for_page {
rvstartid = $fetch_from,
rvlimit = 500,
pageids = $id,
+
+# let the mediawiki knows that we support the latest API
+continue = '',
};
 
my $revnum = 0;
@@ -640,8 +643,15 @@ sub fetch_mw_revisions_for_page {
push(@page_revs, $page_rev_ids);
$revnum++;
}
-   last if (!$result-{'query-continue'});
-   $query-{rvstartid} = 
$result-{'query-continue'}-{revisions}-{rvstartid};
+
+if ($result-{'query-continue'}) { # For legacy APIs
+$query-{rvstartid} = 
$result-{'query-continue'}-{revisions}-{rvstartid};
+} elsif ($result-{continue}) { # For newer APIs
+$query-{rvstartid} = $result-{continue}-{rvcontinue};
+$query-{continue} = $result-{continue}-{continue};
+} else {
+last;
+}
}
if ($shallow_import  @page_revs) {
print {*STDERR}   Found 1 revision (shallow import).\n;
diff --git a/contrib/mw-to-git/t/t9365-continuing-queries.sh 
b/contrib/mw-to-git/t/t9365-continuing-queries.sh
new file mode 100755
index 000..6fb5df4
--- /dev/null
+++ b/contrib/mw-to-git/t/t9365-continuing-queries.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='Test the Git Mediawiki remote helper: queries w/ more than 
500 results'
+
+. ./test-gitmw-lib.sh
+. ./push-pull-tests.sh
+. $TEST_DIRECTORY/test-lib.sh
+
+test_check_precond
+
+test_expect_success 'creating page w/ 500 revisions' '
+   wiki_reset 
+   for i in $(seq 1 501)
+   do
+   echo creating revision $i
+   wiki_editpage foo revision $ibr/ true
+   done
+'
+
+test_expect_success 'cloning page w/ 500 revisions' '
+   git clone mediawiki::'$WIKI_URL' mw_dir
+'
+
+test_done
-- 
1.8.4.GIT

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

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

2013-09-22 Thread Jonathon Mah 
  HEAD:/some/path/to/foo.txt
  HEAD:some/path/to/foo.txt
 
 With my patch it prints the latter.
 
 This is because get_sha1_with_context(HEAD:...) returns an empty
 'path' string.  The code decides to use ':' as the delimiter in that
 case, but it sees there already is one at the end of HEAD:.

A few days ago I came across the same surprising output of git-grep, tried to 
adjust the code to print git show-able object names, and ran into similar 
subtleties. I just found this thread, and Jeff's code handles more cases than 
mine did (I didn't try Phil's initial patch), but I can add some more test 
cases with non-showable output (again related to git-grep's path scoping):

$ git grep -l cache HEAD:./ | head -1
HEAD:./:.gitignore

$ cd Documentation
$ git grep -l cache HEAD | head -1
HEAD:CodingGuidelines

$ git grep -l cache HEAD:Documentation/CodingGuidelines
../HEAD:Documentation/CodingGuidelines
(woah!)

Sorry that I don't yet have anything useful to suggest! But I can tell the 
story of my use case:

I have a large repository (1.6GB bare) which I don't work on, but which 
contains code that I need to refer to. A checkout is ~600MB and 27k files, 
which I'd like to avoid (it's redundant data, and would slow down backups of my 
drive). I found myself git-grepping through parts of the tree, looking 
through the results, and then git-showing interesting files. Having a real 
object name in the grep output allows copy-and-paste of the object path.



Jonathon Mah
m...@jonathonmah.com


--
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: suspected bug(s) in git-cvsexportcommit.perl

2013-09-22 Thread Robin Rosenberg 


- Ursprungligt meddelande -
 Från: David Loyall david.loy...@nebraska.gov
 Till: git@vger.kernel.org
 Skickat: onsdag, 18 sep 2013 21:00:46
 Ämne: suspected bug(s) in git-cvsexportcommit.perl
 
 Hello.
 
 I don't believe that git-cvsexportcommit.perl is working properly.
 
 First off, invocations of git-apply fail on my system.  git apply works.
   (The aforementioned script uses the former.)

git-apply is only available from scripts invoked by the git wrapper. Same 
applies
to all the other gitdash-scripts.
 
 Second, please have a look at this output (specific strings have been
 replaced with 'foo')
 
 hobbes@metalbaby:~/src/foo$ git cvsexportcommit -v
 fecc8b4bb3d91d204f4eb3ebd112f6cec6004819
 Applying to CVS commit fecc8b4bb3d91d204f4eb3ebd112f6cec6004819 from parent
 695a544fbdcf7e0614c35d1dab9a3eac0cc57b4c
 Checking if patch will apply
 cvs status: nothing known about `WebContent/WEB-INF/lib/commons-lang-2.6.jar'
 cvs status: nothing known about
 `WebContent/WEB-INF/lib/commons-configuration-1.9.jar'
 cvs status: nothing known about `JavaSource/foo/ConfigManager.java'
 cvs status: nothing known about `JavaSource/config.xml'
 Huh? Status 'Unknown' reported for unexpected file 'no file
 commons-lang-2.6.jar'
 Huh? Status 'Unknown' reported for unexpected file 'no file
 commons-configuration-1.9.jar'
 Huh? Status 'Unknown' reported for unexpected file 'no file
 ConfigManager.java'
 Huh? Status 'Unknown' reported for unexpected file 'no file config.xml'
 Applying
 stdin:7: trailing whitespace.
 ?xml version=1.0 encoding=UTF-8 ?
 stdin:8: trailing whitespace.
 config
 stdin:9: trailing whitespace.
     masterthread
 stdin:10: trailing whitespace.
     queuemappings
 stdin:11: trailing whitespace.
     mapping threadtype=foo
     service=foo
     action=foo_test /
 error: patch failed: JavaSource/foo/QueueMapping.java:67
 error: JavaSource/foo/QueueMapping.java: patch does not apply
 Context reduced to (2/2) to apply fragment at 43
 cannot patch at /usr/lib/git-core/git-cvsexportcommit line 333.
 hobbes@metalbaby:~/src/foo$

Your CVS directory is not up to date or you are, in some other way,
trying to export something that does not match the content of your CVS
directory. Keep in mind that you cannot export any commit, it must be 
possible to apply the changes in the commit to the CVS checkout, much
as patch(1) would.

The -u flag to cvsexportcommit will perform an update from CVS
for you, unless you want to do it manually.

 Even after I altered my copy of the script to invoke 'git apply' in two
 places, I still get the erroneous unknown lines.  My suspicion is that it
 has something to do with the $basename stuff, but, who am I kidding? I don't
 grok perl.

 I haven't yet explored the error lines after Applying.

Same explanation as for the Unknown status. It's not that the status is not
known. The status is Uknown, as in CVS does not know about the file.

 So, what do I want?  I want somebody to test git-cvsexportcommit.perl.  If it
 passes tests, then I'd like help creating a test that it does fail. (Perhaps
 my workspace can help.  Perhaps it is as simple as the presence of a new
 file in a subdirectory.)

-- robin
--
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?] inconsistent `git reflog show` output, possibly `git fsck` output

2013-09-22 Thread Roberto Tyley 

On 21/09/2013 23:16, Keshav Kini wrote:

[SNIP]
This situation came about because the BFG Repo-Cleaner doesn't write new
reflog entries after creating its new objects and moving refs around.


True enough - I don't think the BFG does write new entires to the
reflog when it does the final ref-update, and it would be nicer if it 
did. I'll get that fixed.


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

Re: [PATCH/RFC 0/7] Support for Ruby

2013-09-22 Thread Junio C Hamano 
[on vacaion, with only gmail webmail UI; please excuse me if this message comes
out badly formatted or gets dropped by vger.kernel.org]

On Sat, Sep 21, 2013 at 4:56 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:
 On Sat, Sep 21, 2013 at 05:52:05PM -0500, Felipe Contreras wrote:
 On Sat, Sep 21, 2013 at 4:29 PM, brian m. carlson
 sand...@crustytoothpaste.net wrote:
  As Junio has also pointed out in the past, there are people who aren't
  able to use Ruby in the same way that they are Perl and Python.  If it's
  announced now, Git 2.0 might be a good time to start accepting Ruby
  scripts, as that will give people time to plan for its inclusion.

In the very beginning, the codebase and development community of Git was
very small. In order to give usability and also easy availability of minimally
sufficient features, we used shell and Perl for quicker turn-around and
implementation and included these Porcelain scripts written in higher level
languages in the same package as the core Git.

We should look at use of shell and Perl as necessary evil in that context,
not as an enabler for people who do not want to write in C. It is no longer
2005 and the enabler side has a much more suited project for it these days.

Namely, it is better served by various language-binding efforts around libgit2.
Binding that takes advantage of each specific language is better done over
there, I think. Cf. http://www.youtube.com/watch?v=4ZWqr6iih3s

If anything, I think the core side should be focused on three things
(in addition
to bug-fixes, of course) in the longer term:

 - Defining and implementing necessary improvements to the core on-file and
   on-the-wire data structures and the protocols to serve as the canonical
   implementation.

 - Moving away from higher-level scripting languages such as shell and Perl.
   Recent clean --interactive may have added some code that could be
   reused for a rewrite of add -i (which I think is in Perl), for example.
   The minimum You need to have these to use Git should be made more
   portable by doing *less* in shell or Perl, not by adding more in the higher-
   level languages, and certainly not by adding other languages, be it Ruby or
   Lua.

 - Giving solid interface to the outside world, e.g. remote-helpers, credential-
   helpers API, and let the users and developers that want to use them do their
   own projects, without adding things to contrib/.

In other words, now the Git user and developer community are strong
and thriving,
we should strive to make the core smaller, not larger, and encourage people to
form more third party communities that specialise in the areas the
participant of
these communities are stronger than those who are involved in the core
(e.g. like
myself, Peff, Nico, Jonathan, etc.). For programs that talk remote-helper or
credential-helper protocols, for example, it is wasteful to have them
in our contrib/
and have the changes to them go through my tree, with the same coding style
standard applied to the core, which would in the longer term only add
unnecessary overhead to what they want to do and what their effort supply the
users with.

For that, defining a good inter-system interface boundary is essential
on the core
side, but we do not need to (and I do not want to see us on the core side doing)
design and implement the other side that talks to us, which people may write in
their favorite higher-level languages.

We define a reasonably robust object  history transfer mechanism over SSH
connection with set of necessary hooks to customize what happens when a push
and a fetch is requested, and Sitaram built Gitolite totally outside
the core. I think
that is the kind of thing we want to see *more* in this community. Was it better
if we defined in the core how hosting service is to be done and implemented
one ourselves? I do not think so. Similarly for Michael Haggerty's iMerge, which
was done outside the core.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 0/7] Support for Ruby

2013-09-22 Thread brian m. carlson 
On Sun, Sep 22, 2013 at 05:00:44PM -0700, Junio C Hamano wrote:
  - Moving away from higher-level scripting languages such as shell and Perl.
Recent clean --interactive may have added some code that could be
reused for a rewrite of add -i (which I think is in Perl), for example.
The minimum You need to have these to use Git should be made more
portable by doing *less* in shell or Perl, not by adding more in the 
 higher-
level languages, and certainly not by adding other languages, be it Ruby or
Lua.

I can certainly go for that.  C is faster and the codebase can be more
consistent (and more portable to non-Unix).  My concern was that if
we're going to be adding additional languages, some previous warning
would be appropriate.  As I said, I wouldn't be able to deploy a git
using Ruby immediately, and I'm sure I'm not the only one.  If we're not
going to be adding another language, then obviously the issue becomes
moot.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: [PATCH 0/9] transport-helper: updates

2013-09-22 Thread Richard Hansen 
On 2013-08-29 11:23, Felipe Contreras wrote:
 Here are the patches that allow transport helpers to be completely 
 transparent;
 renaming branches, deleting them, custom refspecs, --force, --dry-run,
 reporting forced update, everything works.

What is the status of these patches?

I would like to be able to 'git push old:new' and 'git push -f' to a
bzr repo, and these are a prerequisite.  I imagine changes to
git-remote-bzr will also be required; I'm willing write up such
changes to git-remote-bzr, but only if there's interest in picking up
this patch series.

Thanks,
Richard
--
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/9] transport-helper: add support to delete branches

2013-09-22 Thread Richard Hansen 
On 2013-08-29 11:23, Felipe Contreras wrote:
 For remote-helpers that use 'export' to push.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  t/t5801-remote-helpers.sh |  8 
  transport-helper.c| 11 ++-
  2 files changed, 14 insertions(+), 5 deletions(-)
 
[...]
 diff --git a/transport-helper.c b/transport-helper.c
 index c7135ef..5490796 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport 
 *transport,
   }
   free(private);
  
 - if (ref-deletion)
 - die(remote-helpers do not support ref deletion);
 -

The above deleted lines actually appear twice in transport-helper.c due
to an incorrect merge conflict resolution in
99d9ec090677c925c534001f01cbaf303a31cb82.  The other copy of those lines should
also be deleted:

diff --git a/transport-helper.c b/transport-helper.c
index 859131f..bbf4e7c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -874,9 +874,6 @@ static int push_refs_with_export(struct transport 
*transport,
char *private;
unsigned char sha1[20];

-   if (ref-deletion)
-   die(remote-helpers do not support ref deletion);
-
private = apply_refspecs(data-refspecs, data-refspec_nr, 
ref-name);
if (private  !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
--
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/9] fast-export: add new --refspec option

2013-09-22 Thread Richard Hansen 
On 2013-08-29 11:23, Felipe Contreras wrote:
 So that we can covert the exported ref names.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/git-fast-export.txt |  4 
  builtin/fast-export.c | 30 ++
  t/t9350-fast-export.sh|  7 +++
  3 files changed, 41 insertions(+)
 
 diff --git a/Documentation/git-fast-export.txt 
 b/Documentation/git-fast-export.txt
 index 85f1f30..221506b 100644
 --- a/Documentation/git-fast-export.txt
 +++ b/Documentation/git-fast-export.txt
 @@ -105,6 +105,10 @@ marks the same across runs.
   in the commit (as opposed to just listing the files which are
   different from the commit's first parent).
  
 +--refspec::
 + Apply the specified refspec to each ref exported. Multiple of them can
 + be specified.
 +

Do you mean '--refspec=refspec' and/or '--refspec refspec'?

How are the multiple refspecs specified?  Space/comma/colon separated
list?  Or multiple '--refspec' arguments with one refspec per '--refspec'?

 diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
 index 34c2d8f..dcf 100755
 --- a/t/t9350-fast-export.sh
 +++ b/t/t9350-fast-export.sh
 @@ -504,4 +504,11 @@ test_expect_success 'refs are updated even if no commits 
 need to be exported' '
   test_cmp expected actual
  '
  
 +test_expect_success 'use refspec' '
 + git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
 + grep ^commit  | sort | uniq  actual 
 + echo commit refs/heads/foobar  expected 
 + test_cmp expected actual
 +'
 +
  test_done
 

I think it'd be good to add a test for multiple refspecs.

-Richard
--
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 0/9] transport-helper: updates

2013-09-22 Thread Felipe Contreras 
Richard Hansen wrote:
 On 2013-08-29 11:23, Felipe Contreras wrote:
  Here are the patches that allow transport helpers to be completely 
  transparent;
  renaming branches, deleting them, custom refspecs, --force, --dry-run,
  reporting forced update, everything works.
 
 What is the status of these patches?

Ask Junio. All the valid feedback has been addressed, there's no reason not to
apply them.

 I would like to be able to 'git push old:new' and 'git push -f' to a
 bzr repo, and these are a prerequisite.  I imagine changes to
 git-remote-bzr will also be required; I'm willing write up such
 changes to git-remote-bzr, but only if there's interest in picking up
 this patch series.

There's no need to modify git-remote-bzr, if theese patches are applied,
old:new would work for all remote-helpers.

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

Re: [PATCH 8/9] transport-helper: add support to delete branches

2013-09-22 Thread Felipe Contreras 
Richard Hansen wrote:
 On 2013-08-29 11:23, Felipe Contreras wrote:
  For remote-helpers that use 'export' to push.
  
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  ---
   t/t5801-remote-helpers.sh |  8 
   transport-helper.c| 11 ++-
   2 files changed, 14 insertions(+), 5 deletions(-)
  
 [...]
  diff --git a/transport-helper.c b/transport-helper.c
  index c7135ef..5490796 100644
  --- a/transport-helper.c
  +++ b/transport-helper.c
  @@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport 
  *transport,
  }
  free(private);
   
  -   if (ref-deletion)
  -   die(remote-helpers do not support ref deletion);
  -
 
 The above deleted lines actually appear twice in transport-helper.c due
 to an incorrect merge conflict resolution in
 99d9ec090677c925c534001f01cbaf303a31cb82.  The other copy of those lines 
 should
 also be deleted:
 
 diff --git a/transport-helper.c b/transport-helper.c
 index 859131f..bbf4e7c 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -874,9 +874,6 @@ static int push_refs_with_export(struct transport 
 *transport,
   char *private;
   unsigned char sha1[20];
 
 - if (ref-deletion)
 - die(remote-helpers do not support ref deletion);
 -
   private = apply_refspecs(data-refspecs, data-refspec_nr, 
 ref-name);
   if (private  !get_sha1(private, sha1)) {
   strbuf_addf(buf, ^%s, private);

Right.

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

Re: [PATCH 1/9] transport-helper: add 'force' to 'export' helpers

2013-09-22 Thread Richard Hansen 
On 2013-08-29 11:23, Felipe Contreras wrote:
 Otherwise they cannot know when to force the push or not (other than
 hacks).
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  transport-helper.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/transport-helper.c b/transport-helper.c
 index 63cabc3..62051a6 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport 
 *transport,
   die(helper %s does not support dry-run, data-name);
   }
  
 + if (flags  TRANSPORT_PUSH_FORCE)
 + set_helper_option(transport, force, true);

Should the return value of set_helper_option() be checked?

Also, should there be a #define TRANS_OPT_FORCE force with
TRANS_OPT_FORCE added to boolean_options[]?

Thanks,
Richard
--
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/9] fast-export: add new --refspec option

2013-09-22 Thread Felipe Contreras 
Richard Hansen wrote:
 On 2013-08-29 11:23, Felipe Contreras wrote:
  So that we can covert the exported ref names.
  
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  ---
   Documentation/git-fast-export.txt |  4 
   builtin/fast-export.c | 30 ++
   t/t9350-fast-export.sh|  7 +++
   3 files changed, 41 insertions(+)
  
  diff --git a/Documentation/git-fast-export.txt 
  b/Documentation/git-fast-export.txt
  index 85f1f30..221506b 100644
  --- a/Documentation/git-fast-export.txt
  +++ b/Documentation/git-fast-export.txt
  @@ -105,6 +105,10 @@ marks the same across runs.
  in the commit (as opposed to just listing the files which are
  different from the commit's first parent).
   
  +--refspec::
  +   Apply the specified refspec to each ref exported. Multiple of them can
  +   be specified.
  +
 
 Do you mean '--refspec=refspec' and/or '--refspec refspec'?

I mean --refspec refspec1 --refspec refspec2.

  diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
  index 34c2d8f..dcf 100755
  --- a/t/t9350-fast-export.sh
  +++ b/t/t9350-fast-export.sh
  @@ -504,4 +504,11 @@ test_expect_success 'refs are updated even if no 
  commits need to be exported' '
  test_cmp expected actual
   '
   
  +test_expect_success 'use refspec' '
  +   git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
  +   grep ^commit  | sort | uniq  actual 
  +   echo commit refs/heads/foobar  expected 
  +   test_cmp expected actual
  +'
  +
   test_done
  
 
 I think it'd be good to add a test for multiple refspecs.

Maybe. If these patches were to actually be applied.

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

Re: [PATCH 1/9] transport-helper: add 'force' to 'export' helpers

2013-09-22 Thread Felipe Contreras 
Richard Hansen wrote:
 On 2013-08-29 11:23, Felipe Contreras wrote:
  Otherwise they cannot know when to force the push or not (other than
  hacks).
  
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  ---
   transport-helper.c | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/transport-helper.c b/transport-helper.c
  index 63cabc3..62051a6 100644
  --- a/transport-helper.c
  +++ b/transport-helper.c
  @@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport 
  *transport,
  die(helper %s does not support dry-run, data-name);
  }
   
  +   if (flags  TRANSPORT_PUSH_FORCE)
  +   set_helper_option(transport, force, true);
 
 Should the return value of set_helper_option() be checked?

Yeah, it would make sense. I guess we want to die() if the user does
'git push -f' and the remote helper doesn't support that.

 Also, should there be a #define TRANS_OPT_FORCE force with

I don't see the point of that. Defines are useful when you want to change the
value string, so you don't have to change the string everywhere, but we
definitely don't want to do that, as that would break backwards compatibility,
so TRANS_OPT_KEEP would always be keep so it's just a way to tire our
fingers.

 TRANS_OPT_FORCE added to boolean_options[]?

I don't see how that would help us, the only thing that would achieve is to map:


  set_helper_option(transport, force, 1);

to

  set_helper_option(transport, force, true);

But we are already doing that.

Moreover, the code is already doing something similar for all the other options.

  set_helper_option(t, progress, t-progress ? true : false);
  set_helper_option(t, verbosity, buf);
  set_helper_option(transport, servpath, exec);
  set_helper_option(transport, dry-run, true);

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

[PATCH 04/15] contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly

2013-09-22 Thread Brandon Casey 
If the correct arguments were not specified, this program should exit
non-zero.  Let's do so.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index ad23dbf..5459ba7 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -396,7 +396,7 @@ int main(int argc, char *argv[])
 
if (!argv[1]) {
usage(argv[0]);
-   goto out;
+   exit(EXIT_FAILURE);
}
 
/* lookup operation callback */
-- 
1.8.4.489.g545bc72

--
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 06/15] contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t

2013-09-22 Thread Brandon Casey 
Also, initialization is not necessary since it is assigned before it is
used.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 5779770..1081224 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -307,7 +307,7 @@ static void credential_clear(struct credential *c)
 static int credential_read(struct credential *c)
 {
charbuf[1024];
-   ssize_t line_len = 0;
+   size_t line_len;
char   *key  = buf;
char   *value;
 
-- 
1.8.4.489.g545bc72

--
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 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros

2013-09-22 Thread Brandon Casey 
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

 contrib/credential/gnome-keyring/Makefile  |   4 +-
 .../gnome-keyring/git-credential-gnome-keyring.c   | 280 -
 2 files changed, 158 insertions(+), 126 deletions(-)

-- 
1.8.4.489.g545bc72

--
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 01/15] contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations

2013-09-22 Thread Brandon Casey 
These are all defined before they are used, so it is not necessary to
pre-declare them.  Remove the pre-declarations.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../credential/gnome-keyring/git-credential-gnome-keyring.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index f2cdefe..15b0a1c 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -46,11 +46,6 @@ struct credential
 #define CREDENTIAL_INIT \
   { NULL,NULL,0,NULL,NULL,NULL }
 
-void credential_init(struct credential *c);
-void credential_clear(struct credential *c);
-int  credential_read(struct credential *c);
-void credential_write(const struct credential *c);
-
 typedef int (*credential_op_cb)(struct credential*);
 
 struct credential_operation
@@ -62,14 +57,6 @@ struct credential_operation
 #define CREDENTIAL_OP_END \
   { NULL,NULL }
 
-/*
- * Table with operation callbacks is defined in concrete
- * credential helper implementation and contains entries
- * like { get, function_to_get_credential } terminated
- * by CREDENTIAL_OP_END.
- */
-struct credential_operation const credential_helper_ops[];
-
 /*  common helper functions - */
 
 static inline void free_password(char *password)
-- 
1.8.4.489.g545bc72

--
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 10/15] contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords

2013-09-22 Thread Brandon Casey 
gnome-keyring provides functions to allocate non-pageable memory (if
possible).  Let's use them to allocate memory that may be used to hold
secure data read from the keyring.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../credential/gnome-keyring/git-credential-gnome-keyring.c  | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index ff2f48c..94a65b2 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -289,12 +289,14 @@ static void credential_clear(struct credential *c)
 
 static int credential_read(struct credential *c)
 {
-   charbuf[1024];
+   char*buf;
size_t line_len;
-   char   *key  = buf;
+   char   *key;
char   *value;
 
-   while (fgets(buf, sizeof(buf), stdin))
+   key = buf = gnome_keyring_memory_alloc(1024);
+
+   while (fgets(buf, 1024, stdin))
{
line_len = strlen(buf);
 
@@ -307,6 +309,7 @@ static int credential_read(struct credential *c)
value = strchr(buf,'=');
if(!value) {
warning(invalid credential line: %s, key);
+   gnome_keyring_memory_free(buf);
return -1;
}
*value++ = '\0';
@@ -339,6 +342,9 @@ static int credential_read(struct credential *c)
 * learn new lines, and the helpers are updated to match.
 */
}
+
+   gnome_keyring_memory_free(buf);
+
return 0;
 }
 
-- 
1.8.4.489.g545bc72

--
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 08/15] contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object()

2013-09-22 Thread Brandon Casey 
Rather than carefully allocating memory for sprintf() to write into,
let's make use of the glib helper function g_strdup_printf(), which
makes things a lot easier and less error-prone.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 8ae2eab..7565765 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -112,21 +112,13 @@ static inline char *xstrdup(const char *str)
 /* create a special keyring option string, if path is given */
 static char* keyring_object(struct credential *c)
 {
-   char* object = NULL;
-
if (!c-path)
-   return object;
-
-   object = (char*) malloc(strlen(c-host)+strlen(c-path)+8);
-   if(!object)
-   die_errno(errno);
+   return NULL;
 
if(c-port)
-   sprintf(object,%s:%hd/%s,c-host,c-port,c-path);
-   else
-   sprintf(object,%s/%s,c-host,c-path);
+   return g_strdup_printf(%s:%hd/%s, c-host, c-port, c-path);
 
-   return object;
+   return g_strdup_printf(%s/%s, c-host, c-path);
 }
 
 static int keyring_get(struct credential *c)
-- 
1.8.4.489.g545bc72

--
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 11/15] contrib/git-credential-gnome-keyring.c: use glib memory allocation functions

2013-09-22 Thread Brandon Casey 
Rather than roll our own, let's use the memory allocation/free routines
provided by glib.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 48 --
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 94a65b2..5b10e3e 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -27,7 +27,6 @@
 #include string.h
 #include stdarg.h
 #include stdlib.h
-#include errno.h
 #include glib.h
 #include gnome-keyring.h
 #include gnome-keyring-memory.h
@@ -83,21 +82,6 @@ static inline void error(const char *fmt, ...)
va_end(ap);
 }
 
-static inline void die_errno(int err)
-{
-   error(%s, strerror(err));
-   exit(EXIT_FAILURE);
-}
-
-static inline char *xstrdup(const char *str)
-{
-   char *ret = strdup(str);
-   if (!ret)
-   die_errno(errno);
-
-   return ret;
-}
-
 /* - GNOME Keyring functions - */
 
 /* create a special keyring option string, if path is given */
@@ -134,7 +118,7 @@ static int keyring_get(struct credential *c)
c-port,
entries);
 
-   free(object);
+   g_free(object);
 
if (result == GNOME_KEYRING_RESULT_NO_MATCH)
return EXIT_SUCCESS;
@@ -154,7 +138,7 @@ static int keyring_get(struct credential *c)
c-password = gnome_keyring_memory_strdup(password_data-password);
 
if (!c-username)
-   c-username = xstrdup(password_data-user);
+   c-username = g_strdup(password_data-user);
 
gnome_keyring_network_password_list_free(entries);
 
@@ -192,7 +176,7 @@ static int keyring_store(struct credential *c)
c-password,
item_id);
 
-   free(object);
+   g_free(object);
return EXIT_SUCCESS;
 }
 
@@ -226,7 +210,7 @@ static int keyring_erase(struct credential *c)
c-port,
entries);
 
-   free(object);
+   g_free(object);
 
if (result == GNOME_KEYRING_RESULT_NO_MATCH)
return EXIT_SUCCESS;
@@ -278,10 +262,10 @@ static void credential_init(struct credential *c)
 
 static void credential_clear(struct credential *c)
 {
-   free(c-protocol);
-   free(c-host);
-   free(c-path);
-   free(c-username);
+   g_free(c-protocol);
+   g_free(c-host);
+   g_free(c-path);
+   g_free(c-username);
gnome_keyring_memory_free(c-password);
 
credential_init(c);
@@ -315,22 +299,22 @@ static int credential_read(struct credential *c)
*value++ = '\0';
 
if (!strcmp(key, protocol)) {
-   free(c-protocol);
-   c-protocol = xstrdup(value);
+   g_free(c-protocol);
+   c-protocol = g_strdup(value);
} else if (!strcmp(key, host)) {
-   free(c-host);
-   c-host = xstrdup(value);
+   g_free(c-host);
+   c-host = g_strdup(value);
value = strrchr(c-host,':');
if (value) {
*value++ = '\0';
c-port = atoi(value);
}
} else if (!strcmp(key, path)) {
-   free(c-path);
-   c-path = xstrdup(value);
+   g_free(c-path);
+   c-path = g_strdup(value);
} else if (!strcmp(key, username)) {
-   free(c-username);
-   c-username = xstrdup(value);
+   g_free(c-username);
+   c-username = g_strdup(value);
} else if (!strcmp(key, password)) {
gnome_keyring_memory_free(c-password);
c-password = gnome_keyring_memory_strdup(value);
-- 
1.8.4.489.g545bc72

--
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 13/15] contrib/git-credential-gnome-keyring.c: report failure to store password

2013-09-22 Thread Brandon Casey 
Produce an error message when we fail to store a password to the keyring.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index a6369a3..6fa1278 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -125,6 +125,7 @@ static int keyring_store(struct credential *c)
 {
guint32 item_id;
char  *object = NULL;
+   GnomeKeyringResult result;
 
/*
 * Sanity check that what we are storing is actually sensible.
@@ -139,7 +140,7 @@ static int keyring_store(struct credential *c)
 
object = keyring_object(c);
 
-   gnome_keyring_set_network_password_sync(
+   result = gnome_keyring_set_network_password_sync(
GNOME_KEYRING_DEFAULT,
c-username,
NULL /* domain */,
@@ -152,6 +153,12 @@ static int keyring_store(struct credential *c)
item_id);
 
g_free(object);
+
+   if (result != GNOME_KEYRING_RESULT_OK) {
+   g_critical(%s, gnome_keyring_result_to_message(result));
+   return EXIT_FAILURE;
+   }
+
return EXIT_SUCCESS;
 }
 
-- 
1.8.4.489.g545bc72

--
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 05/15] contrib/git-credential-gnome-keyring.c: set Gnome application name

2013-09-22 Thread Brandon Casey 
Since this is a Gnome application, let's set the application name to
something reasonable.  This will be displayed in Gnome dialog boxes
e.g. the one that prompts for the user's keyring password.

We add an include statement for glib.h and add the glib-2.0 cflags and
libs to the compilation arguments, but both of these are really noops
since glib is already a dependency of gnome-keyring.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 contrib/credential/gnome-keyring/Makefile   | 4 ++--
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/contrib/credential/gnome-keyring/Makefile 
b/contrib/credential/gnome-keyring/Makefile
index e6561d8..c3c7c98 100644
--- a/contrib/credential/gnome-keyring/Makefile
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -8,8 +8,8 @@ CFLAGS = -g -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-INCS:=$(shell pkg-config --cflags gnome-keyring-1)
-LIBS:=$(shell pkg-config --libs gnome-keyring-1)
+INCS:=$(shell pkg-config --cflags gnome-keyring-1 glib-2.0)
+LIBS:=$(shell pkg-config --libs gnome-keyring-1 glib-2.0)
 
 SRCS:=$(MAIN).c
 OBJS:=$(SRCS:.c=.o)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 5459ba7..5779770 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -28,6 +28,7 @@
 #include stdarg.h
 #include stdlib.h
 #include errno.h
+#include glib.h
 #include gnome-keyring.h
 
 /*
@@ -399,6 +400,8 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
 
+   g_set_application_name(Git Credential Helper);
+
/* lookup operation callback */
while(try_op-name  strcmp(argv[1], try_op-name))
try_op++;
-- 
1.8.4.489.g545bc72

--
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 02/15] contrib/git-credential-gnome-keyring.c: remove unused die() function

2013-09-22 Thread Brandon Casey 
Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../credential/gnome-keyring/git-credential-gnome-keyring.c| 10 --
 1 file changed, 10 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 15b0a1c..4334f23 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -91,16 +91,6 @@ static inline void error(const char *fmt, ...)
va_end(ap);
 }
 
-static inline void die(const char *fmt, ...)
-{
-   va_list ap;
-
-   va_start(ap,fmt);
-   error(fmt, ap);
-   va_end(ap);
-   exit(EXIT_FAILURE);
-}
-
 static inline void die_errno(int err)
 {
error(%s, strerror(err));
-- 
1.8.4.489.g545bc72

--
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 12/15] contrib/git-credential-gnome-keyring.c: use glib messaging functions

2013-09-22 Thread Brandon Casey 
Rather than roll our own, let's use the messaging functions provided
by glib.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 33 +++---
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 5b10e3e..a6369a3 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -25,7 +25,6 @@
 
 #include stdio.h
 #include string.h
-#include stdarg.h
 #include stdlib.h
 #include glib.h
 #include gnome-keyring.h
@@ -58,30 +57,6 @@ struct credential_operation
 #define CREDENTIAL_OP_END \
   { NULL,NULL }
 
-/*  common helper functions - */
-
-static inline void warning(const char *fmt, ...)
-{
-   va_list ap;
-
-   va_start(ap, fmt);
-   fprintf(stderr, warning: );
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n );
-   va_end(ap);
-}
-
-static inline void error(const char *fmt, ...)
-{
-   va_list ap;
-
-   va_start(ap, fmt);
-   fprintf(stderr, error: );
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n );
-   va_end(ap);
-}
-
 /* - GNOME Keyring functions - */
 
 /* create a special keyring option string, if path is given */
@@ -127,7 +102,7 @@ static int keyring_get(struct credential *c)
return EXIT_SUCCESS;
 
if (result != GNOME_KEYRING_RESULT_OK) {
-   error(%s,gnome_keyring_result_to_message(result));
+   g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
 
@@ -220,7 +195,7 @@ static int keyring_erase(struct credential *c)
 
if (result != GNOME_KEYRING_RESULT_OK)
{
-   error(%s,gnome_keyring_result_to_message(result));
+   g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
 
@@ -234,7 +209,7 @@ static int keyring_erase(struct credential *c)
 
if (result != GNOME_KEYRING_RESULT_OK)
{
-   error(%s,gnome_keyring_result_to_message(result));
+   g_critical(%s, gnome_keyring_result_to_message(result));
return EXIT_FAILURE;
}
 
@@ -292,7 +267,7 @@ static int credential_read(struct credential *c)
 
value = strchr(buf,'=');
if(!value) {
-   warning(invalid credential line: %s, key);
+   g_warning(invalid credential line: %s, key);
gnome_keyring_memory_free(buf);
return -1;
}
-- 
1.8.4.489.g545bc72

--
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 14/15] contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring

2013-09-22 Thread Brandon Casey 
The gnome-keyring lib distributed with RHEL 5.X is ancient and does
not provide a few of the functions/defines that more recent versions
do, but mostly the API is the same.  Let's provide the missing bits
via macro definitions and function implementation.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 58 ++
 1 file changed, 58 insertions(+)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 6fa1278..f8f4df9 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -28,8 +28,66 @@
 #include stdlib.h
 #include glib.h
 #include gnome-keyring.h
+
+#ifdef GNOME_KEYRING_DEFAULT
+
+   /* Modern gnome-keyring */
+
 #include gnome-keyring-memory.h
 
+#else
+
+   /*
+* Support ancient gnome-keyring, circ. RHEL 5.X.
+* GNOME_KEYRING_DEFAULT seems to have been introduced with Gnome 2.22,
+* and the other features roughly around Gnome 2.20, 6 months before.
+* Ubuntu 8.04 used Gnome 2.22 (I think).  Not sure any distro used 2.20.
+* So the existence/non-existence of GNOME_KEYRING_DEFAULT seems like
+* a decent thing to use as an indicator.
+*/
+
+#define GNOME_KEYRING_DEFAULT NULL
+
+/*
+ * ancient gnome-keyring returns DENIED when an entry is not found.
+ * Setting NO_MATCH to DENIED will prevent us from reporting denied
+ * errors during get and erase operations, but we will still report
+ * DENIED errors during a store.
+ */
+#define GNOME_KEYRING_RESULT_NO_MATCH GNOME_KEYRING_RESULT_DENIED
+
+#define gnome_keyring_memory_alloc g_malloc
+#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)
+{
+   switch (result) {
+   case GNOME_KEYRING_RESULT_OK:
+   return OK;
+   case GNOME_KEYRING_RESULT_DENIED:
+   return Denied;
+   case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON:
+   return No Keyring Daemon;
+   case GNOME_KEYRING_RESULT_ALREADY_UNLOCKED:
+   return Already UnLocked;
+   case GNOME_KEYRING_RESULT_NO_SUCH_KEYRING:
+   return No Such Keyring;
+   case GNOME_KEYRING_RESULT_BAD_ARGUMENTS:
+   return Bad Arguments;
+   case GNOME_KEYRING_RESULT_IO_ERROR:
+   return IO Error;
+   case GNOME_KEYRING_RESULT_CANCELLED:
+   return Cancelled;
+   case GNOME_KEYRING_RESULT_ALREADY_EXISTS:
+   return Already Exists;
+   default:
+   return Unknown Error;
+   }
+}
+
+#endif
+
 /*
  * This credential struct and API is simplified from git's credential.{h,c}
  */
-- 
1.8.4.489.g545bc72

--
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 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing

2013-09-22 Thread Brandon Casey 
Ensure buffer length is non-zero before attempting to access the last
element.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 1081224..8ae2eab 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -315,7 +315,7 @@ static int credential_read(struct credential *c)
{
line_len = strlen(buf);
 
-   if(buf[line_len-1]=='\n')
+   if(line_len  buf[line_len-1] == '\n')
buf[--line_len]='\0';
 
if(!line_len)
-- 
1.8.4.489.g545bc72

--
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 15/15] contrib/git-credential-gnome-keyring.c: support really ancient gnome-keyring

2013-09-22 Thread Brandon Casey 
The gnome-keyring lib (0.4) distributed with RHEL 4.X is really ancient
and does not provide most of the synchronous functions that even ancient
releases do.  Thankfully, we're only using one function that is missing.
Let's emulate gnome_keyring_item_delete_sync() by calling the asynchronous
function and then triggering the event loop processing until our
callback is called.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 39 ++
 1 file changed, 39 insertions(+)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index f8f4df9..ce2ddee 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -86,6 +86,45 @@ static const char* 
gnome_keyring_result_to_message(GnomeKeyringResult result)
}
 }
 
+/*
+ * 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 ??
+ */
+#if GLIB_MAJOR_VERSION == 2  GLIB_MINOR_VERSION  8
+
+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];
+
+   *r = result;
+   *done = 1;
+}
+
+static void wait_for_request_completion(int *done)
+{
+   GMainContext *mc = g_main_context_default();
+   while (!*done)
+   g_main_context_iteration(mc, TRUE);
+}
+
+static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, 
guint32 id)
+{
+   int done = 0;
+   GnomeKeyringResult result;
+   gpointer data[] = { done, result };
+
+   gnome_keyring_item_delete(keyring, id, gnome_keyring_done_cb, data,
+   NULL);
+
+   wait_for_request_completion(done);
+
+   return result;
+}
+
+#endif
 #endif
 
 /*
-- 
1.8.4.489.g545bc72

--
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 09/15] contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds

2013-09-22 Thread Brandon Casey 
gnome-keyring provides functions for allocating non-pageable memory (if
possible) intended to be used for storing passwords.  Let's use them.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c| 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 7565765..ff2f48c 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -30,6 +30,7 @@
 #include errno.h
 #include glib.h
 #include gnome-keyring.h
+#include gnome-keyring-memory.h
 
 /*
  * This credential struct and API is simplified from git's credential.{h,c}
@@ -60,16 +61,6 @@ struct credential_operation
 
 /*  common helper functions - */
 
-static inline void free_password(char *password)
-{
-   char *c = password;
-   if (!password)
-   return;
-
-   while (*c) *c++ = '\0';
-   free(password);
-}
-
 static inline void warning(const char *fmt, ...)
 {
va_list ap;
@@ -159,8 +150,8 @@ static int keyring_get(struct credential *c)
/* pick the first one from the list */
password_data = (GnomeKeyringNetworkPasswordData *) entries-data;
 
-   free_password(c-password);
-   c-password = xstrdup(password_data-password);
+   gnome_keyring_memory_free(c-password);
+   c-password = gnome_keyring_memory_strdup(password_data-password);
 
if (!c-username)
c-username = xstrdup(password_data-user);
@@ -291,7 +282,7 @@ static void credential_clear(struct credential *c)
free(c-host);
free(c-path);
free(c-username);
-   free_password(c-password);
+   gnome_keyring_memory_free(c-password);
 
credential_init(c);
 }
@@ -338,8 +329,8 @@ static int credential_read(struct credential *c)
free(c-username);
c-username = xstrdup(value);
} else if (!strcmp(key, password)) {
-   free_password(c-password);
-   c-password = xstrdup(value);
+   gnome_keyring_memory_free(c-password);
+   c-password = gnome_keyring_memory_strdup(value);
while (*value) *value++ = '\0';
}
/*
-- 
1.8.4.489.g545bc72

--
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 03/15] contrib/git-credential-gnome-keyring.c: add static where applicable

2013-09-22 Thread Brandon Casey 
Mark global variable and functions as static.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 4334f23..ad23dbf 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -128,7 +128,7 @@ static char* keyring_object(struct credential *c)
return object;
 }
 
-int keyring_get(struct credential *c)
+static int keyring_get(struct credential *c)
 {
char* object = NULL;
GList *entries;
@@ -178,7 +178,7 @@ int keyring_get(struct credential *c)
 }
 
 
-int keyring_store(struct credential *c)
+static int keyring_store(struct credential *c)
 {
guint32 item_id;
char  *object = NULL;
@@ -212,7 +212,7 @@ int keyring_store(struct credential *c)
return EXIT_SUCCESS;
 }
 
-int keyring_erase(struct credential *c)
+static int keyring_erase(struct credential *c)
 {
char  *object = NULL;
GList *entries;
@@ -277,7 +277,7 @@ int keyring_erase(struct credential *c)
  * Table with helper operation callbacks, used by generic
  * credential helper main function.
  */
-struct credential_operation const credential_helper_ops[] =
+static struct credential_operation const credential_helper_ops[] =
 {
{ get,   keyring_get   },
{ store, keyring_store },
@@ -287,12 +287,12 @@ struct credential_operation const credential_helper_ops[] 
=
 
 /* -- credential functions -- */
 
-void credential_init(struct credential *c)
+static void credential_init(struct credential *c)
 {
memset(c, 0, sizeof(*c));
 }
 
-void credential_clear(struct credential *c)
+static void credential_clear(struct credential *c)
 {
free(c-protocol);
free(c-host);
@@ -303,7 +303,7 @@ void credential_clear(struct credential *c)
credential_init(c);
 }
 
-int credential_read(struct credential *c)
+static int credential_read(struct credential *c)
 {
charbuf[1024];
ssize_t line_len = 0;
@@ -358,14 +358,14 @@ int credential_read(struct credential *c)
return 0;
 }
 
-void credential_write_item(FILE *fp, const char *key, const char *value)
+static void credential_write_item(FILE *fp, const char *key, const char *value)
 {
if (!value)
return;
fprintf(fp, %s=%s\n, key, value);
 }
 
-void credential_write(const struct credential *c)
+static void credential_write(const struct credential *c)
 {
/* only write username/password, if set */
credential_write_item(stdout, username, c-username);
-- 
1.8.4.489.g545bc72

--
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 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing

2013-09-22 Thread Felipe Contreras 
Brandon Casey wrote:
 Ensure buffer length is non-zero before attempting to access the last
 element.
 
 Signed-off-by: Brandon Casey draf...@gmail.com
 ---
  contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
 b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
 index 1081224..8ae2eab 100644
 --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
 +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
 @@ -315,7 +315,7 @@ static int credential_read(struct credential *c)
   {
   line_len = strlen(buf);
  
 - if(buf[line_len-1]=='\n')
 + if(line_len  buf[line_len-1] == '\n')

The style is if ().

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