Re: [PATCH/RFC 0/7] Support for Ruby
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
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
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
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
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)
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
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.
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)
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
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
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
- 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
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
[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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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