Re: Announcing git-reparent
Hello, On Mon, Jan 14, 2013 at 8:16 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi Mark, Mark Lodato wrote: Create a new commit object that has the same tree and commit message as HEAD but with a different set of parents. If ``--no-reset`` is given, the full object id of this commit is printed and the program exits I've been wishing for something like this for a long time. I used to fake it using cat-file commit, sed, and hash-object -w when stitching together poorly imported history using git replace. Just wondering, is the result different than something like git checkout commit_to_reparent cp -r * ../snapshot/ git reset --hard new_parent rm -r * cp -r ../snapshot/* . git add -A (assumes 1 parent, does not cope with .dot files, and has probably other small problems) -- Piotr Krukowiecki -- 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: Announcing git-reparent
Jonathan Nieder jrnie...@gmail.com writes: Hi Mark, Mark Lodato wrote: Create a new commit object that has the same tree and commit message as HEAD but with a different set of parents. If ``--no-reset`` is given, the full object id of this commit is printed and the program exits I've been wishing for something like this for a long time. I used to fake it using cat-file commit, sed, and hash-object -w when stitching together poorly imported history using git replace. Thanks for writing it. Ciao, Jonathan The scriptq is simple enough, and from a cursory look, it may be fine to throw it in contrib/ after some minor style fixes and such, if many find it useful. -- 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
What ever these people remember to pay for the actual title oftiffany jewellery ？
What ever these people remember to pay for the actual title associated with tiffany necklace http://www.cheaptiffanyclub.co.uk ? Consider source from the Tiffany bands? Perhaps you have any kind of indisputable fact that may immediately, Tiffany Company are simply provided available letter head, till 1853 tiffany company offered the actual program within jewelry, hyperlinks associated with birmingham jewellery progressively get to be the main metallic superb, that additionally appreciate excellent recognition. Exactly why is Tiffany Ear-rings therefore well-known? the reason why all the individuals presently very pleased every single child take advantage of is really because of the fact tiffany rings, tiffany 1837 band? Precisely what imply dosage of 1, 837 Tiffany jewellery? At this time keep in mind the actual senior citizens times, the way in which this changes Tiffany? Within 1837 Tiffany may be released through Charles Lewis Tiffany. two. Within the divulguer related to fifth Method Calle 57 (New york, Completely new You are able to) is often a Tiffany is primary shop. This particular shop is actually well-known internationally, due to Breakfast from Tiffany is inch together with Audrey Hepburn, Gentlemen Choose Blondes inch along with Marilyn Monroe Sweet House The state of alabama inch. 3. Within 1845 these people released the very first Tiffany list, known as Blue Guide inch (Tiffany Bands continues to be released these days). Within 1862 the corporation outfitted the actual Marriage military as well as banners swords. Not much (Sept 2007), the business opened it's second shop, In the end it's 2nd shop within Kuala Lumpur, additionally within birmingham is Heathrow airport airport terminal Airport terminal 4 in the total associated with 03 08 opened up the outside tents associated with Tiffany. Presently, approximately sixty four shops in the united states. right now, anybody can purchase low cost tiffany on the internet. www.cheaptiffanyclub.co.uk -- View this message in context: http://git.661346.n2.nabble.com/What-ever-these-people-remember-to-pay-for-the-actual-title-oftiffany-jewellery-tp7575004.html Sent from the git mailing list archive at Nabble.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
pandora bracelet charms may well do any daydream glance valid
Just about every single baby trademark to make sure you scratching commendable add on located at him / her ceremony; pandora bracelet charms http://www.cheapcharmpandora.net may well do any daydream glance valid. Gotten married adventure fantastic, however, the key blithe connections add on is normally cutting edge pet owners as necessary all the awesome relationship about connections precious jewelry, and additionally avant-garde physiology pandora about consent to and buying is normally employed, accessories. To make sure you an individual's essence, consistent with figure out, connections team add on and additionally ambit frequently. Accordingly, all the exclusively clothes taken Pandora connections precious jewelry, endure, is normally as necessary curved visual appearance -- still all the wind it manually is mostly a smaller Pandora jewelry, cutting edge pet owners to make sure you equally prefer about dye-in-the-wood exclusively temperament. http://www.cheapcharmpandora.net -- View this message in context: http://git.661346.n2.nabble.com/pandora-bracelet-charms-may-well-do-any-daydream-glance-valid-tp7575005.html Sent from the git mailing list archive at Nabble.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] archive-tar: fix sanity check in config parsing
Jeff King wrote: On Sun, Jan 13, 2013 at 06:42:01PM +0100, René Scharfe wrote: When parsing these config variable names, we currently check that the second dot is found nine characters into the name, disallowing filter names with a length of five characters. Additionally, git archive crashes when the second dot is omitted: $ ./git -c tar.foo=bar archive HEAD /dev/null fatal: Data too large to fit into virtual memory space. Instead we should check if the second dot exists at all, or if we only found the first one. Eek. Thanks for finding it. Your fix is obviously correct. --- a/archive-tar.c +++ b/archive-tar.c @@ -335,7 +335,7 @@ static int tar_filter_config(const char *var, const char *value, void *data) if (prefixcmp(var, tar.)) return 0; dot = strrchr(var, '.'); - if (dot == var + 9) + if (dot == var + 3) return 0; For the curious, the original version of the patch[1] read: + if (prefixcmp(var, tarfilter.)) + return 0; + dot = strrchr(var, '.'); + if (dot == var + 9) + return 0; and when I shortened the config section to tar in a re-roll of the series, I missed the corresponding change to the offset. Wouldn't it then be better ti use strlen(tar) rather than a 3? Or at least a comment? Bye, Jojo -- 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 01/14] imap-send.c: remove msg_data::flags, which was always zero
On 01/14/2013 06:57 AM, Jonathan Nieder wrote:
Michael Haggerty wrote:
This removes the need for function imap_make_flags(), so delete it,
too.
[...]
--- a/imap-send.c
+++ b/imap-send.c
[...]
box = gctx-name;
prefix = !strcmp(box, INBOX) ? : ctx-prefix;
cb.create = 0;
-ret = imap_exec_m(ctx, cb, APPEND \%s%s\ %s, prefix, box, flagstr);
+ret = imap_exec_m(ctx, cb, APPEND \%s%s\, prefix, box);
Before this change, the command is
APPEND SP mailbox SP { msglen } CRLF
. After this change, it leaves out the space before the brace. If I
understand RFC3501 correctly, the space is required. Intentional?
No, not intentional. I simply didn't follow far enough how the string
was used and mistakenly thought it was an entire command. Thanks for
finding and fixing this.
(It probably would be less error-prone if v_issue_imap_cmd() would be
responsible for adding the extra space in the second branch of the
following if statement:
if (!cmd-cb.data)
bufl = nfsnprintf(buf, sizeof(buf), %d %s\r\n, cmd-tag,
cmd-cmd);
else
bufl = nfsnprintf(buf, sizeof(buf), %d %s{%d%s}\r\n,
cmd-tag, cmd-cmd, cmd-cb.dlen,
CAP(LITERALPLUS) ? + : );
but that's a design choice that was already in the original version and
I don't care enough to change it.)
With the below squashed in,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
diff --git i/imap-send.c w/imap-send.c
index 451d5027..f1c8f5a5 100644
--- i/imap-send.c
+++ w/imap-send.c
@@ -1296,7 +1296,7 @@ static int imap_store_msg(struct store *gctx, struct
msg_data *msg)
box = gctx-name;
prefix = !strcmp(box, INBOX) ? : ctx-prefix;
cb.create = 0;
- ret = imap_exec_m(ctx, cb, APPEND \%s%s\, prefix, box);
+ ret = imap_exec_m(ctx, cb, APPEND \%s%s\ , prefix, box);
imap-caps = imap-rcaps;
if (ret != DRV_OK)
return ret;
ACK.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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 06/14] imap-send.c: remove some unused fields from struct store
On 01/14/2013 07:19 AM, Jonathan Nieder wrote:
Michael Haggerty wrote:
--- a/imap-send.c
+++ b/imap-send.c
[...]
@@ -772,13 +767,10 @@ static int get_cmd_result(struct imap_store *ctx,
struct imap_cmd *tcmd)
!strcmp(NO, arg) || !strcmp(BYE, arg)) {
if ((resp = parse_response_code(ctx, NULL,
cmd)) != RESP_OK)
return resp;
-} else if (!strcmp(CAPABILITY, arg))
+} else if (!strcmp(CAPABILITY, arg)) {
parse_capability(imap, cmd);
-else if ((arg1 = next_arg(cmd))) {
-if (!strcmp(EXISTS, arg1))
-ctx-gen.count = atoi(arg);
-else if (!strcmp(RECENT, arg1))
-ctx-gen.recent = atoi(arg);
+} else if ((arg1 = next_arg(cmd))) {
+/* unused */
Neat. Let me try to understand what was going on here:
When opening a mailbox with the SELECT command, an IMAP server
responds with tagged data indicating how many messages exist and how
many are marked Recent. But git imap-send never reads mailboxes and
in particular never uses the SELECT command, so there is no need for
us to parse or record such responses.
Out of paranoia we are keeping the parsing for now, but the parsed
response is unused, hence the comment above.
If I've understood correctly so far (a big assumption), I still am not
sure what it would mean if we hit this ((arg1 = next_arg(cmd))) case.
Does it mean:
A. The server has gone haywire and given a tagged response where
one is not allowed, but let's tolerate it because we always have
done so? Or
B. This is a perfectly normal response to some of the commands we
send, and we have always been deliberately ignoring it because it
is not important for what imap-send does?
Honestly, I didn't bother to look into this. I was just doing some
brainless elimination of obviously unused code.
No doubt a deeper analysis (like yours) could find more code to discard,
but I didn't want to invest that much time and this code has absolutely
no tests, so I stuck to the obvious (and even then you found a mistake
in my changes :-( ).
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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 09/14] imap-send.c: remove namespace fields from struct imap
On 01/14/2013 07:43 AM, Jonathan Nieder wrote:
Michael Haggerty wrote:
[...]
@@ -722,9 +667,9 @@ static int get_cmd_result(struct imap_store *ctx, struct
imap_cmd *tcmd)
}
if (!strcmp(NAMESPACE, arg)) {
-imap-ns_personal = parse_list(cmd);
-imap-ns_other = parse_list(cmd);
-imap-ns_shared = parse_list(cmd);
+skip_list(cmd);
+skip_list(cmd);
+skip_list(cmd);
This codepath would only be triggered if we emit a NAMESPACE command,
right? If I am understanding correctly, a comment could make this
less mystifying:
/* rfc2342 NAMESPACE response. */
skip_list(cmd);/* Personal mailboxes */
skip_list(cmd);/* Others' mailboxes */
skip_list(cmd);/* Shared mailboxes */
Yes, the comments are useful.
Though that's probably academic since hopefully this if branch
will die soon. :)
Not by my hand :-) Maybe somebody more familiar with the IMAP protocol
wants to take the code culling further...
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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 00/14] Remove unused code from imap-send.c
On 01/14/2013 07:57 AM, Jonathan Nieder wrote: Michael Haggerty wrote: imap-send.c | 286 +--- 1 file changed, 39 insertions(+), 247 deletions(-) See my replies for comments on patches 1, 6, 9, 11, and 12. The rest are Reviewed-by: Jonathan Nieder jrnie...@gmail.com The series is tasteful and easy to follow and it's hard to argue with the resulting code reduction. Thanks for a pleasant read. Thanks for your careful review. I will re-roll the patch series as soon as I get the chance. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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 2/8] git_remote_helpers: fix input when running under Python 3
On Mon, Jan 14, 2013 at 05:48:18AM +0100, Michael Haggerty wrote:
On 01/13/2013 05:17 PM, John Keeping wrote:
On Sun, Jan 13, 2013 at 04:26:39AM +0100, Michael Haggerty wrote:
On 01/12/2013 08:23 PM, John Keeping wrote:
Although 2to3 will fix most issues in Python 2 code to make it run under
Python 3, it does not handle the new strict separation between byte
strings and unicode strings. There is one instance in
git_remote_helpers where we are caught by this.
Fix it by explicitly decoding the incoming byte string into a unicode
string. In this instance, use the locale under which the application is
running.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git_remote_helpers/git/importer.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git_remote_helpers/git/importer.py
b/git_remote_helpers/git/importer.py
index e28cc8f..6814003 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -20,7 +20,7 @@ class GitImporter(object):
Returns a dictionary with refs.
args = [git, --git-dir= + gitdir, for-each-ref,
refs/heads]
-lines = check_output(args).strip().split('\n')
+lines = check_output(args).decode().strip().split('\n')
refs = {}
for line in lines:
value, name = line.split(' ')
Won't this change cause an exception if the branch names are not all
valid strings in the current locale's encoding? I don't see how this
assumption is justified (e.g., see git-check-ref-format(1) for the rules
governing reference names).
Yes it will. The problem is that for Python 3 we need to decode the
byte string into a unicode string, which means we need to know what
encoding it is.
I don't think we can just say git-for-each-ref will print refs in
UTF-8 since AFAIK git doesn't care what encoding the refs are in - I
suspect that's determined by the filesystem which in the end probably
maps to whatever bytes the shell fed git when the ref was created.
That's why I chose the current locale in this case. I'm hoping someone
here will correct me if we can do better, but I don't see any way of
avoiding choosing some encoding here if we want to support Python 3
(which I think we will, even if we don't right now).
I'm not just trying to be a nuisance here;
You're not being - I think this is a difficult issue and I don't know
myself what the right answer is.
I'm struggling myself to
understand how a program that cares about strings-vs-bytes (e.g., a
Python3 script) should coexist with a program that doesn't (e.g., git
[1]). I think this will become a big issue if my Python version of the
commit email script ever gets integrated and then made compatible with
Python3.
You claim for Python 3 we need to decode the byte string into a unicode
string. I understand that Python 3 strings are Unicode, but why/when
is it necessary to decode data into a Unicode string as opposed to
leaving it as a byte sequence?
In this particular case (from a cursory look over the code) it seems to
me that (1) decoding to Unicode will sometimes fail for data that git
considers valid and (2) there is no obvious reason that the data cannot
be processed as byte sequences.
I've been thinking about this overnight and I think you're right that
treating them as byte strings in Python is most correct. Having said
that, when I'm programming in Python I would find it quite surprising
that I had to treat ref strings specially - and as soon as I want to use
one in a string context (e.g. printing it as part of a message to the
user) I'm back to the same problem.
So I think we should try to solve the problem once rather than forcing
everyone who wants to use the library to solve it individually. I just
wish it was obvious what we should do!
John
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Bug in git stash
Am 1/14/2013 10:18, schrieb Nikolay Frantsev:
nikolay@localhost:~/Desktop/git-stash_bug/bug$ git status
# On branch master
# Changes to be committed:
# (use git reset HEAD file... to unstage)
#
# new file: 3
#
# Changes not staged for commit:
# (use git add file... to update what will be committed)
# (use git checkout -- file... to discard changes in working directory)
#
# modified: 1
# modified: 2
#
nikolay@localhost:~/Desktop/git-stash_bug/bug$ git stash save --keep-index one
Saved working directory and index state On master: one
HEAD is now at 7e495f9 files added
...
nikolay@localhost:~/Desktop/git-stash_bug/bug$ git stash pop stash@{1}
# On branch master
# Changes to be committed:
# (use git reset HEAD file... to unstage)
#
# new file: 3
#
# Changes not staged for commit:
# (use git add file... to update what will be committed)
# (use git checkout -- file... to discard changes in working directory)
#
# modified: 1
# modified: 2
#
Dropped stash@{1} (7926ab7285753c179a368a3a7e8ebfb0f39d0437)
Why there a new empty file named 3?
It is by design. --keep-index only achieves that your staged changes are
not reverted, but nevertheless all changes are stashed away. Therefore,
when you later apply the stash, you also get back the modified index.
-- Hannes
--
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
Error:non-monotonic index after failed recursive sed command
Hi All,
I've managed to corrupt my very valuable repository with a recursive
sed which went wrong.
I wanted to convert all tabs to spaces with the following command:
find ./ -name '*.*' -exec sed -i 's/\t//g' {} \;
I think that has changed not only the files in the repo, but the data
files in .git directory itself. As a result, my index became
corrupted, and almost every single command dies:
git log
error: non-monotonic index
.git/objects/pack/pack-314b1944adebea645526b6724b2044c1313241f5.idx
error: non-monotonic index
.git/objects/pack/pack-75c95b0defe1968b61e4f4e1ab7040d35110bfdc.idx
...
error: non-monotonic index
.git/objects/pack/pack-3da0da48d05140b55f4af1cf87c55a2d7898bdd5.idx
fatal: bad object HEAD
Output for git fsck is even worse:
git fsck
error: non-monotonic index
.git/objects/pack/pack-434f8445672a92f123accffce651bdb693bd8fcb.idx
...
error: non-monotonic index
.git/objects/pack/pack-0c9d5ae4e2b46dd78dace7533adf6cdfe10326ef.idx
error: non-monotonic index
.git/objects/pack/pack-e8bd5c7f85e96e7e548a62954a8f7c7f223ba9e0.idx
Segmentation fault (core dumped)
Any advice? I've lost about 2 weeks worth of work.
Is there anything better I can try to do other then trying to
reconstruct the data manually from git blobs?
--
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: Error:non-monotonic index after failed recursive sed command
George Karpenkov geo...@metaworld.ru writes:
Hi All,
I've managed to corrupt my very valuable repository with a recursive
sed which went wrong.
I wanted to convert all tabs to spaces with the following command:
find ./ -name '*.*' -exec sed -i 's/\t//g' {} \;
Clearly, this is a dangerous command as it impacts .git/. However, Git
partially protects you from this kind of error, since object files and
pack files are read-only by default.
My obvious first advice is: make backups of your corrupted repository.
Yes, I said backup_s_, better safe than sorry.
Then, the errors you get are in *.idx files, which are basically index
for pack files, for quicker access. You can try removing these files,
and then running git index-pack on each pack file, like
$ rm .git/objects/pack/pack-*.idx
$ git index-pack
.git/objects/pack/pack-4745076928ca4df932a3727b8cc25e83574560bb.pack
4745076928ca4df932a3727b8cc25e83574560bb
$ git index-pack
.git/objects/pack/pack-c74a6514f653d0269cdcdf9c1c102d326706bbda.pack
c74a6514f653d0269cdcdf9c1c102d326706bbda
--
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
Re: Error:non-monotonic index after failed recursive sed command
On Mon, Jan 14, 2013 at 01:06:16PM +0100, Matthieu Moy wrote:
George Karpenkov geo...@metaworld.ru writes:
I've managed to corrupt my very valuable repository with a recursive
sed which went wrong.
I wanted to convert all tabs to spaces with the following command:
find ./ -name '*.*' -exec sed -i 's/\t//g' {} \;
Clearly, this is a dangerous command as it impacts .git/. However, Git
partially protects you from this kind of error, since object files and
pack files are read-only by default.
My obvious first advice is: make backups of your corrupted repository.
Yes, I said backup_s_, better safe than sorry.
Having backed up the corrupt state, another option is to just try
running the reverse command:
find .git -name '*.*' -exec sed -i 's//\t/g' {} \;
I had a quick grep over some pack indices here and '' doesn't occur
in any of mine whereas '\t' is very common so you may find that the only
'' sequences are the ones you introduced.
John
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error:non-monotonic index after failed recursive sed command
Am 1/14/2013 12:40, schrieb George Karpenkov:
I've managed to corrupt my very valuable repository with a recursive
sed which went wrong.
I wanted to convert all tabs to spaces with the following command:
find ./ -name '*.*' -exec sed -i 's/\t//g' {} \;
I think that has changed not only the files in the repo, but the data
files in .git directory itself. As a result, my index became
corrupted, and almost every single command dies:
git log
error: non-monotonic index
..git/objects/pack/pack-314b1944adebea645526b6724b2044c1313241f5.idx
error: non-monotonic index
..git/objects/pack/pack-75c95b0defe1968b61e4f4e1ab7040d35110bfdc.idx
error: non-monotonic index
..git/objects/pack/pack-3da0da48d05140b55f4af1cf87c55a2d7898bdd5.idx
fatal: bad object HEAD
Output for git fsck is even worse:
git fsck
error: non-monotonic index
..git/objects/pack/pack-434f8445672a92f123accffce651bdb693bd8fcb.idx
error: non-monotonic index
..git/objects/pack/pack-0c9d5ae4e2b46dd78dace7533adf6cdfe10326ef.idx
error: non-monotonic index
..git/objects/pack/pack-e8bd5c7f85e96e7e548a62954a8f7c7f223ba9e0.idx
Segmentation fault (core dumped)
Any advice? I've lost about 2 weeks worth of work.
Is there anything better I can try to do other then trying to
reconstruct the data manually from git blobs?
First things first: MAKE A COPY OF THE CURRENT STATE of the repository,
including the worktree, so that you have something to go back to if things
get worse later. (At the very least, we want to debug the segfault that we
see above.)
So far that's all I can say about your case. Everything that follows is
just a shot in the dark, and others may have better ideas. Also, look
here:
https://github.com/gitster/git/blob/master/Documentation/howto/recover-corrupted-blob-object.txt
You can remove the *.idx files, because they do not carry essential
information. Now try git fsck; git repack.
Try the reverse edit:
find .git -name '*.*' -exec sed -i 's//\t/g' {} \;
Remove .git/index; it can be reconstructed (of course, assuming you
started all this with a clean index; if not, you lose the staged changes).
-- Hannes
--
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] archive-tar: fix sanity check in config parsing
On Mon, Jan 14, 2013 at 09:17:57AM +0100, Joachim Schmitz wrote:
For the curious, the original version of the patch[1] read:
+ if (prefixcmp(var, tarfilter.))
+ return 0;
+ dot = strrchr(var, '.');
+ if (dot == var + 9)
+ return 0;
and when I shortened the config section to tar in a re-roll of the
series, I missed the corresponding change to the offset.
Wouldn't it then be better ti use strlen(tar) rather than a 3? Or
at least a comment?
Then you are relying on the two strings being the same, rather than the
string and the length being the same. If you wanted to DRY it up, it
would look like:
diff --git a/archive-tar.c b/archive-tar.c
index d1cce46..a7c0690 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -332,15 +332,17 @@ static int tar_filter_config(const char *var, const char
*value, void *data)
const char *type;
int namelen;
- if (prefixcmp(var, tar.))
+#define SECTION tar
+ if (prefixcmp(var, SECTION .))
return 0;
dot = strrchr(var, '.');
- if (dot == var + 9)
+ if (dot == var + strlen(SECTION))
return 0;
- name = var + 4;
+ name = var + strlen(SECTION) + 1;
namelen = dot - name;
type = dot + 1;
+#undef SECTION
ar = find_tar_filter(name, namelen);
if (!ar) {
(of course there are other variants where you do not use a macro, but
then you need to manually check for the . after the prefixcmp call).
I dunno. It is technically more robust in that the offsets are computed,
but I think it is a little harder to read. Of course, I wrote the
original so I am probably not a good judge.
We could also potentially encapsulate it in a function. I think the diff
code has a very similar block.
-Peff
--
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] Possible bug in `remote set-url --add --push`
Jardel Weyrich venit, vidit, dixit 12.01.2013 10:33: On Sat, Jan 12, 2013 at 6:44 AM, Sascha Cunz sascha...@babbelbox.org wrote: Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano: Jardel Weyrich jweyr...@gmail.com writes: I believe `remote set-url --add --push` has a bug. Performed tests with v1.8.0.1 and v1.8.1 (Mac OS X). Quoting the relevant part of the documentation: set-url Changes URL remote points to. Sets first URL remote points to matching regex oldurl (first URL if no oldurl is given) to newurl. If oldurl doesn’t match any URL, error occurs and nothing is changed. With --push, push URLs are manipulated instead of fetch URLs. With --add, instead of changing some URL, new URL is added. With --delete, instead of changing some URL, all URLs matching regex url are deleted. Trying to delete all non-push URLs is an error. Here are some steps to reproduce: 1. Show the remote URLs jweyrich@pharao:test_clone1 [* master]$ git remote -v origin /Volumes/sandbox/test (fetch) origin /Volumes/sandbox/test (push) 2. Add a new push URL for origin jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \ /Volumes/sandbox/test_clone2 3. Check what happened jweyrich@pharao:test_clone1 [* master]$ git remote -v origin /Volumes/sandbox/test (fetch) origin /Volumes/sandbox/test_clone2 (push) The original pushurl was replaced with the additional one, instead of being left and the new one getting added. That looks certainly wrong. However, the result of applying the attached patch (either to v1.7.12 or v1.8.1) still passes the test and I do not think it is doing anything differently from what you described above. What do you get from git config -l | grep '^remote\.origin' in steps 1. and 3. in your procedure? This question is trying to tell if your bug is in git remote -v or in git remote set-url. I'm not sure, if there is a bug at all. According to man git-push: The pushurl is used for pushes only. It is optional and defaults to url. (From the section REMOTES - Named remote in configuration file) the command: git remote add foo g...@foo-fetch.org/some.git will set remote.foo.url to g...@foo-fetch.org. Subsequently, fetch and push will use g...@foo-fetch.org as url. Fetch will use this url, because remote.foo.url explicitly sets this. push will use it in absence of a remote.foo.pushurl. Now, we're adding a push-url: git remote set-url --add --push foo g...@foo-push.org/some.git Relevant parts of config are now looking like: [remote foo] url = g...@foo-fetch.org/some.git pushurl = g...@foo-push.org/some.git Since, pushurl is now given explicitly, git push will use that one (and only that one). If we add another push-url now, git remote set-url --add --push foo g...@foo-push-also.org/some.git the next git-push will push to foo-push.org and foo-push-also.org. Now, using --set-url --delete on both of these urls restores the original state: only remote.foo.url is set; meaning implicitly pushurl defaults to url again. To me this is exactly what Jardel was observing: In step 2, Git replaced the original push URL instead of adding a new one. But it seems to happen only the first time I use `remote set-url --add --push`. Re-adding the original URL using the same command seems to work properly. And FWIW, if I delete (with set-url --delete) both URLs push, Git restores the original URL. Or am I missing something here? You're right. However, as I quoted earlier, the git-remote man-page states: set-url Changes URL remote points to. suppressed With --push, push URLs are manipulated instead of fetch URLs. With --add, instead of changing some URL, new URL is added. It explicitly mentions that it should **add a new URL**. So when I do `git remote set-url --add --push origin git://another/repo.git`, I expect git-push to use both the default push URL and the new one. Or am I misinterpreting the man-page? Might be that the bug actually is that the expectation was git remote add foo g...@foo-fetch.org/some.git should have created a config like: [remote foo] url = g...@foo-fetch.org/some.git pushurl = g...@foo-fetch.org/some.git since that is what git remote -v reports. If that is the case, we might want to amend the output of 'git remote -v' with the information that a pushurl is not explicitly given and thus defaults to url. Correct. Adding a remote doesn't automatically generate a pushurl for it. To me, it seems that git-push checks for the existence of pushurl's in the config, and if it finds any, it ignores the defaul push URL during the actual push. In better words, it pushes only to pushurls, if it finds any, otherwise it pushes to the default URL. To comply with the statements in the
Re: [PATCH] rebase --preserve-merges keeps empty merge commits
On Sat, Jan 12, 2013 at 03:46:01PM -0500, Phil Hord wrote:
Since 90e1818f9a (git-rebase: add keep_empty flag, 2012-04-20)
'git rebase --preserve-merges' fails to preserve empty merge commits
unless --keep-empty is also specified. Merge commits should be
preserved in order to preserve the structure of the rebased graph,
even if the merge commit does not introduce changes to the parent.
Teach rebase not to drop merge commits only because they are empty.
A special case which is not handled by this change is for a merge commit
whose parents are now the same commit because all the previous different
parents have been dropped as a result of this rebase or some previous
operation.
---
git-rebase--interactive.sh | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 44901d5..8ed7fcc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -190,6 +190,11 @@ is_empty_commit() {
test $tree = $ptree
}
+is_merge_commit()
+{
+ git rev-parse --verify --quiet $1^2 /dev/null 21
+}
+
# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
# GIT_AUTHOR_DATE exported from the current environment.
do_with_author () {
@@ -874,7 +879,7 @@ git rev-list $merges_option --pretty=oneline
--abbrev-commit \
while read -r shortsha1 rest
do
- if test -z $keep_empty is_empty_commit $shortsha1
+ if test -z $keep_empty is_empty_commit $shortsha1 !
is_merge_commit $shortsha1
then
comment_out=#
else
--
1.8.1.dirty
Seems reasonable
Acked-by: Neil Horman nhor...@tuxdriver.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] rebase --preserve-merges keeps empty merge commits
Phil Hord ho...@cisco.com writes: Subject: [PATCH] rebase --preserve-merges keeps empty merge commits I would rephrase it as rebase --preserve-merges: keep empty merge commits we usually give orders in commit messages, not state facts (it's not clear from the existing subject line whether keeping merge commit is the new behavior or a bug that the commit tries to fix). -- 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
Re: [PATCH] archive-tar: fix sanity check in config parsing
On Mon, Jan 14, 2013 at 04:44:24AM -0800, Jeff King wrote: Wouldn't it then be better ti use strlen(tar) rather than a 3? Or at least a comment? [...] We could also potentially encapsulate it in a function. I think the diff code has a very similar block. Here's a series that does that, with a few other cleanups on top. The diffstat actually ends up a few lines longer, but that is mostly because of comments and function declarations. More importantly, though, the call-sites are much easier to read. Having written this series, though, I can't help but wonder if the world would be a better place if config_fn_t looked more like: typedef int (*config_fn_t)(const char *full_var, const char *section, const char *subsection, const char *key, const char *value, void *data); It's just as easy for the config parser to do this ahead of time, and by handing off real C-strings (instead of ending up with a ptr/len pair for the subsection), it makes the lives of the callbacks much easier (e.g., the final patch below contorts a bit to use string_list with the subsection). I can look into that, but here is the less invasive cleanup: [1/6]: config: add helper function for parsing key names [2/6]: archive-tar: use match_config_key when parsing config [3/6]: convert some config callbacks to match_config_key [4/6]: userdiff: drop parse_driver function [5/6]: submodule: use match_config_key when parsing config [6/6]: submodule: simplify memory handling in config parsing -Peff -- 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 1/6] config: add helper function for parsing key names
The config callback functions get keys of the general form:
section.subsection.key
(where the subsection may be contain arbitrary data, or may
be missing). For matching keys without subsections, it is
simple enough to call strcmp. Matching keys with
subsections is a little more complicated, and each callback
does it in an ad-hoc way, usually involving error-prone
pointer arithmetic.
Let's provide a helper that keeps the pointer arithmetic all
in one place.
Signed-off-by: Jeff King p...@peff.net
---
No users yet; they come in future patches.
cache.h | 15 +++
config.c | 33 +
2 files changed, 48 insertions(+)
diff --git a/cache.h b/cache.h
index c257953..14003b8 100644
--- a/cache.h
+++ b/cache.h
@@ -1164,6 +1164,21 @@ extern int git_config_include(const char *name, const
char *value, void *data);
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
+/*
+ * Match and parse a config key of the form:
+ *
+ * section.(subsection.)?key
+ *
+ * (i.e., what gets handed to a config_fn_t). The caller provides the section;
+ * we return -1 if it does not match, 0 otherwise. The subsection and key
+ * out-parameters are filled by the function (and subsection is NULL if it is
+ * missing).
+ */
+extern int match_config_key(const char *var,
+const char *section,
+const char **subsection, int *subsection_len,
+const char **key);
+
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index 7b444b6..18d9c0e 100644
--- a/config.c
+++ b/config.c
@@ -1667,3 +1667,36 @@ int config_error_nonbool(const char *var)
{
return error(Missing value for '%s', var);
}
+
+int match_config_key(const char *var,
+const char *section,
+const char **subsection, int *subsection_len,
+const char **key)
+{
+ int section_len = strlen(section);
+ const char *dot;
+
+ /* Does it start with section. ? */
+ if (prefixcmp(var, section) || var[section_len] != '.')
+ return -1;
+
+ /*
+* Find the key; we don't know yet if we have a subsection, but we must
+* parse backwards from the end, since the subsection may have dots in
+* it, too.
+*/
+ dot = strrchr(var, '.');
+ *key = dot + 1;
+
+ /* Did we have a subsection at all? */
+ if (dot == var + section_len) {
+ *subsection = NULL;
+ *subsection_len = 0;
+ }
+ else {
+ *subsection = var + section_len + 1;
+ *subsection_len = dot - *subsection;
+ }
+
+ return 0;
+}
--
1.8.1.rc1.10.g7d71f7b
--
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 2/6] archive-tar: use match_config_key when parsing config
This is fewer lines of code, but more importantly, fixes a
bogus pointer offset. We are looking for tar. in the
section, but later assume that the dot we found is at offset
9, not 3. This is a holdover from an earlier iteration of
767cf45 which called the section tarfilter.
As a result, we could erroneously reject some filters with
dots in their name, as well as read uninitialized memory.
Reported by (and test by) René Scharfe.
Signed-off-by: Jeff King p...@peff.net
---
This obviously replaces René's patch. It might make more sense to just
put his patch onto maint, and build this on top; then this patch can get
squashed into the next one (which just updates the uninteresting
callsites).
archive-tar.c | 10 +-
t/t5000-tar-tree.sh | 3 ++-
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 0ba3f25..68dbe59 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -325,20 +325,12 @@ static int tar_filter_config(const char *var, const char
*value, void *data)
static int tar_filter_config(const char *var, const char *value, void *data)
{
struct archiver *ar;
- const char *dot;
const char *name;
const char *type;
int namelen;
- if (prefixcmp(var, tar.))
+ if (match_config_key(var, tar, name, namelen, type) 0 || !name)
return 0;
- dot = strrchr(var, '.');
- if (dot == var + 9)
- return 0;
-
- name = var + 4;
- namelen = dot - name;
- type = dot + 1;
ar = find_tar_filter(name, namelen);
if (!ar) {
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index ecf00ed..517ae04 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -283,7 +283,8 @@ test_expect_success 'setup tar filters' '
test_expect_success 'setup tar filters' '
git config tar.tar.foo.command tr ab ba
git config tar.bar.command tr ab ba
- git config tar.bar.remote true
+ git config tar.bar.remote true
+ git config tar.invalid baz
'
test_expect_success 'archive --list mentions user filter' '
--
1.8.1.rc1.10.g7d71f7b
--
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 3/6] convert some config callbacks to match_config_key
This is easier to read and avoids magic offset constants
which need to be in sync with the section-name we provide.
Signed-off-by: Jeff King p...@peff.net
---
convert.c | 6 +-
ll-merge.c | 6 +-
userdiff.c | 13 +++--
3 files changed, 5 insertions(+), 20 deletions(-)
diff --git a/convert.c b/convert.c
index 6602155..e3ecb30 100644
--- a/convert.c
+++ b/convert.c
@@ -465,10 +465,8 @@ static int read_convert_config(const char *var, const char
*value, void *cb)
* External conversion drivers are configured using
* filter.name.variable.
*/
- if (prefixcmp(var, filter.) || (ep = strrchr(var, '.')) == var + 6)
+ if (match_config_key(var, filter, name, namelen, ep) 0 || !name)
return 0;
- name = var + 7;
- namelen = ep - name;
for (drv = user_convert; drv; drv = drv-next)
if (!strncmp(drv-name, name, namelen) !drv-name[namelen])
break;
@@ -479,8 +477,6 @@ static int read_convert_config(const char *var, const char
*value, void *cb)
user_convert_tail = (drv-next);
}
- ep++;
-
/*
* filter.name.smudge and filter.name.clean specifies
* the command line:
diff --git a/ll-merge.c b/ll-merge.c
index acea33b..d4c4ff6 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -236,15 +236,13 @@ static int read_merge_config(const char *var, const char
*value, void *cb)
* especially, we do not want to look at variables such as
* merge.summary, merge.tool, and merge.verbosity.
*/
- if (prefixcmp(var, merge.) || (ep = strrchr(var, '.')) == var + 5)
+ if (match_config_key(var, merge, name, namelen, ep) 0 || !name)
return 0;
/*
* Find existing one as we might be processing merge.name.var2
* after seeing merge.name.var1.
*/
- name = var + 6;
- namelen = ep - name;
for (fn = ll_user_merge; fn; fn = fn-next)
if (!strncmp(fn-name, name, namelen) !fn-name[namelen])
break;
@@ -256,8 +254,6 @@ static int read_merge_config(const char *var, const char
*value, void *cb)
ll_user_merge_tail = (fn-next);
}
- ep++;
-
if (!strcmp(name, ep)) {
if (!value)
return error(%s: lacks value, var);
diff --git a/userdiff.c b/userdiff.c
index ed958ef..1a6a0fa 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -188,20 +188,13 @@ static struct userdiff_driver *parse_driver(const char
*var,
const char *value, const char *type)
{
struct userdiff_driver *drv;
- const char *dot;
- const char *name;
+ const char *name, *key;
int namelen;
- if (prefixcmp(var, diff.))
- return NULL;
- dot = strrchr(var, '.');
- if (dot == var + 4)
- return NULL;
- if (strcmp(type, dot+1))
+ if (match_config_key(var, diff, name, namelen, key) 0 ||
+ strcmp(type, key))
return NULL;
- name = var + 5;
- namelen = dot - name;
drv = userdiff_find_by_namelen(name, namelen);
if (!drv) {
ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
--
1.8.1.rc1.10.g7d71f7b
--
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 4/6] userdiff: drop parse_driver function
When we parse userdiff config, we generally assume that
diff.name.key
will affect the key value of the name driver. However,
without checking the key, we conflict with the ancient
diff.color.* namespace. The current code is careful not to
even create a driver struct if we do not see a key that is
known by the diff-driver code.
However, this carefulness is unnecessary; the default driver
with no keys set behaves exactly the same as having no
driver at all. We can simply set up the driver struct as
soon as we see we have a config key that looks like a
driver. This makes the code a bit more readable.
Signed-off-by: Jeff King p...@peff.net
---
This is not strictly related to the series, but I noticed it as a
cleanup while doing the previous patch.
userdiff.c | 50 +-
1 file changed, 21 insertions(+), 29 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index 1a6a0fa..c6cdec4 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -184,28 +184,6 @@ static struct userdiff_driver
*userdiff_find_by_namelen(const char *k, int len)
return NULL;
}
-static struct userdiff_driver *parse_driver(const char *var,
- const char *value, const char *type)
-{
- struct userdiff_driver *drv;
- const char *name, *key;
- int namelen;
-
- if (match_config_key(var, diff, name, namelen, key) 0 ||
- strcmp(type, key))
- return NULL;
-
- drv = userdiff_find_by_namelen(name, namelen);
- if (!drv) {
- ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
- drv = drivers[ndrivers++];
- memset(drv, 0, sizeof(*drv));
- drv-name = xmemdupz(name, namelen);
- drv-binary = -1;
- }
- return drv;
-}
-
static int parse_funcname(struct userdiff_funcname *f, const char *k,
const char *v, int cflags)
{
@@ -233,20 +211,34 @@ int userdiff_config(const char *k, const char *v)
int userdiff_config(const char *k, const char *v)
{
struct userdiff_driver *drv;
+ const char *name, *type;
+ int namelen;
+
+ if (match_config_key(k, diff, name, namelen, type) || !name)
+ return 0;
+
+ drv = userdiff_find_by_namelen(name, namelen);
+ if (!drv) {
+ ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
+ drv = drivers[ndrivers++];
+ memset(drv, 0, sizeof(*drv));
+ drv-name = xmemdupz(name, namelen);
+ drv-binary = -1;
+ }
- if ((drv = parse_driver(k, v, funcname)))
+ if (!strcmp(type, funcname))
return parse_funcname(drv-funcname, k, v, 0);
- if ((drv = parse_driver(k, v, xfuncname)))
+ if (!strcmp(type, xfuncname))
return parse_funcname(drv-funcname, k, v, REG_EXTENDED);
- if ((drv = parse_driver(k, v, binary)))
+ if (!strcmp(type, binary))
return parse_tristate(drv-binary, k, v);
- if ((drv = parse_driver(k, v, command)))
+ if (!strcmp(type, command))
return git_config_string(drv-external, k, v);
- if ((drv = parse_driver(k, v, textconv)))
+ if (!strcmp(type, textconv))
return git_config_string(drv-textconv, k, v);
- if ((drv = parse_driver(k, v, cachetextconv)))
+ if (!strcmp(type, cachetextconv))
return parse_bool(drv-textconv_want_cache, k, v);
- if ((drv = parse_driver(k, v, wordregex)))
+ if (!strcmp(type, wordregex))
return git_config_string(drv-word_regex, k, v);
return 0;
--
1.8.1.rc1.10.g7d71f7b
--
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 5/6] submodule: use match_config_key when parsing config
This makes the code a lot simpler to read by dropping a
whole bunch of constant offsets.
As a bonus, it means we also feed the whole config variable
name to our error functions:
[before]
$ git -c submodule.foo.fetchrecursesubmodules=bogus checkout
fatal: bad foo.fetchrecursesubmodules argument: bogus
[after]
fatal: bad submodule.foo.fetchrecursesubmodules argument: bogus
Signed-off-by: Jeff King p...@peff.net
---
submodule.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/submodule.c b/submodule.c
index 2f55436..4361207 100644
--- a/submodule.c
+++ b/submodule.c
@@ -126,15 +126,17 @@ int parse_submodule_config_option(const char *var, const
char *value)
int parse_submodule_config_option(const char *var, const char *value)
{
- int len;
struct string_list_item *config;
struct strbuf submodname = STRBUF_INIT;
+ const char *name, *key;
+ int namelen;
- var += 10; /* Skip submodule. */
+ if (match_config_key(var, submodule, name, namelen, key) 0 ||
+ !name)
+ return 0;
- len = strlen(var);
- if ((len 5) !strcmp(var + len - 5, .path)) {
- strbuf_add(submodname, var, len - 5);
+ if (!strcmp(key, path)) {
+ strbuf_add(submodname, name, namelen);
config = unsorted_string_list_lookup(config_name_for_path,
value);
if (config)
free(config-util);
@@ -142,22 +144,22 @@ int parse_submodule_config_option(const char *var, const
char *value)
config = string_list_append(config_name_for_path,
xstrdup(value));
config-util = strbuf_detach(submodname, NULL);
strbuf_release(submodname);
- } else if ((len 23) !strcmp(var + len - 23,
.fetchrecursesubmodules)) {
- strbuf_add(submodname, var, len - 23);
+ } else if (!strcmp(key, fetchrecursesubmodules)) {
+ strbuf_add(submodname, name, namelen);
config =
unsorted_string_list_lookup(config_fetch_recurse_submodules_for_name,
submodname.buf);
if (!config)
config =
string_list_append(config_fetch_recurse_submodules_for_name,
strbuf_detach(submodname,
NULL));
config-util = (void
*)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
strbuf_release(submodname);
- } else if ((len 7) !strcmp(var + len - 7, .ignore)) {
+ } else if (!strcmp(key, ignore)) {
if (strcmp(value, untracked) strcmp(value, dirty)
strcmp(value, all) strcmp(value, none)) {
warning(Invalid parameter \%s\ for config option
\submodule.%s.ignore\, value, var);
return 0;
}
- strbuf_add(submodname, var, len - 7);
+ strbuf_add(submodname, name, namelen);
config = unsorted_string_list_lookup(config_ignore_for_name,
submodname.buf);
if (config)
free(config-util);
--
1.8.1.rc1.10.g7d71f7b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git list files
On Sun, Jan 13, 2013 at 11:08:32PM -0800, Jonathan Nieder wrote: Jeff King wrote: As far as I recall, that script works. However, I have a pure-C blame-tree implementation that is much faster, which may also be of interest. I need to clean up and put a few finishing touches on it to send it to the list, but it has been in production at GitHub for a few months. You can find it here: git://github.com/peff/git jk/blame-tree Oh, neat. Would that make sense as an item in https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools? I'd rather finish cleaning it up and actually get it merged. It's on my todo list. -Peff -- 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] pretty: use prefixcmp instead of memcmp on NUL-terminated strings
This conversion avoids the need for magic string length numbers in the
code. And unlike memcmp(), prefixcmp() is careful to not run over the
end of a string.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
pretty.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/pretty.c b/pretty.c
index 92c839f..01795de 100644
--- a/pretty.c
+++ b/pretty.c
@@ -966,7 +966,7 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
if (!end)
return 0;
- if (!memcmp(begin, auto,, 5)) {
+ if (!prefixcmp(begin, auto,)) {
if (!want_color(c-pretty_ctx-color))
return end - placeholder + 1;
begin += 5;
@@ -1301,7 +1301,7 @@ static void pp_header(const struct pretty_print_context
*pp,
continue;
}
- if (!memcmp(line, parent , 7)) {
+ if (!prefixcmp(line, parent )) {
if (linelen != 48)
die(bad parent line in commit);
continue;
@@ -1325,11 +1325,11 @@ static void pp_header(const struct pretty_print_context
*pp,
* FULL shows both authors but not dates.
* FULLER shows both authors and dates.
*/
- if (!memcmp(line, author , 7)) {
+ if (!prefixcmp(line, author )) {
strbuf_grow(sb, linelen + 80);
pp_user_info(pp, Author, sb, line + 7, encoding);
}
- if (!memcmp(line, committer , 10)
+ if (!prefixcmp(line, committer )
(pp-fmt == CMIT_FMT_FULL || pp-fmt == CMIT_FMT_FULLER)) {
strbuf_grow(sb, linelen + 80);
pp_user_info(pp, Commit, sb, line + 10, encoding);
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] convert some config callbacks to match_config_key
Jeff King wrote:
--- a/convert.c
+++ b/convert.c
@@ -465,10 +465,8 @@ static int read_convert_config(const char *var, const
char *value, void *cb)
* External conversion drivers are configured using
* filter.name.variable.
*/
- if (prefixcmp(var, filter.) || (ep = strrchr(var, '.')) == var + 6)
+ if (match_config_key(var, filter, name, namelen, ep) 0 || !name)
return 0;
Hm, I actually find the preimage more readable here.
I like the idea of having a function to do this, though. Here are a
couple of ideas for making the meaning obvious again for people like
me:
Rename match_config_key() to something like parse_config_key()?
match_ makes it sound like its main purpose is to match it against a
pattern, but really it is more about decomposing into constituent
parts.
Rename ep to something like 'key' or 'filtertype'? Without the
explicit string processing, it is not obvious what ep is the end of.
[...]
--- a/userdiff.c
+++ b/userdiff.c
@@ -188,20 +188,13 @@ static struct userdiff_driver *parse_driver(const char
*var,
const char *value, const char *type)
{
struct userdiff_driver *drv;
- const char *dot;
- const char *name;
+ const char *name, *key;
int namelen;
- if (prefixcmp(var, diff.))
- return NULL;
- dot = strrchr(var, '.');
- if (dot == var + 4)
- return NULL;
- if (strcmp(type, dot+1))
+ if (match_config_key(var, diff, name, namelen, key) 0 ||
+ strcmp(type, key))
return NULL;
- name = var + 5;
- namelen = dot - name;
drv = userdiff_find_by_namelen(name, namelen);
What happens in the !name case? (Honest question --- I haven't checked.)
Generally I like the cleanup. Thanks for tasteful patch.
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] convert some config callbacks to match_config_key
On Mon, Jan 14, 2013 at 08:55:28AM -0800, Jonathan Nieder wrote: Jeff King wrote: --- a/convert.c +++ b/convert.c @@ -465,10 +465,8 @@ static int read_convert_config(const char *var, const char *value, void *cb) * External conversion drivers are configured using * filter.name.variable. */ - if (prefixcmp(var, filter.) || (ep = strrchr(var, '.')) == var + 6) + if (match_config_key(var, filter, name, namelen, ep) 0 || !name) return 0; Hm, I actually find the preimage more readable here. The big thing to me is getting rid of the manual 6 there, which is bad for maintainability (it must match the length of filter, and there is no compile-time verification). Rename match_config_key() to something like parse_config_key()? match_ makes it sound like its main purpose is to match it against a pattern, but really it is more about decomposing into constituent parts. There is already a git_config_parse_key, but it does something else. I wanted to avoid confusion there. And I was trying to indicate that this was not just about parsing, but also about matching the section. Basically I was trying to encapsulate the current technique and have as little code change as possible. But maybe: struct config_key k; parse_config_key(k, var); if (strcmp(k.section, filter) || k.subsection)) return 0; would be a better start (or having git_config do the first two lines itself before triggering the callback). Rename ep to something like 'key' or 'filtertype'? Without the explicit string processing, it is not obvious what ep is the end of. Ah, so that is what ep stands for. I was thinking it is a terrible variable name, but I was trying to keep the patch minimal. - if (prefixcmp(var, diff.)) - return NULL; - dot = strrchr(var, '.'); - if (dot == var + 4) - return NULL; - if (strcmp(type, dot+1)) + if (match_config_key(var, diff, name, namelen, key) 0 || + strcmp(type, key)) return NULL; - name = var + 5; - namelen = dot - name; drv = userdiff_find_by_namelen(name, namelen); What happens in the !name case? (Honest question --- I haven't checked.) Segfault, I expect. Thanks for catching. I actually wrote this correctly once, coupled with patch 4, but screwed it up when teasing it apart into two patches. It should be: if (match_config_key(var, diff, name, namelen, key) 0 || !name || strcmp(type, key)) return NULL; Patch 4 replaces this with correct code (as it moves it into userdiff_config). -Peff -- 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] rebase --preserve-merges keeps empty merge commits
Matthieu Moy matthieu@grenoble-inp.fr writes:
Phil Hord ho...@cisco.com writes:
Subject: [PATCH] rebase --preserve-merges keeps empty merge commits
I would rephrase it as
rebase --preserve-merges: keep empty merge commits
we usually give orders in commit messages, not state facts (it's not
clear from the existing subject line whether keeping merge commit is the
new behavior or a bug that the commit tries to fix).
Thanks for giving a concise rationale on our use of imperative mood.
Phil, I think you meant to and forgot to sign-off; here is what I'll
queue.
Thanks.
-- 8 --
From: Phil Hord ho...@cisco.com
Date: Sat, 12 Jan 2013 15:46:01 -0500
Subject: [PATCH] rebase --preserve-merges: keep all merge commits including
empty ones
Since 90e1818f9a (git-rebase: add keep_empty flag, 2012-04-20)
'git rebase --preserve-merges' fails to preserve empty merge commits
unless --keep-empty is also specified. Merge commits should be
preserved in order to preserve the structure of the rebased graph,
even if the merge commit does not introduce changes to the parent.
Teach rebase not to drop merge commits only because they are empty.
A special case which is not handled by this change is for a merge commit
whose parents are now the same commit because all the previous different
parents have been dropped as a result of this rebase or some previous
operation.
Signed-off-by: Phil Hord ho...@cisco.com
Acked-by: Neil Horman nhor...@tuxdriver.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-rebase--interactive.sh | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..2fed92f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -175,6 +175,11 @@ is_empty_commit() {
test $tree = $ptree
}
+is_merge_commit()
+{
+ git rev-parse --verify --quiet $1^2 /dev/null 21
+}
+
# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
# GIT_AUTHOR_DATE exported from the current environment.
do_with_author () {
@@ -796,7 +801,7 @@ git rev-list $merges_option --pretty=oneline
--abbrev-commit \
while read -r shortsha1 rest
do
- if test -z $keep_empty is_empty_commit $shortsha1
+ if test -z $keep_empty is_empty_commit $shortsha1 !
is_merge_commit $shortsha1
then
comment_out=#
else
--
1.8.1.1.338.g126d652
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git list files
I went through your initial thread about blame-tree, and it is really very very (+very) close to answer my question. Thanks for writing it, if it comes one day to git, I will use it. As for: 'I guess people's eyes and brains are trained by the old school file boundaries matter way of thinking' -- Junio C Hamano at http://thread.gmane.org/gmane.comp.version-control.git/168323; http://article.gmane.org/gmane.comp.version-control.git/168333 I think it's not the case, Mr. Hamano. From my point of view, it is just to have a quick picture of what came from where in this current directory, which is a normal reaction of human beings, I think. Speaking of which I can't help thinking that this feature could be provided by $git rev-list (HEAD) --no-walk -- paths, just don't stop at first commit, but at first commit for each of the paths. Or maybe diff could have an option to not compare against a specific point, but actually do his job and go downstears and find where the _diff_erence for _each_ path happened finally. (... applicable for $git status -l (--list) --porcelain ... but thats a whim, sorry.) Anyway, thank you all for your time, it was a real pleasure for me, Blind. 2013/1/14 Jeff King p...@peff.net: On Sun, Jan 13, 2013 at 11:08:32PM -0800, Jonathan Nieder wrote: Jeff King wrote: As far as I recall, that script works. However, I have a pure-C blame-tree implementation that is much faster, which may also be of interest. I need to clean up and put a few finishing touches on it to send it to the list, but it has been in production at GitHub for a few months. You can find it here: git://github.com/peff/git jk/blame-tree Oh, neat. Would that make sense as an item in https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools? I'd rather finish cleaning it up and actually get it merged. It's on my todo list. -Peff -- 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 2/3] push: Add support for pre-push hooks
Aaron Schrab aa...@schrab.com writes: Add support for a pre-push hook which can be used to determine if the set of refs to be pushed is suitable for the target repository. The hook is run with two arguments specifying the name and location of the destination repository. Information about what is to be pushed is provided by sending lines of the following form to the hook's standard input: local ref SP local sha1 SP remote ref SP remote sha1 LF If the hook exits with a non-zero status, the push will be aborted. This will allow the script to determine if the push is acceptable based on the target repository and branch(es), the commits which are to be pushed, and even the source branches in some cases. Signed-off-by: Aaron Schrab aa...@schrab.com --- Documentation/githooks.txt | 29 ++ builtin/push.c | 1 + t/t5571-pre-push-hook.sh | 129 + transport.c| 60 + transport.h| 1 + 5 files changed, 220 insertions(+) create mode 100755 t/t5571-pre-push-hook.sh diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index b9003fe..d839233 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -176,6 +176,35 @@ save and restore any form of metadata associated with the working tree (eg: permissions/ownership, ACLS, etc). See contrib/hooks/setgitperms.perl for an example of how to do this. +pre-push + + +This hook is called by 'git push' and can be used to prevent a push from taking +place. The hook is called with two parameters which provide the name and +location of the destination remote, if a named remote is not being used both +values will be the same. + +Information about what is to be pushed is provided on the hook's standard +input with lines of the form: + + local ref SP local sha1 SP remote ref SP remote sha1 LF + +For instance, if the command +git push origin master:foreign+ were run the Just being curious, but why use +monospace text+ here? Most of the new text use `monospace text literally` instead in this patch. +hook would receive a line like the following: + + refs/heads/master 67890 refs/heads/foreign 12345 + +although the full, 40-character SHA1s would be supplied. Perhaps ellipses are called for here? refs/heads/master 67890... refs/heads/foreign 12345... (the above abbreviates full 40-hexdigits for illustration purposes only) +If the foreign ref +does not yet exist the `remote SHA1` will be 40 `0`. If a ref is to be +deleted, the `local ref` will be supplied as `(delete)` and the `local +SHA1` will be 40 `0`. If the local commit was specified by something other +than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be +supplied as it was originally given. + +If this hook exits with a non-zero status, 'git push' will abort without +pushing anything. Information about why the push is rejected may be sent +to the user by writing to standard error. s/standard error/ of the hook/; perhaps? It is unclear who does the writing and it can be misunderstood that git-push will write to standard error upon seeing your hook that silently exits. diff --git a/builtin/push.c b/builtin/push.c index 8491e43..b158028 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char *prefix) OPT_BOOL(0, progress, progress, N_(force progress reporting)), OPT_BIT(0, prune, flags, N_(prune locally removed refs), TRANSPORT_PUSH_PRUNE), + OPT_BIT(0, no-verify, flags, N_(bypass pre-push hook), TRANSPORT_PUSH_NO_HOOK), OPT_END() }; So to countermand this, you have to say --no-no-verify? Wouldn't it be more natural to introduce a --verify option that turns the bit on, which automatically gives you --no-verify to turn it off? A bit in a flag word can be initialized to true before the flag word is given to the parse_options() machinery to make the field default to true, no? -- 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 3/3] Add sample pre-push hook script
Aaron Schrab aa...@schrab.com writes: Create a sample of a script for a pre-push hook. The main purpose is to illustrate how a script may parse the information which is supplied to such a hook. The script may also be useful to some people as-is for avoiding to push commits which are marked as a work in progress. Signed-off-by: Aaron Schrab aa...@schrab.com --- templates/hooks--pre-push.sample | 53 1 file changed, 53 insertions(+) create mode 100644 templates/hooks--pre-push.sample diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample new file mode 100644 index 000..15ab6d8 --- /dev/null +++ b/templates/hooks--pre-push.sample @@ -0,0 +1,53 @@ +#!/bin/sh + +# An example hook script to verify what is about to be pushed. Called by git +# push after it has checked the remote status, but before anything has been +# pushed. If this script exits with a non-zero status nothing will be pushed. +# +# This hook is called with the following parameters: +# +# $1 -- Name of the remote to which the push is being done +# $2 -- URL to which the push is being done +# +# If pushing without using a named remote those arguments will be equal. +# +# Information about the commits which are being pushed is supplied as lines to +# the standard input in the form: +# +# local ref local sha1 remote ref remote sha1 +# +# This sample shows how to prevent push of commits where the log message starts +# with WIP (work in progress). An example for a plausible use case is nice to have. I would prefer to see any new shell script to follow the Git style, though. +remote=$1 +url=$2 + +z40= + +IFS=' ' +while read local_ref local_sha remote_ref remote_sha +do + if [ $local_sha = $z40 ] + then + # Handle delete ... by doing what? + else + if [ $remote_sha = $z40 ] + then + # New branch, examine all commits + range=$local_sha + else + # Update to existing branch, examine new commits + range=$remote_sha..$local_sha + fi + + # Check for WIP commit + commit=`git rev-list -n 1 --grep '^WIP' $range` + if [ -n $commit ] + then + echo Found WIP commit in $local_ref, not pushing + exit 1 + fi + fi +done + +exit 0 -- 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 0/3] pre-push hook support
Aaron Schrab aa...@schrab.com writes: Main changes since the initial version: * The first patch converts the existing hook callers to use the new find_hook() function. * Information about what is to be pushed is now sent over a pipe rather than passed as command-line parameters. Aaron Schrab (3): hooks: Add function to check if a hook exists push: Add support for pre-push hooks Add sample pre-push hook script Getting much nicer. Thanks. -- 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] rebase --preserve-merges keeps empty merge commits
Junio C Hamano wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Phil Hord ho...@cisco.com writes: Subject: [PATCH] rebase --preserve-merges keeps empty merge commits I would rephrase it as rebase --preserve-merges: keep empty merge commits we usually give orders in commit messages, not state facts (it's not clear from the existing subject line whether keeping merge commit is the new behavior or a bug that the commit tries to fix). Thanks for giving a concise rationale on our use of imperative mood. Phil, I think you meant to and forgot to sign-off; here is what I'll queue. Thanks. Looks good. Thanks for the help. Phil -- 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] t0050: mark TC merge (case change) as success
On 14.01.13 00:24, Junio C Hamano wrote: Torsten Bögershausen tbo...@web.de writes: The test merge (case change) passes on a case ignoring file system Use test_expect_success to remove the known breakage vanished Signed-off-by: Torsten Bögershausen tbo...@web.de --- Interesting. When did this change? Do you happen to have a bisection? This seems to be the commit: commit 6aad47dec7a72bb36c64afb6c43f4dbcaa49e7f9 Merge: e13067a 0047dd2 Author: Junio C Hamano gits...@pobox.com Date: Fri May 23 16:05:52 2008 -0700 Merge branch 'sp/ignorecase' * sp/ignorecase: t0050: Fix merge test on case sensitive file systems t0050: Add test for case insensitive add t0050: Set core.ignorecase case to activate case insensitivity t0050: Test autodetect core.ignorecase git-init: autodetect core.ignorecase Which comes from here: commit 0047dd2fd1fc1980913901c5fa098357482c2842 Author: Steffen Prohaska proha...@zib.de Date: Thu May 15 07:19:54 2008 +0200 t0050: Fix merge test on case sensitive file systems On a case sensitive filesystem, git reset --hard might refuse to overwrite a file whose name differs only by case, even if core.ignorecase is set. It is not clear which circumstances cause this behavior. This commit simply works around the problem by removing the case changing file before running git reset --hard. Signed-off-by: Steffen Prohaska proha...@zib.de Signed-off-by: Junio C Hamano gits...@pobox.com === Or did the test pass from the very beginning? Hm, reading the commit, it seems as if the root problem still exist: git reset --hard does not change the case of an existing file What is the exist behvior? My feeling is that the test as such deserves some more improvements, the result of the merge is not checked, files are empty so that the content is not checked. Another improvement: Running under Linux gives: not ok 6 - add (with different case) # TODO known breakage (and running under mingw failes) Please stay tuned for more updates, thanks for reviewing. /Torsten -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] convert some config callbacks to match_config_key
On Mon, Jan 14, 2013 at 09:06:10AM -0800, Jeff King wrote:
struct config_key k;
parse_config_key(k, var);
if (strcmp(k.section, filter) || k.subsection))
return 0;
would be a better start (or having git_config do the first two lines
itself before triggering the callback).
Here's what that looks like, along with the cleanups in submodule.c that
are made possible by it.
I cheat a little and use a static buffer when parsing the config key,
so that the caller does not have to deal with freeing it. It makes using
the parser literally as simple as the lines above, but it does mean it
isn't re-entrant (and worse, it has to be invoked from a config
callback, since the static buffer is tied to the config file stack).
None of that is a problem for the use here, but it is not a
generally-callable function. For that reason, it might make more sense
to have the config parser just provide the config_key, and not have a
public function at all. The downside to that is that we have to update
the function signature of all of the config callbacks.
---
cache.h | 7 ++
config.c| 35 ++
submodule.c | 41 ++-
3 files changed, 58 insertions(+), 25 deletions(-)
diff --git a/cache.h b/cache.h
index c257953..df756e6 100644
--- a/cache.h
+++ b/cache.h
@@ -1119,6 +1119,13 @@ extern int update_server_info(int);
#define CONFIG_INVALID_PATTERN 6
#define CONFIG_GENERIC_ERROR 7
+struct config_key {
+ const char *section;
+ const char *subsection;
+ const char *key;
+};
+void config_key_parse(struct config_key *, const char *);
+
typedef int (*config_fn_t)(const char *, const char *, void *);
extern int git_default_config(const char *, const char *, void *);
extern int git_config_from_file(config_fn_t fn, const char *, void *);
diff --git a/config.c b/config.c
index 7b444b6..7b8df3e 100644
--- a/config.c
+++ b/config.c
@@ -18,6 +18,7 @@ typedef struct config_file {
int eof;
struct strbuf value;
struct strbuf var;
+ struct strbuf key_parse_buf;
} config_file;
static config_file *cf;
@@ -899,6 +900,7 @@ int git_config_from_file(config_fn_t fn, const char
*filename, void *data)
top.eof = 0;
strbuf_init(top.value, 1024);
strbuf_init(top.var, 1024);
+ strbuf_init(top.key_parse_buf, 1024);
cf = top;
ret = git_parse_file(fn, data);
@@ -906,6 +908,7 @@ int git_config_from_file(config_fn_t fn, const char
*filename, void *data)
/* pop config-file parsing state stack */
strbuf_release(top.value);
strbuf_release(top.var);
+ strbuf_release(top.key_parse_buf);
cf = top.prev;
fclose(f);
@@ -1667,3 +1670,35 @@ int config_error_nonbool(const char *var)
{
return error(Missing value for '%s', var);
}
+
+void config_key_parse(struct config_key *key, const char *var)
+{
+ /*
+* We want to use a static buffer so the caller does not have to worry
+* about memory ownership. But since config parsing can happen
+* recursively, we must use storage from the stack of config files.
+*/
+ struct strbuf *sb = cf-key_parse_buf;
+ char *dot;
+ char *rdot;
+
+ strbuf_reset(sb);
+ strbuf_addstr(sb, var);
+
+ dot = strchr(sb-buf, '.');
+ rdot = strrchr(sb-buf, '.');
+ /* Should never happen because our keys come from git_parse_file. */
+ if (!dot)
+ die(BUG: config_key_parse was fed a bogus key);
+ key-section = sb-buf;
+ *dot = '\0';
+ key-key = rdot + 1;
+
+ if (rdot == dot)
+ key-subsection = NULL;
+ else {
+ *rdot = '\0';
+ key-subsection = dot + 1;
+ }
+
+}
diff --git a/submodule.c b/submodule.c
index 2f55436..4894718 100644
--- a/submodule.c
+++ b/submodule.c
@@ -11,9 +11,9 @@
#include sha1-array.h
#include argv-array.h
-static struct string_list config_name_for_path;
-static struct string_list config_fetch_recurse_submodules_for_name;
-static struct string_list config_ignore_for_name;
+static struct string_list config_name_for_path = STRING_LIST_INIT_DUP;
+static struct string_list config_fetch_recurse_submodules_for_name =
STRING_LIST_INIT_DUP;
+static struct string_list config_ignore_for_name = STRING_LIST_INIT_DUP;
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
static struct string_list changed_submodule_paths;
static int initialized_fetch_ref_tips;
@@ -126,47 +126,38 @@ int parse_submodule_config_option(const char *var, const
char *value)
int parse_submodule_config_option(const char *var, const char *value)
{
- int len;
+ struct config_key key;
struct string_list_item *config;
- struct strbuf submodname = STRBUF_INIT;
- var += 10; /* Skip
Re: [PATCH 1/6] config: add helper function for parsing key names
Jeff King p...@peff.net writes:
The config callback functions get keys of the general form:
section.subsection.key
(where the subsection may be contain arbitrary data, or may
be missing). For matching keys without subsections, it is
simple enough to call strcmp. Matching keys with
subsections is a little more complicated, and each callback
does it in an ad-hoc way, usually involving error-prone
pointer arithmetic.
Let's provide a helper that keeps the pointer arithmetic all
in one place.
Signed-off-by: Jeff King p...@peff.net
---
No users yet; they come in future patches.
cache.h | 15 +++
config.c | 33 +
2 files changed, 48 insertions(+)
diff --git a/cache.h b/cache.h
index c257953..14003b8 100644
--- a/cache.h
+++ b/cache.h
@@ -1164,6 +1164,21 @@ extern int git_config_include(const char *name, const
char *value, void *data);
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void
*data);
+/*
+ * Match and parse a config key of the form:
+ *
+ * section.(subsection.)?key
+ *
+ * (i.e., what gets handed to a config_fn_t). The caller provides the
section;
+ * we return -1 if it does not match, 0 otherwise. The subsection and key
+ * out-parameters are filled by the function (and subsection is NULL if it is
+ * missing).
+ */
+extern int match_config_key(const char *var,
+ const char *section,
+ const char **subsection, int *subsection_len,
+ const char **key);
+
I agree with Jonathan about the naming s/match/parse/.
After looking at the callers in your later patches, I think the
counted interface to subsection is probably fine. The caller can
check !subsection to see if it is a two- or three- level name, and
if (parse_config_key(var, submodule, name, namelen, key) 0 ||
!name)
return 0;
is very easy to follow (that is the result of your 5th step).
--
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] clone: do not export and unexport GIT_CONFIG
Earlier, dc87183 (use GIT_CONFIG only in git config, not other
programs, 2008-06-30) made sure that the environment variable is
never used outside git config, but git clone, after creating a
directory for the new repository and until the init_db() function
populates its .git/ directory, exported the variable for no good
reason. No hook will run from init_db() and more importantly no
hook can run until init_db() finishes creation of the new
repository, so it cannot be used by any invocation of git config
by definition.
Stop doing the useless export/unexport.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clone.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index a4d8d25..6f0c1c6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -714,8 +714,6 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
atexit(remove_junk);
sigchain_push_common(remove_junk_on_signal);
- setenv(CONFIG_ENVIRONMENT, mkpath(%s/config, git_dir), 1);
-
if (safe_create_leading_directories_const(git_dir) 0)
die(_(could not create leading directories of '%s'), git_dir);
@@ -732,13 +730,6 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
init_db(option_template, INIT_DB_QUIET);
write_config(option_config);
- /*
-* At this point, the config exists, so we do not need the
-* environment variable. We actually need to unset it, too, to
-* re-enable parsing of the global configs.
-*/
- unsetenv(CONFIG_ENVIRONMENT);
-
git_config(git_default_config, NULL);
if (option_bare) {
--
1.8.1.407.g91cb4ac
--
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] remote-hg: store converted URL
Max Horn m...@quendi.de writes: From: Felipe Contreras felipe.contre...@gmail.com Mercurial might convert the URL to something more appropriate, like an absolute path. What it is converted *TO* is fairly clear with , like an ..., but from the first reading it was unclear to me what it is converted *FROM* and *WHEN* the conversion happens. Do you mean that the user gives git clone an URL ../hg-repo via the command line (e.g. the argument to git clone is spelled something like hg::../hg-repo), and that ../hg-repo is rewritten to something else (an absolute path, e.g. /srv/project/hg-repo)? Lets store that instead of the original URL, which won't work from a different working directory if it's relative. What is lacking from this description is why it even needs to work from a different working directory. I am guessing that remote-hg later creates a hidden Hg repository or something in a different place and still tries to use the URL to interact with the upstream, and that is what breaks, but with only the above description without looking at your original report, people who will read the git log output and find this change will not be able to tell why this was needed, I am afraid. Of course, the above guess of mine may even be wrong, but then that is yet another reason that the log needs to explain the change better. Suggested-by: Max Horn m...@quendi.de Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Max Horn m...@quendi.de --- For a discussion of the problem, see also http://article.gmane.org/gmane.comp.version-control.git/210250 I do not see any discussion; only your problem report. Was this work done outside the list? I just want to make sure this patch is not something Felipe did not want to sign off for whatever reason but you are passing it to the list as a patch signed off by him. -- 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/3] config: Introduce diff.algorithm variable
Michal Privoznik mpriv...@redhat.com writes:
Some users or projects prefer different algorithms over others, e.g.
patience over myers or similar. However, specifying appropriate
argument every time diff is to be used is impractical. Moreover,
creating an alias doesn't play nicely with other tools based on diff
(git-show for instance). Hence, a configuration variable which is able
to set specific algorithm is needed. For now, these four values are
accepted: 'myers' (which has the same effect as not setting the config
variable at all), 'minimal', 'patience' and 'histogram'.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Documentation/diff-config.txt | 17 +
contrib/completion/git-completion.bash | 1 +
diff.c | 23 +++
3 files changed, 41 insertions(+)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 4314ad0..be31276 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -155,3 +155,20 @@ diff.tool::
kompare. Any other value is treated as a custom diff tool,
and there must be a corresponding `difftool.tool.cmd`
option.
+
+diff.algorithm::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use patience diff algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to support
+ low-occurrence common elements.
+--
++
Sounds sensible.
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 383518c..33e25dc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1839,6 +1839,7 @@ _git_config ()
diff.suppressBlankEmpty
diff.tool
diff.wordRegex
+ diff.algorithm
difftool.
difftool.prompt
fetch.recurseSubmodules
diff --git a/diff.c b/diff.c
index 732d4c2..ddae5c4 100644
--- a/diff.c
+++ b/diff.c
@@ -36,6 +36,7 @@ static int diff_no_prefix;
static int diff_stat_graph_width;
static int diff_dirstat_permille_default = 30;
static struct diff_options default_diff_options;
+static long diff_algorithm = 0;
Please do not initialize a static explicitly to a zero and instead
let BSS do its thing.
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -143,6 +144,20 @@ static int git_config_rename(const char *var, const char
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
+static long parse_algorithm_value(const char *value)
+{
+ if (!value || !strcasecmp(value, myers))
+ return 0;
+ else if (!strcasecmp(value, minimal))
+ return XDF_NEED_MINIMAL;
+ else if (!strcasecmp(value, patience))
+ return XDF_PATIENCE_DIFF;
+ else if (!strcasecmp(value, histogram))
+ return XDF_HISTOGRAM_DIFF;
+ else
+ return -1;
+}
+
/*
* These are to give UI layer defaults.
* The core-level commands such as git-diff-files should
@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (!strcmp(var, diff.algorithm)) {
+ diff_algorithm = parse_algorithm_value(value);
+ if (diff_algorithm 0)
+ return -1;
+ return 0;
+ }
+
if (git_color_config(var, value, cb) 0)
return -1;
@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)
options-add_remove = diff_addremove;
options-use_color = diff_use_color_default;
options-detect_rename = diff_detect_rename_default;
+ options-xdl_opts |= diff_algorithm;
if (diff_no_prefix) {
options-a_prefix = options-b_prefix = ;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] diff: Introduce --diff-algorithm command line option
Michal Privoznik mpriv...@redhat.com writes:
Since command line options have higher priority than config file
variables and taking previous commit into account, we need a way
how to specify myers algorithm on command line.
Yes. We cannot stop at [2/3] without this patch.
However,
inventing `--myers` is not the right answer. We need far more
general option, and that is `--diff-algorithm`.
Yes, --diff-algorithm=default would let people defeat a configured
algo without knowing how exactly to spell myers.
The older options
(`--minimal`, `--patience` and `--histogram`) are kept for
backward compatibility.
That is phrasing it too strongly to be acceptable.
People who do not care any configuration can keep using --histogram
without having to retrain their fingers to type a much longer form
you introduced (i.e. --diff-algorithm=histogram). It is and will
always be just as valid a way to ask for --histogram as the new
longhand is.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Documentation/diff-options.txt | 22 ++
contrib/completion/git-completion.bash | 11 +++
diff.c | 12 +++-
diff.h | 2 ++
merge-recursive.c | 6 ++
5 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 39f2c50..4091f52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -45,6 +45,9 @@ ifndef::git-format-patch[]
Synonym for `-p --raw`.
endif::git-format-patch[]
+The next three options are kept for compatibility reason. You should use the
+`--diff-algorithm` option instead.
+
Drop this.
@@ -55,6 +58,25 @@ endif::git-format-patch[]
--histogram::
Generate a diff using the histogram diff algorithm.
+--diff-algorithm={patience|minimal|histogram|myers}::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use patience diff algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to support
+ low-occurrence common elements.
+--
++
+You should prefer this option over the `--minimal`, `--patience` and
+`--histogram` which are kept just for backwards compatibility.
Drop the last one, and replace it with something like:
If you configured diff.algorithm to a non-default value and
want to use the default one, you have to use this form and
choose myers, i.e. --diff-algorithm=myers, as we do not have
a short-and-sweet --myers option.
(but the above is a bit too verbose, so please shorten it appropriately).
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 33e25dc..d592cf9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1021,6 +1021,8 @@ _git_describe ()
__gitcomp_nl $(__git_refs)
}
+__git_diff_algorithms=myers minimal patience histogram
+
__git_diff_common_options=--stat --numstat --shortstat --summary
--patch-with-stat --name-only --name-status --color
--no-color --color-words --no-renames --check
@@ -1035,6 +1037,7 @@ __git_diff_common_options=--stat --numstat --shortstat
--summary
--raw
--dirstat --dirstat= --dirstat-by-file
--dirstat-by-file= --cumulative
+ --diff-algorithm=
_git_diff ()
@@ -1042,6 +1045,10 @@ _git_diff ()
__git_has_doubledash return
case $cur in
+ --diff-algorithm=*)
+ __gitcomp $__git_diff_algorithms
${cur##--diff-algorithm=}
+ return
+ ;;
--*)
__gitcomp --cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs --no-index
@@ -2114,6 +2121,10 @@ _git_show ()
${cur#*=}
return
;;
+ --diff-algorithm=*)
+ __gitcomp $__git_diff_algorithms
${cur##--diff-algorithm=}
+ return
+ ;;
--*)
__gitcomp --pretty= --format= --abbrev-commit --oneline
$__git_diff_common_options
diff --git a/diff.c b/diff.c
index ddae5c4..6418076 100644
--- a/diff.c
+++ b/diff.c
@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
-static long parse_algorithm_value(const char *value)
+long parse_algorithm_value(const char *value)
{
if (!value || !strcasecmp(value, myers))
return 0;
@@ -3633,6 +3633,16 @@ int
Re: [PATCH 00/14] Remove unused code from imap-send.c
Michael Haggerty mhag...@alum.mit.edu writes: On 01/14/2013 07:57 AM, Jonathan Nieder wrote: Michael Haggerty wrote: imap-send.c | 286 +--- 1 file changed, 39 insertions(+), 247 deletions(-) See my replies for comments on patches 1, 6, 9, 11, and 12. The rest are Reviewed-by: Jonathan Nieder jrnie...@gmail.com The series is tasteful and easy to follow and it's hard to argue with the resulting code reduction. Thanks for a pleasant read. Thanks for your careful review. I will re-roll the patch series as soon as I get the chance. Thanks, all of you. The numbers and the graph look very nice indeed ;-) -- 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: Error:non-monotonic index after failed recursive sed command
Johannes Sixt j.s...@viscovery.net writes:
Am 1/14/2013 12:40, schrieb George Karpenkov:
I've managed to corrupt my very valuable repository with a recursive
sed which went wrong.
I wanted to convert all tabs to spaces with the following command:
find ./ -name '*.*' -exec sed -i 's/\t//g' {} \;
I think that has changed not only the files in the repo, but the data
files in .git directory itself. As a result, my index became
corrupted, and almost every single command dies:
git log
error: non-monotonic index
..git/objects/pack/pack-314b1944adebea645526b6724b2044c1313241f5.idx
error: non-monotonic index
..git/objects/pack/pack-75c95b0defe1968b61e4f4e1ab7040d35110bfdc.idx
...
Try the reverse edit:
find .git -name '*.*' -exec sed -i 's//\t/g' {} \;
Remove .git/index; it can be reconstructed (of course, assuming you
started all this with a clean index; if not, you lose the staged changes).
Everybody seems to be getting an impression that .idx is the only
thing that got corrupt. Where does that come from?
I do not see anything that prevents the original command line from
touching *.pack files. Loose objects may have been left unmolested
as their names do not match '*.*', though.
--
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] Possible bug in `remote set-url --add --push`
Michael J Gruber g...@drmicha.warpmail.net writes: It seems to me that everything works as designed, and that the man page talk about push URLs can be read in two ways,... Hmph, but I had an impression that Jardel's original report was that one of the --add --pushurl was not adding but was replacing. If that was a false alarm, everything you said makes sense to me. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] diff: Introduce --diff-algorithm command line option
Junio C Hamano gits...@pobox.com writes:
Michal Privoznik mpriv...@redhat.com writes:
...
diff --git a/merge-recursive.c b/merge-recursive.c
index d882060..53d8feb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2068,6 +2068,12 @@ int parse_merge_opt(struct merge_options *o, const
char *s)
o-xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);
else if (!strcmp(s, histogram))
o-xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);
+else if (!strcmp(s, diff-algorithm=)) {
+long value = parse_algorithm_value(s+15);
+if (value 0)
+return -1;
+o-xdl_opts |= value;
Isn't this new hunk wrong? DIFF_WITH_ALG() removes the previously
chosen algorithm choice before OR'ing the new one in, so that
diff --histogram --patience
would not end up with a value PATIENCE|HISTOGRAM OR'ed together in
the algo field.
I misspoke; this is not diff, but merge-recursive. The issue
may be the same here, though.
--
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: Error:non-monotonic index after failed recursive sed command
Junio C Hamano gits...@pobox.com writes: Everybody seems to be getting an impression that .idx is the only thing that got corrupt. Where does that come from? It's the only thing that appear in the error message. This does not imply that it is the only corrupt thing, but gives a little hope that it may still be the case. Actually, I thought the read-only protection should have protected files in object/ directory, but a little testing shows that sed -i gladly accepts to modify read-only files (technically, it does not modify it, but creates a temporary file with the new content, and then renames it to the new location). So, the hope that pack files are uncorrupted is rather thin unfortunately. -- 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] builtin/reset.c: Fix a sparse warning
In particular, sparse issues an symbol not declared. Should it be
static? warning for the 'parse_args' function. Since this function
does not require greater than file visibility, we suppress this
warning by simply adding the static modifier to it's decalaration.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Martin,
When you re-roll your reset patches (branch 'mz/reset-misc'), could
you please squash this into commit b24b3654 (reset.c: extract function
for parsing arguments, 09-01-2013). Note that this patch is on top of
the pu branch (@75ea4ed3), which includes Junio's style fix patch
(commit 9f45c39f, [SQUASH???] style fix, 10-01-2013).
Thanks!
ATB,
Ramsay Jones
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index a37044e..c69f9da 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -171,7 +171,7 @@ static void die_if_unmerged_cache(int reset_type)
}
-const char **parse_args(int argc, const char **argv, const char *prefix, const
char **rev_ret)
+static const char **parse_args(int argc, const char **argv, const char
*prefix, const char **rev_ret)
{
const char *rev = HEAD;
unsigned char unused[20];
--
1.8.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] builtin/reset.c: Fix a sparse warning
On Mon, Jan 14, 2013 at 11:28 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: In particular, sparse issues an symbol not declared. Should it be static? warning for the 'parse_args' function. Since this function does not require greater than file visibility, we suppress this warning by simply adding the static modifier to it's decalaration. Oops, how did I miss that? Will fix in re-roll. Thanks. Martin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff
Even though --patience was already there, we missed --minimal and --histogram for some reason. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index a4c48e1..383518c 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1031,7 +1031,7 @@ __git_diff_common_options=--stat --numstat --shortstat --summary --no-ext-diff --no-prefix --src-prefix= --dst-prefix= --inter-hunk-context= - --patience + --patience --histogram --minimal --raw --dirstat --dirstat= --dirstat-by-file --dirstat-by-file= --cumulative -- 1.8.0.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] config: Introduce diff.algorithm variable
Some users or projects prefer different algorithms over others, e.g.
patience over myers or similar. However, specifying appropriate
argument every time diff is to be used is impractical. Moreover,
creating an alias doesn't play nicely with other tools based on diff
(git-show for instance). Hence, a configuration variable which is able
to set specific algorithm is needed. For now, these four values are
accepted: 'myers' (which has the same effect as not setting the config
variable at all), 'minimal', 'patience' and 'histogram'.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Documentation/diff-config.txt | 17 +
contrib/completion/git-completion.bash | 1 +
diff.c | 23 +++
3 files changed, 41 insertions(+)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 4314ad0..be31276 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -155,3 +155,20 @@ diff.tool::
kompare. Any other value is treated as a custom diff tool,
and there must be a corresponding `difftool.tool.cmd`
option.
+
+diff.algorithm::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use patience diff algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to support
+ low-occurrence common elements.
+--
++
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 383518c..33e25dc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1839,6 +1839,7 @@ _git_config ()
diff.suppressBlankEmpty
diff.tool
diff.wordRegex
+ diff.algorithm
difftool.
difftool.prompt
fetch.recurseSubmodules
diff --git a/diff.c b/diff.c
index 732d4c2..e9a7e4d 100644
--- a/diff.c
+++ b/diff.c
@@ -36,6 +36,7 @@ static int diff_no_prefix;
static int diff_stat_graph_width;
static int diff_dirstat_permille_default = 30;
static struct diff_options default_diff_options;
+static long diff_algorithm;
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -143,6 +144,20 @@ static int git_config_rename(const char *var, const char
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
+static long parse_algorithm_value(const char *value)
+{
+ if (!value || !strcasecmp(value, myers))
+ return 0;
+ else if (!strcasecmp(value, minimal))
+ return XDF_NEED_MINIMAL;
+ else if (!strcasecmp(value, patience))
+ return XDF_PATIENCE_DIFF;
+ else if (!strcasecmp(value, histogram))
+ return XDF_HISTOGRAM_DIFF;
+ else
+ return -1;
+}
+
/*
* These are to give UI layer defaults.
* The core-level commands such as git-diff-files should
@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char *value,
void *cb)
return 0;
}
+ if (!strcmp(var, diff.algorithm)) {
+ diff_algorithm = parse_algorithm_value(value);
+ if (diff_algorithm 0)
+ return -1;
+ return 0;
+ }
+
if (git_color_config(var, value, cb) 0)
return -1;
@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)
options-add_remove = diff_addremove;
options-use_color = diff_use_color_default;
options-detect_rename = diff_detect_rename_default;
+ options-xdl_opts |= diff_algorithm;
if (diff_no_prefix) {
options-a_prefix = options-b_prefix = ;
--
1.8.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] diff: Introduce --diff-algorithm command line option
Since command line options have higher priority than config file
variables and taking previous commit into account, we need a way
how to specify myers algorithm on command line. However,
inventing `--myers` is not the right answer. We need far more
general option, and that is `--diff-algorithm`.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Documentation/diff-options.txt | 23 +++
contrib/completion/git-completion.bash | 11 +++
diff.c | 12 +++-
diff.h | 2 ++
merge-recursive.c | 9 +
5 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 39f2c50..01b97b3 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -55,6 +55,29 @@ endif::git-format-patch[]
--histogram::
Generate a diff using the histogram diff algorithm.
+--diff-algorithm={patience|minimal|histogram|myers}::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use patience diff algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to support
+ low-occurrence common elements.
+--
++
+For instance, if you configured diff.algorithm variable to a
+non-default value and want to use the default one, then you
+have to use `--diff-algorithm=myers` option.
+
+You should prefer this option over the `--minimal`, `--patience` and
+`--histogram` which are kept just for backwards compatibility.
+
--stat[=width[,name-width[,count]]]::
Generate a diffstat. By default, as much space as necessary
will be used for the filename part, and the rest for the graph
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 33e25dc..d592cf9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1021,6 +1021,8 @@ _git_describe ()
__gitcomp_nl $(__git_refs)
}
+__git_diff_algorithms=myers minimal patience histogram
+
__git_diff_common_options=--stat --numstat --shortstat --summary
--patch-with-stat --name-only --name-status --color
--no-color --color-words --no-renames --check
@@ -1035,6 +1037,7 @@ __git_diff_common_options=--stat --numstat --shortstat
--summary
--raw
--dirstat --dirstat= --dirstat-by-file
--dirstat-by-file= --cumulative
+ --diff-algorithm=
_git_diff ()
@@ -1042,6 +1045,10 @@ _git_diff ()
__git_has_doubledash return
case $cur in
+ --diff-algorithm=*)
+ __gitcomp $__git_diff_algorithms
${cur##--diff-algorithm=}
+ return
+ ;;
--*)
__gitcomp --cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs --no-index
@@ -2114,6 +2121,10 @@ _git_show ()
${cur#*=}
return
;;
+ --diff-algorithm=*)
+ __gitcomp $__git_diff_algorithms
${cur##--diff-algorithm=}
+ return
+ ;;
--*)
__gitcomp --pretty= --format= --abbrev-commit --oneline
$__git_diff_common_options
diff --git a/diff.c b/diff.c
index e9a7e4d..3e021d5 100644
--- a/diff.c
+++ b/diff.c
@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
-static long parse_algorithm_value(const char *value)
+long parse_algorithm_value(const char *value)
{
if (!value || !strcasecmp(value, myers))
return 0;
@@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
options-xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, --histogram))
options-xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
+ else if (!prefixcmp(arg, --diff-algorithm=)) {
+ long value = parse_algorithm_value(arg+17);
+ if (value 0)
+ return error(option diff-algorithm accepts \myers\,
+\minimal\, \patience\ and
\histogram\);
+ /* clear out previous settings */
+ DIFF_XDL_CLR(options, NEED_MINIMAL);
+ options-xdl_opts = ~XDF_DIFF_ALGORITHM_MASK;
+ options-xdl_opts |= value;
+ }
/* flags options */
else if (!strcmp(arg, --binary)) {
diff --git a/diff.h b/diff.h
index a47bae4..54c2590 100644
--- a/diff.h
+++ b/diff.h
@@
[PATCH v2 0/3] Rework git-diff algorithm selection
It's been a while I was trying to get this in. Recently, I realized how important this is. Please keep me CC'ed as I am not subscribed to the list. diff to v1: -Junio's suggestions worked in Michal Privoznik (3): git-completion.bash: Autocomplete --minimal and --histogram for git-diff config: Introduce diff.algorithm variable diff: Introduce --diff-algorithm command line option Documentation/diff-config.txt | 17 + Documentation/diff-options.txt | 23 +++ contrib/completion/git-completion.bash | 14 +- diff.c | 33 + diff.h | 2 ++ merge-recursive.c | 9 + 6 files changed, 97 insertions(+), 1 deletion(-) -- 1.8.0.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Announcing git-reparent
Piotr Krukowiecki piotr.krukowie...@gmail.com writes: Just wondering, is the result different than something like git checkout commit_to_reparent cp -r * ../snapshot/ git reset --hard new_parent rm -r * cp -r ../snapshot/* . git add -A I think you are looking for git reset --soft new_parent, which keeps both the index and the working tree intact. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- 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: Announcing git-reparent
On Mon, Jan 14, 2013 at 3:03 AM, Piotr Krukowiecki piotr.krukowie...@gmail.com wrote: Just wondering, is the result different than something like git checkout commit_to_reparent cp -r * ../snapshot/ git reset --hard new_parent rm -r * cp -r ../snapshot/* . git add -A (assumes 1 parent, does not cope with .dot files, and has probably other small problems) The result is similar, but your script would also lose the commit message and author. I think the following would do exactly as my script does (untested): git checkout commit_to_reparent git branch tmp git reset --soft new_parent git commit -C tmp git branch -D tmp I actually contemplated using the above method in my script, rather than git-commit-tree and git-reset. In the end, I decided to stick with my original approach because it does not create any intermediate state; either an early command fails and nothing changes, or the git reset works and everything is done. Using the above might be cleaner for the --edit flag since it allows the git-commit cleanup of the commit message, but this would require much more careful error handling, and might make the reflog uglier. I'd be interested to hear a git expert's opinion on the choice. -- 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 2/3] config: Introduce diff.algorithm variable
Michal Privoznik mpriv...@redhat.com writes:
+static long parse_algorithm_value(const char *value)
+{
+ if (!value || !strcasecmp(value, myers))
+ return 0;
[diff]
algorithm
should probably error out. Also it is rather unusual to parse the
keyword values case insensitively.
+ else if (!strcasecmp(value, minimal))
+ return XDF_NEED_MINIMAL;
+ else if (!strcasecmp(value, patience))
+ return XDF_PATIENCE_DIFF;
+ else if (!strcasecmp(value, histogram))
+ return XDF_HISTOGRAM_DIFF;
+ else
+ return -1;
+}
+
/*
* These are to give UI layer defaults.
* The core-level commands such as git-diff-files should
@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (!strcmp(var, diff.algorithm)) {
+ diff_algorithm = parse_algorithm_value(value);
+ if (diff_algorithm 0)
+ return -1;
+ return 0;
+ }
+
if (git_color_config(var, value, cb) 0)
return -1;
@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)
options-add_remove = diff_addremove;
options-use_color = diff_use_color_default;
options-detect_rename = diff_detect_rename_default;
+ options-xdl_opts |= diff_algorithm;
if (diff_no_prefix) {
options-a_prefix = options-b_prefix = ;
--
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 3/3] diff: Introduce --diff-algorithm command line option
Michal Privoznik mpriv...@redhat.com writes:
+--diff-algorithm={patience|minimal|histogram|myers}::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use patience diff algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to support
+ low-occurrence common elements.
+--
++
+For instance, if you configured diff.algorithm variable to a
+non-default value and want to use the default one, then you
+have to use `--diff-algorithm=myers` option.
+
+You should prefer this option over the `--minimal`, `--patience` and
+`--histogram` which are kept just for backwards compatibility.
Much better; I'd drop the last paragraph, though.
I also think we really should consider default synonym for
whichever happens to be the built-in default (currently myers).
diff --git a/diff.c b/diff.c
index e9a7e4d..3e021d5 100644
--- a/diff.c
+++ b/diff.c
@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char
*value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
-static long parse_algorithm_value(const char *value)
+long parse_algorithm_value(const char *value)
{
if (!value || !strcasecmp(value, myers))
return 0;
@@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
options-xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, --histogram))
options-xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
+ else if (!prefixcmp(arg, --diff-algorithm=)) {
+ long value = parse_algorithm_value(arg+17);
+ if (value 0)
+ return error(option diff-algorithm accepts \myers\,
+ \minimal\, \patience\ and
\histogram\);
+ /* clear out previous settings */
+ DIFF_XDL_CLR(options, NEED_MINIMAL);
+ options-xdl_opts = ~XDF_DIFF_ALGORITHM_MASK;
+ options-xdl_opts |= value;
This makes me wonder if other places that use DIFF_WITH_ALG() also
need to worry about clearing NEED_MINIMAL?
--
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] am: invoke perl's strftime in C locale
Dmitry V. Levin l...@altlinux.org writes:
This fixes hg patch format support for locales other than C and en_*,
see https://bugzilla.altlinux.org/show_bug.cgi?id=28248
Signed-off-by: Dmitry V. Levin l...@altlinux.org
---
Thanks.
The reference URL is not very friendly, and you should be able to
state it here on a single line in English instead, I think.
The patch looks correct, though.
git-am.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index c682d34..64b88e4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,7 @@ split_patches () {
# Since we cannot guarantee that the commit message is
in
# git-friendly format, we put no Subject: line and just
consume
# all of the message as the body
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN {
$subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print
; }
elsif (/^\# Date /) {
--
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] Make git selectively and conditionally ignore certain stat fields
Robin Rosenberg robin.rosenb...@dewire.com writes: diff --git a/read-cache.c b/read-cache.c index fda78bc..f7fe15d 100644 --- a/read-cache.c +++ b/read-cache.c @@ -197,8 +197,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st) } if (ce-ce_mtime.sec != (unsigned int)st-st_mtime) changed |= MTIME_CHANGED; - if (trust_ctime ce-ce_ctime.sec != (unsigned int)st-st_ctime) - changed |= CTIME_CHANGED; + if ((trust_ctime || ((check_nonzero_statCHECK_NONZERO_STAT_CTIME) ce-ce_ctime.sec))) One SP is required on each side of a binary operator; please have one after check_nonzero_stat and after the after it. I wonder if we should lose the trust_ctime variable and use this check_nonzero_stat bitset exclusively, provided that this were a good direction to go? -- 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] Make git selectively and conditionally ignore certain stat fields
Robin Rosenberg robin.rosenb...@dewire.com writes:
@@ -566,6 +566,31 @@ static int git_default_core_config(const char *var,
const char *value)
trust_ctime = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, core.ignorezerostat)) {
+ char *copy, *tok;
+ git_config_string(copy, core.ignorezerostat, value);
+ check_nonzero_stat = CHECK_NONZERO_STAT_MASK;
+ tok = strtok(value, ,);
+ while (tok) {
+ if (strcasecmp(tok, uid) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_UID;
+ else if (strcasecmp(tok, gid) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_GID;
+ else if (strcasecmp(tok, ctime) == 0) {
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_CTIME;
+ trust_ctime = 0;
+ } else if (strcasecmp(tok, ino) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_INO;
+ else if (strcasecmp(tok, dev) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_DEV;
+ else
+ die_bad_config(var);
+ tok = strtok(NULL, ,);
+ }
+ if (check_nonzero_stat = CHECK_NONZERO_STAT_MASK)
+ die_bad_config(var);
+ free(copy);
+ }
Also I am getting these:
config.c: In function 'git_default_core_config':
config.c:571: error: passing argument 1 of 'git_config_string' from
incompatible pointer type
config.c:540: note: expected 'const char **' but argument is of type 'char **'
config.c:573: error: passing argument 1 of 'strtok' discards qualifiers from
pointer target type
--
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
What's cooking in git.git (Jan 2013, #06; Mon, 14)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. As usual, this cycle is expected to last for 8 to 10 weeks, with a preview -rc0 sometime in the middle of next month. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * jc/cvsimport-upgrade (2013-01-14) 8 commits - t9600: adjust for new cvsimport - t9600: further prepare for sharing - cvsimport-3: add a sample test - cvsimport: make tests reusable for cvsimport-3 - cvsimport: start adding cvsps 3.x support - cvsimport: introduce a version-switch wrapper - cvsimport: allow setting a custom cvsps (2.x) program name - Makefile: add description on PERL/PYTHON_PATH The most important part of this series is the addition of the new cvsimport by Eric Raymond that works with cvsps 3.x. Given some distros have inertia to be conservative, Git with cvsimport that does not work with both 3.x will block adoption of cvsps 3.x by them, and shipping Git with cvsimport that does not work with cvsps 2.x will block such a version of Git, so we'll do the proven both old and new are available, but we aim to deprecate and remove the old one in due time strategy that we used successfully in the past. * as/pre-push-hook (2013-01-14) 3 commits - Add sample pre-push hook script - push: Add support for pre-push hooks - hooks: Add function to check if a hook exists Add an extra hook so that git push that is run without making sure what is being pushed is sane can be checked and rejected (as opposed to the user deciding not pushing). * dl/am-hg-locale (2013-01-14) 1 commit - am: invoke perl's strftime in C locale Datestamp recorded in Hg format patch was reformatted incorrectly to an e-mail looking date using locale dependant strftime, causing patch application to fail. * jk/config-parsing-cleanup (2013-01-14) 7 commits - [DONTMERGE] reroll coming - submodule: simplify memory handling in config parsing - submodule: use match_config_key when parsing config - userdiff: drop parse_driver function - convert some config callbacks to match_config_key - archive-tar: use match_config_key when parsing config - config: add helper function for parsing key names Expecting a reroll. * mp/diff-algo-config (2013-01-14) 3 commits - diff: Introduce --diff-algorithm command line option - config: Introduce diff.algorithm variable - git-completion.bash: Autocomplete --minimal and --histogram for git-diff Add diff.algorithm configuration so that the user does not type diff --histogram. Expecting a reroll. * ph/rebase-preserve-all-merges (2013-01-14) 1 commit - rebase --preserve-merges: keep all merge commits including empty ones An earlier change to add --keep-empty option broke git rebase --preserve-merges and lost merge commits that end up being the same as its parent. Will merge to 'next'. * rs/archive-tar-config-parsing-fix (2013-01-14) 1 commit - archive-tar: fix sanity check in config parsing Configuration parsing for tar.* configuration variables were broken; Peff's config parsing clean-up topic will address the same breakage, so this may be superseded by that other topic. * rs/pretty-use-prefixcmp (2013-01-14) 1 commit - pretty: use prefixcmp instead of memcmp on NUL-terminated strings Will merge to 'next'. -- [Graduated to master] * ap/status-ignored-in-ignored-directory (2013-01-07) 3 commits (merged to 'next' on 2013-01-10 at 20f7476) + status: always report ignored tracked directories (merged to 'next' on 2013-01-07 at 2a20b19) + git-status: Test --ignored behavior + dir.c: Make git-status --ignored more consistent Output from git status --ignored showed an unexpected interaction with --untracked. * fc/remote-testgit-feature-done (2012-10-29) 1 commit (merged to 'next' on 2013-01-10 at 3132a60) + remote-testgit: properly check for errors In the longer term, tightening rules is a good thing to do, and because nobody who has worked in the remote helper area seems to be interested in reviewing this, I would assume they do not think such a retroactive tightening will affect their remote helpers. So let's advance this topic to see what happens. * jc/blame-no-follow (2012-09-21) 2 commits (merged to 'next' on 2013-01-10 at 201c7f4) + blame: pay attention to --no-follow + diff: accept --no-follow option Teaches --no-follow option to git blame to disable its whole-file rename detection. * jc/format-patch-reroll (2013-01-03) 9 commits (merged to 'next' on 2013-01-07 at 0e007e6) + format-patch: give --reroll-count a short synonym -v + format-patch: document and test --reroll-count + format-patch: add --reroll-count=$N option + get_patch_filename(): split into two
[PATCH v2] am: invoke perl's strftime in C locale
This fixes hg patch format support for locales other than C and en_*.
Before the change, git-am was making Date: line from hg changeset
metadata according to the current locale, and this line was rejected
later with invalid date format diagnostics because localized date
strings are not supported.
Reported-by: Gleb Fotengauer-Malinovskiy gle...@altlinux.org
Signed-off-by: Dmitry V. Levin l...@altlinux.org
---
v2: replaced unfriendly URL with a short description
git-am.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index c682d34..64b88e4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,7 @@ split_patches () {
# Since we cannot guarantee that the commit message is
in
# git-friendly format, we put no Subject: line and just
consume
# all of the message as the body
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN {
$subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print
; }
elsif (/^\# Date /) {
--
ldv
--
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
git grep performance regression
Hi, I have noticed a performance regression in git grep between v1.8.1 and v1.8.1.1: On the kernel tree: For git 1.8.1: $ time git grep foodsgsg real 0m0.158s user 0m0.290s sys0m0.207s For git 1.8.1.1: $ time /tmp/g/bin/git grep foodsgsg real 0m0.501s user 0m0.707s sys0m0.493s A bisect seems to indicate that it was introduced by 94bc67: commit 94bc671a1f2e8610de475c2494d2763355a99f65 Author: Jean-Noël AVILA avila...@gmail.com Date: Sat Dec 8 21:04:39 2012 +0100 Add directory pattern matching to attributes The manpage of gitattributes says: The rules how the pattern matches paths are the same as in .gitignore files and the gitignore pattern matching has a pattern ending with / for directory matching. This rule is specifically relevant for the 'export-ignore' rule used for git archive. Signed-off-by: Jean-Noel Avila jn.av...@free.fr Signed-off-by: Junio C Hamano gits...@pobox.com Regards -- Ross Lagerwall -- 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 0/3] pre-push hook support
Junio C Hamano gits...@pobox.com writes: Aaron Schrab aa...@schrab.com writes: Main changes since the initial version: * The first patch converts the existing hook callers to use the new find_hook() function. * Information about what is to be pushed is now sent over a pipe rather than passed as command-line parameters. Aaron Schrab (3): hooks: Add function to check if a hook exists push: Add support for pre-push hooks Add sample pre-push hook script Getting much nicer. Thanks. Hmph, t5571 seems to be flaky in that it sometimes fails but passes when run again. Something timing dependent is going on??? -- 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: Error:non-monotonic index after failed recursive sed command
Thanks everyone!
Progress so far:
After executing reverse sed command:
find .git -name '*.*' -exec sed -i 's//\t/g' {} \;
And trying to switch the branch I get:
git checkout X
error: failed to read object 51a980792f26875d00acb79a19f043420f542cfa
at offset 41433013 from
.git/objects/pack/pack-8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack
fatal: packed object 51a980792f26875d00acb79a19f043420f542cfa (stored
in .git/objects/pack/pack-
8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack) is corrupt
So the actual .pack file is corrupt, unfortunately.
On Tue, Jan 15, 2013 at 6:13 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Junio C Hamano gits...@pobox.com writes:
Everybody seems to be getting an impression that .idx is the only
thing that got corrupt. Where does that come from?
It's the only thing that appear in the error message. This does not
imply that it is the only corrupt thing, but gives a little hope that it
may still be the case.
Actually, I thought the read-only protection should have protected
files in object/ directory, but a little testing shows that sed -i
gladly accepts to modify read-only files (technically, it does not
modify it, but creates a temporary file with the new content, and then
renames it to the new location). So, the hope that pack files are
uncorrupted is rather thin unfortunately.
--
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
[RFC/PATCH] ignore memcmp() overreading in bsearch() callback
It appears that memcmp() uses the usual one word at a time
comparison and triggers valgrind in a callback of bsearch() used in
the refname search. I can easily trigger problems in any script
with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v)
without this suppression.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/valgrind/default.supp | 8
1 file changed, 8 insertions(+)
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 0a6724f..032332f 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -49,3 +49,11 @@
Memcheck:Addr4
fun:copy_ref
}
+
+{
+ ignore-memcmp-reading-too-much-in-bsearch-callback
+ Memcheck:Addr4
+ fun:ref_entry_cmp_sslice
+ fun:bsearch
+ fun:search_ref_dir
+}
--
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] Make git selectively and conditionally ignore certain stat fields
- Ursprungligt meddelande - Robin Rosenberg robin.rosenb...@dewire.com writes: diff --git a/read-cache.c b/read-cache.c index fda78bc..f7fe15d 100644 --- a/read-cache.c +++ b/read-cache.c @@ -197,8 +197,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st) } if (ce-ce_mtime.sec != (unsigned int)st-st_mtime) changed |= MTIME_CHANGED; - if (trust_ctime ce-ce_ctime.sec != (unsigned int)st-st_ctime) - changed |= CTIME_CHANGED; + if ((trust_ctime || ((check_nonzero_statCHECK_NONZERO_STAT_CTIME) ce-ce_ctime.sec))) One SP is required on each side of a binary operator; please have one after check_nonzero_stat and after the after it. I wonder if we should lose the trust_ctime variable and use this check_nonzero_stat bitset exclusively, provided that this were a good direction to go? Semantically they're somewhat different. My flags are for ignoring a value when it's not used as indicated by the value zero, while trustctime is for ignoring untrustworthy, non-zero, values. From 1ce4790bf5e: A new configuration variable 'core.trustctime' is introduced to allow ignoring st_ctime information when checking if paths in the working tree has changed, because there are situations where it produces too much false positives. Like when file system crawlers keep changing it when scanning and using the ctime for marking scanned files. (your second mail) Also I am getting these: config.c: In function 'git_default_core_config': config.c:571: error: passing argument 1 of 'git_config_string' from incompatible pointer type config.c:540: note: expected 'const char **' but argument is of type 'char **' config.c:573: error: passing argument 1 of 'strtok' discards qualifiers from pointer target type Different compilers have different defaults. I'm on OS X (mountain lion), or am I missing something? I do get a warning. Am I allowed to modify the value, like strtok does? Seems I missed the opportunity to use the copy rather then the original value. Another thing that I noticed, is that I probably wanto to be able to filter on the precision of timestamps. Again, this i JGit-related. Current JGit has milliseconds precision (max), whereas Git has down to nanosecond precision in timestamps. Newer JGits may get nanoseconds timestamps too, but on current Linux versions JGit gets only integral seconds regardless of file system. Would the names, milli, micro, nano be good for ignoring the tail when zero, or n1..n9 (obviously n2 would be ok too). nN = ignore all but first N nsec digits if they are zero)? -- 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: Error:non-monotonic index after failed recursive sed command
From: George Karpenkov geo...@metaworld.ru
Sent: Monday, January 14, 2013 10:57 PM
Thanks everyone!
Progress so far:
After executing reverse sed command:
find .git -name '*.*' -exec sed -i 's//\t/g' {} \;
Have you counted how many substitutions there are in the pack file(s).
It may be sufficiently small that you can simply try all the possible
combinations of fwd and reverse substitutions. Even if it takes a few
days the computer won't get bored ;-)
And trying to switch the branch I get:
git checkout X
error: failed to read object 51a980792f26875d00acb79a19f043420f542cfa
at offset 41433013 from
.git/objects/pack/pack-8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack
fatal: packed object 51a980792f26875d00acb79a19f043420f542cfa (stored
in .git/objects/pack/pack-
8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack) is corrupt
So the actual .pack file is corrupt, unfortunately.
On Tue, Jan 15, 2013 at 6:13 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Junio C Hamano gits...@pobox.com writes:
Everybody seems to be getting an impression that .idx is the only
thing that got corrupt. Where does that come from?
It's the only thing that appear in the error message. This does not
imply that it is the only corrupt thing, but gives a little hope that
it
may still be the case.
Actually, I thought the read-only protection should have protected
files in object/ directory, but a little testing shows that sed -i
gladly accepts to modify read-only files (technically, it does not
modify it, but creates a temporary file with the new content, and
then
renames it to the new location). So, the hope that pack files are
uncorrupted is rather thin unfortunately.
--
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 v3] Make git selectively and conditionally ignore certain stat fields
Specifically the fields uid, gid, ctime, ino and dev are set to zero
by JGit. Any stat checking by git will then need to check content,
which may be very slow, particularly on Windows. Since mtime and size
is typically enough we should allow the user to tell git to avoid
checking these fields if they are set to zero in the index.
This change introduces a core.ignorezerostat config option where the
user can list the fields to ignore using the names above.
Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
---
Documentation/config.txt | 9 +
cache.h | 8
config.c | 26 ++
environment.c| 1 +
read-cache.c | 29 ++---
5 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..7f34c94 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -235,6 +235,15 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
+core.ignorezerostat::
+ Affects the interpretation of some fields in the index. If
+ unset has no effect. When set to a comma separated list of fields,
+ each of the fields in the index will be excluded from comparison with
+ working tree if the index value is zero. The following fields
+ are recognzed: `uid', `gid', `ctime', `ino' and `dev'. When ctime is
ignored
+ the setting of 'core.trustctime' is overridden by by this config
+ value.
+
core.quotepath::
The commands that output paths (e.g. 'ls-files',
'diff'), when not given the `-z` option, will quote
diff --git a/cache.h b/cache.h
index c257953..524e49a 100644
--- a/cache.h
+++ b/cache.h
@@ -536,6 +536,14 @@ extern int delete_ref(const char *, const unsigned char
*sha1, int delopt);
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
extern int trust_ctime;
+extern int check_nonzero_stat;
+#define CHECK_NONZERO_STAT_UID (10)
+#define CHECK_NONZERO_STAT_GID (11)
+#define CHECK_NONZERO_STAT_CTIME (12)
+#define CHECK_NONZERO_STAT_INO (13)
+#define CHECK_NONZERO_STAT_DEV (14)
+#define CHECK_NONZERO_STAT_MASK ((15)-1)
+
extern int quote_path_fully;
extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
diff --git a/config.c b/config.c
index 7b444b6..6b617bc 100644
--- a/config.c
+++ b/config.c
@@ -566,6 +566,32 @@ static int git_default_core_config(const char *var, const
char *value)
trust_ctime = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, core.ignorezerostat)) {
+ const char *copy;
+ const char *tok;
+ git_config_string(copy, core.ignorezerostat, value);
+ check_nonzero_stat = CHECK_NONZERO_STAT_MASK;
+ tok = strtok((char*)copy, ,);
+ while (tok) {
+ if (strcasecmp(tok, uid) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_UID;
+ else if (strcasecmp(tok, gid) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_GID;
+ else if (strcasecmp(tok, ctime) == 0) {
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_CTIME;
+ trust_ctime = 0;
+ } else if (strcasecmp(tok, ino) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_INO;
+ else if (strcasecmp(tok, dev) == 0)
+ check_nonzero_stat = ~CHECK_NONZERO_STAT_DEV;
+ else
+ die_bad_config(var);
+ tok = strtok(NULL, ,);
+ }
+ if (check_nonzero_stat = CHECK_NONZERO_STAT_MASK)
+ die_bad_config(var);
+ free((char*)copy);
+ }
if (!strcmp(var, core.quotepath)) {
quote_path_fully = git_config_bool(var, value);
diff --git a/environment.c b/environment.c
index 85edd7f..e90b52f 100644
--- a/environment.c
+++ b/environment.c
@@ -13,6 +13,7 @@
int trust_executable_bit = 1;
int trust_ctime = 1;
+int check_nonzero_stat = CHECK_NONZERO_STAT_MASK;
int has_symlinks = 1;
int minimum_abbrev = 4, default_abbrev = 7;
int ignore_case;
diff --git a/read-cache.c b/read-cache.c
index fda78bc..c4226ee 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -197,21 +197,27 @@ static int ce_match_stat_basic(struct cache_entry *ce,
struct stat *st)
}
if (ce-ce_mtime.sec != (unsigned int)st-st_mtime)
changed |= MTIME_CHANGED;
- if (trust_ctime ce-ce_ctime.sec != (unsigned int)st-st_ctime)
- changed |= CTIME_CHANGED;
+ if ((trust_ctime || ((check_nonzero_stat CHECK_NONZERO_STAT_CTIME)
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
Hi Junio, On Mon, 14 Jan 2013, Junio C Hamano wrote: It appears that memcmp() uses the usual one word at a time comparison and triggers valgrind in a callback of bsearch() used in the refname search. I can easily trigger problems in any script with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v) without this suppression. I have no way to replicate that issue, but I take your word for it. With that in mind, here is my ACK. Ciao, Johannes -- 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] Make git selectively and conditionally ignore certain stat fields
Robin Rosenberg robin.rosenb...@dewire.com writes: Semantically they're somewhat different. My flags are for ignoring a value when it's not used as indicated by the value zero, while trustctime is for ignoring untrustworthy, non-zero, values. Yeah, I realized that after writing that message. Another thing that I noticed, is that I probably wanto to be able to filter on the precision of timestamps. Again, this i JGit-related. Current JGit has milliseconds precision (max), whereas Git has down to nanosecond precision in timestamps. Newer JGits may get nanoseconds timestamps too, but on current Linux versions JGit gets only integral seconds regardless of file system. Would the names, milli, micro, nano be good for ignoring the tail when zero, or n1..n9 (obviously n2 would be ok too). nN = ignore all but first N nsec digits if they are zero)? It somehow starts to sound like over-engineering to solve a wrong problem. I'd say a simplistic ignore if zero is stored or even ignore this as one of the systems that shares this file writes crap in it may be sufficient, and if this is a jGit specific issue, it might even make sense to introduce a single configuration variable with string jgit somewhere in its name and bypass the stat field comparison for known-problematic fields, instead of having the user know and list what stat fields need special attention. Is this the user edits in eclipse and then runs 'git status' from the terminal problem? -- 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 0/3] pre-push hook support
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: Aaron Schrab aa...@schrab.com writes: Main changes since the initial version: * The first patch converts the existing hook callers to use the new find_hook() function. * Information about what is to be pushed is now sent over a pipe rather than passed as command-line parameters. Aaron Schrab (3): hooks: Add function to check if a hook exists push: Add support for pre-push hooks Add sample pre-push hook script Getting much nicer. Thanks. Hmph, t5571 seems to be flaky in that it sometimes fails but passes when run again. Something timing dependent is going on??? With this patch applied, repeatedly try to - make sure foreign ref does not exist; and - attempt pushing the HEAD:foreign to create the foreign ref until it fails, I can get it stop before the output scrolls off of my 114 line terminal. Then when I revert the changes to transport.[ch] and builtin/push.c in this series, the test will keep going. Wait. The sample hook used in the test _is_ fed some input but it exits without reading any. What happens when we fork it, and it completes execution before we even have a chance to feed a single byte? Wont' we get a sigpipe and die? Yup, I think that is what is missing from run_pre_push_hook() implementation. diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh index d68fed7..050318b 100755 --- a/t/t5571-pre-push-hook.sh +++ b/t/t5571-pre-push-hook.sh @@ -16,8 +16,15 @@ test_expect_success 'setup' ' git init --bare repo1 git remote add parent1 repo1 test_commit one - git push parent1 HEAD:foreign + while : + do + git push parent1 :refs/heads/foreign + git push parent1 HEAD:foreign || break + done ' + +exit + write_script $HOOK EOF exit 1 EOF -- 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 2/3] push: Add support for pre-push hooks
Aaron Schrab aa...@schrab.com writes: t/t5571-pre-push-hook.sh | 129 + diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh new file mode 100755 index 000..d68fed7 --- /dev/null +++ b/t/t5571-pre-push-hook.sh @@ -0,0 +1,129 @@ +#!/bin/sh + +test_description='check pre-push hooks' +. ./test-lib.sh + +# Setup hook that always succeeds +HOOKDIR=$(git rev-parse --git-dir)/hooks +HOOK=$HOOKDIR/pre-push +mkdir -p $HOOKDIR +write_script $HOOK EOF +exit 0 +EOF As this script is expected to read from the pipe, if this exits before the parent has a chance to write to the pipe, the parent can be killed with sigpipe. At least the attached patch is necessary. In the longer term, we may want to discuss what should happen when the hook exited without even reading what we fed. My gut feeling is that we can still trust its exit status (a hook that was badly coded so it wanted to read from us and use that information to decide but somehow died before fully reading from us is not likely to exit with zero status, so we wouldn't diagnosing breakage as a success), but there may be downsides for being that lax. If we decide we want to be lax, then the call site of this hook and the pre-receive hook (is there any other take info from the standard input hook?) need to be modified so that they ignore sigpipe, I think. There was a related discussion around this issue about a year ago. http://thread.gmane.org/gmane.comp.version-control.git/180346/focus=186291 t/t5571-pre-push-hook.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh index d68fed7..577d252 100755 --- a/t/t5571-pre-push-hook.sh +++ b/t/t5571-pre-push-hook.sh @@ -8,6 +8,7 @@ HOOKDIR=$(git rev-parse --git-dir)/hooks HOOK=$HOOKDIR/pre-push mkdir -p $HOOKDIR write_script $HOOK EOF +cat /dev/null exit 0 EOF @@ -19,6 +20,7 @@ test_expect_success 'setup' ' git push parent1 HEAD:foreign ' write_script $HOOK EOF +cat /dev/null exit 1 EOF @@ -38,6 +40,7 @@ COMMIT2=$(git rev-parse HEAD) export COMMIT2 write_script $HOOK 'EOF' +cat /dev/null echo $1 actual echo $2 actual cat actual -- 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] Make git selectively and conditionally ignore certain stat fields
Is this the user edits in eclipse and then runs 'git status' from the terminal problem? Yes. Of course not just status, but any command that validates the index. On Unix this is usually bearable, though slow, but on Windows I often see git status take minutes (yes large files...). -- 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
[PATCH 00/14] git p4 branch handling fixes
There are multiple oddities in how git-p4 treats multiple p4 branches, as created with clone or sync and the '--branch' argument. Olivier reported some of these recently in http://thread.gmane.org/gmane.comp.version-control.git/212613 There are two observable behavior changes, but they are in the category of bug fixes in my opinion: - p4/HEAD symbolic ref is always created now; it used to be created only after the first sync operation after a clone - using clone --branch now checks out files; it used to complain that there was no p4/master ref Pete Wyckoff (14): git p4: test sync/clone --branch behavior git p4: rearrange and simplify hasOrigin handling git p4: add comments to p4BranchesInGit git p4: inline listExistingP4GitBranches git p4: create p4/HEAD on initial clone git p4: verify expected refs in clone --bare test git p4: clone --branch should checkout master git p4 doc: fix branch detection example git p4: allow short ref names to --branch git p4: rearrange self.initialParent use git p4: fail gracefully on sync with no master branch git p4: fix sync --branch when no master branch git p4 test: keep P4CLIENT changes inside subshells git p4: fix submit when no master branch Documentation/git-p4.txt | 22 +-- git-p4.py | 152 -- t/t9800-git-p4-basic.sh | 9 ++- t/t9806-git-p4-options.sh | 128 -- 4 files changed, 253 insertions(+), 58 deletions(-) -- 1.8.1.350.gdbf6fd0 -- 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/14] git p4: test sync/clone --branch behavior
Add failing tests to document behavior when there are multiple p4 branches, as created using the --branch option. In particular: Using clone --branch populates the specified branch correctly, but dies with an error when trying to checkout master. Calling sync without a master branch dies with an error looking for master. When there are two or more branches, a sync does nothing due to branch detection code, but that is expected. Using sync --branch to try to update just a particular branch updates no branch, but appears to succeed. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9806-git-p4-options.sh | 53 +++ 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh index fa40cc8..844aae0 100755 --- a/t/t9806-git-p4-options.sh +++ b/t/t9806-git-p4-options.sh @@ -27,14 +27,59 @@ test_expect_success 'clone no --git-dir' ' test_must_fail git p4 clone --git-dir=xx //depot ' -test_expect_success 'clone --branch' ' +test_expect_failure 'clone --branch should checkout master' ' git p4 clone --branch=refs/remotes/p4/sb --dest=$git //depot test_when_finished cleanup_git ( cd $git - git ls-files files - test_line_count = 0 files - test_path_is_file .git/refs/remotes/p4/sb + git rev-parse refs/remotes/p4/sb sb + git rev-parse refs/heads/master master + test_cmp sb master + git rev-parse HEAD head + test_cmp sb head + ) +' + +test_expect_failure 'sync when branch is not called master should work' ' + git p4 clone --branch=refs/remotes/p4/sb --dest=$git //depot@2 + test_when_finished cleanup_git + ( + cd $git + git p4 sync + git show -s --format=%s refs/remotes/p4/sb show + grep change 3 show + ) +' + +# engages --detect-branches code, which will do filename filtering so +# no sync to either b1 or b2 +test_expect_success 'sync when two branches but no master should noop' ' + test_when_finished cleanup_git + ( + cd $git + git init + git p4 sync --branch=refs/remotes/p4/b1 //depot@2 + git p4 sync --branch=refs/remotes/p4/b2 //depot@2 + git p4 sync + git show -s --format=%s refs/remotes/p4/b1 show + grep Initial import show + git show -s --format=%s refs/remotes/p4/b2 show + grep Initial import show + ) +' + +test_expect_failure 'sync --branch updates specified branch' ' + test_when_finished cleanup_git + ( + cd $git + git init + git p4 sync --branch=refs/remotes/p4/b1 //depot@2 + git p4 sync --branch=refs/remotes/p4/b2 //depot@2 + git p4 sync --branch=refs/remotes/p4/b2 + git show -s --format=%s refs/remotes/p4/b1 show + grep Initial import show + git show -s --format=%s refs/remotes/p4/b2 show + grep change 3 show ) ' -- 1.8.1.350.gdbf6fd0 -- 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/14] git p4: rearrange and simplify hasOrigin handling
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 69f1452..68f7458 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2754,23 +2754,23 @@ class P4Sync(Command, P4UserMap):
self.changeRange =
self.initialParent =
self.previousDepotPaths = []
+self.hasOrigin = False
# map from branch depot path to parent branch
self.knownBranches = {}
self.initialParents = {}
-self.hasOrigin = originP4BranchesExist()
-if not self.syncWithOrigin:
-self.hasOrigin = False
if self.importIntoRemotes:
self.refPrefix = refs/remotes/p4/
else:
self.refPrefix = refs/heads/p4/
-if self.syncWithOrigin and self.hasOrigin:
-if not self.silent:
-print Syncing with origin first by calling git fetch origin
-system(git fetch origin)
+if self.syncWithOrigin:
+self.hasOrigin = originP4BranchesExist()
+if self.hasOrigin:
+if not self.silent:
+print 'Syncing with origin first, using git fetch origin'
+system(git fetch origin)
if len(self.branch) == 0:
self.branch = self.refPrefix + master
--
1.8.1.350.gdbf6fd0
--
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/14] git p4: add comments to p4BranchesInGit
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 68f7458..03680b0 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -553,27 +553,36 @@ def gitConfigList(key):
_gitConfig[key] = read_pipe(git config --get-all %s % key,
ignore_error=True).strip().split(os.linesep)
return _gitConfig[key]
-def p4BranchesInGit(branchesAreInRemotes = True):
+def p4BranchesInGit(branchesAreInRemotes=True):
+Find all the branches whose names start with p4/, looking
+ in remotes or heads as specified by the argument. Return
+ a dictionary of { branch: revision } for each one found.
+ The branch names are the short names, without any
+ p4/ prefix.
+
branches = {}
cmdline = git rev-parse --symbolic
if branchesAreInRemotes:
-cmdline += --remotes
+cmdline += --remotes
else:
-cmdline += --branches
+cmdline += --branches
for line in read_pipe_lines(cmdline):
line = line.strip()
-## only import to p4/
-if not line.startswith('p4/') or line == p4/HEAD:
+# only import to p4/
+if not line.startswith('p4/'):
+continue
+# special symbolic ref to p4/master
+if line == p4/HEAD:
continue
-branch = line
-# strip off p4
-branch = re.sub (^p4/, , line)
+# strip off p4/ prefix
+branch = line[len(p4/):]
branches[branch] = parseRevision(line)
+
return branches
def findUpstreamBranchPoint(head = HEAD):
--
1.8.1.350.gdbf6fd0
--
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/14] git p4: inline listExistingP4GitBranches
It is four lines of code used in only one place. Simplify by
including it where it is used.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 03680b0..8814049 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2518,13 +2518,6 @@ class P4Sync(Command, P4UserMap):
branch = branch[len(self.projectName):]
self.knownBranches[branch] = branch
-def listExistingP4GitBranches(self):
-# branches holds mapping from name to commit
-branches = p4BranchesInGit(self.importIntoRemotes)
-self.p4BranchesInGit = branches.keys()
-for branch in branches.keys():
-self.initialParents[self.refPrefix + branch] = branches[branch]
-
def updateOptionDict(self, d):
option_keys = {}
if self.keepRepoPath:
@@ -2805,7 +2798,12 @@ class P4Sync(Command, P4UserMap):
if args == []:
if self.hasOrigin:
createOrUpdateBranchesFromOrigin(self.refPrefix, self.silent)
-self.listExistingP4GitBranches()
+
+# branches holds mapping from branch name to sha1
+branches = p4BranchesInGit(self.importIntoRemotes)
+self.p4BranchesInGit = branches.keys()
+for branch in branches.keys():
+self.initialParents[self.refPrefix + branch] = branches[branch]
if len(self.p4BranchesInGit) 1:
if not self.silent:
--
1.8.1.350.gdbf6fd0
--
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/14] git p4: create p4/HEAD on initial clone
There is code to create a symbolic reference from p4/HEAD to p4/master. This allows saying git show p4 as a shortcut to git show p4/master, for example. But this reference was only created on the second git p4 sync (or first sync after a clone). Make it work on the initial clone or sync. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 12 t/t9806-git-p4-options.sh | 23 +++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/git-p4.py b/git-p4.py index 8814049..537eac6 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2778,10 +2778,7 @@ class P4Sync(Command, P4UserMap): self.branch = self.refPrefix + master if gitBranchExists(refs/heads/p4) and self.importIntoRemotes: system(git update-ref %s refs/heads/p4 % self.branch) -system(git branch -D p4); -# create it /after/ importing, when master exists -if not gitBranchExists(self.refPrefix + HEAD) and self.importIntoRemotes and gitBranchExists(self.branch): -system(git symbolic-ref %sHEAD %s % (self.refPrefix, self.branch)) +system(git branch -D p4) # accept either the command-line option, or the configuration variable if self.useClientSpec: @@ -3013,6 +3010,13 @@ class P4Sync(Command, P4UserMap): read_pipe(git update-ref -d %s % branch) os.rmdir(os.path.join(os.environ.get(GIT_DIR, .git), self.tempBranchLocation)) +# Create a symbolic ref p4/HEAD pointing to p4/branch to allow +# a convenient shortcut refname p4. +if self.importIntoRemotes: +head_ref = self.refPrefix + HEAD +if not gitBranchExists(head_ref) and gitBranchExists(self.branch): +system([git, symbolic-ref, head_ref, self.branch]) + return True class P4Rebase(Command): diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh index 844aae0..4900aef 100755 --- a/t/t9806-git-p4-options.sh +++ b/t/t9806-git-p4-options.sh @@ -83,6 +83,29 @@ test_expect_failure 'sync --branch updates specified branch' ' ) ' +# allows using the refname p4 as a short name for p4/master +test_expect_success 'clone creates HEAD symbolic reference' ' + git p4 clone --dest=$git //depot + test_when_finished cleanup_git + ( + cd $git + git rev-parse --verify refs/remotes/p4/master master + git rev-parse --verify p4 p4 + test_cmp master p4 + ) +' + +test_expect_success 'clone --branch creates HEAD symbolic reference' ' + git p4 clone --branch=refs/remotes/p4/sb --dest=$git //depot + test_when_finished cleanup_git + ( + cd $git + git rev-parse --verify refs/remotes/p4/sb sb + git rev-parse --verify p4 p4 + test_cmp sb p4 + ) +' + test_expect_success 'clone --changesfile' ' test_when_finished rm cf printf 1\n3\n cf -- 1.8.1.350.gdbf6fd0 -- 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/14] git p4: verify expected refs in clone --bare test
Make sure that the standard branches are created as expected. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9800-git-p4-basic.sh | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh index 8c59796..166e752 100755 --- a/t/t9800-git-p4-basic.sh +++ b/t/t9800-git-p4-basic.sh @@ -160,9 +160,12 @@ test_expect_success 'clone --bare should make a bare repository' ' test_when_finished cleanup_git ( cd $git - test ! -d .git - bare=`git config --get core.bare` - test $bare = true + test_path_is_missing .git + git config --get --bool core.bare true + git rev-parse --verify refs/remotes/p4/master + git rev-parse --verify refs/remotes/p4/HEAD + git rev-parse --verify refs/heads/master + git rev-parse --verify HEAD ) ' -- 1.8.1.350.gdbf6fd0 -- 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/14] git p4: allow short ref names to --branch
For a clone or sync, --branch says where the newly imported branch should go, or which existing branch to sync up. It takes an argument, which is currently either something that starts with refs/, or if not, refs/heads/p4 is prepended. Putting it in heads seems like a bad default; these should go in remotes/p4/ in most situations. Make that the new default, and be more liberal in the form of the branch name. Signed-off-by: Pete Wyckoff p...@padd.com --- Documentation/git-p4.txt | 7 +-- git-p4.py | 12 +++- t/t9806-git-p4-options.sh | 21 + 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 7c5230e..7bd5c29 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -173,8 +173,11 @@ subsequent 'sync' operations. --branch branch:: Import changes into given branch. If the branch starts with - 'refs/', it will be used as is, otherwise the path 'refs/heads/' - will be prepended. The default branch is 'p4/master'. + 'refs/', it will be used as is. Otherwise if it does not start + with 'p4/', that prefix is added. The branch is assumed to + name a remote tracking, but this can be modified using + '--import-local', or by giving a full ref name. The default + branch is 'master'. + This example imports a new remote p4/proj2 into an existing git repository: diff --git a/git-p4.py b/git-p4.py index d92f00c..5dcb527 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2847,8 +2847,18 @@ class P4Sync(Command, P4UserMap): if not self.silent and not self.detectBranches: print Performing incremental import into %s git branch % self.branch +# accept multiple ref name abbreviations: +#refs/foo/bar/branch - use it exactly +#p4/branch - prepend refs/remotes/ or refs/heads/ +#branch - prepend refs/remotes/p4/ or refs/heads/p4/ if not self.branch.startswith(refs/): -self.branch = refs/heads/ + self.branch +if self.importIntoRemotes: +prepend = refs/remotes/ +else: +prepend = refs/heads/ +if not self.branch.startswith(p4/): +prepend += p4/ +self.branch = prepend + self.branch if len(args) == 0 and self.depotPaths: if not self.silent: diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh index 2ad3a3e..c0d4433 100755 --- a/t/t9806-git-p4-options.sh +++ b/t/t9806-git-p4-options.sh @@ -51,6 +51,27 @@ test_expect_failure 'sync when branch is not called master should work' ' ) ' +test_expect_success 'sync --branch builds the full ref name correctly' ' + test_when_finished cleanup_git + ( + cd $git + git init + + git p4 sync --branch=b1 //depot + git rev-parse --verify refs/remotes/p4/b1 + git p4 sync --branch=p4/b2 //depot + git rev-parse --verify refs/remotes/p4/b2 + + git p4 sync --import-local --branch=h1 //depot + git rev-parse --verify refs/heads/p4/h1 + git p4 sync --import-local --branch=p4/h2 //depot + git rev-parse --verify refs/heads/p4/h2 + + git p4 sync --branch=refs/stuff //depot + git rev-parse --verify refs/stuff + ) +' + # engages --detect-branches code, which will do filename filtering so # no sync to either b1 or b2 test_expect_success 'sync when two branches but no master should noop' ' -- 1.8.1.350.gdbf6fd0 -- 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/14] git p4: fail gracefully on sync with no master branch
If --branch was used to build a repository with no
refs/remotes/p4/master, future syncs will not know
which branch to sync. Notice this situation and
print a helpful error message.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 29 +++--
t/t9806-git-p4-options.sh | 9 -
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 9b07ddd..390d3f1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -585,6 +585,17 @@ def p4BranchesInGit(branchesAreInRemotes=True):
return branches
+def branch_exists(branch):
+Make sure that the given ref name really exists.
+
+cmd = [ git, rev-parse, --symbolic, --verify, branch ]
+p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+out, _ = p.communicate()
+if p.returncode:
+return False
+# expect exactly one line of output: the branch name
+return out.rstrip() == branch
+
def findUpstreamBranchPoint(head = HEAD):
branches = p4BranchesInGit()
# map from depot-path to branch name
@@ -2774,6 +2785,7 @@ class P4Sync(Command, P4UserMap):
print 'Syncing with origin first, using git fetch origin'
system(git fetch origin)
+branch_arg_given = bool(self.branch)
if len(self.branch) == 0:
self.branch = self.refPrefix + master
if gitBranchExists(refs/heads/p4) and self.importIntoRemotes:
@@ -2967,8 +2979,21 @@ class P4Sync(Command, P4UserMap):
else:
# catch git p4 sync with no new branches, in a repo that
# does not have any existing p4 branches
-if len(args) == 0 and not self.p4BranchesInGit:
-die(No remote p4 branches. Perhaps you never did \git
p4 clone\ in here.);
+if len(args) == 0:
+if not self.p4BranchesInGit:
+die(No remote p4 branches. Perhaps you never did
\git p4 clone\ in here.)
+
+# The default branch is master, unless --branch is used to
+# specify something else. Make sure it exists, or complain
+# nicely about how to use --branch.
+if not self.detectBranches:
+if not branch_exists(self.branch):
+if branch_arg_given:
+die(Error: branch %s does not exist. %
self.branch)
+else:
+die(Error: no branch %s; perhaps specify one
with --branch. %
+self.branch)
+
if self.verbose:
print Getting p4 changes for %s...%s % (',
'.join(self.depotPaths),
self.changeRange)
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index c0d4433..a51f122 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -40,14 +40,13 @@ test_expect_success 'clone --branch should checkout master'
'
)
'
-test_expect_failure 'sync when branch is not called master should work' '
- git p4 clone --branch=refs/remotes/p4/sb --dest=$git //depot@2
+test_expect_success 'sync when no master branch prints a nice error' '
test_when_finished cleanup_git
+ git p4 clone --branch=refs/remotes/p4/sb --dest=$git //depot@2
(
cd $git
- git p4 sync
- git show -s --format=%s refs/remotes/p4/sb show
- grep change 3 show
+ test_must_fail git p4 sync 2err
+ grep Error: no branch refs/remotes/p4/master err
)
'
--
1.8.1.350.gdbf6fd0
--
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/14] git p4: fix sync --branch when no master branch
It is legal to sync a branch with a different name than refs/remotes/p4/master, and to do so even when master does not exist. Signed-off-by: Pete Wyckoff p...@padd.com --- Documentation/git-p4.txt | 5 + git-p4.py | 14 +++--- t/t9806-git-p4-options.sh | 8 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 7bd5c29..e79d046 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -112,6 +112,11 @@ will be fetched and consulted first during a 'git p4 sync'. Since importing directly from p4 is considerably slower than pulling changes from a git remote, this can be useful in a multi-developer environment. +If there are multiple branches, doing 'git p4 sync' will automatically +use the BRANCH DETECTION algorithm to try to partition new changes +into the right branch. This can be overridden with the '--branch' +option to specify just a single branch to update. + Rebase ~~ diff --git a/git-p4.py b/git-p4.py index 390d3f1..77bde59 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2810,14 +2810,22 @@ class P4Sync(Command, P4UserMap): # branches holds mapping from branch name to sha1 branches = p4BranchesInGit(self.importIntoRemotes) -self.p4BranchesInGit = branches.keys() -for branch in branches.keys(): -self.initialParents[self.refPrefix + branch] = branches[branch] + +# restrict to just this one, disabling detect-branches +if branch_arg_given: +short = self.branch.split(/)[-1] +if short in branches: +self.p4BranchesInGit = [ short ] +else: +self.p4BranchesInGit = branches.keys() if len(self.p4BranchesInGit) 1: if not self.silent: print Importing from/into multiple branches self.detectBranches = True +for branch in branches.keys(): +self.initialParents[self.refPrefix + branch] = \ +branches[branch] if self.verbose: print branches: %s % self.p4BranchesInGit diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh index a51f122..3bf 100755 --- a/t/t9806-git-p4-options.sh +++ b/t/t9806-git-p4-options.sh @@ -88,14 +88,14 @@ test_expect_success 'sync when two branches but no master should noop' ' ) ' -test_expect_failure 'sync --branch updates specified branch' ' +test_expect_success 'sync --branch updates specific branch, no detection' ' test_when_finished cleanup_git ( cd $git git init - git p4 sync --branch=refs/remotes/p4/b1 //depot@2 - git p4 sync --branch=refs/remotes/p4/b2 //depot@2 - git p4 sync --branch=refs/remotes/p4/b2 + git p4 sync --branch=b1 //depot@2 + git p4 sync --branch=b2 //depot@2 + git p4 sync --branch=b2 git show -s --format=%s refs/remotes/p4/b1 show grep Initial import show git show -s --format=%s refs/remotes/p4/b2 show -- 1.8.1.350.gdbf6fd0 -- 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/14] git p4: fix submit when no master branch
It finds its upstream and applies the commit properly, but the sync step will fail unless it is told which branch to work on. Signed-off-by: Pete Wyckoff p...@padd.com --- Documentation/git-p4.txt | 5 + git-p4.py | 6 +- t/t9806-git-p4-options.sh | 25 + 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index e79d046..f70ef9d 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -294,6 +294,11 @@ These options can be used to modify 'git p4 submit' behavior. to bypass the prompt, causing conflicting commits to be automatically skipped, or to quit trying to apply commits, without prompting. +--branch branch:: + After submitting, sync this named branch instead of the default + p4/master. See the Sync options section above for more + information. + Rebase options ~~ These options can be used to modify 'git p4 rebase' behavior. diff --git a/git-p4.py b/git-p4.py index 77bde59..2da5649 100755 --- a/git-p4.py +++ b/git-p4.py @@ -927,7 +927,8 @@ class P4Submit(Command, P4UserMap): optparse.make_option(--dry-run, -n, dest=dry_run, action=store_true), optparse.make_option(--prepare-p4-only, dest=prepare_p4_only, action=store_true), optparse.make_option(--conflict, dest=conflict_behavior, - choices=self.conflict_behavior_choices) + choices=self.conflict_behavior_choices), +optparse.make_option(--branch, dest=branch), ] self.description = Submit changes from git to the perforce depot. self.usage += [name of git branch to submit into perforce depot] @@ -940,6 +941,7 @@ class P4Submit(Command, P4UserMap): self.isWindows = (platform.system() == Windows) self.exportLabels = False self.p4HasMoveCommand = p4_has_move_command() +self.branch = None def check(self): if len(p4CmdList(opened ...)) 0: @@ -1676,6 +1678,8 @@ class P4Submit(Command, P4UserMap): print All commits applied! sync = P4Sync() +if self.branch: +sync.branch = self.branch sync.run([]) rebase = P4Rebase() diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh index 8d914a5..4f077ee 100755 --- a/t/t9806-git-p4-options.sh +++ b/t/t9806-git-p4-options.sh @@ -251,6 +251,31 @@ test_expect_success 'clone --use-client-spec' ' ) ' +test_expect_success 'submit works with no p4/master' ' + test_when_finished cleanup_git + git p4 clone --branch=b1 //depot@1,2 --destination=$git + ( + cd $git + test_commit submit-1-branch + git config git-p4.skipSubmitEdit true + git p4 submit --branch=b1 + ) +' + +# The sync/rebase part post-submit will engage detect-branches +# machinery which will not do anything in this particular test. +test_expect_success 'submit works with two branches' ' + test_when_finished cleanup_git + git p4 clone --branch=b1 //depot@1,2 --destination=$git + ( + cd $git + git p4 sync --branch=b2 //depot@1,3 + test_commit submit-2-branches + git config git-p4.skipSubmitEdit true + git p4 submit + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.8.1.350.gdbf6fd0 -- 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: Error:non-monotonic index after failed recursive sed command
Happy ending!
Turns out i have actually made a backup 3 days ago.
My other work was on a branch + in a stash. Commits done on a branch
were already present in a backup.
I was able to get the stash working by copying corrupted .pack files
from the backup, luckily all the new work wasn't packed yet.
So i've just verified the log messages to see that no new commits were
made, created a patch from the corrupted git repo of the stash,
applied it on the backup, and wo-hooo, everything worked.
And then I've pushed to origin to avoid such silliness in the future.
Thanks and Regards,
George
On Tue, Jan 15, 2013 at 10:45 AM, Philip Oakley philipoak...@iee.org wrote:
From: George Karpenkov geo...@metaworld.ru
Sent: Monday, January 14, 2013 10:57 PM
Thanks everyone!
Progress so far:
After executing reverse sed command:
find .git -name '*.*' -exec sed -i 's//\t/g' {} \;
Have you counted how many substitutions there are in the pack file(s). It
may be sufficiently small that you can simply try all the possible
combinations of fwd and reverse substitutions. Even if it takes a few days
the computer won't get bored ;-)
And trying to switch the branch I get:
git checkout X
error: failed to read object 51a980792f26875d00acb79a19f043420f542cfa
at offset 41433013 from
.git/objects/pack/pack-8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack
fatal: packed object 51a980792f26875d00acb79a19f043420f542cfa (stored
in .git/objects/pack/pack-
8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack) is corrupt
So the actual .pack file is corrupt, unfortunately.
On Tue, Jan 15, 2013 at 6:13 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Junio C Hamano gits...@pobox.com writes:
Everybody seems to be getting an impression that .idx is the only
thing that got corrupt. Where does that come from?
It's the only thing that appear in the error message. This does not
imply that it is the only corrupt thing, but gives a little hope that it
may still be the case.
Actually, I thought the read-only protection should have protected
files in object/ directory, but a little testing shows that sed -i
gladly accepts to modify read-only files (technically, it does not
modify it, but creates a temporary file with the new content, and then
renames it to the new location). So, the hope that pack files are
uncorrupted is rather thin unfortunately.
--
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
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
Johannes Schindelin johannes.schinde...@gmx.de writes: On Mon, 14 Jan 2013, Junio C Hamano wrote: It appears that memcmp() uses the usual one word at a time comparison and triggers valgrind in a callback of bsearch() used in the refname search. I can easily trigger problems in any script with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v) without this suppression. I have no way to replicate that issue, but I take your word for it. With that in mind, here is my ACK. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git grep performance regression
On Tue, Jan 15, 2013 at 5:38 AM, Ross Lagerwall rosslagerw...@gmail.com wrote: Hi, I have noticed a performance regression in git grep between v1.8.1 and v1.8.1.1: On the kernel tree: For git 1.8.1: $ time git grep foodsgsg real 0m0.158s user 0m0.290s sys0m0.207s For git 1.8.1.1: $ time /tmp/g/bin/git grep foodsgsg real 0m0.501s user 0m0.707s sys0m0.493s Interesting. I see the regression on linux-2.6 too (0.6s real vs 0.9s). A bisect seems to indicate that it was introduced by 94bc67: commit 94bc671a1f2e8610de475c2494d2763355a99f65 Author: Jean-Noël AVILA avila...@gmail.com Date: Sat Dec 8 21:04:39 2012 +0100 Add directory pattern matching to attributes The manpage of gitattributes says: The rules how the pattern matches paths are the same as in .gitignore files and the gitignore pattern matching has a pattern ending with / for directory matching. This rule is specifically relevant for the 'export-ignore' rule used for git archive. Not the real contributor to the regression, but it should be noted that glibc's strrchr does not employ a typical loop. Instead it advances with strchr with a note that strchr is much faster. It looks like strchr compares a word at a time instead of a byte. We might want to do the same. I don't have time to look into details now, but by enabling DEBUG_ATTR, it looks like this commit makes it push and pop patterns a lot more than without the commit. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git grep performance regression
Ross Lagerwall rosslagerw...@gmail.com writes: I have noticed a performance regression in git grep between v1.8.1 and v1.8.1.1: On the kernel tree: For git 1.8.1: $ time git grep foodsgsg real 0m0.158s user 0m0.290s sys0m0.207s For git 1.8.1.1: $ time /tmp/g/bin/git grep foodsgsg real 0m0.501s user 0m0.707s sys0m0.493s A bisect seems to indicate that it was introduced by 94bc67: commit 94bc671a1f2e8610de475c2494d2763355a99f65 Author: Jean-Noël AVILA avila...@gmail.com Date: Sat Dec 8 21:04:39 2012 +0100 Add directory pattern matching to attributes The manpage of gitattributes says: The rules how the pattern matches paths are the same as in .gitignore files and the gitignore pattern matching has a pattern ending with / for directory matching. This rule is specifically relevant for the 'export-ignore' rule used for git archive. Signed-off-by: Jean-Noel Avila jn.av...@free.fr Signed-off-by: Junio C Hamano gits...@pobox.com Hmph, that looks really bad, especially given that in the normal codepath like git grep, we would never care about directories (the attributes are normally applied to real paths with contents, and the use by archive is an anomaly) and the implementation should be done in a way not to impose such excess and useless overhead. We may end up reverting that patch for the time being X-. Jean-Noël, ideas? -- 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] attr: make it build with DEBUG_ATTR again
Commit 82dce99 (attr: more matching optimizations from .gitignore -
2012-10-15) changed match_attr structure but it did not update
DEBUG_ATTR-specific code. This fixes it.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index 097ae87..0575bf7 100644
--- a/attr.c
+++ b/attr.c
@@ -693,7 +693,7 @@ static int fill_one(const char *what, struct match_attr *a,
int rem)
if (*n == ATTR__UNKNOWN) {
debug_set(what,
- a-is_macro ? a-u.attr-name : a-u.pattern,
+ a-is_macro ? a-u.attr-name :
a-u.pat.pattern,
attr, v);
*n = v;
rem--;
--
1.8.0.rc3.18.g0d9b108
--
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 (Jan 2013, #06; Mon, 14)
Junio C Hamano gits...@pobox.com writes: [New Topics] * jc/cvsimport-upgrade (2013-01-14) 8 commits - t9600: adjust for new cvsimport - t9600: further prepare for sharing - cvsimport-3: add a sample test - cvsimport: make tests reusable for cvsimport-3 - cvsimport: start adding cvsps 3.x support - cvsimport: introduce a version-switch wrapper - cvsimport: allow setting a custom cvsps (2.x) program name - Makefile: add description on PERL/PYTHON_PATH The most important part of this series is the addition of the new cvsimport by Eric Raymond that works with cvsps 3.x. Given some distros have inertia to be conservative, Git with cvsimport that does not work with both 3.x will block adoption of cvsps 3.x by them, and shipping Git with cvsimport that does not work with cvsps 2.x will block such a version of Git, so we'll do the proven both old and new are available, but we aim to deprecate and remove the old one in due time strategy that we used successfully in the past. My reading of the review discussion of this series, and the discussion in the $gmane/213170 thread, is that the approach outlined in this series is something Git-side is comfortable working with. I personally think it will be slightly less work on your side to keep the cvsps 3.x + new cvsimport combo improving, because you no longer need to worry about punting to the old cvsimport. In addition, I think the new layout would make it easier for the new combo to gain trust of existing Git userbase over time by adding more t965x series of tests that correspond to the tests in the t960x series, working on the same (simple) CVS histories, demonstrating that the result would be what users expect, and guarding the code from future breakage. By giving options to pick and choose both old and new cvsps, I think it will make it easier for distros to include cvsps 3.x sooner, promoting its adoption, which will in turn benefit us. I converted one of Chris's follow-up test tweaks to this to illustrate how it can be done without breaking tests for the original cvsimport, but didn't do all of them. Chris, is this a foundation we can work together on top? Even though I assigned Author: to the start adding cvsps 3 patch, I forgot to forge Eric's sign-off to it. If Eric is OK with the direction this series is going, I'll do so and advance the rerolled series to 'next'. Thanks. -- 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 2/3] push: Add support for pre-push hooks
Junio C Hamano gits...@pobox.com writes: At least the attached patch is necessary. Sorry, but the last hunk (see below) is not. It breaks the hook. In the longer term, we may want to discuss what should happen when the hook exited without even reading what we fed. My gut feeling is that we can still trust its exit status (a hook that was badly coded so it wanted to read from us and use that information to decide but somehow died before fully reading from us is not likely to exit with zero status, so we wouldn't diagnosing breakage as a success), but there may be downsides for being that lax. If we decide we want to be lax, then the call site of this hook and the pre-receive hook (is there any other take info from the standard input hook?) need to be modified so that they ignore sigpipe, I think. There was a related discussion around this issue about a year ago. http://thread.gmane.org/gmane.comp.version-control.git/180346/focus=186291 ... @@ -38,6 +40,7 @@ COMMIT2=$(git rev-parse HEAD) export COMMIT2 write_script $HOOK 'EOF' +cat /dev/null echo $1 actual echo $2 actual cat actual As this one wants to keep the incoming data to actual, we do not want the extra cat to slurp everything in. Sorry for not being careful. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git grep performance regression
On Tue, Jan 15, 2013 at 9:46 AM, Duy Nguyen pclo...@gmail.com wrote:
I don't have time to look into details now, but by enabling
DEBUG_ATTR, it looks like this commit makes it push and pop patterns a
lot more than without the commit.
I think the culprit is at this chunk:
static void prepare_attr_stack(const char *path)
{
struct attr_stack *elem, *info;
int dirlen, len;
const char *cp;
- cp = strrchr(path, '/');
- if (!cp)
- dirlen = 0;
- else
- dirlen = cp - path;
+ dirlen = find_basename(path) - path;
dirlen is not expected to include the trailing slash, but
find_basename() does that. It messes up with the path filters for
push/pop in the next code. This brings grep performance closely back
to before for me. Ross, can you check (patch could be whitespace
damaged by gmail)?
-- 8 --
diff --git a/attr.c b/attr.c
index b05110d..1e96e26 100644
--- a/attr.c
+++ b/attr.c
@@ -583,6 +583,9 @@ static void prepare_attr_stack(const char *path)
dirlen = find_basename(path) - path;
+ if (dirlen)
+ dirlen--;
+
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
-- 8 --
--
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] Possible bug in `remote set-url --add --push`
On 14/01/2013, at 17:09, Junio C Hamano gits...@pobox.com wrote: Michael J Gruber g...@drmicha.warpmail.net writes: It seems to me that everything works as designed, and that the man page talk about push URLs can be read in two ways,... Hmph, but I had an impression that Jardel's original report was that one of the --add --pushurl was not adding but was replacing. If that was a false alarm, everything you said makes sense to me. Thanks. I failed to explain my reasoning. But I learned quite a bit from this discussion. I understood that the defaul push url is not used by git-push when there's at least one pushurl for a given remote. If that's by design, I still fail to comprehend the exact reason. If you allow me, I'd like you to forget about the concepts for a minute, and focus on the user experience. Imagine a simple hypothetical scenario in which the user wants to push to 2 distinct repositories. He already has cloned the repo from the 1st repository, thus (theoretically) all he needs to do, is to add a new repository for push. He then uses `remote set-url --add --push 2nd-repo` (which I personally thought would suffice). However, if he tries to push a new commit to this remote, it would be pushed _only_ to the 2nd-repo. This is exactly what I thought to be a bug. If it's intended to work the way I described in the previous scenario, I'll have to ask and/or research to understand the reason behind this -- Why does having a pushurl make git-push _not_ to push to the default push location (the 1st repo in my scenario) as well? Could you describe a scenario in which that behavior is useful and/or better than the behavior I expected? Please, pardon me for not being as clear as needed. I appreciate your time on this. Thank you all. Sent from my mobile.-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, argv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) if (pathspec) in place of if (i argc).
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 65413d0..045c960 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -153,26 +153,15 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int interactive_reset(const char *revision, const char **argv,
-const char *prefix)
-{
- const char **pathspec = NULL;
-
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- return run_add_interactive(revision, --patch=reset, pathspec);
-}
-
-static int read_from_tree(const char *prefix, const char **argv,
- unsigned char *tree_sha1, int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
+ int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
- diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), opt);
+ diff_tree_setup_paths(pathspec, opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
const char *rev = HEAD;
unsigned char sha1[20], *orig = NULL, sha1_orig[20],
*old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
struct commit *commit;
struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not parse object '%s'.), rev);
hashcpy(sha1, commit-object.sha1);
+ if (i argc)
+ pathspec = get_pathspec(prefix, argv + i);
+
if (patch_mode) {
if (reset_type != NONE)
die(_(--patch is incompatible with
--{hard,mixed,soft}));
- return interactive_reset(rev, argv + i, prefix);
+ return run_add_interactive(rev, --patch=reset, pathspec);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (i argc) {
+ if (pathspec) {
if (reset_type == MIXED)
warning(_(--mixed with paths is deprecated; use 'git
reset -- paths' instead.));
else if (reset_type != NONE)
die(_(Cannot do %s reset with paths.),
_(reset_type_names[reset_type]));
- return read_from_tree(prefix, argv + i, sha1,
+ return read_from_tree(pathspec, sha1,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/19] reset: don't allow git reset -- $pathspec in bare repo
Running e.g. git reset . in a bare repo results in an index file being created from the HEAD commit. The differences compared to the index are then printed as usual, but since there is no worktree, it will appear as if all files are deleted. For example, in a bare clone of git.git: Unstaged changes after reset: D .gitattributes D .gitignore D .mailmap ... This happens because the check for is_bare_repository() happens after we branch off into read_from_tree() to reset with paths. Fix by moving the branching point after the check. Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- builtin/reset.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 045c960..664fad9 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) else if (reset_type != NONE) die(_(Cannot do %s reset with paths.), _(reset_type_names[reset_type])); - return read_from_tree(pathspec, sha1, - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); } if (reset_type == NONE) reset_type = MIXED; /* by default */ @@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_(%s reset is not allowed in a bare repository), _(reset_type_names[reset_type])); + if (pathspec) + return read_from_tree(pathspec, sha1, + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + /* Soft reset does not touch the index file nor the working tree * at all, but requires them in a good order. Other resets reset * the index file to the tree object we are switching to. */ -- 1.8.1.1.454.gce43f05 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/19] reset $pathspec: no need to discard index
Since 34110cd (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06), the index no longer gets clobbered by
do_diff_cache() and we can remove the code for discarding and
re-reading it.
There are two paths to update_index_refresh() from cmd_reset(), but on
both paths, either read_cache() or read_cache_unmerged() will have
been called, so the call to read_cache() in this method is redundant
(although practically free).
This speeds up git reset -- . a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real0m0.093s0m0.080s
user0m0.040s0m0.020s
sys 0m0.050s0m0.050s
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..8cc7c72 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file
*index_lock, int flags)
fd = hold_locked_index(index_lock, 1);
}
- if (read_cache() 0)
- return error(_(Could not read index));
-
result = refresh_index(the_index, (flags), NULL, NULL,
_(Unstaged changes after reset:)) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
@@ -141,12 +138,6 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
- int *discard_flag = data;
-
- /* do_diff_cache() mangled the index */
- discard_cache();
- *discard_flag = 1;
- read_cache();
for (i = 0; i q-nr; i++) {
struct diff_filespec *one = q-queue[i]-one;
@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char
**argv,
unsigned char *tree_sha1, int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd, index_was_discarded = 0;
+ int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- opt.format_callback_data = index_was_discarded;
index_fd = hold_locked_index(lock, 1);
- index_was_discarded = 0;
read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char
**argv,
diff_flush(opt);
diff_tree_release_paths(opt);
- if (!index_was_discarded)
- /* The index is still clobbered from do_diff_cache() */
- discard_cache();
return update_index_refresh(index_fd, lock, refresh_flags);
}
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/19] reset $pathspec: exit with code 0 if successful
git reset $pathspec currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.
This makes the 4 disambiguation test cases in t7102 clearer since
they all used to fail, 3 of which failed due to changes in the
work tree. Now only the ambiguous one fails.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 8 +++-
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 18 --
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
{
- int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
- result = refresh_index(the_index, (flags), NULL, NULL,
- _(Unstaged changes after reset:)) ? 1 : 0;
+ refresh_index(the_index, (flags), NULL, NULL,
+ _(Unstaged changes after reset:));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error (Could not refresh index);
- return result;
+ return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success 'reset submodule updates the index' '
git update-index --refresh
git diff-files --quiet
git diff-index --quiet --cached HEAD
- test_must_fail git reset HEAD^ submodule
+ git reset HEAD^ submodule
test_must_fail git diff-files --quiet
git reset submodule
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed paths' '
echo 4 file4
echo 5 file1
git add file1 file3 file4
- test_must_fail git reset HEAD -- file1 file2 file3
+ git reset HEAD -- file1 file2 file3
+ test_must_fail git diff --quiet
git diff output
test_cmp output expect
git diff --cached output
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give
paths' '
sub/file2
git update-index --add sub/file1 sub/file2
T=$(git write-tree)
- test_must_fail git reset HEAD sub/file2
+ git reset HEAD sub/file2
+ test_must_fail git diff --quiet
U=$(git write-tree)
echo $T
echo $U
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is
unmerged' '
echo 100644 $F3 3 file2
} | git update-index --index-info
git ls-files -u
- test_must_fail git reset HEAD file2
+ git reset HEAD file2
+ test_must_fail git diff --quiet
git diff-index --exit-code --cached HEAD
'
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard
secondfile
git add secondfile
- test_must_fail git reset secondfile
+ git reset secondfile
+ test_must_fail git diff --quiet -- secondfile
test -z $(git diff --cached --name-only)
test -f secondfile
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
secondfile
git add secondfile
rm -f secondfile
- test_must_fail git reset HEAD secondfile
+ git reset HEAD secondfile
+ test_must_fail git diff --quiet
test -z $(git diff --cached --name-only)
test ! -f secondfile
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
secondfile
git add secondfile
rm -f secondfile
- test_must_fail git reset -- secondfile
+ git reset -- secondfile
+ test_must_fail git diff --quiet
test -z $(git diff --cached --name-only)
test ! -f secondfile
'
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish
Resetting with paths does not update HEAD and there is nothing else
that a commit should be needed for. Relax the argument parsing so only
a tree is required.
The sha1 is only passed to read_from_tree(), which already only
requires a tree.
The rev variable we pass to run_add_interactive() will resolve to a
tree. This is fine since interactive_reset only needs the parameter to
be a treeish and doesn't use it for display purposes.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 48 +++-
t/t7102-reset.sh | 8
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 520c1a5..b776867 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -178,9 +178,10 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
/*
* Possible arguments are:
*
-* git reset [-opts] rev paths...
-* git reset [-opts] rev -- paths...
-* git reset [-opts] -- paths...
+* git reset [-opts] [rev]
+* git reset [-opts] tree [paths...]
+* git reset [-opts] tree -- [paths...]
+* git reset [-opts] -- [paths...]
* git reset [-opts] paths...
*
* At this point, argv points immediately after [-opts].
@@ -195,11 +196,13 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
}
/*
* Otherwise, argv[0] could be either rev or paths and
-* has to be unambiguous.
+* has to be unambiguous. If there is a single argument, it
+* can not be a tree
*/
- else if (!get_sha1_committish(argv[0], unused)) {
+ else if ((!argv[1] !get_sha1_committish(argv[0], unused)) ||
+(argv[1] !get_sha1_treeish(argv[0], unused))) {
/*
-* Ok, argv[0] looks like a rev; it should not
+* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
@@ -241,7 +244,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
- struct commit *commit;
const struct option options[] = {
OPT__QUIET(quiet, N_(be quiet, only report errors)),
OPT_SET_INT(0, mixed, reset_type,
@@ -263,19 +265,23 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, rev);
- if (get_sha1_committish(rev, sha1))
- die(_(Failed to resolve '%s' as a valid ref.), rev);
-
- /*
-* NOTE: As git reset $treeish -- $path should be usable on
-* any tree-ish, this is not strictly correct. We are not
-* moving the HEAD to any commit; we are merely resetting the
-* entries in the index to that of a treeish.
-*/
- commit = lookup_commit_reference(sha1);
- if (!commit)
- die(_(Could not parse object '%s'.), rev);
- hashcpy(sha1, commit-object.sha1);
+ if (!pathspec) {
+ struct commit *commit;
+ if (get_sha1_committish(rev, sha1))
+ die(_(Failed to resolve '%s' as a valid revision.),
rev);
+ commit = lookup_commit_reference(sha1);
+ if (!commit)
+ die(_(Could not parse object '%s'.), rev);
+ hashcpy(sha1, commit-object.sha1);
+ } else {
+ struct tree *tree;
+ if (get_sha1_treeish(rev, sha1))
+ die(_(Failed to resolve '%s' as a valid tree.), rev);
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_(Could not parse object '%s'.), rev);
+ hashcpy(sha1, tree-object.sha1);
+ }
if (patch_mode) {
if (reset_type != NONE)
@@ -340,7 +346,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD !update_ref_status !quiet)
- print_new_head_line(commit);
+ print_new_head_line(lookup_commit_reference(sha1));
remove_branch_state();
}
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 81b2570..1fa2a5f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '
test ! -f secondfile
'
+test_expect_success 'reset with paths accepts tree' '
+ # for simpler tests, drop last commit containing
[PATCH v2 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given
Thanks to b65982b (Optimize diff-index --cached using cache-tree,
2009-05-20), resetting with paths is much faster than resetting
without paths. Some timings for the linux-2.6 repo to illustrate this
(best of five, warm cache):
reset reset .
real0m0.219s0m0.080s
user0m0.140s0m0.040s
sys 0m0.070s0m0.030s
These two commands should do the same thing, so instead of having the
user type the trailing . to get the faster do_diff_cache()-based
implementation, always use it when doing a mixed reset, with or
without paths (so git reset $rev would also be faster).
Timing git reset shows that it indeed becomes as fast as
git reset . after this patch.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
It seems like a better solution would be for unpack_trees() learn the
same tricks as do_diff_cache(). I'm leaving that a challange for the
reader :-). I did have a look a unpack_trees(), but it looked rather
overwhelming.
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 45b01eb..921afbe 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -322,7 +322,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- if (pathspec) {
+ if (reset_type == MIXED) {
if (read_from_tree(pathspec, sha1))
return 1;
} else {
--
1.8.1.1.454.gce43f05
--
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
