* Re: What's cooking in git.git (Sep 2011, #04; Mon, 12)
From: Junio C Hamano @ 2011-09-14 20:57 UTC (permalink / raw)
To: Jonathon Mah; +Cc: Dan McGee, David Aguilar, git
In-Reply-To: <D3CA81F2-647B-4AD0-A4FC-4C22772FD791@JonathonMah.com>
Jonathon Mah <me@JonathonMah.com> writes:
> On 2011-09-12, at 14:59, Junio C Hamano wrote:
>
>>> [Stalled]
>>>
>>> * jm/mergetool-pathspec (2011-06-22) 2 commits
>>> - mergetool: Don't assume paths are unmerged
>>> - mergetool: Add tests for filename with whitespace
>>
>> What's the plan for this series? Do we still want to pursue it within the
>> timeframe for the next round?
>>
>> Is there any mergetool/difftool expert who volunteers to help moving this
>> topic forward?
>
> I'd love this to stay alive. As I've mentioned before, my relationship
> with shell is tenuous. My biggest problem is...
Hopefully volunteers can help moving this forward with "coding".
http://thread.gmane.org/gmane.comp.version-control.git/176215
http://thread.gmane.org/gmane.comp.version-control.git/176216
There were issues with the patches that were _not_ about the coding but
about the documentation and design.
Topics with a stale version in 'pu' that are stalled are not even alive--
they are zombies that need to be wiped at some point, or replaced with
updated series, whichever comes first.
^ permalink raw reply
* Re: [Survey] Signed push
From: Jonathan Nieder @ 2011-09-14 21:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CACsJy8Dwu2U-7eEZU-VYmcrA7JwtvUkJS5SywXjZWoE1twchhQ@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> On Wed, Sep 14, 2011 at 2:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> An alternative that I am considering is to let the requester say this
>> instead:
>>
>> are available in the git repository at:
>> git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
[...]
> Stupid question, if we agree to go with signed push, can we also sign
> pull requests and verify them when we pull? I suppose most of the
> time, pulling can be done automatically by extracting pull url from
> the request. This would make pull/push both signed.
>
> BTW, there's a third way (rsync is obsolete) to carry changes away in
> human-unreadable way: bundles. Should we also sign the bundles too (I
> guess we could just do the same as in signed push).
If I understand you correctly, then ordinary PGP email signing[1]
should work for that already. In your first example, the receiver can
make sure whatever process grabs a pull request verifies it, and in
the second example, the receiver checks the signature on her email
before saving a bundle and passing it to "git fetch".
[1] http://www.phildev.net/pgp/gpgmua.html
^ permalink raw reply
* Re: Helping on Git development
From: Jonathan Nieder @ 2011-09-14 21:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Ardill, Eduardo D'Avila, git, git
In-Reply-To: <7vehzjugdz.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> As to contributing to the project, right now, I think we have enough
> people who want to write code and documentation for Git, but what we lack
> are bandwidth to (this is not meant to be an exhaustive list):
[...]
> - distilling random wishes from the end user community while winnowing
> chaffs that are unrealistic or do not fit well with the grand scheme of
> things, to come up with a concrete proposal and a patch series
[...]
> - dig list archives to point people at age-old discussions to non-issues
> that have long been resolved to squelch noise; and
>
> - remind original submitter, people who were involved in the discussion,
> and people who should have been involved but who weren't, of a worthy
> but stalled topics from time to time.
I also should (reluctantly) mention that the Debian bug tracker has
been accepting bugs from outsiders and provides a service like this.
Caveats:
- it only tracks bugs that affect Debian (usually meaning
platform-independent bugs). Occasionally bugs from Windows users
have been reported there and it's been okay.
- the interface might seem quirky if you're not used to it.
Documentation is at http://www.debian.org/Bugs/
- no guarantee of a quick response. When there is a response,
usually it is "here are some thoughts; now let's take this to
git@vger.kernel.org".
- if the bugtracker gets swamped with reports from outside without
manpower to match, the policy re bugs from outside Debian might
change.
Bug listing: [1].
To subscribe to receive bug reports by email: [2].
Thoughts welcome, as always.
Jonathan
[1] http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=git;include=tags:upstream;exclude=tags:fixed-upstream;exclude=tags:moreinfo;exclude=severity:wishlist
[2] http://www.debian.org/doc/manuals/developers-reference/resources.html#pts-commands
Summary: an email to pts@qa.debian.org whose body contains the two
lines "subscribe git", "keyword git = bts" would do the trick.
^ permalink raw reply
* Re: [PATCH 0/5] credential helper super fun pak
From: Junio C Hamano @ 2011-09-14 21:37 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Here's a mixed bag of bugfix commits to go on top of what's in
> jk/http-auth-keyring.
Heh, it indeed is a super-fun-pak. Will queue.
^ permalink raw reply
* Re: [PATCH] contrib: add a credential helper for Mac OS X's keychain
From: John Szakmeister @ 2011-09-14 22:23 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <CAG+J_Dw-vf7FtyT-vPpj-LHBo0rCBJi39bHh=8vWjc52QBMM2A@mail.gmail.com>
On Wed, Sep 14, 2011 at 9:31 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
[snip]
> Nope, you snipped out too much context. That command is turned into a
> string and then sent to /usr/bin/security on its stdin. It is
> absolutely not passed on the command-line.
Bah. I see it now. Thanks.
-John
^ permalink raw reply
* Re: [Survey] Signed push
From: Nguyen Thai Ngoc Duy @ 2011-09-14 22:42 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20110914210512.GA20294@elie>
On Thu, Sep 15, 2011 at 7:05 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>> On Wed, Sep 14, 2011 at 2:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>>> An alternative that I am considering is to let the requester say this
>>> instead:
>>>
>>> are available in the git repository at:
>>> git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
> [...]
>> Stupid question, if we agree to go with signed push, can we also sign
>> pull requests and verify them when we pull? I suppose most of the
>> time, pulling can be done automatically by extracting pull url from
>> the request. This would make pull/push both signed.
>>
>> BTW, there's a third way (rsync is obsolete) to carry changes away in
>> human-unreadable way: bundles. Should we also sign the bundles too (I
>> guess we could just do the same as in signed push).
>
> If I understand you correctly, then ordinary PGP email signing[1]
> should work for that already. In your first example, the receiver can
> make sure whatever process grabs a pull request verifies it, and in
> the second example, the receiver checks the signature on her email
> before saving a bundle and passing it to "git fetch".
Yes, I think we can do that already. It's just more convenient to
teach "git fetch/pull" to take pull requests and automatically verify
them. Some repositories may also want to enforce signing and we can do
that by setting config file and fetch/pull refuses if pull requests
are not signed. We can also store the sign as git notes, just like in
git-push (extra work if it has to be done manually).
> [1] http://www.phildev.net/pgp/gpgmua.html
--
Duy
^ permalink raw reply
* [PATCH] credential-osxkeychain: load Security framework dynamically
From: Jay Soffian @ 2011-09-14 22:55 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, Junio C Hamano, Jeff King, John Szakmeister
In-Reply-To: <1316023117-84755-1-git-send-email-jaysoffian@gmail.com>
Use dlopen() / dysym() instead of dynmically linking to the
Security framework. A followup commit will refactor things such
that git-credential-osxkeychain can be hardlinked to git.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
> Hmm, maybe it can be if I dlopen the security framework instead of linking
> against it.
Something like this. I'm going to pause here for feedback. Is the (not yet
existant) followup commit referenced above allowing git-credential-osxkeychain
to be a hard link to git a worthwhile endeavor? Or would a better approach be
to make git-credential-osxkeychain.c not use any git code?
contrib/credential-osxkeychain/Makefile | 12 +++-
.../credential-osxkeychain/generate_security.py | 73 ++++++++++++++++++++
2 files changed, 82 insertions(+), 3 deletions(-)
create mode 100755 contrib/credential-osxkeychain/generate_security.py
diff --git a/contrib/credential-osxkeychain/Makefile b/contrib/credential-osxkeychain/Makefile
index dc6bbbc3f9..001d695cb8 100644
--- a/contrib/credential-osxkeychain/Makefile
+++ b/contrib/credential-osxkeychain/Makefile
@@ -25,11 +25,17 @@ ifndef V
endif
endif
-git-credential-osxkeychain: git-credential-osxkeychain.o $(GIT_LIBS)
- $(QUIET_LINK)$(CC) -o $@ $< $(LIBS) -Wl,-framework -Wl,Security
+git-credential-osxkeychain: git-credential-osxkeychain.o security.o $(GIT_LIBS)
+ $(QUIET_LINK)$(CC) -o $@ $< security.o $(LIBS)
git-credential-osxkeychain.o: git-credential-osxkeychain.c
$(QUIET_CC)$(CC) -c $(CFLAGS) $<
+security.o: security.c
+ $(QUIET_CC)$(CC) -c $(CFLAGS) $<
+
+security.c: generate_security.py
+ python generate_security.py
+
clean:
- $(RM) git-credential-osxkeychain git-credential-osxkeychain.o
+ $(RM) git-credential-osxkeychain git-credential-osxkeychain.o security.?
diff --git a/contrib/credential-osxkeychain/generate_security.py b/contrib/credential-osxkeychain/generate_security.py
new file mode 100755
index 0000000000..db94672e95
--- /dev/null
+++ b/contrib/credential-osxkeychain/generate_security.py
@@ -0,0 +1,73 @@
+#!/usr/bin/python
+
+import re
+
+func_decls = """
+OSStatus SecKeychainAddInternetPassword(SecKeychainRef keychain, UInt32 serverNameLength, const char *serverName, UInt32 securityDomainLength, const char *securityDomain, UInt32 accountNameLength, const char *accountName, UInt32 pathLength, const char *path, UInt16 port, SecProtocolType protocol, SecAuthenticationType authenticationType, UInt32 passwordLength, const void *passwordData, SecKeychainItemRef *itemRef);
+OSStatus SecKeychainFindInternetPassword(CFTypeRef keychainOrArray, UInt32 serverNameLength, const char *serverName, UInt32 securityDomainLength, const char *securityDomain, UInt32 accountNameLength, const char *accountName, UInt32 pathLength, const char *path, UInt16 port, SecProtocolType protocol, SecAuthenticationType authenticationType, UInt32 *passwordLength, void **passwordData, SecKeychainItemRef *itemRef);
+OSStatus SecKeychainItemCopyContent(SecKeychainItemRef itemRef, SecItemClass *itemClass, SecKeychainAttributeList *attrList, UInt32 *length, void **outData);
+OSStatus SecKeychainItemDelete(SecKeychainItemRef itemRef);
+OSStatus SecKeychainItemFreeContent(SecKeychainAttributeList *attrList, void *data);
+OSStatus SecKeychainItemModifyContent(SecKeychainItemRef itemRef, const SecKeychainAttributeList *attrList, UInt32 length, const void *data);
+"""
+
+header = r"""
+#include <dlfcn.h>
+#include <Security/Security.h>
+#include "cache.h"
+
+const char *security_framework =
+ "/System/Library/Frameworks/Security.framework/Security";
+
+void *load_security()
+{
+ static void *security;
+ if (!security) {
+ if (!(security = dlopen(security_framework, RTLD_LAZY)))
+ die(_("dlopen(\"%s\") failed: %s"),
+ security_framework, dlerror());
+ }
+ return security;
+}
+"""
+
+func_tmpl = """
+%(func_decl)s
+{
+ %(func_rv)s (*func)(%(arg_types)s) =
+ dlsym(load_security(), "%(func_name)s");
+ if (!func)
+ die(_("dlsym(%(func_name)s) failed: %%s"), dlerror());
+ return func(%(arg_names)s);
+}
+"""
+
+def generate_func(decl):
+ func_rv, func_name, func_args = re.search(
+ r'^(.*?)\s+([^(]+)\((.*)\);$', decl).groups()
+ func_args = [s.strip() for s in func_args.split(',')]
+ arg_types = []
+ arg_names = []
+ for arg in func_args:
+ arg_type, arg_name = re.search(r'^(.*?)([a-zA-Z]+)$', arg).groups()
+ arg_types.append(arg_type.strip())
+ arg_names.append(arg_name.strip())
+ return func_tmpl % dict(
+ func_decl=decl.rstrip(';'),
+ func_name=func_name,
+ func_rv=func_rv,
+ arg_types=', '.join(arg_types),
+ arg_names=', '.join(arg_names),
+ )
+
+def main():
+ f = open('security.c', 'w')
+ f.write(header)
+ for decl in func_decls.splitlines():
+ decl = decl.strip()
+ if decl:
+ f.write(generate_func(decl))
+ f.close()
+
+if __name__ == '__main__':
+ main()
--
1.7.7.rc1.1.g011e1
^ permalink raw reply related
* Re: [PATCH] credential-osxkeychain: load Security framework dynamically
From: Jeff King @ 2011-09-14 23:08 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <1316040926-89429-1-git-send-email-jaysoffian@gmail.com>
On Wed, Sep 14, 2011 at 06:55:26PM -0400, Jay Soffian wrote:
> Something like this. I'm going to pause here for feedback. Is the (not yet
> existant) followup commit referenced above allowing git-credential-osxkeychain
> to be a hard link to git a worthwhile endeavor? Or would a better approach be
> to make git-credential-osxkeychain.c not use any git code?
To be honest, I was surprised to see you linking against git. I had
always envisioned OS-specific helpers as living outside of the git.git
repo. That's why I provided git-credential-getpass; it should be the
only part of git that helpers really want to reuse.
What are you getting from git that is useful? From my skim of your
patch, it looks like xmalloc/die, parse_options, and credential_getpass.
The first can be replaced with a few trivial lines of code. The second
is overkill, I think. The helper code will always hand you the
"--option=value" form, and I always intended it to stay that way
(whether that is well documented, I'm not sure). But a simple loop with
strncmps would be fine.
The hardest part is credential_getpass. You can call "git
credential-getpass", but you'll have to read the output yourself (though
it's quite simple to parse; see read_credential_response).
I'm not a fan of cutting and pasting code, and this will add a number of
lines to your C program. But I feel like keeping the build completely
separate from core git is probably a good boundary (especially because
this will not be getting built or tested all the time, as most of the
core code is).
-Peff
^ permalink raw reply
* Re: Helping on Git development
From: Jeff King @ 2011-09-14 23:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Andrew Ardill, Eduardo D'Avila
In-Reply-To: <7vehzjugdz.fsf@alter.siamese.dyndns.org>
On Wed, Sep 14, 2011 at 11:29:28AM -0700, Junio C Hamano wrote:
> As to contributing to the project, right now, I think we have enough
> people who want to write code and documentation for Git, but what we lack
> are bandwidth to (this is not meant to be an exhaustive list):
Is there such a thing as enough coders? :)
Two things that got me started on git, and that I think are still
relevant today:
1. Scratch your own itch. Surely git doesn't do something that you
wish it did. Or did it faster. Or whatever. Try to dig up past
discussions on the list to make sure you're not doing something
that has already been tried and rejected, and then start hacking.
Your patches may be terrible at first, but I think there are people
willing to guide you if you actually have running code.
2. Read the list. People will report bugs. Try reproducing them,
bisecting them, creating minimal test cases, narrowing the issues
down to certain configurations or a certain bit of code, etc.
Sometimes that will lead you to propose a solution. Sometimes
you'll just add to the discussion, and then somebody with more
familiarity can pick up the topic from there. But you'll have
helped them by doing some of the work, and you'll have learned more
about how git works.
-Peff
^ permalink raw reply
* Re: [PATCH 3/7] refactor argv_array into generic code
From: Jeff King @ 2011-09-14 23:18 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, Jens Lehmann, git
In-Reply-To: <CAP8UFD1vxP9ABgJpM99hxDWWLeGO_QW7QLVFq1f-teu1fiCftA@mail.gmail.com>
On Wed, Sep 14, 2011 at 07:54:48AM +0200, Christian Couder wrote:
> In sha1-array you called the "push" function "sha1_array_append"
> instead of "sha1_array_push", so I wonder why here you call them
> "*_push*" instead of "*_append*"?
I dunno. It just seemed natural to write "push" in the context of argv.
Maybe too much perl (push, pop, shift, unshift).
argv_array_append does make sense. One could argue that
sha1_array_append actually doesn't. True, it does append to the end of
the array, but after writing the docs for it yesterday, I realized that
it less of an array, and more of a set container. Because the point of
using it is the optimized lookup/unique function, which is going to sort
it. The array is really just an implementation detail.
So arguably it should be "struct sha1_set", and "sha1_set_insert" or
something. I'm not sure if it's really worth changing (because this is
C, our data structures tend to be a little leaky, anyway, and you _can_
use sha1_array as an ordered list if you want; just don't call the
lookup or sorting functions).
-Peff
^ permalink raw reply
* Re: [PATCH] credential-osxkeychain: load Security framework dynamically
From: Junio C Hamano @ 2011-09-14 23:18 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Jeff King, John Szakmeister
In-Reply-To: <1316040926-89429-1-git-send-email-jaysoffian@gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> Something like this. I'm going to pause here for feedback. Is the (not yet
> existant) followup commit referenced above allowing git-credential-osxkeychain
> to be a hard link to git a worthwhile endeavor? Or would a better approach be
> to make git-credential-osxkeychain.c not use any git code?
Most definitely the latter, I would think.
The whole point of making the Git credential code talk with a defined
interface with external programs is so that these keychain helpers can be
written independently from the rest of Git.
If the reason why your keychain helper benefits from linking with the rest
of Git is because some pieces of information you need in order to respond
to the requests from credential interface is hard to get if your helper is
built as an independent program, that is a sign that we are not exposing
enough information to scripts, iow, the failure in the design of the
credential interface. If that is the case (and I doubt it is), we would
need to fix the interface (either the credential interface, or perhaps
"git config") so that such an independent program does not have to peek
inside the internals of Git.
If the reason is because you want to reuse some generic C API we have that
are not necessarily tied to Git (e.g. strbuf, string-list, etc.), on the
other hand, please resist the temptation to do so. It would not help your
program to serve as an example of independent external keychain helpers,
i.e. a demonstration of how simple to write them.
^ permalink raw reply
* Re: Fwd: [Survey] Signed push
From: Philip Oakley @ 2011-09-14 22:51 UTC (permalink / raw)
To: Johan Herland, Linus Torvalds
Cc: Git List, Matthieu Moy, Michael Haggerty, Junio C Hamano
In-Reply-To: <201109141814.04752.johan@herland.net>
From: "Johan Herland" <johan@herland.net>
.
> _that_ SHA1. As I said, that _can_ be done with the notes
> infrastructure, but - as Ted noted - there might be better solutions to
> storing branch descriptions.
>
Is one option to store the branch description (if any) on line two of the
<branch name> file in .git\refs\heads.
That is, from byte 42 onward, after the 40 byte sha1 and its LF.
Older systems would simply overwrite it, while newer systems would be able
to read it. The fixed format of the first 41 chars alllows sensible checks
in the various places it is used.
Philip Oakley
^ permalink raw reply
* Re: Fwd: [Survey] Signed push
From: Linus Torvalds @ 2011-09-14 23:30 UTC (permalink / raw)
To: Philip Oakley
Cc: Johan Herland, Git List, Matthieu Moy, Michael Haggerty,
Junio C Hamano
In-Reply-To: <E9E05FA85D0F4461BAE9ECAFE25CD84E@PhilipOakley>
On Wed, Sep 14, 2011 at 3:51 PM, Philip Oakley <philipoakley@iee.org> wrote:
>
> Is one option to store the branch description (if any) on line two of the
> <branch name> file in .git\refs\heads.
Or even on line one.
We already basically do that for the magic FETCH_HEAD branch, and use
it to populate the merge commit. Extending that kind of thing to all
branches might be a nice idea.
Of course, then the question becomes "what about packed refs"? Do you
just leave the unpacked ref in place for those?
Linus
^ permalink raw reply
* Re: Helping on Git development
From: Junio C Hamano @ 2011-09-14 23:34 UTC (permalink / raw)
To: Jeff King; +Cc: git, Andrew Ardill, Eduardo D'Avila
In-Reply-To: <20110914231427.GA5611@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Sep 14, 2011 at 11:29:28AM -0700, Junio C Hamano wrote:
>
>> As to contributing to the project, right now, I think we have enough
>> people who want to write code and documentation for Git, but what we lack
>> are bandwidth to (this is not meant to be an exhaustive list):
>
> Is there such a thing as enough coders? :)
Ever heard of the Mythical Man-Month ;-)?
I thought it was clear from my message but probably I wasn't clear enough,
so let's make it clear. I didn't mean to ay "we have enough -- we need no
more -- we reject new applicants".
I was simply saying that there already are many people who scratch his own
real itch, and we are short of the bandwidth to review them all. It would
not help the project at all to add more people who scratch some random
non-itches that nobody is actually interested in (e.g. an entry in an
unmaintained "bug tracker" that may list irrelevant and stale non issues).
> 2. Read the list. People will report bugs. Try reproducing them,
> bisecting them, creating minimal test cases, narrowing the issues
> down to certain configurations or a certain bit of code, etc.
> Sometimes that will lead you to propose a solution. Sometimes
> you'll just add to the discussion, and then somebody with more
> familiarity can pick up the topic from there. But you'll have
> helped them by doing some of the work, and you'll have learned more
> about how git works.
Yes. In the earlier steps in the above, you may find out that the report
was actually not a bug at all (e.g. old issue that has long been fixed,
pebcak, or wrong expectation), but even in such a case, reporting your
finding would help others.
^ permalink raw reply
* Re: Fwd: [Survey] Signed push
From: Junio C Hamano @ 2011-09-14 23:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Philip Oakley, Johan Herland, Git List, Matthieu Moy,
Michael Haggerty, Junio C Hamano
In-Reply-To: <CA+55aFxFnAjpSAd+uB25BuZXBJGvN59qNMmF3fzvky8XK_DP0A@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Sep 14, 2011 at 3:51 PM, Philip Oakley <philipoakley@iee.org> wrote:
>>
>> Is one option to store the branch description (if any) on line two of the
>> <branch name> file in .git\refs\heads.
>
> Or even on line one.
>
> We already basically do that for the magic FETCH_HEAD branch, and use
> it to populate the merge commit. Extending that kind of thing to all
> branches might be a nice idea.
>
> Of course, then the question becomes "what about packed refs"? Do you
> just leave the unpacked ref in place for those?
Seriously, storage format is not an issue at all and you know it. The
semantics is.
What commands use the description and for what purpose, how it is updated
when the branch is repurposed, how it is propagated to other repositories,
if there is a situation where two descriptions need to be merged, and if
so how that merge happens, etc., etc.
We could store it in unused part of loose refs. We could add [branch
"master"] description = ... in the .git/config. The latter would even be
easier for humans to edit by hand.
If we want to use the description when merging locally, for example,
fmt-merge-msg needs to be taught to read it, which would mean we would
need an internal API "read_branch_description()", regardless of what
storage format we choose to use. If we want to use it for "git pull", then
the transport layer needs to become aware of it.
^ permalink raw reply
* Re: [PATCH] credential-osxkeychain: load Security framework dynamically
From: Jay Soffian @ 2011-09-14 23:56 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <20110914230816.GA5754@sigill.intra.peff.net>
On Wed, Sep 14, 2011 at 7:08 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Sep 14, 2011 at 06:55:26PM -0400, Jay Soffian wrote:
>
> To be honest, I was surprised to see you linking against git. I had
> always envisioned OS-specific helpers as living outside of the git.git
> repo. That's why I provided git-credential-getpass; it should be the
> only part of git that helpers really want to reuse.
Okay, I think I was led astray by the fact that
credential-{cache,store}.c (at least the latter of which is meant as
nothing more than an example helper right?) links with git.
> What are you getting from git that is useful? From my skim of your
> patch, it looks like xmalloc/die, parse_options, and credential_getpass.
Correct.
> The first can be replaced with a few trivial lines of code. The second
> is overkill, I think. The helper code will always hand you the
> "--option=value" form, and I always intended it to stay that way
> (whether that is well documented, I'm not sure). But a simple loop with
> strncmps would be fine.
>
> The hardest part is credential_getpass. You can call "git
> credential-getpass", but you'll have to read the output yourself (though
> it's quite simple to parse; see read_credential_response).
>
> I'm not a fan of cutting and pasting code, and this will add a number of
> lines to your C program. But I feel like keeping the build completely
> separate from core git is probably a good boundary (especially because
> this will not be getting built or tested all the time, as most of the
> core code is).
Okay.
j.
^ permalink raw reply
* Re: Helping on Git development
From: Jeff King @ 2011-09-15 0:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Andrew Ardill, Eduardo D'Avila
In-Reply-To: <7vd3f2snox.fsf@alter.siamese.dyndns.org>
On Wed, Sep 14, 2011 at 04:34:38PM -0700, Junio C Hamano wrote:
> > Is there such a thing as enough coders? :)
>
> Ever heard of the Mythical Man-Month ;-)?
I thought git was a silver bullet. :)
> I was simply saying that there already are many people who scratch his
> own real itch, and we are short of the bandwidth to review them all.
> It would not help the project at all to add more people who scratch
> some random non-itches that nobody is actually interested in (e.g. an
> entry in an unmaintained "bug tracker" that may list irrelevant and
> stale non issues).
Yeah, that may be. But I don't look at it as "we have enough
itch-scratchers, so we don't need more". I see it as survival of the
fittest. You may post a patch series that needs a lot of help, but
nobody else cares, and it dies off. Or your series may be interesting
enough that it draws attention, to the detriment of somebody else's
series (which may take longer to get reviewed and merged). But natural
selection only works if we have a diverse population to select from.
The downside, of course, is that somebody may end up wasting time going
down a fruitless road. But for a new contributor, hopefully they learn
something in the process.
> > 2. Read the list. People will report bugs. Try reproducing them,
> [...]
>
> Yes. In the earlier steps in the above, you may find out that the
> report was actually not a bug at all (e.g. old issue that has long
> been fixed, pebcak, or wrong expectation), but even in such a case,
> reporting your finding would help others.
Very much agreed. I think big organizations like mozilla have people who
do nothing but bug triage. We are not that big, but it has proven to be
one area that is easy to break out and distribute to other people.
-Peff
^ permalink raw reply
* Re: [PATCH] credential-osxkeychain: load Security framework dynamically
From: Jeff King @ 2011-09-15 0:16 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <CAG+J_Dxenu7GDsTzUCCQiYg38Vto8CtD8ODX1J9mA8o1n2_YJA@mail.gmail.com>
On Wed, Sep 14, 2011 at 07:56:42PM -0400, Jay Soffian wrote:
> Okay, I think I was led astray by the fact that
> credential-{cache,store}.c (at least the latter of which is meant as
> nothing more than an example helper right?) links with git.
No, credential-store is meant to be used. It's just that it has a
security tradeoff that makes it the wrong choice for most cases. So it's
meant to be used sparingly. :)
As for those helpers being linked against git, I guess it doesn't make
them the best example code. But I wanted them to be always available as
a lowest common denominator (because even if you have a fancy local
keychain, it is likely that you'll end up at some point using git across
an ssh connection, and I wanted to provide _something_ there).
Not having any external dependencies, those helpers don't pollute our
code base too much. Building and testing them with the rest of git keeps
the code fresh and unbroken. Maybe it would be better if they provided a
clearer separation as an example. I'm open to that if people think it's
worth splitting them out. I suspect I could write credential-store as
something like 10 lines of perl.
-Peff
^ permalink raw reply
* git gc exit with out of memory, malloc failed error
From: Alexander Kostikov @ 2011-09-15 1:33 UTC (permalink / raw)
To: git
I'm new to git and I'm getting the following out of memory error on git gc:
$ git gc
Counting objects: 80818, done.
Delta compression using up to 8 threads.
fatal: Out of memory, malloc failed (tried to allocate 24359675 bytes)
error: failed to run repack
The only advice I found in the internet suggested to run repack with
--window-memory parameter specified. But this call also fails:
$ git repack -adf --window-memory=0
Counting objects: 80818, done.
Delta compression using up to 8 threads.
warning: suboptimal pack - out of memory
fatal: Out of memory, malloc failed (tried to allocate 24356363 bytes)
How do I cleanup my repository?
$ git version
git version 1.7.6.msysgit.0
OS: Windows Server 2008 R2 SP1 (x64)
Physical memory: 24 GB
The commands listed were executed under x64 console process.
--
Thanks,
Alexander Kostikov
^ permalink raw reply
* Fwd: vcs-svn and friends
From: David Michael Barr @ 2011-09-15 1:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Dmitry Ivankov, git
Hi,
Thanks to the work of Dmitry, we now have a simple front-end
that exercises the yet unmerged changes to vcs-svn that Jonathan
and I authored a few months ago. I think there's still some work
to be done before we can bless an integrated branch for inclusion.
I'd like to bring attention to just how far we have diverged; see the
email below.
--
David Barr
---------- Forwarded message ----------
From: David Michael Barr <davidbarr@google.com>
Date: Thu, Sep 15, 2011 at 11:37 AM
Subject: Re: vcs-svn and friends
To: Dmitry Ivankov <divanorama@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>
We now have 56 interesting commits pending:
vcs-svn: add fast_export_note to create notes
vcs-svn,svn-fe: add --incremental option
vcs-svn,svn-fe: allow to disable 'progress' lines
vcs-svn,svn-fe: convert REPORT_FILENO to an option
vcs-svn,svn-fe: allow to specify dump destination ref
vcs-svn: move commit parameters logic to svndump.c
vcs-svn: make svndump_init parameters a struct
svn-fe,test-svn-fe: use parse-options
fast-import: allow top directory as an argument for some commands
fast-import: be saner with temporary trees
svn-fe: reuse import-marks in remote-svn-alpha
svn-fe: import incrementally in svn-remote-alpha
svn-fe: write svnrev notes in remote-svn-alpha
svn-fe: use proper refspec in remote-svn-alpha
svn-fe: use svn-fe --no-progress in remote-svn-alpha
svn-fe: add a test for remote-svn-alpha
svn-fe: allow svnadmin instead of svnrdump in remote-svn-alpha
svn-fe: avoid error on no-op imports in remote-svn-alpha
svn-fe: add man target to Makefile
svn-fe: use svnrdump --quiet in remote-svn-alpha
vcs-svn: reset first_commit_done in fast_export_init
svn-fe: use tabs to indent in remote helper script
svn-fe: do not rely on /bin/env utility to launch remote helper
Add alpha version of remote-svn helper
Arrange a backflow pipe from fast-importer to remote helper stdin
vcs-svn: do not initialize report_buffer twice
vcs-svn: avoid hangs from corrupt deltas
vcs-svn: guard against overflow when computing preimage length
vcs-svn: cap number of bytes read from sliding view
test-svn-fe: split off "test-svn-fe -d" into a separate function
vcs-svn: implement text-delta handling
vcs-svn: let deltas use data from preimage
vcs-svn: let deltas use data from postimage
vcs-svn: verify that deltas consume all inline data
vcs-svn: implement copyfrom_data delta instruction
vcs-svn: read instructions from deltas
vcs-svn: read inline data from deltas
vcs-svn: read the preimage when applying deltas
vcs-svn: parse svndiff0 window header
vcs-svn: skeleton of an svn delta parser
vcs-svn: make buffer_read_binary API more convenient
vcs-svn: learn to maintain a sliding view of a file
Makefile: list one vcs-svn/xdiff object or header per line
vcs-svn: avoid using ls command twice
vcs-svn: drop obj_pool
vcs-svn: drop treap
vcs-svn: drop string_pool
vcs-svn: pass paths through to fast-import
vcs-svn: use mark from previous import for parent commit
vcs-svn: handle filenames with dq correctly
vcs-svn: quote paths correctly for ls command
vcs-svn: eliminate repo_tree structure
vcs-svn: add a comment before each commit
vcs-svn: save marks for imported commits
vcs-svn: use higher mark numbers for blobs
vcs-svn: set up channel to read fast-import cat-blob response
There are a lot of svn-fe tests failing on my integration branch.
One upside is that gph/master..db/svn-fe-pu only contains
relevant commits. I think a little more polish is needed before
we can suggest a pull to jch. In particular, I think we should
include remote-svn-alpha and the test should work out of the
box.
--
David Barr
^ permalink raw reply
* [PATCH 0/4] Honor core.ignorecase for attribute patterns
From: Brandon Casey @ 2011-09-15 1:59 UTC (permalink / raw)
To: peff; +Cc: git, gitster, sunshine, bharrosh, trast, zapped
In-Reply-To: <5XXEFw0WjtXKd9dpXSxpkskCcgVyG9Db1_zzVSEBNey-kpXSBbmQfYaxZ2Szg6Pbck6hZZTQ5hHzBwG4rhKYXshrdmveEFLPZ9W0V8P_lw@cipher.nrlssc.navy.mil>
On 09/13/2011 11:22 AM, Brandon Casey wrote:
> On 09/13/2011 11:05 AM, Jeff King wrote:
>> On Tue, Sep 13, 2011 at 10:15:15AM -0500, Brandon Casey wrote:
>>
>>> ...and I see there is already an fnmatch_icase() in dir.c which adds
>>> FNM_CASEFOLD when the global var ignore_case is set. So, maybe it's as
>>> easy as:
>>> [...]
>>> - return (fnmatch(pattern, basename, 0) == 0);
>>> + return (fnmatch_icase(pattern, basename, 0) == 0);
>>
>> OK, wow. That's exactly the level of easy I was hoping for. Do you want
>> to roll that up into a patch with some tests?
>
> I haven't even tested that it runs. :) No, I was hoping someone
> who was more interested would finish it, and maybe even test on
> an affected system.
Ok, I lied. Here's a series that needs testing by people on a
case-insensitive filesystem and some comments.
The first three patches are just housekeeping and can be accepted
independently of the fourth patch which is marked WIP.
The last patch implements the case-insensitive matching of attribute
patterns, but I discovered that bad things can happen if git_config()
is called more than once. Details are in the patch email.
-Brandon
[PATCH 1/4] attr.c: avoid inappropriate access to strbuf "buf"
[PATCH 2/4] cleanup: use internal memory allocation wrapper
[PATCH 3/4] builtin/mv.c: plug miniscule memory leak
[PATCH 4/4] attr.c: respect core.ignorecase when matching attribute
^ permalink raw reply
* [PATCH 1/4] attr.c: avoid inappropriate access to strbuf "buf" member
From: Brandon Casey @ 2011-09-15 1:59 UTC (permalink / raw)
To: peff; +Cc: git, gitster, sunshine, bharrosh, trast, zapped, Brandon Casey
In-Reply-To: <1316051979-19671-1-git-send-email-drafnel@gmail.com>
This code sequence performs a strcpy into the buf member of a strbuf
struct. The strcpy may move the position of the terminating nul of the
string and effectively change the length of string so that it does not
match the len member of the strbuf struct.
Currently, this sequence works since the strbuf was given a hint when it
was initialized to allocate enough space to accomodate the string that will
be strcpy'ed, but this is an implementation detail of strbufs, not a
guarantee.
So, lets rework this sequence so that the strbuf is only manipulated by
strbuf functions, and direct modification of its "buf" member is not
necessary.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
attr.c | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/attr.c b/attr.c
index 33cb4e4..206f233 100644
--- a/attr.c
+++ b/attr.c
@@ -552,7 +552,6 @@ static void prepare_attr_stack(const char *path)
{
struct attr_stack *elem, *info;
int dirlen, len;
- struct strbuf pathbuf;
const char *cp;
cp = strrchr(path, '/');
@@ -561,8 +560,6 @@ static void prepare_attr_stack(const char *path)
else
dirlen = cp - path;
- strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));
-
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
@@ -607,27 +604,29 @@ static void prepare_attr_stack(const char *path)
* Read from parent directories and push them down
*/
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
- while (1) {
- char *cp;
+ struct strbuf pathbuf = STRBUF_INIT;
+ while (1) {
len = strlen(attr_stack->origin);
if (dirlen <= len)
break;
- strbuf_reset(&pathbuf);
- strbuf_add(&pathbuf, path, dirlen);
+ cp = memchr(path + len + 1, '/', dirlen - len - 1);
+ if (!cp)
+ cp = path + dirlen;
+ strbuf_add(&pathbuf, path, cp - path);
strbuf_addch(&pathbuf, '/');
- cp = strchr(pathbuf.buf + len + 1, '/');
- strcpy(cp + 1, GITATTRIBUTES_FILE);
+ strbuf_add(&pathbuf, GITATTRIBUTES_FILE,
+ strlen(GITATTRIBUTES_FILE));
elem = read_attr(pathbuf.buf, 0);
- *cp = '\0';
- elem->origin = strdup(pathbuf.buf);
+ strbuf_setlen(&pathbuf, cp - path);
+ elem->origin = strbuf_detach(&pathbuf, NULL);
elem->prev = attr_stack;
attr_stack = elem;
debug_push(elem);
}
- }
- strbuf_release(&pathbuf);
+ strbuf_release(&pathbuf);
+ }
/*
* Finally push the "info" one at the top of the stack.
--
1.7.6.2
^ permalink raw reply related
* [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
From: Brandon Casey @ 2011-09-15 1:59 UTC (permalink / raw)
To: peff; +Cc: git, gitster, sunshine, bharrosh, trast, zapped, Brandon Casey
In-Reply-To: <1316051979-19671-1-git-send-email-drafnel@gmail.com>
The "x"-prefixed versions of strdup, malloc, etc. will check whether the
allocation was successful and terminate the process otherwise.
A few uses of malloc were left alone since they already implemented a
graceful path of failure or were in a quasi external library like xdiff.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
attr.c | 2 +-
builtin/mv.c | 2 +-
compat/mingw.c | 2 +-
compat/qsort.c | 2 +-
compat/win32/syslog.c | 2 +-
remote.c | 2 +-
show-index.c | 2 +-
transport-helper.c | 4 ++--
8 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/attr.c b/attr.c
index 206f233..3359b39 100644
--- a/attr.c
+++ b/attr.c
@@ -533,7 +533,7 @@ static void bootstrap_attr_stack(void)
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
elem = read_attr(GITATTRIBUTES_FILE, 1);
- elem->origin = strdup("");
+ elem->origin = xstrdup("");
elem->prev = attr_stack;
attr_stack = elem;
debug_push(elem);
diff --git a/builtin/mv.c b/builtin/mv.c
index 40f33ca..e9d191f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -29,7 +29,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
to_copy--;
if (to_copy != length || base_name) {
char *it = xmemdupz(result[i], to_copy);
- result[i] = base_name ? strdup(basename(it)) : it;
+ result[i] = base_name ? xstrdup(basename(it)) : it;
}
}
return get_pathspec(prefix, result);
diff --git a/compat/mingw.c b/compat/mingw.c
index 6ef0cc4..8947418 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1183,7 +1183,7 @@ static int WSAAPI getaddrinfo_stub(const char *node, const char *service,
}
ai->ai_addrlen = sizeof(struct sockaddr_in);
if (hints && (hints->ai_flags & AI_CANONNAME))
- ai->ai_canonname = h ? strdup(h->h_name) : NULL;
+ ai->ai_canonname = h ? xstrdup(h->h_name) : NULL;
else
ai->ai_canonname = NULL;
diff --git a/compat/qsort.c b/compat/qsort.c
index d93dce2..9574d53 100644
--- a/compat/qsort.c
+++ b/compat/qsort.c
@@ -55,7 +55,7 @@ void git_qsort(void *b, size_t n, size_t s,
msort_with_tmp(b, n, s, cmp, buf);
} else {
/* It's somewhat large, so malloc it. */
- char *tmp = malloc(size);
+ char *tmp = xmalloc(size);
msort_with_tmp(b, n, s, cmp, tmp);
free(tmp);
}
diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
index 42b95a9..197ab34 100644
--- a/compat/win32/syslog.c
+++ b/compat/win32/syslog.c
@@ -38,7 +38,7 @@ void syslog(int priority, const char *fmt, ...)
return;
}
- str = malloc(str_len + 1);
+ str = xmalloc(str_len + 1);
va_start(ap, fmt);
vsnprintf(str, str_len + 1, fmt, ap);
va_end(ap);
diff --git a/remote.c b/remote.c
index b8ecfa5..7840d2f 100644
--- a/remote.c
+++ b/remote.c
@@ -840,7 +840,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
refspec->dst, &ret))
return ret;
} else if (!strcmp(refspec->src, name))
- return strdup(refspec->dst);
+ return xstrdup(refspec->dst);
}
return NULL;
}
diff --git a/show-index.c b/show-index.c
index 4c0ac13..63f9da5 100644
--- a/show-index.c
+++ b/show-index.c
@@ -48,7 +48,7 @@ int main(int argc, char **argv)
unsigned char sha1[20];
uint32_t crc;
uint32_t off;
- } *entries = malloc(nr * sizeof(entries[0]));
+ } *entries = xmalloc(nr * sizeof(entries[0]));
for (i = 0; i < nr; i++)
if (fread(entries[i].sha1, 20, 1, stdin) != 1)
die("unable to read sha1 %u/%u", i, nr);
diff --git a/transport-helper.c b/transport-helper.c
index 4eab844..0713126 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -183,7 +183,7 @@ static struct child_process *get_helper(struct transport *transport)
ALLOC_GROW(refspecs,
refspec_nr + 1,
refspec_alloc);
- refspecs[refspec_nr++] = strdup(capname + strlen("refspec "));
+ refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec "));
} else if (!strcmp(capname, "connect")) {
data->connect = 1;
} else if (!prefixcmp(capname, "export-marks ")) {
@@ -445,7 +445,7 @@ static int fetch_with_import(struct transport *transport,
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
- private = strdup(posn->name);
+ private = xstrdup(posn->name);
read_ref(private, posn->old_sha1);
free(private);
}
--
1.7.6.2
^ permalink raw reply related
* [PATCH 3/4] builtin/mv.c: plug miniscule memory leak
From: Brandon Casey @ 2011-09-15 1:59 UTC (permalink / raw)
To: peff; +Cc: git, gitster, sunshine, bharrosh, trast, zapped, Brandon Casey
In-Reply-To: <1316051979-19671-1-git-send-email-drafnel@gmail.com>
The "it" string would not be free'ed if base_name was non-NULL.
Let's free it.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
builtin/mv.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index e9d191f..5efe6c5 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -29,7 +29,11 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
to_copy--;
if (to_copy != length || base_name) {
char *it = xmemdupz(result[i], to_copy);
- result[i] = base_name ? xstrdup(basename(it)) : it;
+ if (base_name) {
+ result[i] = xstrdup(basename(it));
+ free(it);
+ } else
+ result[i] = it;
}
}
return get_pathspec(prefix, result);
--
1.7.6.2
^ permalink raw reply related
* [PATCH 4/4] attr.c: respect core.ignorecase when matching attribute patterns
From: Brandon Casey @ 2011-09-15 1:59 UTC (permalink / raw)
To: peff; +Cc: git, gitster, sunshine, bharrosh, trast, zapped, Brandon Casey
In-Reply-To: <1316051979-19671-1-git-send-email-drafnel@gmail.com>
When core.ignorecase is true, the file globs configured in the
.gitattributes file should be matched case-insensitively against the paths
in the working directory.
---
Two points:
1)
I think these two changes of fnmatch to fnmatch_icase should be all
that is necessary. There are a number of uses of strncmp where the
"origin" path of an attribute entry is compared to the prefix of a file
path, but since the attribute stack is built from the same path string
that it is being compared against, we shouldn't have to do
strncmp_icase everywhere. The case of the two strings should
necessarily match.
This needs some testing by someone on a case-insensitive filesystem.
Also, notice some of the new tests are marked with a CASE_INSENSITIVE_FS
pre-requisite. I tested on a USB thumb drive, but it would be nice if
someone tested on a platform that is natively case-insensitive. Maybe
CASE_INSENSITIVE_FS should be moved to test-lib.sh, and t0005(others?)
should be updated to use it?
2)
The bad news, this breaks t8005. The breakage is caused by
git_attr_config calling git_default_config, and stomping on the
git_log_output_encoding set by setup_revisions() when it parsed the
--encoding command line option.
What happens is cmd_blame() calls git_config() which parses the config
files and sets up the global config variables like
git_log_output_encoding, then later blame calls setup_revisions() which
parses the command line option --encoding and overrides the value in
git_log_output_encoding, then, even later, userdiff looks up an
attribute on a path and calls git_check_attr() which calls git_config
_again_, which resets git_log_output_encoding to the value in the config
file (stomping on the value set by --encoding on the git blame command
line).
Since fnmatch_icase depends on the ignore_case global variable being set
correctly, the obvious thing for me to do was to allow
git_default_config to call git_default_core_config and parse the
core.ignorecase config option. So I modified git_attr_config so it fell
back to git_default_config. But that can have the undesired side effect
described above.
It's easy to work around this issue. I could just parse core.ignorecase
in git_attr_config() and set ignore_case myself like:
if (!strcmp(var, "core.ignorecase")) {
ignore_case = git_config_bool(var, value);
return 0;
}
The big question is whether it should be safe to call git_config()
multiple times? Right now, it is not. We also don't protect against
git_config() being called multiple times either.
I suspect that setup_revisions() is not the only place where a command
line option overrides a global config variable and it would be a big
can of worms to try to fix them all.
Thoughts?
---
attr.c | 7 +++--
t/t0003-attributes.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/attr.c b/attr.c
index 3359b39..b8ed7cf 100644
--- a/attr.c
+++ b/attr.c
@@ -11,6 +11,7 @@
#include "cache.h"
#include "exec_cmd.h"
#include "attr.h"
+#include "dir.h"
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
@@ -499,7 +500,7 @@ static int git_attr_config(const char *var, const char *value, void *dummy)
if (!strcmp(var, "core.attributesfile"))
return git_config_pathname(&attributes_file, var, value);
- return 0;
+ return git_default_config(var, value, dummy);
}
static void bootstrap_attr_stack(void)
@@ -643,7 +644,7 @@ static int path_matches(const char *pathname, int pathlen,
/* match basename */
const char *basename = strrchr(pathname, '/');
basename = basename ? basename + 1 : pathname;
- return (fnmatch(pattern, basename, 0) == 0);
+ return (fnmatch_icase(pattern, basename, 0) == 0);
}
/*
* match with FNM_PATHNAME; the pattern has base implicitly
@@ -657,7 +658,7 @@ static int path_matches(const char *pathname, int pathlen,
return 0;
if (baselen != 0)
baselen++;
- return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+ return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
}
static int macroexpand_one(int attr_nr, int rem);
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index ae2f1da..47a70c4 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -9,7 +9,7 @@ attr_check () {
path="$1"
expect="$2"
- git check-attr test -- "$path" >actual 2>err &&
+ git $3 check-attr test -- "$path" >actual 2>err &&
echo "$path: test: $2" >expect &&
test_cmp expect actual &&
test_line_count = 0 err
@@ -27,6 +27,7 @@ test_expect_success 'setup' '
echo "onoff test -test"
echo "offon -test test"
echo "no notest"
+ echo "A/e/F test=A/e/F"
) >.gitattributes &&
(
echo "g test=a/g" &&
@@ -93,6 +94,63 @@ test_expect_success 'attribute test' '
'
+test_expect_success 'attribute matching is case sensitive when core.ignorecase=0' '
+
+ test_must_fail attr_check F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/c/F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/G a/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/B/g a/b/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/G a/b/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/H a/b/h "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=0" &&
+ test_must_fail attr_check oNoFf unset "-c core.ignorecase=0" &&
+ test_must_fail attr_check oFfOn set "-c core.ignorecase=0" &&
+ attr_check NO unspecified "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=0" &&
+ attr_check a/b/d/YES a/b/d/* "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/E/f "A/e/F" "-c core.ignorecase=0"
+
+'
+
+test_expect_success 'attribute matching is case insensitive when core.ignorecase=1' '
+
+ attr_check F f "-c core.ignorecase=1" &&
+ attr_check a/F f "-c core.ignorecase=1" &&
+ attr_check a/c/F f "-c core.ignorecase=1" &&
+ attr_check a/G a/g "-c core.ignorecase=1" &&
+ attr_check a/B/g a/b/g "-c core.ignorecase=1" &&
+ attr_check a/b/G a/b/g "-c core.ignorecase=1" &&
+ attr_check a/b/H a/b/h "-c core.ignorecase=1" &&
+ attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check oNoFf unset "-c core.ignorecase=1" &&
+ attr_check oFfOn set "-c core.ignorecase=1" &&
+ attr_check NO unspecified "-c core.ignorecase=1" &&
+ attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check a/b/d/YES unspecified "-c core.ignorecase=1" &&
+ attr_check a/E/f "A/e/F" "-c core.ignorecase=1"
+
+'
+
+test_expect_success 'check whether FS is case-insensitive' '
+ mkdir junk &&
+ echo good >junk/CamelCase &&
+ echo bad >junk/camelcase &&
+ if test "$(cat junk/CamelCase)" != good
+ then
+ test_set_prereq CASE_INSENSITIVE_FS
+ fi
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'additional case insensitivity tests' '
+ test_must_fail attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=0" &&
+ test_must_fail attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=0" &&
+ attr_check A/b/h a/b/h "-c core.ignorecase=0" &&
+ attr_check A/b/h a/b/h "-c core.ignorecase=1" &&
+ attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=1"
+'
+
test_expect_success 'unnormalized paths' '
attr_check ./f f &&
--
1.7.6
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox