Re: Subtree: My Status
- Mail original - Hi folks, I apologize for being off the grid for a while. We had a baby and unexpectedly ended up in the NICU. We just got him home a week ago. Everyone is doing fine but I had to pretty much drop all non-essential work for a month or so. Good to here that everything is well on your side and congratulation for the baby, Rest assured that I have all of the git-subtree-related mail sitting in my inbox. It will take me some time to process it all -- looks ike there has been a lot of good work! no worries, there is no emergency here... Please remember that I don't consider myself a gatekeeper to git subtree. In fact I could use some help reviewing and approving patches. If anyone thinks a patch looks good, let Junio know. It's my responsibility to object to things, not your responsibility to wait for I guess it's more or less everybody's responsability to review patches, but it seems to me that for the actual gate-keeping, Junio considers you in charge of git-subtree... Maybe there is an organisational quirk to sort- out here... Junio ? -- 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 6.5/7] builtin/log.c: make usage string consistent with doc
Replace 'since..until' with 'revision range', in accordance with
the documentation.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Junio: sorry I missed this detail. Can you squeeze this patch
between [6/7] and [7/7] so that the commit message in [7/7] makes
sense?
Thanks.
builtin/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index ad46f72..6e56a50 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,7 +37,7 @@ static const char *fmt_patch_subject_prefix = PATCH;
static const char *fmt_pretty;
static const char * const builtin_log_usage[] = {
- N_(git log [options] [since..until] [[--] path...]\n)
+ N_(git log [options] [revision range] [[--] path...]\n)
N_( or: git show [options] object...),
NULL
};
--
1.8.2.1.423.g9a53c75.dirty
--
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 1/8] Add new git-cc-cmd helper to contrib
Would it make sense to integrate this in git shortlog, which already does something similar? Conceptually, yes, but the end result will be much larger in scope. I am not sure if shortlog is still a good label for it. since we are throwing ideas around... The first place where I would logically look for such a feature would be in git-blame --cc-list or something like that. git-blame seems to me as a logical place for all look at history and give me a list of names type commands -- 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
[ITCH] Mixed-case aliases
Hi, My .gitconfig has aliases like the following: [alias] bD = branch -D bM = branch -M Unfortunately, since the current config-parsing code unconditionally runs tolower() on all keys, these aliases don't work. My proposal is to modify alias.c::alias_lookup() to call a git_config_DONTMANGLECASE() variant, and modify the various functions in config.c appropriately. On a related note, it might be useful to warn the user that we are an ignoring alias.builtin-name key when executing the corresponding builtin. It's not really my itch, but I suspect a lot of beginners get tripped up by this. What do you think? Ram -- 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: Premerging topics (was: [RFD] annnotating a pair of commit objects?)
Any comment on that ? I think anyone using a Topic Workflow could use that feature and that it would be a nice addition to the project. Maybe I'm totally wrong in the proposal below (please tell me !), but there are some unanswered question that prevents me from starting (and I'd really like this to be discussed before actually starting). On Wed, Apr 10, 2013 at 10:35 PM, Antoine Pelisse apeli...@gmail.com wrote: The goal is to propose a structure for storing and pre-merging pairs of commits. Data-structure == We could use a note ref to store the pre-merge information. Each commit would be annotated with a blob containing the list of pre-merges (one sha1 per line with sha1 pointing to a merge commit). The commit on the other side of a merge would also be annotated. The choice of the refname could be done like we do with notes: - Have a default value - Have a default value configured in config - Use a specific value when merging/creating the pre-merges Here are my concerns: Pros 1. Notes allow dynamic annotation a commit 2. If we manage to fix 4, we can easily download all pre-merges from a remote host by fetching the ref (or clean by deleting the ref). 3. Conflicts on pre-merge notes could probably be resolved by concatenation. Cons 4. Checking connectivity means opening the blob and parsing it 5. Regular notes and pre-merge notes have to be handled separately because of 4. I'm hoping we can keep the pros and avoid the cons, but I'm kind of stuck here. Help would be really appreciated (or maybe this is a totally wrong direction, and I would also like to know ;) Merging (Using what we saved) = The goal is to merge branches J and B using existing pre-merges. E0. Create an empty stack S E1. Create set of commits 'J..B' and 'B..J' (that is probably already done) E2. For each commit C in smallest(J..B, B..J), execute E3 E3. For each premerge P in notes-premerge(C), execute E4 E4. If one of both parents of P belongs to biggest(J..B, B..J), stack P in S E5. Merge J and B using all pre-merges from S Let's consider that |J..B| is smaller than |B..J|. E0 is executed only once E1 is O(|J..B| + |B..J|) E2 is O(|J..B|) E3 is O(|J..B| x the average number of pre-merge per commits P_avg) E4 is executed for each parent (let's say it's two/constant, after all the topic is pair of commits), so still O(|J..B| x P_avg) E5 I don't know (how it can be done, and what would be the resulting time complexity) So the time cost for steps E0 to E4 is O(|J..B| + |B..J| x P_avg) Tools (Save the pre-merges) === Of course we need several tools to maintain the list of premerges, and to easily compute them. For example, it would be nice to be able to do something like: $ git pre-merge topicA topicB topicC to find, resolve and store all interactions between the topics. We could then easily derive to something that would allow to pre-merge a new topic with all topics already merged in master..pu (for example). Anyway, this task is left for latter. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] Silent data loss on merge with uncommited changes + renames
Hi, Following the discussion on merge with uncommited changes inside the git pull --autostash thread, I did a bit of testing, and encountered a case with silent data loss. In short: merge a branch introducing changes to a file. If the file has been renamed in the current branch, then git merge follows the rename and brings changes to the renamed file, but uncommited changes in this file are overriden silently. I could have expected git merge --abort to fail, but the problem is really more serious here: data loss is done silently before giving me an opportunity to do or abort anything. Reproduction script below: #! /bin/sh # Create repo git init git.$$ cd git.$$ echo init test.txt git add test.txt git commit -m init # Make a branch changing test.txt git checkout -b branch echo new test.txt git commit -am new # Move test.txt on master git checkout master git mv test.txt moved.txt git commit -m move # Make uncommited changes to moved.txt echo precious moved.txt # Merge loses uncommited content precious in moved.txt silently git merge --no-edit branch ls # lists just moved.txt git status # nothing to commit, working directory clean cat moved.txt # Says new. -- 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 4/5] git-log.txt: rewrite note on why -- may be required
Junio C Hamano gits...@pobox.com writes: This is a minimalistic patch to fix the formatting. I removed the extra sentence after the enumeration and moved it to the end of the main text, but somebody may have a better idea to persuade AsciiDoc to format it in a more reasonable way while keeping the sentence there. -- 8 -- Subject: line-log: fix documentation formatting The second paragraph of the added description for the -L option start and end can take one of these forms:, and the list of forms that follow the headline, were indented one level too short, due to the missing + to signal that the next paragraph continues the previous one. Also You can specify this option more than once is about the -L option, not about its various forms of starting and ending points. Move it to the end of the main text. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-log.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 4850226..0959f9d 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -76,12 +76,11 @@ produced by --stat etc. not give any pathspec limiters. This is currently limited to a walk starting from a single revision, i.e., you may only give zero or one positive revision arguments. - + You can specify this option more than once. ++ start and end can take one of these forms: include::line-range-format.txt[] -You can specify this option more than once. - Sorry for being a bit late to this. I think it's a good solution; putting You can specify this option more than once after all the other text was probably worse because it gets lost down there. As for --full-line-diff:: Always print the interesting range even if the current commit That's just stale and not currently implemented. Sigh. We should remove it. -- 8 -- Subject: [PATCH] git-log(1): remove --full-line-diff description This option is a remnant of an earlier log -L version, and not currently implemented. Remove it until (if at all) it is implemented again. Signed-off-by: Thomas Rast tr...@inf.ethz.ch --- Documentation/git-log.txt | 4 1 file changed, 4 deletions(-) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 0959f9d..65707ce 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -82,10 +82,6 @@ produced by --stat etc. include::line-range-format.txt[] ---full-line-diff:: - Always print the interesting range even if the current commit - does not change any line of the range. - [\--] path...:: Show only commits that are enough to explain how the files that match the specified paths came to be. See History -- 1.8.2.1.844.g59e84de.dirty -- 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-to-mail notes resurrected
Hi,
As some might remember, I made a script that writes notes (as in
git-notes) linking patches to emails back in 2009:
http://thread.gmane.org/gmane.comp.version-control.git/109074
I resurrected this idea the other week, using a faster implementation
(the N command in fast-import is great!) and generating better links.
I am regularly pushing the results to
git://github.com/trast/git.git
git://repo.or.cz/git/trast.git
branches
notes/gmanea link to the gmane thread view, focused on this patch
notes/message-id the raw message-id
Point your notes.displayRef at one or both to use them.
It still fails to match some commits, so consider this WIP, but I think
it's quite useful already.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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/6] show: obey --textconv for blobs
Jeff King venit, vidit, dixit 21.04.2013 05:37: On Sat, Apr 20, 2013 at 03:38:53PM +0200, Michael J Gruber wrote: Wait, this does the opposite of the last patch. If we do want to do this, shouldn't the last one have been an expect_failure? The last patch just documents the status quo, which is not a bug per se. Therefore, no failure, but change in the definition of success. IMHO, the series is easier to review if you it does not go back and forth. If you have one patch that says X is the right behavior, and then another patch that flips it to say Y is the right behavior, the reviewer would read each in sequence and want to be convinced by your arguments for X and Y. But you probably cannot make a good argument for X if you are trying to end up at Y. :) So I'd much rather see the test introduced with the desired end behavior, marked as expect_failure, and the commit message contain an argument about why Y is a good thing (and squashing the tests in with the actual fix is often even better, because the fix itself would want to contain the same argument). Just my two cents as a reviewer. My reasoning is twofold: - consistency between git show commit and git show blob I'm not sure I agree with this line of reasoning. git show commit is showing a diff, not the file contents; textconv has always been about munging the contents to produce a textual diff. It may be reasonable to extend its definition to this is the preferred human view of this content, and that happens to be what you would want to produce a diff. But I do not think it is necessarily inconsistent not to apply it for the blob case. - git show is a user facing command, and as such should produce output consumable by humans; whereas git cat-file is plumbing and should produce raw data unless told otherwise (-p, --textconv). That holds if the textconv is the only (or best) human-readable version of the file. And maybe that is reasonable. But is it possible that somebody uses textconv to produce a better diff of some already human-readable format? For example, imagine I define a textconv for XML files that normalizes the formatting to make diffs less noisy. When I am not looking at a diff, what is the best human-readable version? The original, or the normalized one? I'm not sure. Note that I'm somewhat playing devil's advocate here. For the cases where I have used textconv in the real world, I think I would probably prefer seeing the converted contents, and I am happy to call it user error if I use git show HEAD:foo.jpg bar.jpg accidentally. But I also want to make sure we are not regressing somebody else unnecessarily. Yes, the thing is that textconv helps diff by converting content (to text) before the (textual) diff. So it's somehow a double-faced beast. It's clearly activated by a diff attribute; so that would be a strong argument against my patch, at least against defaulting to --textconv for blobs. OTOH, textconv does have this aspect of converting text to a form digestable by humans (pre-diff, granted), which is the argument for defaulting to --textconv in porcellain. We could use a separate attribute show in addition to diff, but I don't think it's worth going there, unless there is a strong use case for diff-specific textconv which one would not want to apply when showing just the content. Michael -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes: was checking it out: a 'git log pathspec', when referring to a file inside the subtree, doesn't work as expected: it only displays the HEAD commit. This is somehow expected: the subtree merge changed the filename during merge (it is subtree/file.txt after the merge, and just file.txt before), so git log without --follow just considers the file appeared. OTOH, I think this is a known limitation of git log --follow that it does not follow renames done by subtree merges. -- 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] t4202 (log): add failing test for log with subtree
Matthieu Moy wrote: This is somehow expected: the subtree merge changed the filename during merge (it is subtree/file.txt after the merge, and just file.txt before), so git log without --follow just considers the file appeared. No, a merge does not change any filenames. The history of the file is very much present: run a git log HEAD^2 to see the entire history of the subtree. Even a git blame (without -M or -C) works just fine. OTOH, I think this is a known limitation of git log --follow that it does not follow renames done by subtree merges. Um, no. I think --follow is entirely orthogonal to the issue: unless I'm mistaken, it looks for other blobs in history with heuristically similar content. The real issue has nothing to do with log itself: it has to do with how rev-parse handles pathspecs. A 'git rev-parse HEAD:subproject/README' works fine, but 'git rev-parse HEAD^2:subproject/README' fails. However, 'git rev-parse HEAD^2:README' works, but it is assuming that the path README is present in /, when it is actually present in subproject/. Now, I'm not sure rev-parse is doing something unexpected, which is why I filed the bug in log. -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes: Matthieu Moy wrote: This is somehow expected: the subtree merge changed the filename during merge (it is subtree/file.txt after the merge, and just file.txt before), so git log without --follow just considers the file appeared. No, a merge does not change any filenames. Read again my message, especially the it is subtree/file.txt after the merge, and just file.txt before part. Replace subtree with bar and file.txt with ichi. -- 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] t4202 (log): add failing test for log with subtree
Thomas Rast wrote: Umm, it should follow the rename. There is no rename. Unless there is a commit with the following diff (with heuristically similar content), I don't see how --follow can work: diff --git a/README b/README deleted file mode 100644 diff --git a/subproject/README b/subproject/README new file mode 100644 Here, we just created one tiny merge commit after reading a tree into a non-/ prefix. There is no diff associated. -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra wrote: Unless there is a commit with the following diff (with heuristically similar content), I don't see how --follow can work: diff --git a/README b/README deleted file mode 100644 diff --git a/subproject/README b/subproject/README new file mode 100644 In other words, git diff-tree HEAD^2 HEAD is absolute nonsense in the subtree case. Let me outline how I think --follow works: A 'git log --follow foo' executes a diff-tree with historical trees until it finds one that removes (or adds, depending on which way you look at it) the 'foo' entry. It then inspects all the trees in the corresponding commit for a blob that is heuristically similar to the HEAD:foo blob, and follows it. How can you logically extend this concept to handle my case? -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes:
So after a long IRC discussion trying to hash out what you *want* it to
do, here's the summary for everyone else. This test is wrong on several
counts. For simplicity I'll denote by M the subtree merge, called
$new_h in the actual test.
+test_expect_failure 'log pathspec in tree read into prefix' '
+ git checkout --orphan subtree
+ git rm -rf .
+ echo foodle ichi
'ichi' also exists in M^1 because you reused a name from another test.
So rename detection will never pair the eventual 'bar/ichi' with this
'ichi', because the 'ichi' path was *modified*, not deleted, w.r.t. M^1.
Just to clarify, rename detection is based on seeing
A foo
D bar
and changing that to
R bar - foo
assuming the blobs were reasonably similar. And indeed, *copy*
detection (-C) is able to figure it out, because it considers all paths
that were modified as possible candidates for a copy source.
But --follow only uses rename detection.
+ git add ichi
+ test_tick
+ git commit -m foom
+ echo moodle unrelated
+ git add unrelated
+ test_tick
+ git commit -m quux
+ subtree_h=$(git rev-parse HEAD)
+ git checkout master
+ orig_h=$(git rev-parse HEAD)
+ git read-tree --prefix=bar $subtree_h
You need to supply a trailing / for the prefix, it's not implied.
+ new_t=$(git write-tree)
+ new_h=$(echo new subtree |
+ git commit-tree $new_t -p $orig_h -p $subtree_h)
+ git reset --hard $new_h
+ (
+ cd bar
+ git log --oneline ichi ../actual
You need to use --follow, as otherwise the pathspec filtering is
considered fixed.
Note that as discussed in the rest of the thread, --follow is fairly
limited and doesn't *really* solve the problem. But it does work
assuming the filenames are different and you only have one subtree
merge, like in this case.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] t4202 (log): add failing test for log with subtree
Thomas Rast tr...@inf.ethz.ch writes:
+test_expect_failure 'log pathspec in tree read into prefix' '
+git checkout --orphan subtree
+git rm -rf .
+echo foodle ichi
'ichi' also exists in M^1 because you reused a name from another test.
So rename detection will never pair the eventual 'bar/ichi' with this
'ichi', because the 'ichi' path was *modified*, not deleted, w.r.t. M^1.
Argh, that should read 'w.r.t. M^2', i.e. the subtree side.
The subtree side brings its own 'ichi', but it is moved to bar/ichi, so
there is a large difference between M:ichi (which came from M^1) and
M^2:ichi.
PS: As mentioned on IRC, even if you fix all that, a one-line file is
probably too small to pass the rename detection heuristics.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] Silent data loss on merge with uncommited changes + renames
Am 4/22/2013 11:24, schrieb Matthieu Moy: Following the discussion on merge with uncommited changes inside the git pull --autostash thread, I did a bit of testing, and encountered a case with silent data loss. In short: merge a branch introducing changes to a file. If the file has been renamed in the current branch, then git merge follows the rename and brings changes to the renamed file, but uncommited changes in this file are overriden silently. Can you check whether your case is already covered by one of: git grep expect_failure t/*merge* and if not, contribute a test case? -- 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] t4202 (log): add failing test for log with subtree
Thomas Rast wrote: [...] I think you've misunderstood the whole thing. The histories of M^1 and M^2 are completely unrelated: they're from different projects altogether. Considering the /ichi in M^2 a rename of the /ichi in M^1 is completely wrong. They have nothing to do with each other. I intentionally named it ichi in my orphan branch just to drive my point. I suspect you've got confused because I used an orphan branch to emulate a different project's history. If you want an end-user understanding of the problem, use git subtree: $ cd /tmp $ git clone gh:artagnon/varlog $ cd varlog $ git subtree add --prefix=clayoven \ gh:artagnon/clayoven master $ cd clayoven $ git log README.md What do you expect? The same output you would get if you cloned gh:artagnon/clayoven separately and executed 'git log README.md' on it. Now, clayoven's README.md (the one in HEAD^2) has nothing to do with varlog's README.md (the one in HEAD^1). It's just incidental that both projects have a README.md. I repeat: clayoven and varlog have _nothing_ to do with each other. If I say git log --follow README.md in the above example, I don't even get the HEAD commit. And I wouldn't expect to either. I will repeat this: --follow has nothing to do with the problem I've specified. And it is not tied to renaming (ie. changing the name/path of a file) as you've made it look. If you're still not convinced, I've included a testcase for --follow following over a merge commit (include it after the --follow test in the t4202). Try it without the --follow and you'll see what I mean. Neither the filename nor the filepath of ichi has changed in this example. -- 8 -- test_expect_success '--follow over merge' ' git checkout -b featurebranch echo foodle ichi git add ichi test_tick git commit -m add a line to the end of ichi echo moodle unrelated git add unrelated test_tick git commit -m quux git checkout master mv ichi ichi.bak echo gooble ichi cat ichi.bak ichi git add ichi test_tick git commit -m add a line to the beginning of ichi git merge featurebranch git log --follow --oneline ichi actual cat expect -\EOF df26551 add a line to the beginning of ichi 882d8d9 add a line to the end of ichi 2fbe8c0 third f7dab8e second 3a2fdcb initial EOF test_cmp expect 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: Staging Individual Lines
- Original Message - From: ode Date: 4/21/2013 4:22 PM I went looking on Google and found git-cola gui client which works for staging individual lines to the commit. The 'git gui' that ships with Git also stages individual lines and groups of lines, FWIW. -Josh -- 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] Silent data loss on merge with uncommited changes + renames
Johannes Sixt j.s...@viscovery.net writes: Am 4/22/2013 11:24, schrieb Matthieu Moy: Following the discussion on merge with uncommited changes inside the git pull --autostash thread, I did a bit of testing, and encountered a case with silent data loss. In short: merge a branch introducing changes to a file. If the file has been renamed in the current branch, then git merge follows the rename and brings changes to the renamed file, but uncommited changes in this file are overriden silently. Can you check whether your case is already covered by one of: git grep expect_failure t/*merge* Indeed, it's already here: test_expect_failure 'will not overwrite unstaged changes in renamed file' ' git reset --hard c1 git mv c1.c other.c git commit -m rename cp important other.c git merge c1a test_cmp important other.c ' -- 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 4/5] git-log.txt: rewrite note on why -- may be required
Thomas Rast tr...@inf.ethz.ch writes:
Thomas Rast tr...@inf.ethz.ch writes:
-- 8 --
Subject: [PATCH] git-log(1): remove --full-line-diff description
BTW, I generated this with your jc/format-patch, but it stopped working
after fc/send-email-annotate made it into next; I need this on top. Am
I missing something?
No, the topic has been stalled and left behind and needs to be
rebased on top of that other topic with your patch. Thanks.
It also needs a lot more work to de-mime its output to be eligible
for 'next', though.
-- 8 --
Subject: [PATCH] FIXUP jc/format-patch: adapt for fc/send-email-annotate
2a4c260 (format-patch: add format.coverLetter configuration variable,
2013-04-07) changed the coverletter variable to -1 by default, so the
die(... incompatible) always triggers. Test if it is 0 instead.
---
builtin/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 4804229..c972e62 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1247,7 +1247,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
/* Set defaults and check incompatible options */
if (rev.inline_single) {
use_stdout = 1;
- if (cover_letter)
+ if (cover_letter 0)
die(_(inline-single and cover-letter are
incompatible.));
if (thread)
die(_(inline-single and thread are incompatible.));
--
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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra wrote: $ git log README.md What do you expect? The same output you would get if you cloned gh:artagnon/clayoven separately and executed 'git log README.md' on it. And the reason I brought up rev-parse in the first place is because this problem is not unique to log. Try a 'git shortlog README.md' if you're not convinced. The entire revision-walking-pathspec-filtering mechanism in the log-like-family is broken on trees that are read into a non-/ prefix. And no, it doesn't have anything to do with renames or rename detection. If it did, 'git shortlog README.md' should atleast give me the commit that was responsible for the rename. And if you're still wondering about --follow, 'git shortlog --follow' (undocumented) is completely broken. -- 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] t4202 (log): add test for --follow over a merge
The --follow feature can be used to follow the history of a file over a merge commit, and is useful even when the file hasn't been copied/renamed. Add a test to show off this feature. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- I haven't updated the documentation, because I can't claim to understand --follow fully without reading the code (doing that now). In the meantime, I'm contributing a patch I wrote out originally as an example for Thomas Rast to prove a point. Where should this go? What is t4206-log-follow-harder-copies (it just has one test which isn't even that special)? Should we move all the --follow tests out of this file and put it in a separate file? t/t4202-log.sh | 29 + 1 file changed, 29 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 523c1be..5b041fd 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -168,6 +168,35 @@ test_expect_success 'git log --follow' ' ' +test_expect_success '--follow over merge' ' + git checkout -b featurebranch + echo foodle ichi + git add ichi + test_tick + git commit -m add a line to the end of ichi + echo moodle unrelated + git add unrelated + test_tick + git commit -m quux + git checkout master + mv ichi ichi.bak + echo gooble ichi + cat ichi.bak ichi + git add ichi + test_tick + git commit -m add a line to the beginning of ichi + git merge featurebranch + git log --follow --oneline ichi actual + cat expect -\EOF + df26551 add a line to the beginning of ichi + 882d8d9 add a line to the end of ichi + 2fbe8c0 third + f7dab8e second + 3a2fdcb initial + EOF + test_cmp expect actual +' + test_expect_failure 'log pathspec in tree read into prefix' ' git checkout --orphan subtree git rm -rf . -- 1.8.2.1.546.gea3475a -- 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] Silent data loss on merge with uncommited changes + renames
Matthieu Moy matthieu@grenoble-inp.fr writes: I could have expected git merge --abort to fail, but the problem is really more serious here: data loss is done silently before giving me an opportunity to do or abort anything. I think this is a well known and longstanding failure case in the recursive merge. As it does not perform its internal operation while handling renames in clean and distinct steps (i.e. figure out what goes to where before touching any index entry or working tree, then check if a proposed change to the index or the working tree conflicts with local changes, and finally carry out the proposed change), it is somewhat hard to fix it correctly in the current implementation, even though you probably could patch these up case by case basis. -- 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-to-mail notes resurrected
Thomas Rast tr...@student.ethz.ch writes: As some might remember, I made a script that writes notes (as in git-notes) linking patches to emails back in 2009: http://thread.gmane.org/gmane.comp.version-control.git/109074 I resurrected this idea the other week, using a faster implementation (the N command in fast-import is great!) and generating better links. I am regularly pushing the results to git://github.com/trast/git.git git://repo.or.cz/git/trast.git branches notes/gmanea link to the gmane thread view, focused on this patch notes/message-id the raw message-id Point your notes.displayRef at one or both to use them. It still fails to match some commits, so consider this WIP, but I think it's quite useful already. 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes:
Thomas Rast wrote:
[...]
I think you've misunderstood the whole thing. The histories of M^1
and M^2 are completely unrelated: they're from different projects
altogether. Considering the /ichi in M^2 a rename of the /ichi in
M^1 is completely wrong. They have nothing to do with each other. I
intentionally named it ichi in my orphan branch just to drive my
point. I suspect you've got confused because I used an orphan branch
to emulate a different project's history. If you want an end-user
understanding of the problem, use git subtree:
$ cd /tmp
$ git clone gh:artagnon/varlog
$ cd varlog
$ git subtree add --prefix=clayoven \
gh:artagnon/clayoven master
$ cd clayoven
$ git log README.md
What do you expect? The same output you would get if you cloned
gh:artagnon/clayoven separately and executed 'git log README.md' on
it.
No, I don't. But that's probably because I know a few things about how
git-log works that your hypothetical $USER doesn't.
At the risk of restating what everyone agrees on: It's a design
principle of git that it only stores tree states, and anything about
diffs, files, renames, etc. is purely in the imagination of the user.
We support that imagination by having analysis tools with which some
things can be found out, but others can't.
So (I think?) in the above you claim that $USER interprets
git log -- README.md
as
Show me the history of README.md.
But there's no such thing as the history of a file! The command instead
says
If I filter all history for only changes affecting a path 'README.md'
in the root of the repository[1], then what does it look like?
So please don't write tests that go contrary to that definition, because
they're *wrong*. The current implementation precisely matches the
current definition of pathspec filtering.
You can try arguing for changing the definition, but unless you find one
that can be implemented fast enough to be generally usable, I will
oppose that change.
The only thing that's broken in any of this is that I think, as
explained on IRC, that a hypothetical fixed --follow -C should be able
to figure out this case. By spending extra cycles on analysis,
naturally.
[1] and also skipping lines of history that seem uninteresting at this
point already, compare --simplify-merges
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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/6] show: obey --textconv for blobs
Jeff King p...@peff.net writes: Just my two cents as a reviewer. My reasoning is twofold: - consistency between git show commit and git show blob I'm not sure I agree with this line of reasoning. git show commit is showing a diff, not the file contents; textconv has always been about munging the contents to produce a textual diff. It may be reasonable to extend its definition to this is the preferred human view of this content, and that happens to be what you would want to produce a diff. But I do not think it is necessarily inconsistent not to apply it for the blob case. True. Applying textconv to otherwise unreadable blobs is often useful, but I agree that it is unexpected if it is done by default, especially given that many people have learned to do: git show HEAD~4:binary-gob old-binary-gob to recover old version of binary contents to a temporary file when checking the sanity of or restoring the breakage in the new one. It of course does _not_ forbid git show --textconv HEAD~4:binary-gob | less but I doubt it is a good idea to turn it on by default this late in the game. -- 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/6] show: obey --textconv for blobs
On Mon, Apr 22, 2013 at 08:25:41AM -0700, Junio C Hamano wrote: True. Applying textconv to otherwise unreadable blobs is often useful, but I agree that it is unexpected if it is done by default, especially given that many people have learned to do: git show HEAD~4:binary-gob old-binary-gob to recover old version of binary contents to a temporary file when checking the sanity of or restoring the breakage in the new one. It of course does _not_ forbid git show --textconv HEAD~4:binary-gob | less but I doubt it is a good idea to turn it on by default this late in the game. Exactly. I certainly do not mind it as an option, and I am on the fence regarding it as a default (I think it might have been a sane thing to do from the start, but at this point the change-of-behavior makes me hesitate). So I am perfectly willing to go either way, depending on what others think. I'm going to be out of email contact the rest of the week, so I'll let you two talk it out. :) -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] Filenames with single colon being treated as remote repository
On Sun, Apr 21, 2013 at 11:01:58AM -0700, Junio C Hamano wrote: I think the rule could be something like: 1. If it looks like a URL (^scheme://), it is. 2. Otherwise, if it is a path in the filesystem, it is. 3. Otherwise, if it has a colon, it's host:path 4. Otherwise, barf. where the interesting bit is the ordering of 2 and 3. It seems like git clone follows the order above with get_repo_path. But we do not seem to follow it in git_connect, where we prefer 3 over 2. At least for a string whose host part does not have any slash, switching the rules 2 and 3 in git_connect() would be a regression, no? frotz:/srv/git/git.git has been the way to talk to host frotz for a long time, and if you want to talk to a local directory that is a subdirectory of frotz:/ directory you have in your $cwd, you can disambiguate by saying ./frotz:/srv/git/git.git or something. Yeah, it would be a regression for fetch, though git clone frotz:/srv is already broken if that file exists; it turns into `realpath frotz:/srv` before we even feed it into the fetch machinery. So I think one reasonable path would be: 1. Do not treat host:path as ssh if host has a slash, which should not regress anybody. It does not allow unadorned relative paths with colons, but it lets you use absolute paths or ./ to disambiguate. 2. Teach git-clone to ask the transport code to parse the source repo spec, and decide from that whether it is local or not. That would harmonize the implementations and avoid errors when you _did_ mean to use ssh, but host:path happens to exist in your filesystem. I also would not be surprised if there are problems with URL-encoding, but maybe clone handles that properly (I didn't check). And the host contains slash rule is pretty easy to explain in the documentation, which is good. -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 2/6] show: obey --textconv for blobs
git show --textconv HEAD~4:binary-gob | less but I doubt it is a good idea to turn it on by default this late in the game. Exactly. I certainly do not mind it as an option, and I am on the fence regarding it as a default (I think it might have been a sane thing to do from the start, but at this point the change-of-behavior makes me hesitate). So I am perfectly willing to go either way, depending on what others think. some features detect if they are piping to a terminal... couldn't we do something like that ? -- 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] t4202 (log): add failing test for log with subtree
Thomas Rast wrote:
So (I think?) in the above you claim that $USER interprets
git log -- README.md
as
Show me the history of README.md.
But there's no such thing as the history of a file!
I made no such claims. I might not know as much as you or the others
on the list about git, but I can certainly grok how git stores
history.
There needs to be some amount of mutual respect for a healthy
conversation: if you start assuming midway that I don't understand
what history is, we have a problem.
So please don't write tests that go contrary to that definition, because
they're *wrong*. The current implementation precisely matches the
current definition of pathspec filtering.
Who said anything about changing any definitions? Where are you
getting all this from?
How does git log HEAD~3 -- README work? It sets up a revision walk
to start from HEAD~3 going all the way down to the root commit. In
each of these commits, it looks for the entry README in the
corresponding tree. It then runs diff-tree with the previous commit's
tree to see if the object (blob) corresponding to the README entry
is different: if so, it selects the commit and displays it.
Now, what am I saying? I'm saying that this approach assumes that all
trees are read into /. A pathspec subproject/README is _only_
present in the subtree-merge commit^{tree} and nowhere else. The
current log algorithm might try to look for the entry
subproject/README (your pathspec) in all the commit^{tree}s of the
commits leading up to M^2. That is _not_ the problem, as I have
already illustrated that --follow follows over merges. The problem is
looking for the pathspec subproject/README in the first place: those
commit^{trees}s have the entry stored as README.
Am I making any sense, or are you going to accuse me of not
understanding trees now?
--
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] t4202 (log): add failing test for log with subtree
Thomas Rast wrote:
The only thing that's broken in any of this is that I think, as
explained on IRC, that a hypothetical fixed --follow -C should be able
to figure out this case. By spending extra cycles on analysis,
naturally.
For the 100th time, nothing has been copied. There is no need to
spend time on any analysis. It's a very straightforward problem that
requires no computation or heuristics: it just requires you to strip
the leading subproject/ when looking for pathspecs in the M^2
commit^{tree}s. Done.
--
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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra wrote:
For the 100th time, nothing has been copied. There is no need to
spend time on any analysis. It's a very straightforward problem that
requires no computation or heuristics: it just requires you to strip
the leading subproject/ when looking for pathspecs in the M^2
commit^{tree}s. Done.
And if you're still not convinced, run 'git log HEAD^2 -- README.md'
from the toplevel directory. You'll get the log of README.md from the
subproject.
--
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/6] show: obey --textconv for blobs
Jeremy Rosen jeremy.ro...@openwide.fr writes: some features detect if they are piping to a terminal... couldn't we do something like that ? That's OK for convenience features like colors or so, but that would be really, really unexpected to have $ git show HEAD:file foo $ git show HEAD:file tmp $ cat tmp bar IMHO. -- 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: [BUG] Filenames with single colon being treated as remote repository
Jeff King p...@peff.net writes: So I think one reasonable path would be: 1. Do not treat host:path as ssh if host has a slash, which should not regress anybody. It does not allow unadorned relative paths with colons, but it lets you use absolute paths or ./ to disambiguate. 2. Teach git-clone to ask the transport code to parse the source repo spec, and decide from that whether it is local or not. That would harmonize the implementations and avoid errors when you _did_ mean to use ssh, but host:path happens to exist in your filesystem. I also would not be surprised if there are problems with URL-encoding, but maybe clone handles that properly (I didn't check). And the host contains slash rule is pretty easy to explain in the documentation, which is good. Sounds like a good direction to take us. -- 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] cherry-pick/revert: make usage say 'commit-ish...'
The usage string for cherry-pick and revert has never been updated to
reflect their ability to handle multiple commits. Other documentation is
already correct.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
builtin/revert.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index c5e36b9..0401fdb 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -19,13 +19,13 @@
*/
static const char * const revert_usage[] = {
- N_(git revert [options] commit-ish),
+ N_(git revert [options] commit-ish...),
N_(git revert subcommand),
NULL
};
static const char * const cherry_pick_usage[] = {
- N_(git cherry-pick [options] commit-ish),
+ N_(git cherry-pick [options] commit-ish...),
N_(git cherry-pick subcommand),
NULL
};
--
1.8.2.255.g39c5835
--
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] zlib: fix compilation failures with Sun C Compilaer
Do this by removing a couple of useless return statements. Without this change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15 2010/08/11) fails with the following message: zlib.c, line 192: void function cannot return value zlib.c, line 201: void function cannot return value cc: acomp failed for zlib.c Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com --- zlib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zlib.c b/zlib.c index bbaa081..61e6df0 100644 --- a/zlib.c +++ b/zlib.c @@ -189,7 +189,7 @@ void git_deflate_init_gzip(git_zstream *strm, int level) * Use default 15 bits, +16 is to generate gzip header/trailer * instead of the zlib wrapper. */ - return do_git_deflate_init(strm, level, 15 + 16); + do_git_deflate_init(strm, level, 15 + 16); } void git_deflate_init_raw(git_zstream *strm, int level) @@ -198,7 +198,7 @@ void git_deflate_init_raw(git_zstream *strm, int level) * Use default 15 bits, negate the value to get raw compressed * data without zlib header and trailer. */ - return do_git_deflate_init(strm, level, -15); + do_git_deflate_init(strm, level, -15); } int git_deflate_abort(git_zstream *strm) -- 1.8.1.rc3.897.gb3600c3 -- 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] t4202 (log): add failing test for log with subtree
Thomas Rast tr...@inf.ethz.ch writes: So (I think?) in the above you claim that $USER interprets git log -- README.md as Show me the history of README.md. But there's no such thing as the history of a file! The command instead says If I filter all history for only changes affecting a path 'README.md' in the root of the repository[1], then what does it look like? Correct. The design principle I did not quote from your message comes from one of the most important messages in the list archive ($gmane/217). Three issues with --follow are: (1) It uses the given pathname as single pathspec and drill down the same way without --follow until it notices the path disappears and until then there is no attempt to detect renames is made. And it only does -M variant of rename detection (2) The same way in (1) includes the merge simplification to cull side branches if one parent matches the end result. In a history where the first parent of a merge M, i.e. M^1, did not have path F, its second parent M^2 had path F, and the merge result M deposited the contents of M^2:F at M:D/F, then running log --follow F starting from M would notice that the end result M did not have F in it, which is shared with its first parent M^1, and culls the side branch M^2. The --full-history option may let you keep the history leading to M^2, though. (3) When (1) notices that the path being followed did not exist in any of the parents (be it a merge or a non-merge) and finds a different path with a similar looking content, it _switches_ the pathspec to it, but the single pathspec it uses is a global state and affects traversals of other ancestry paths at the same time. Because of this, --follow will not work correctly in a history that contains merges. It often _appears_ to work only by accident. The first two are relatively minor (the second is not even an issue). To solve the last one, you would need to keep track of which path it is following per traversal path. -- 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] zlib: fix compilation failures with Sun C Compilaer
Stefano Lattarini stefano.lattar...@gmail.com writes: Do this by removing a couple of useless return statements. Without this change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15 2010/08/11) fails with the following message: zlib.c, line 192: void function cannot return value zlib.c, line 201: void function cannot return value cc: acomp failed for zlib.c Thanks for catching a recent regression in the mainline before any tagged release is made out of it. Very much appreciated. Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com --- zlib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zlib.c b/zlib.c index bbaa081..61e6df0 100644 --- a/zlib.c +++ b/zlib.c @@ -189,7 +189,7 @@ void git_deflate_init_gzip(git_zstream *strm, int level) * Use default 15 bits, +16 is to generate gzip header/trailer * instead of the zlib wrapper. */ - return do_git_deflate_init(strm, level, 15 + 16); + do_git_deflate_init(strm, level, 15 + 16); } void git_deflate_init_raw(git_zstream *strm, int level) @@ -198,7 +198,7 @@ void git_deflate_init_raw(git_zstream *strm, int level) * Use default 15 bits, negate the value to get raw compressed * data without zlib header and trailer. */ - return do_git_deflate_init(strm, level, -15); + do_git_deflate_init(strm, level, -15); } int git_deflate_abort(git_zstream *strm) -- 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] zlib: fix compilation failures with Sun C Compilaer
Am 22.04.2013 18:18, schrieb Stefano Lattarini: Do this by removing a couple of useless return statements. Without this change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15 2010/08/11) fails with the following message: zlib.c, line 192: void function cannot return value zlib.c, line 201: void function cannot return value cc: acomp failed for zlib.c Hmm, what was I thinking when I introduced these returns in c3c2e1a0? :-/ Thanks for catching! René -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra wrote: And if you're still not convinced, run 'git log HEAD^2 -- README.md' from the toplevel directory. You'll get the log of README.md from the subproject. On IRC, Thomas explained to me that mixing in changes from various branches into the pathspec will break this so-called determinism. To try it out for yourself, do: $ cd /tmp $ git clone gh:trast/subtree-mainline-example $ cd subtree-mainline-example $ git log HEAD^2 -- sub # only lists the side changes $ git log -- dir/sub # only lists the mainline changes What we should really expect is a mix of the two. -- 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] zlib: fix compilation failures with Sun C Compilaer
On 04/22/2013 06:48 PM, Junio C Hamano wrote: Stefano Lattarini stefano.lattar...@gmail.com writes: Do this by removing a couple of useless return statements. Without this change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15 2010/08/11) fails with the following message: zlib.c, line 192: void function cannot return value zlib.c, line 201: void function cannot return value cc: acomp failed for zlib.c Thanks for catching a recent regression in the mainline before any tagged release is made out of it. Very much appreciated. Actually, I tried to build the bleeding-edge git on Solaris to use it myself, rather than to test it ;-) So, thanks to you and all the git contributors for continuously improving the package, thus making it worth to try to build and use the bleeding-edge version. Best regards, Stefano -- 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] t4202 (log): add failing test for log with subtree
Junio C Hamano wrote: [...] (3) When (1) notices that the path being followed did not exist in any of the parents (be it a merge or a non-merge) and finds a different path with a similar looking content, it _switches_ the pathspec to it, but the single pathspec it uses is a global state and affects traversals of other ancestry paths at the same time. Because of this, --follow will not work correctly in a history that contains merges. It often _appears_ to work only by accident. This explanation is all very nice, but isn't it completely tangential to the issue at hand? Let's say I have a subtree merge M located at HEAD~4. I ask for the log of 'HEAD~4 -- README' with --follow. It follows until it gets to M: at M, M^1:README is missing, but M^2:README is present. Should it follow down and show the history of M^2:README? You can reserve the discussion about --follow working in the general case for another thread. Meanwhile, you're evading the issue of assuming that all trees are read into /, and are really representing the same project's history, while this is not the case. -- 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 4/5] check-ignore: allow incremental streaming of queries via --stdin
Adam Spiers g...@adamspiers.org writes: On Thu, Apr 11, 2013 at 03:11:32PM -0400, Jeff King wrote: I always get a little nervous with sleeps in the test suite, as they are indicative that we are trying to avoid some race condition, which means that the test can fail when the system is under load, or when a tool like valgrind is used which drastically alters the timing (e.g., if check-ignore takes longer than 1 second to produce its answer, we may fail here). Agreed, especially here where my btrfs filesystems see fit to kindly freeze my system for a few seconds many times each day :-/ Is there a simpler way to test this? Like: ... I'll re-roll using your approach. I think I missed this one and it already is in 'next'. I'll hold it back so please make your re-roll into an incremental update. 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes: Meanwhile, you're evading the issue of assuming that all trees are read into /, and are really representing the same project's history, while this is not the case. This is fundamenally how Git works. Git works with commit objects, each commit object points to a tree object that represent / at this commit. When you do a subtree merge, you include the tree that was in / in a subtree of the master project. Files used to exist as /file, and now exist as /subtree/file. There is nothing recording that the new /subtree/file comes from /file in its second parent. Call this a renaming or not, but git log subtree/file won't show you changes touching /file by default, and this is the case for the history of the subproject you are merging. A subtree merge is really a rename of the subproject's file plus a merge, done in the same commit. Try doing something like mkdir -p subproject git mv * subproject/ git commit in your subproject before doing a merge, you'll get the same result, except there will be one more commit. -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: [...] (3) When (1) notices that the path being followed did not exist in any of the parents (be it a merge or a non-merge) and finds a different path with a similar looking content, it _switches_ the pathspec to it, but the single pathspec it uses is a global state and affects traversals of other ancestry paths at the same time. Because of this, --follow will not work correctly in a history that contains merges. It often _appears_ to work only by accident. This explanation is all very nice, but isn't it completely tangential to the issue at hand? I think you are talking mostly about (2), but the primary purpose of my message was not about your specific issue. It was to give a larger picture to people who may be inclined to tackle, and may be capable of tackling, the real issues in the --follow codepath. Anybody who wants to update it needs to be aware of all three. -- 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] t4202 (log): add failing test for log with subtree
Ramkumar Ramachandra artag...@gmail.com writes:
Ramkumar Ramachandra wrote:
And if you're still not convinced, run 'git log HEAD^2 -- README.md'
from the toplevel directory. You'll get the log of README.md from the
subproject.
On IRC, Thomas explained to me that mixing in changes from various
branches into the pathspec will break this so-called determinism. To
try it out for yourself, do:
$ cd /tmp
$ git clone gh:trast/subtree-mainline-example
$ cd subtree-mainline-example
$ git log HEAD^2 -- sub # only lists the side changes
$ git log -- dir/sub # only lists the mainline changes
What we should really expect is a mix of the two.
So after lots of confusing misunderstandings across the entire thread,
and a long IRC discussion, I do have one take-away that I think is worth
writing down:
There is a market for a rename detection that works at the tree level.
Bear with me while I explain.
The average subtree merge (after the initial one) looks like this,
focusing on the subtree:
Mnew state in sub/
| \
| * new state in /
|
* old state in sub/
(The example in the quote above additionally complicates the issue by
changing sub/ in the mainline.)
Ideally we'd like our hypothetical fixed --follow to accurately track a
pathspec 'sub/foo' so that in the mainline it remains the same, but in
the side it becomes 'foo'. Because of reasons already explained in
earlier mails, -M does not suffice for all cases (in particular, it
fails if there is a /foo in the mainline too). -C works, but is slow.
So how can we fix that? We could try to somehow figure out that M:sub/
refers to the same thing as M^2:/, by looking at them at the tree level.
Let's provisionally call this --follow-tree-rename.
I don't quite know how to heuristically[1] detect such a rename, but
since 'merge -ssubtree' does it (if you don't specify a tree prefix
explicitly), it can't be That Hard(tm). If we're extra lucky it's fast
enough to be enabled by default.
In the special case where the subtree was not modified in the mainline
since the last merge, the problem is pretty trivial: simply check if any
subtree of M agrees with the root tree of each merge parent; if so, diff
those trees instead of the root trees.
That should then help subtree users to get better logs.
[1] the quoted example shows that you can't just go looking for
identical trees in the general case, so it is really a heuristic
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-imap-send.txt: remove the use of sslverify=false
Since SSL provides no protection if the certificates aren't verified it's better not to include sslverify=false in the examples. Also in the post 1.8.2.1 era git is able to properly verify the validity of a certificate as well it's origin. Signed-off-by: Barbu Paul - Gheorghe barbu.paul.gheor...@gmail.com --- Documentation/git-imap-send.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 875d283..0d72977 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -108,7 +108,6 @@ Using direct mode with SSL: user = bob pass = p4ssw0rd port = 123 -sslverify = false .. @@ -123,7 +122,6 @@ to specify your account settings: host = imaps://imap.gmail.com user = u...@gmail.com port = 993 - sslverify = false - You might need to instead use: folder = [Google Mail]/Drafts if you get an error -- Barbu Paul - Gheorghe Common sense is not so common - Voltaire Visit My GitHub profile to see my open-source projects - https://github.com/paullik -- 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] Make --full-history consider more merges
History simplification previously always treated merges as TREESAME
if they were TREESAME to any parent.
While the desired default behaviour, this could be extremely unhelpful
when searching detailed history, and could not be overridden. For
example, if a merge had ignored a change, as if by -s ours, then:
git log -m -p --full-history -Schange file
would successfully locate change's addition but would not locate the
merge that resolved against it.
This patch changes the simplification so that when --full-history is
specified, a merge is treated as TREESAME only if it is TREESAME to
_all_ parents. This means the command above locates a merge that dropped
change.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
This would address my problem case - it passes existing tests, and covers
my (all-too-common) problem. But it would also need documentation changes and
a new test.
revision.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index eb98128..96fe3f5 100644
--- a/revision.c
+++ b/revision.c
@@ -516,8 +516,14 @@ static void try_to_simplify_commit(struct rev_info *revs,
struct commit *commit)
}
die(bad tree compare for commit %s,
sha1_to_hex(commit-object.sha1));
}
- if (tree_changed !tree_same)
- return;
+
+ if (tree_changed) {
+ if (!tree_same)
+ return;
+
+ if (!revs-simplify_history !revs-simplify_merges)
+ return;
+ }
commit-object.flags |= TREESAME;
}
--
1.8.2.255.g39c5835
--
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] Make --full-history consider more merges
Kevin Bracey ke...@bracey.fi writes:
diff --git a/revision.c b/revision.c
index eb98128..96fe3f5 100644
--- a/revision.c
+++ b/revision.c
@@ -516,8 +516,14 @@ static void try_to_simplify_commit(struct rev_info
*revs, struct commit *commit)
}
die(bad tree compare for commit %s,
sha1_to_hex(commit-object.sha1));
}
- if (tree_changed !tree_same)
- return;
+
+ if (tree_changed) {
+ if (!tree_same)
+ return;
+
+ if (!revs-simplify_history !revs-simplify_merges)
+ return;
So in addition to have some change and there is no same parent
case, under _some_ condition we avoid marking a merge not worth
showing (i.e. TREESAME) if there is any change.
And the condition is !simplify_history and !simplify_merges, which
would cover --full-history, but I am not sure if requiring
!simplify_merges is correct.
Do you need it and if so why? The --simplify-merges option is
defined as a post-processing operation over what full-history
produces in the list limiting code (which involves the logic the
patch is touching). The --ancestry-path option works the same way
but its post-processing is done inside the limit_list() function.
So it feels more natural if the patch were ignoring simplify_merges
and paid attention only to simplify_history.
+ }
commit-object.flags |= TREESAME;
}
--
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 00/33] Various cleanups around reference packing and peeling
Thanks for the feedback; here is a re-roll. A number of points
discussed on the mailing list were fixed. The main change, in patch
17, is how repack_without_ref() deals with references that cannot be
peeled when re-writing the packed-refs file:
if ISBROKEN:
emit an error and omit reference from the output
else if !has_sha1_file(...):
if there is an overriding loose reference:
silently omit reference from the output
else:
emit an error and omit reference from the output
Please note that this creates a relatively harmless race condition
very similar to the ones discussed for pack-refs; see the commit
message for patch 17 for a long explanation. I would like to fix all
of the races as part of a separate patch series.
For now I left the sleeps in t3210. Given that the problem will be
solved by topic jc/prune-all, building a fancier workaround into this
test for the old broken --expire behavior seems like a waste of time.
I propose that the sleeps be removed when this patch series is merged
with jc/prune-all. (In fact, when jc/prune-all is landed, other tests
can also be simplified.) If this suggestion is not ok, then the
easiest thing would probably be to remove the sleeps immediately and
declare jc/prune-all a prerequisite of this series.
I also removed the trailing comma from the enum peel_status
definition, because a recent email on the mailing list claimed that
some compilers don't like them.
Michael Haggerty (33):
refs: document flags constants REF_*
refs: document the fields of struct ref_value
refs: document do_for_each_ref() and do_one_ref()
refs: document how current_ref is used
refs: define constant PEELED_LINE_LENGTH
do_for_each_ref_in_dirs(): remove dead code
get_packed_ref(): return a ref_entry
peel_ref(): use function get_packed_ref()
repack_without_ref(): use function get_packed_ref()
refs: extract a function ref_resolves_to_object()
refs: extract function peel_object()
peel_object(): give more specific information in return value
peel_ref(): fix return value for non-peelable, not-current reference
refs: extract a function peel_entry()
refs: change the internal reference-iteration API
t3210: test for spurious error messages for dangling packed refs
repack_without_ref(): silence errors for dangling packed refs
search_ref_dir(): return an index rather than a pointer
refs: change how packed refs are deleted
t3211: demonstrate loss of peeled refs if a packed ref is deleted
repack_without_ref(): write peeled refs in the rewritten file
refs: extract a function write_packed_entry()
pack-refs: rename handle_one_ref() to pack_one_ref()
pack-refs: merge code from pack-refs.{c,h} into refs.{c,h}
pack_one_ref(): rename path parameter to refname
refs: use same lock_file object for both ref-packing functions
pack_refs(): change to use do_for_each_entry()
refs: inline function do_not_prune()
pack_one_ref(): use function peel_entry()
pack_one_ref(): use write_packed_entry() to do the writing
pack_one_ref(): do some cheap tests before a more expensive one
refs: change do_for_each_*() functions to take ref_cache arguments
refs: handle the main ref_cache specially
Makefile | 2 -
builtin/clone.c | 1 -
builtin/pack-refs.c | 2 +-
pack-refs.c | 148 ---
pack-refs.h | 18 --
refs.c | 733 +++
refs.h | 35 +++
t/t3210-pack-refs.sh | 36 +++
t/t3211-peel-ref.sh | 9 +
9 files changed, 643 insertions(+), 341 deletions(-)
delete mode 100644 pack-refs.c
delete mode 100644 pack-refs.h
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/33] refs: document flags constants REF_*
Document the bits that can appear in the flags parameter passed to
an each_ref_function and/or in the ref_entry::flag field.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 12 +++-
refs.h | 13 +
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index de2d8eb..30b4bf7 100644
--- a/refs.c
+++ b/refs.c
@@ -158,7 +158,17 @@ struct ref_dir {
struct ref_entry **entries;
};
-/* ISSYMREF=0x01, ISPACKED=0x02, and ISBROKEN=0x04 are public interfaces */
+/*
+ * Bit values for ref_entry::flag. REF_ISSYMREF=0x01,
+ * REF_ISPACKED=0x02, and REF_ISBROKEN=0x04 are public values; see
+ * refs.h.
+ */
+
+/*
+ * The field ref_entry-u.value.peeled of this value entry contains
+ * the correct peeled value for the reference, which might be
+ * null_sha1 if the reference is not a tag or if it is broken.
+ */
#define REF_KNOWS_PEELED 0x08
/* ref_entry represents a directory of references */
diff --git a/refs.h b/refs.h
index a35eafc..17bc1c1 100644
--- a/refs.h
+++ b/refs.h
@@ -10,8 +10,21 @@ struct ref_lock {
int force_write;
};
+/*
+ * Bit values set in the flags argument passed to each_ref_fn():
+ */
+
+/* Reference is a symbolic reference. */
#define REF_ISSYMREF 0x01
+
+/* Reference is a packed reference. */
#define REF_ISPACKED 0x02
+
+/*
+ * Reference cannot be resolved to an object name: dangling symbolic
+ * reference (directly or indirectly), corrupt reference file, or
+ * symbolic reference refers to ill-formatted reference name.
+ */
#define REF_ISBROKEN 0x04
/*
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/33] refs: document do_for_each_ref() and do_one_ref()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 1df1ccd..44cc2fc 100644
--- a/refs.c
+++ b/refs.c
@@ -525,10 +525,14 @@ static void sort_ref_dir(struct ref_dir *dir)
dir-sorted = dir-nr = i;
}
-#define DO_FOR_EACH_INCLUDE_BROKEN 01
+/* Include broken references in a do_for_each_ref*() iteration: */
+#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
static struct ref_entry *current_ref;
+/*
+ * Handle one reference in a do_for_each_ref*()-style iteration.
+ */
static int do_one_ref(const char *base, each_ref_fn fn, int trim,
int flags, void *cb_data, struct ref_entry *entry)
{
@@ -1338,6 +1342,15 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt,
const char *refname)
for_each_rawref(warn_if_dangling_symref, data);
}
+/*
+ * Call fn for each reference in the specified submodule for which the
+ * refname begins with base. If trim is non-zero, then trim that many
+ * characters off the beginning of each refname before passing the
+ * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include
+ * broken references in the iteration. If fn ever returns a non-zero
+ * value, stop the iteration and return that value; otherwise, return
+ * 0.
+ */
static int do_for_each_ref(const char *submodule, const char *base,
each_ref_fn fn,
int trim, int flags, void *cb_data)
{
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/33] refs: document the fields of struct ref_value
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 12
1 file changed, 12 insertions(+)
diff --git a/refs.c b/refs.c
index 30b4bf7..1df1ccd 100644
--- a/refs.c
+++ b/refs.c
@@ -109,7 +109,19 @@ struct ref_entry;
* (ref_entry-flag REF_DIR) is zero.
*/
struct ref_value {
+ /*
+* The name of the object to which this reference resolves
+* (which may be a tag object). If REF_ISBROKEN, this is
+* null. If REF_ISSYMREF, then this is the name of the object
+* referred to by the last reference in the symlink chain.
+*/
unsigned char sha1[20];
+
+ /*
+* If REF_KNOWS_PEELED, then this field holds the peeled value
+* of this reference, or null if the reference is known not to
+* be peelable.
+*/
unsigned char peeled[20];
};
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/33] refs: document how current_ref is used
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 9 + 1 file changed, 9 insertions(+) diff --git a/refs.c b/refs.c index 44cc2fc..efad658 100644 --- a/refs.c +++ b/refs.c @@ -528,6 +528,15 @@ static void sort_ref_dir(struct ref_dir *dir) /* Include broken references in a do_for_each_ref*() iteration: */ #define DO_FOR_EACH_INCLUDE_BROKEN 0x01 +/* + * current_ref is a performance hack: when iterating over references + * using the for_each_ref*() functions, current_ref is set to the + * current reference's entry before calling the callback function. If + * the callback function calls peel_ref(), then peel_ref() first + * checks whether the reference to be peeled is the current reference + * (it usually is) and if so, returns that reference's peeled version + * if it is available. This avoids a refname lookup in a common case. + */ static struct ref_entry *current_ref; /* -- 1.8.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/33] get_packed_ref(): return a ref_entry
Instead of copying the reference's SHA1 into a caller-supplied
variable, just return the ref_entry itself (or NULL if there is no
such entry). This change will allow the function to be used from
elsewhere.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index 7260768..91a2940 100644
--- a/refs.c
+++ b/refs.c
@@ -1100,18 +1100,12 @@ int resolve_gitlink_ref(const char *path, const char
*refname, unsigned char *sh
}
/*
- * Try to read ref from the packed references. On success, set sha1
- * and return 0; otherwise, return -1.
+ * Return the ref_entry for the given refname from the packed
+ * references. If it does not exist, return NULL.
*/
-static int get_packed_ref(const char *refname, unsigned char *sha1)
+static struct ref_entry *get_packed_ref(const char *refname)
{
- struct ref_dir *packed = get_packed_refs(get_ref_cache(NULL));
- struct ref_entry *entry = find_ref(packed, refname);
- if (entry) {
- hashcpy(sha1, entry-u.value.sha1);
- return 0;
- }
- return -1;
+ return find_ref(get_packed_refs(get_ref_cache(NULL)), refname);
}
const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int
reading, int *flag)
@@ -1139,13 +1133,17 @@ const char *resolve_ref_unsafe(const char *refname,
unsigned char *sha1, int rea
git_snpath(path, sizeof(path), %s, refname);
if (lstat(path, st) 0) {
+ struct ref_entry *entry;
+
if (errno != ENOENT)
return NULL;
/*
* The loose reference file does not exist;
* check for a packed reference.
*/
- if (!get_packed_ref(refname, sha1)) {
+ entry = get_packed_ref(refname);
+ if (entry) {
+ hashcpy(sha1, entry-u.value.sha1);
if (flag)
*flag |= REF_ISPACKED;
return refname;
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/33] peel_ref(): use function get_packed_ref()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 91a2940..09b68e4 100644
--- a/refs.c
+++ b/refs.c
@@ -1282,10 +1282,9 @@ int peel_ref(const char *refname, unsigned char *sha1)
return -1;
if ((flag REF_ISPACKED)) {
- struct ref_dir *dir = get_packed_refs(get_ref_cache(NULL));
- struct ref_entry *r = find_ref(dir, refname);
+ struct ref_entry *r = get_packed_ref(refname);
- if (r != NULL r-flag REF_KNOWS_PEELED) {
+ if (r (r-flag REF_KNOWS_PEELED)) {
hashcpy(sha1, r-u.value.peeled);
return 0;
}
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/33] refs: extract function peel_object()
It is a nice, logical unit of work, and putting it in a function
removes the need to use a goto in peel_ref(). Soon it will also have
other uses.
The algorithm is unchanged.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 50 ++
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/refs.c b/refs.c
index 8b554d8..b0ef129 100644
--- a/refs.c
+++ b/refs.c
@@ -1272,11 +1272,38 @@ static int filter_refs(const char *refname, const
unsigned char *sha1, int flags
return filter-fn(refname, sha1, flags, filter-cb_data);
}
+/*
+ * Peel the named object; i.e., if the object is a tag, resolve the
+ * tag recursively until a non-tag is found. Store the result to sha1
+ * and return 0 iff successful. If the object is not a tag or is not
+ * valid, return -1 and leave sha1 unchanged.
+ */
+static int peel_object(const unsigned char *name, unsigned char *sha1)
+{
+ struct object *o = lookup_unknown_object(name);
+
+ if (o-type == OBJ_NONE) {
+ int type = sha1_object_info(name, NULL);
+ if (type 0)
+ return -1;
+ o-type = type;
+ }
+
+ if (o-type != OBJ_TAG)
+ return -1;
+
+ o = deref_tag_noverify(o);
+ if (!o)
+ return -1;
+
+ hashcpy(sha1, o-sha1);
+ return 0;
+}
+
int peel_ref(const char *refname, unsigned char *sha1)
{
int flag;
unsigned char base[20];
- struct object *o;
if (current_ref (current_ref-name == refname
|| !strcmp(current_ref-name, refname))) {
@@ -1286,8 +1313,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
hashcpy(sha1, current_ref-u.value.peeled);
return 0;
}
- hashcpy(base, current_ref-u.value.sha1);
- goto fallback;
+ return peel_object(current_ref-u.value.sha1, sha1);
}
if (read_ref_full(refname, base, 1, flag))
@@ -1302,23 +1328,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
}
}
-fallback:
- o = lookup_unknown_object(base);
- if (o-type == OBJ_NONE) {
- int type = sha1_object_info(base, NULL);
- if (type 0)
- return -1;
- o-type = type;
- }
-
- if (o-type == OBJ_TAG) {
- o = deref_tag_noverify(o);
- if (o) {
- hashcpy(sha1, o-sha1);
- return 0;
- }
- }
- return -1;
+ return peel_object(base, sha1);
}
struct warn_if_dangling_data {
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/33] peel_object(): give more specific information in return value
Instead of just returning a success/failure bit, return an enumeration
value that explains the reason for any failure. This will come in
handy shortly.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index b0ef129..d26e847 100644
--- a/refs.c
+++ b/refs.c
@@ -1272,32 +1272,48 @@ static int filter_refs(const char *refname, const
unsigned char *sha1, int flags
return filter-fn(refname, sha1, flags, filter-cb_data);
}
+enum peel_status {
+ /* object was peeled successfully: */
+ PEEL_PEELED = 0,
+
+ /*
+* object cannot be peeled because the named object (or an
+* object referred to by a tag in the peel chain), does not
+* exist.
+*/
+ PEEL_INVALID = -1,
+
+ /* object cannot be peeled because it is not a tag: */
+ PEEL_NON_TAG = -2
+};
+
/*
* Peel the named object; i.e., if the object is a tag, resolve the
- * tag recursively until a non-tag is found. Store the result to sha1
- * and return 0 iff successful. If the object is not a tag or is not
- * valid, return -1 and leave sha1 unchanged.
+ * tag recursively until a non-tag is found. If successful, store the
+ * result to sha1 and return PEEL_PEELED. If the object is not a tag
+ * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
+ * and leave sha1 unchanged.
*/
-static int peel_object(const unsigned char *name, unsigned char *sha1)
+static enum peel_status peel_object(const unsigned char *name, unsigned char
*sha1)
{
struct object *o = lookup_unknown_object(name);
if (o-type == OBJ_NONE) {
int type = sha1_object_info(name, NULL);
if (type 0)
- return -1;
+ return PEEL_INVALID;
o-type = type;
}
if (o-type != OBJ_TAG)
- return -1;
+ return PEEL_NON_TAG;
o = deref_tag_noverify(o);
if (!o)
- return -1;
+ return PEEL_INVALID;
hashcpy(sha1, o-sha1);
- return 0;
+ return PEEL_PEELED;
}
int peel_ref(const char *refname, unsigned char *sha1)
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/33] peel_ref(): fix return value for non-peelable, not-current reference
The old version was inconsistent: when a reference was
REF_KNOWS_PEELED but with a null peeled value, it returned non-zero
for the current reference but zero for other references. Change the
behavior for non-current references to match that of current_ref,
which is what callers expect. Document the behavior.
Current callers only call peel_ref() from within a for_each_ref-style
iteration and only for the current ref; therefore, the buggy code path
was never reached.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 5 -
refs.h | 8
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index d26e847..2f73189 100644
--- a/refs.c
+++ b/refs.c
@@ -120,7 +120,8 @@ struct ref_value {
/*
* If REF_KNOWS_PEELED, then this field holds the peeled value
* of this reference, or null if the reference is known not to
-* be peelable.
+* be peelable. See the documentation for peel_ref() for an
+* exact definition of peelable.
*/
unsigned char peeled[20];
};
@@ -1339,6 +1340,8 @@ int peel_ref(const char *refname, unsigned char *sha1)
struct ref_entry *r = get_packed_ref(refname);
if (r (r-flag REF_KNOWS_PEELED)) {
+ if (is_null_sha1(r-u.value.peeled))
+ return -1;
hashcpy(sha1, r-u.value.peeled);
return 0;
}
diff --git a/refs.h b/refs.h
index 17bc1c1..ac0ff92 100644
--- a/refs.h
+++ b/refs.h
@@ -74,6 +74,14 @@ extern void add_packed_ref(const char *refname, const
unsigned char *sha1);
extern int ref_exists(const char *);
+/*
+ * If refname is a non-symbolic reference that refers to a tag object,
+ * and the tag can be (recursively) dereferenced to a non-tag object,
+ * store the SHA1 of the referred-to object to sha1 and return 0. If
+ * any of these conditions are not met, return a non-zero value.
+ * Symbolic references are considered unpeelable, even if they
+ * ultimately resolve to a peelable tag.
+ */
extern int peel_ref(const char *refname, unsigned char *sha1);
/** Locks a refs/ ref returning the lock on success and NULL on failure. **/
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/33] refs: extract a function peel_entry()
Peel the entry, and as a side effect store the peeled value in the
entry. Use this function from two places in peel_ref(); a third
caller will be added soon.
Please note that this change can lead to ref_entries for unpacked refs
being peeled. This has no practical benefit but is harmless.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 63 +--
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 2f73189..777a4b7 100644
--- a/refs.c
+++ b/refs.c
@@ -1285,7 +1285,17 @@ enum peel_status {
PEEL_INVALID = -1,
/* object cannot be peeled because it is not a tag: */
- PEEL_NON_TAG = -2
+ PEEL_NON_TAG = -2,
+
+ /* ref_entry contains no peeled value because it is a symref: */
+ PEEL_IS_SYMREF = -3,
+
+ /*
+* ref_entry cannot be peeled because it is broken (i.e., the
+* symbolic reference cannot even be resolved to an object
+* name):
+*/
+ PEEL_BROKEN = -4
};
/*
@@ -1317,31 +1327,56 @@ static enum peel_status peel_object(const unsigned char
*name, unsigned char *sh
return PEEL_PEELED;
}
+/*
+ * Peel the entry (if possible) and return its new peel_status.
+ */
+static enum peel_status peel_entry(struct ref_entry *entry)
+{
+ enum peel_status status;
+
+ if (entry-flag REF_KNOWS_PEELED)
+ return is_null_sha1(entry-u.value.peeled) ?
+ PEEL_NON_TAG : PEEL_PEELED;
+ if (entry-flag REF_ISBROKEN)
+ return PEEL_BROKEN;
+ if (entry-flag REF_ISSYMREF)
+ return PEEL_IS_SYMREF;
+
+ status = peel_object(entry-u.value.sha1, entry-u.value.peeled);
+ if (status == PEEL_PEELED || status == PEEL_NON_TAG)
+ entry-flag |= REF_KNOWS_PEELED;
+ return status;
+}
+
int peel_ref(const char *refname, unsigned char *sha1)
{
int flag;
unsigned char base[20];
if (current_ref (current_ref-name == refname
- || !strcmp(current_ref-name, refname))) {
- if (current_ref-flag REF_KNOWS_PEELED) {
- if (is_null_sha1(current_ref-u.value.peeled))
- return -1;
- hashcpy(sha1, current_ref-u.value.peeled);
- return 0;
- }
- return peel_object(current_ref-u.value.sha1, sha1);
+ || !strcmp(current_ref-name, refname))) {
+ if (peel_entry(current_ref))
+ return -1;
+ hashcpy(sha1, current_ref-u.value.peeled);
+ return 0;
}
if (read_ref_full(refname, base, 1, flag))
return -1;
- if ((flag REF_ISPACKED)) {
+ /*
+* If the reference is packed, read its ref_entry from the
+* cache in the hope that we already know its peeled value.
+* We only try this optimization on packed references because
+* (a) forcing the filling of the loose reference cache could
+* be expensive and (b) loose references anyway usually do not
+* have REF_KNOWS_PEELED.
+*/
+ if (flag REF_ISPACKED) {
struct ref_entry *r = get_packed_ref(refname);
-
- if (r (r-flag REF_KNOWS_PEELED)) {
- if (is_null_sha1(r-u.value.peeled))
- return -1;
+ if (r) {
+ if (peel_entry(r))
+ return -1;
hashcpy(sha1, r-u.value.peeled);
return 0;
}
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/33] repack_without_ref(): silence errors for dangling packed refs
Stop emitting an error message when deleting a packed reference if we
find another dangling packed reference that is overridden by a loose
reference. See the previous commit for a longer explanation of the
issue.
We have to be careful to make sure that the invalid packed reference
really *is* overridden by a loose reference; otherwise what we have
found is repository corruption, which we *should* report.
Please note that this approach is vulnerable to a race condition
similar to the race conditions already known to affect packed
references [1]:
* Process 1 tries to peel packed reference X as part of deleting
another packed reference. It discovers that X does not refer to a
valid object (because the object that it referred to has been
garbage collected).
* Process 2 tries to delete reference X. It starts by deleting the
loose reference X.
* Process 1 checks whether there is a loose reference X. There is not
(it has just been deleted by process 2), so process 1 reports a
spurious error X does not point to a valid object!
The worst case seems relatively harmless, and the fix is identical to
the fix that will be needed for the other race conditions (namely
holding a lock on the packed-refs file during *all* reference
deletions), so we leave the cleaning up of all of them as a future
project.
[1] http://thread.gmane.org/gmane.comp.version-control.git/211956
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 37 +++--
t/t3210-pack-refs.sh | 2 +-
2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index ed54ed4..2957f6d 100644
--- a/refs.c
+++ b/refs.c
@@ -1901,8 +1901,41 @@ static int repack_without_ref_fn(struct ref_entry
*entry, void *cb_data)
if (!strcmp(data-refname, entry-name))
return 0;
- if (!ref_resolves_to_object(entry))
- return 0; /* Skip broken refs */
+ if (entry-flag REF_ISBROKEN) {
+ /* This shouldn't happen to packed refs. */
+ error(%s is broken!, entry-name);
+ return 0;
+ }
+ if (!has_sha1_file(entry-u.value.sha1)) {
+ unsigned char sha1[20];
+ int flags;
+
+ if (read_ref_full(entry-name, sha1, 0, flags))
+ /* We should at least have found the packed ref. */
+ die(Internal error);
+ if ((flags REF_ISSYMREF) || !(flags REF_ISPACKED))
+ /*
+* This packed reference is overridden by a
+* loose reference, so it is OK that its value
+* is no longer valid; for example, it might
+* refer to an object that has been garbage
+* collected. For this purpose we don't even
+* care whether the loose reference itself is
+* invalid, broken, symbolic, etc. Silently
+* omit the packed reference from the output.
+*/
+ return 0;
+ /*
+* There is no overriding loose reference, so the fact
+* that this reference doesn't refer to a valid object
+* indicates some kind of repository corruption.
+* Report the problem, then omit the reference from
+* the output.
+*/
+ error(%s does not point to a valid object!, entry-name);
+ return 0;
+ }
+
len = snprintf(line, sizeof(line), %s %s\n,
sha1_to_hex(entry-u.value.sha1), entry-name);
/* this should not happen but just being defensive */
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index c032d88..559f602 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -142,7 +142,7 @@ test_expect_success 'delete ref with dangling packed
version' '
test_cmp /dev/null result
'
-test_expect_failure 'delete ref while another dangling packed ref' '
+test_expect_success 'delete ref while another dangling packed ref' '
git branch lamb
git commit --allow-empty -m future garbage
git pack-refs --all
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/33] refs: change how packed refs are deleted
Add a function remove_ref(), which removes a single entry from a
reference cache.
Use this function to reimplement repack_without_ref(). The old
version iterated over all refs, packing all of them except for the one
to be deleted, then discarded the entire packed reference cache. The
new version deletes the doomed reference from the cache *before*
iterating.
This has two advantages:
* the code for writing packed-refs becomes simpler, because it doesn't
have to exclude one of the references.
* it is no longer necessary to discard the packed refs cache after
deleting a reference: symbolic refs cannot be packed, so packed
references cannot depend on each other, so the rest of the packed
refs cache remains valid after a reference is deleted.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 84 +-
1 file changed, 68 insertions(+), 16 deletions(-)
diff --git a/refs.c b/refs.c
index 9fd49e8..51915a8 100644
--- a/refs.c
+++ b/refs.c
@@ -467,6 +467,57 @@ static struct ref_entry *find_ref(struct ref_dir *dir,
const char *refname)
}
/*
+ * Remove the entry with the given name from dir, recursing into
+ * subdirectories as necessary. If refname is the name of a directory
+ * (i.e., ends with '/'), then remove the directory and its contents.
+ * If the removal was successful, return the number of entries
+ * remaining in the directory entry that contained the deleted entry.
+ * If the name was not found, return -1. Please note that this
+ * function only deletes the entry from the cache; it does not delete
+ * it from the filesystem or ensure that other cache entries (which
+ * might be symbolic references to the removed entry) are updated.
+ * Nor does it remove any containing dir entries that might be made
+ * empty by the removal. dir must represent the top-level directory
+ * and must already be complete.
+ */
+static int remove_entry(struct ref_dir *dir, const char *refname)
+{
+ int refname_len = strlen(refname);
+ int entry_index;
+ struct ref_entry *entry;
+ int is_dir = refname[refname_len - 1] == '/';
+ if (is_dir) {
+ /*
+* refname represents a reference directory. Remove
+* the trailing slash; otherwise we will get the
+* directory *representing* refname rather than the
+* one *containing* it.
+*/
+ char *dirname = xmemdupz(refname, refname_len - 1);
+ dir = find_containing_dir(dir, dirname, 0);
+ free(dirname);
+ } else {
+ dir = find_containing_dir(dir, refname, 0);
+ }
+ if (!dir)
+ return -1;
+ entry_index = search_ref_dir(dir, refname, refname_len);
+ if (entry_index == -1)
+ return -1;
+ entry = dir-entries[entry_index];
+
+ memmove(dir-entries[entry_index],
+ dir-entries[entry_index + 1],
+ (dir-nr - entry_index - 1) * sizeof(*dir-entries)
+ );
+ dir-nr--;
+ if (dir-sorted entry_index)
+ dir-sorted--;
+ free_ref_entry(entry);
+ return dir-nr;
+}
+
+/*
* Add a ref_entry to the ref_dir (unsorted), recursing into
* subdirectories as necessary. dir must represent the top-level
* directory. Return 0 on success.
@@ -1894,19 +1945,12 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
}
-struct repack_without_ref_sb {
- const char *refname;
- int fd;
-};
-
-static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data)
+static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
{
- struct repack_without_ref_sb *data = cb_data;
+ int *fd = cb_data;
char line[PATH_MAX + 100];
int len;
- if (!strcmp(data-refname, entry-name))
- return 0;
if (entry-flag REF_ISBROKEN) {
/* This shouldn't happen to packed refs. */
error(%s is broken!, entry-name);
@@ -1947,7 +1991,7 @@ static int repack_without_ref_fn(struct ref_entry *entry,
void *cb_data)
/* this should not happen but just being defensive */
if (len sizeof(line))
die(too long a refname '%s', entry-name);
- write_or_die(data-fd, line, len);
+ write_or_die(*fd, line, len);
return 0;
}
@@ -1955,22 +1999,30 @@ static struct lock_file packlock;
static int repack_without_ref(const char *refname)
{
- struct repack_without_ref_sb data;
+ int fd;
struct ref_cache *refs = get_ref_cache(NULL);
struct ref_dir *packed;
if (!get_packed_ref(refname))
return 0; /* refname does not exist in packed refs */
- data.refname = refname;
- data.fd = hold_lock_file_for_update(packlock, git_path(packed-refs),
0);
-
[PATCH v2 21/33] repack_without_ref(): write peeled refs in the rewritten file
When a reference that existed in the packed-refs file is deleted, the
packed-refs file must be rewritten. Previously, the file was
rewritten without any peeled refs, even if the file contained peeled
refs when it was read. This was not a bug, because the packed-refs
file header didn't claim that the file contained peeled values. But
it had a performance cost, because the repository would lose the
benefit of having precomputed peeled references until pack-refs was
run again.
Teach repack_without_ref() to write peeled refs to the packed-refs
file (regardless of whether they were present in the old version of
the file).
This means that if the old version of the packed-refs file was not
fully peeled, then repack_without_ref() will have to peel references.
To avoid the expense of reading lots of loose references, we take two
shortcuts relative to pack-refs:
* If the peeled value of a reference is already known (i.e., because
it was read from the old version of the packed-refs file), then
output that peeled value again without any checks. This is the
usual code path and should avoid any noticeable overhead. (This is
different than pack-refs, which always re-peels references.)
* We don't verify that the packed ref is still current. It could be
that a packed references is overridden by a loose reference, in
which case the packed ref is no longer needed and might even refer
to an object that has been garbage collected. But we don't check;
instead, we just try to peel all references. If peeling is
successful, the peeled value is written out (even though it might
not be needed any more); if not, then the reference is silently
omitted from the output.
The extra overhead of peeling references in repack_without_ref()
should only be incurred the first time the packed-refs file is written
by a version of Git that knows about the fully-peeled attribute.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 23 +++
t/t3211-peel-ref.sh | 2 +-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 51915a8..ff4c5f1 100644
--- a/refs.c
+++ b/refs.c
@@ -876,6 +876,13 @@ void invalidate_ref_cache(const char *submodule)
#define PEELED_LINE_LENGTH 42
/*
+ * The packed-refs header line that we write out. Perhaps other
+ * traits will be added later. The trailing space is required.
+ */
+static const char PACKED_REFS_HEADER[] =
+ # pack-refs with: peeled fully-peeled \n;
+
+/*
* Parse one line from a packed-refs file. Write the SHA1 to sha1.
* Return a pointer to the refname within the line (null-terminated),
* or NULL if there was a problem.
@@ -1390,6 +1397,12 @@ static enum peel_status peel_object(const unsigned char
*name, unsigned char *sh
/*
* Peel the entry (if possible) and return its new peel_status.
+ *
+ * It is OK to call this function with a packed reference entry that
+ * might be stale and might even refer to an object that has since
+ * been garbage-collected. In such a case, if the entry has
+ * REF_KNOWS_PEELED then leave the status unchanged and return
+ * PEEL_PEELED or PEEL_NON_TAG; otherwise, return PEEL_INVALID.
*/
static enum peel_status peel_entry(struct ref_entry *entry)
{
@@ -1992,6 +2005,15 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
if (len sizeof(line))
die(too long a refname '%s', entry-name);
write_or_die(*fd, line, len);
+ if (!peel_entry(entry)) {
+ /* This reference could be peeled; write the peeled value: */
+ if (snprintf(line, sizeof(line), ^%s\n,
+sha1_to_hex(entry-u.value.peeled)) !=
+ PEELED_LINE_LENGTH)
+ die(internal error);
+ write_or_die(*fd, line, PEELED_LINE_LENGTH);
+ }
+
return 0;
}
@@ -2022,6 +2044,7 @@ static int repack_without_ref(const char *refname)
rollback_lock_file(packlock);
return 0;
}
+ write_or_die(fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
do_for_each_entry_in_dir(packed, 0, repack_ref_fn, fd);
return commit_lock_file(packlock);
}
diff --git a/t/t3211-peel-ref.sh b/t/t3211-peel-ref.sh
index cca1acb..3b7caca 100755
--- a/t/t3211-peel-ref.sh
+++ b/t/t3211-peel-ref.sh
@@ -61,7 +61,7 @@ test_expect_success 'refs are peeled outside of refs/tags
(old packed)' '
test_cmp expect actual
'
-test_expect_failure 'peeled refs survive deletion of packed ref' '
+test_expect_success 'peeled refs survive deletion of packed ref' '
git pack-refs --all
cp .git/packed-refs fully-peeled
git branch yadda
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/33] refs: change the internal reference-iteration API
Establish an internal API for iterating over references, which gives
the callback functions direct access to the ref_entry structure
describing the reference. (Do not change the iteration API that is
exposed outside of the module.)
Define a new internal callback signature
int each_ref_entry_fn(struct ref_entry *entry, void *cb_data)
Change do_for_each_ref_in_dir() and do_for_each_ref_in_dirs() to
accept each_ref_entry_fn callbacks, and rename them to
do_for_each_entry_in_dir() and do_for_each_entry_in_dirs(),
respectively. Adapt their callers accordingly.
Add a new function do_for_each_entry() analogous to do_for_each_ref()
but using the new callback style.
Change do_one_ref() into an each_ref_entry_fn that does some
bookkeeping and then calls a wrapped each_ref_fn.
Reimplement do_for_each_ref() in terms of do_for_each_entry(), using
do_one_ref() as an adapter.
Please note that the responsibility for setting current_ref remains in
do_one_ref(), which means that current_ref is *not* set when iterating
over references via the new internal API. This is not a disadvantage,
because current_ref is not needed by callers of the internal API (they
receive a pointer to the current ref_entry anyway). But more
importantly, this change prevents peel_ref() from returning invalid
results in the following scenario:
When iterating via the external API, the iteration always includes
both packed and loose references, and in particular never presents a
packed ref if there is a loose ref with the same name. The internal
API, on the other hand, gives the option to iterate over only the
packed references. During such an iteration, there is no check
whether the packed ref might be hidden by a loose ref of the same
name. But until now the packed ref was recorded in current_ref during
the iteration. So if peel_ref() were called with the reference name
corresponding to current ref, it would return the peeled version of
the packed ref even though there might be a loose ref that peels to a
different value. This scenario doesn't currently occur in the code,
but fix it to prevent things from breaking in a very confusing way in
the future.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 144 +
1 file changed, 83 insertions(+), 61 deletions(-)
diff --git a/refs.c b/refs.c
index 777a4b7..ed54ed4 100644
--- a/refs.c
+++ b/refs.c
@@ -556,22 +556,34 @@ static int ref_resolves_to_object(struct ref_entry *entry)
*/
static struct ref_entry *current_ref;
+typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);
+
+struct ref_entry_cb {
+ const char *base;
+ int trim;
+ int flags;
+ each_ref_fn *fn;
+ void *cb_data;
+};
+
/*
- * Handle one reference in a do_for_each_ref*()-style iteration.
+ * Handle one reference in a do_for_each_ref*()-style iteration,
+ * calling an each_ref_fn for each entry.
*/
-static int do_one_ref(const char *base, each_ref_fn fn, int trim,
- int flags, void *cb_data, struct ref_entry *entry)
+static int do_one_ref(struct ref_entry *entry, void *cb_data)
{
+ struct ref_entry_cb *data = cb_data;
int retval;
- if (prefixcmp(entry-name, base))
+ if (prefixcmp(entry-name, data-base))
return 0;
- if (!(flags DO_FOR_EACH_INCLUDE_BROKEN)
+ if (!(data-flags DO_FOR_EACH_INCLUDE_BROKEN)
!ref_resolves_to_object(entry))
return 0;
current_ref = entry;
- retval = fn(entry-name + trim, entry-u.value.sha1, entry-flag,
cb_data);
+ retval = data-fn(entry-name + data-trim, entry-u.value.sha1,
+ entry-flag, data-cb_data);
current_ref = NULL;
return retval;
}
@@ -580,11 +592,11 @@ static int do_one_ref(const char *base, each_ref_fn fn,
int trim,
* Call fn for each reference in dir that has index in the range
* offset = index dir-nr. Recurse into subdirectories that are in
* that index range, sorting them before iterating. This function
- * does not sort dir itself; it should be sorted beforehand.
+ * does not sort dir itself; it should be sorted beforehand. fn is
+ * called for all references, including broken ones.
*/
-static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset,
- const char *base,
- each_ref_fn fn, int trim, int flags, void
*cb_data)
+static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
+ each_ref_entry_fn fn, void *cb_data)
{
int i;
assert(dir-sorted == dir-nr);
@@ -594,10 +606,9 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int
offset,
if (entry-flag REF_DIR) {
struct ref_dir *subdir = get_ref_dir(entry);
sort_ref_dir(subdir);
- retval =
[PATCH v2 29/33] pack_one_ref(): use function peel_entry()
Change pack_one_ref() to call peel_entry() rather than using its own
code for peeling references. Aside from sharing code, this lets it
take advantage of the optimization introduced by 6c4a060d7d.
Please note that we *could* use any peeled values that happen to
already be stored in the ref_entries, which would avoid some object
lookups for references that were already packed. But doing so would
also propagate any peeling errors across runs of git pack-refs and
give no way to recover from such errors. And git pack-refs isn't
run often enough that the performance cost is a problem. So instead,
add a new option to peel_entry() to force the entry to be re-peeled,
and call it with that option set.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/refs.c b/refs.c
index f78e955..f2d83f3 100644
--- a/refs.c
+++ b/refs.c
@@ -1396,7 +1396,9 @@ static enum peel_status peel_object(const unsigned char
*name, unsigned char *sh
}
/*
- * Peel the entry (if possible) and return its new peel_status.
+ * Peel the entry (if possible) and return its new peel_status. If
+ * repeel is true, re-peel the entry even if there is an old peeled
+ * value that is already stored in it.
*
* It is OK to call this function with a packed reference entry that
* might be stale and might even refer to an object that has since
@@ -1404,13 +1406,19 @@ static enum peel_status peel_object(const unsigned char
*name, unsigned char *sh
* REF_KNOWS_PEELED then leave the status unchanged and return
* PEEL_PEELED or PEEL_NON_TAG; otherwise, return PEEL_INVALID.
*/
-static enum peel_status peel_entry(struct ref_entry *entry)
+static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
{
enum peel_status status;
- if (entry-flag REF_KNOWS_PEELED)
- return is_null_sha1(entry-u.value.peeled) ?
- PEEL_NON_TAG : PEEL_PEELED;
+ if (entry-flag REF_KNOWS_PEELED) {
+ if (repeel) {
+ entry-flag = ~REF_KNOWS_PEELED;
+ hashclr(entry-u.value.peeled);
+ } else {
+ return is_null_sha1(entry-u.value.peeled) ?
+ PEEL_NON_TAG : PEEL_PEELED;
+ }
+ }
if (entry-flag REF_ISBROKEN)
return PEEL_BROKEN;
if (entry-flag REF_ISSYMREF)
@@ -1429,7 +1437,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
if (current_ref (current_ref-name == refname
|| !strcmp(current_ref-name, refname))) {
- if (peel_entry(current_ref))
+ if (peel_entry(current_ref, 0))
return -1;
hashcpy(sha1, current_ref-u.value.peeled);
return 0;
@@ -1449,7 +1457,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
if (flag REF_ISPACKED) {
struct ref_entry *r = get_packed_ref(refname);
if (r) {
- if (peel_entry(r))
+ if (peel_entry(r, 0))
return -1;
hashcpy(sha1, r-u.value.peeled);
return 0;
@@ -1998,7 +2006,7 @@ struct pack_refs_cb_data {
static int pack_one_ref(struct ref_entry *entry, void *cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
- struct object *o;
+ enum peel_status peel_status;
int is_tag_ref;
/* Do not pack symbolic or broken refs: */
@@ -2014,13 +2022,12 @@ static int pack_one_ref(struct ref_entry *entry, void
*cb_data)
fprintf(cb-refs_file, %s %s\n, sha1_to_hex(entry-u.value.sha1),
entry-name);
- o = parse_object_or_die(entry-u.value.sha1, entry-name);
- if (o-type == OBJ_TAG) {
- o = deref_tag(o, entry-name, 0);
- if (o)
- fprintf(cb-refs_file, ^%s\n,
- sha1_to_hex(o-sha1));
- }
+ peel_status = peel_entry(entry, 1);
+ if (peel_status == PEEL_PEELED)
+ fprintf(cb-refs_file, ^%s\n,
sha1_to_hex(entry-u.value.peeled));
+ else if (peel_status != PEEL_NON_TAG)
+ die(internal error peeling reference %s (%s),
+ entry-name, sha1_to_hex(entry-u.value.sha1));
/* If the ref was already packed, there is no need to prune it. */
if ((cb-flags PACK_REFS_PRUNE) !(entry-flag REF_ISPACKED)) {
@@ -2161,7 +2168,7 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
return 0;
}
- peel_status = peel_entry(entry);
+ peel_status = peel_entry(entry, 0);
write_packed_entry(*fd, entry-name, entry-u.value.sha1,
peel_status == PEEL_PEELED ?
entry-u.value.peeled : NULL);
[PATCH v2 30/33] pack_one_ref(): use write_packed_entry() to do the writing
Change pack_refs() to work with a file descriptor instead of a FILE*
(making the file-locking code less awkward) and use
write_packed_entry() to do the writing.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 33 -
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/refs.c b/refs.c
index f2d83f3..df670cb 100644
--- a/refs.c
+++ b/refs.c
@@ -2000,7 +2000,7 @@ struct ref_to_prune {
struct pack_refs_cb_data {
unsigned int flags;
struct ref_to_prune *ref_to_prune;
- FILE *refs_file;
+ int fd;
};
static int pack_one_ref(struct ref_entry *entry, void *cb_data)
@@ -2019,15 +2019,13 @@ static int pack_one_ref(struct ref_entry *entry, void
*cb_data)
!(entry-flag REF_ISPACKED))
return 0;
- fprintf(cb-refs_file, %s %s\n, sha1_to_hex(entry-u.value.sha1),
- entry-name);
-
peel_status = peel_entry(entry, 1);
- if (peel_status == PEEL_PEELED)
- fprintf(cb-refs_file, ^%s\n,
sha1_to_hex(entry-u.value.peeled));
- else if (peel_status != PEEL_NON_TAG)
+ if (peel_status != PEEL_PEELED peel_status != PEEL_NON_TAG)
die(internal error peeling reference %s (%s),
entry-name, sha1_to_hex(entry-u.value.sha1));
+ write_packed_entry(cb-fd, entry-name, entry-u.value.sha1,
+ peel_status == PEEL_PEELED ?
+ entry-u.value.peeled : NULL);
/* If the ref was already packed, there is no need to prune it. */
if ((cb-flags PACK_REFS_PRUNE) !(entry-flag REF_ISPACKED)) {
@@ -2096,32 +2094,17 @@ static struct lock_file packlock;
int pack_refs(unsigned int flags)
{
- int fd;
struct pack_refs_cb_data cbdata;
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- fd = hold_lock_file_for_update(packlock, git_path(packed-refs),
- LOCK_DIE_ON_ERROR);
- cbdata.refs_file = fdopen(fd, w);
- if (!cbdata.refs_file)
- die_errno(unable to create ref-pack file structure);
+ cbdata.fd = hold_lock_file_for_update(packlock,
git_path(packed-refs),
+ LOCK_DIE_ON_ERROR);
- /* perhaps other traits later as well */
- fprintf(cbdata.refs_file, # pack-refs with: peeled fully-peeled \n);
+ write_or_die(cbdata.fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
do_for_each_entry(NULL, , pack_one_ref, cbdata);
- if (ferror(cbdata.refs_file))
- die(failed to write ref-pack file);
- if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
- die_errno(failed to write ref-pack file);
- /*
-* Since the lock file was fdopen()'ed and then fclose()'ed above,
-* assign -1 to the lock file descriptor so that commit_lock_file()
-* won't try to close() it.
-*/
- packlock.fd = -1;
if (commit_lock_file(packlock) 0)
die_errno(unable to overwrite old ref-pack file);
prune_refs(cbdata.ref_to_prune);
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/33] do_for_each_ref_in_dirs(): remove dead code
There is no way to drop out of the while loop. This code has been dead since 432ad41e. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/refs.c b/refs.c index e1e9ddd..7260768 100644 --- a/refs.c +++ b/refs.c @@ -666,13 +666,6 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1, if (retval) return retval; } - if (i1 dir1-nr) - return do_for_each_ref_in_dir(dir1, i1, - base, fn, trim, flags, cb_data); - if (i2 dir2-nr) - return do_for_each_ref_in_dir(dir2, i2, - base, fn, trim, flags, cb_data); - return 0; } /* -- 1.8.2.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] t4202 (log): add failing test for log with subtree
Thomas Rast wrote: There is a market for a rename detection that works at the tree level. Exactly. And making it auto-follow with a low threshold would be a great default. We'll have to deal with D/F conflicts that Junio was talking about in (2), every once in a while. We'll have a lot more incentive to build the D/F conflict resolution process a nice UI. git-core will actually start working with trees the way it works with 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
[PATCH v2 05/33] refs: define constant PEELED_LINE_LENGTH
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index efad658..e1e9ddd 100644
--- a/refs.c
+++ b/refs.c
@@ -805,6 +805,9 @@ void invalidate_ref_cache(const char *submodule)
clear_loose_ref_cache(refs);
}
+/* The length of a peeled reference line in packed-refs, including EOL: */
+#define PEELED_LINE_LENGTH 42
+
/*
* Parse one line from a packed-refs file. Write the SHA1 to sha1.
* Return a pointer to the refname within the line (null-terminated),
@@ -897,8 +900,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
}
if (last
refline[0] == '^'
- strlen(refline) == 42
- refline[41] == '\n'
+ strlen(refline) == PEELED_LINE_LENGTH
+ refline[PEELED_LINE_LENGTH - 1] == '\n'
!get_sha1_hex(refline + 1, sha1)) {
hashcpy(last-u.value.peeled, sha1);
/*
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/33] repack_without_ref(): use function get_packed_ref()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 09b68e4..1c8edb1 100644
--- a/refs.c
+++ b/refs.c
@@ -1820,9 +1820,11 @@ static int repack_without_ref(const char *refname)
{
struct repack_without_ref_sb data;
struct ref_cache *refs = get_ref_cache(NULL);
- struct ref_dir *packed = get_packed_refs(refs);
- if (find_ref(packed, refname) == NULL)
- return 0;
+ struct ref_dir *packed;
+
+ if (!get_packed_ref(refname))
+ return 0; /* refname does not exist in packed refs */
+
data.refname = refname;
data.fd = hold_lock_file_for_update(packlock, git_path(packed-refs),
0);
if (data.fd 0) {
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/33] refs: extract a function ref_resolves_to_object()
It is a nice unit of work and soon will be needed from multiple
locations.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 1c8edb1..8b554d8 100644
--- a/refs.c
+++ b/refs.c
@@ -529,6 +529,22 @@ static void sort_ref_dir(struct ref_dir *dir)
#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
/*
+ * Return true iff the reference described by entry can be resolved to
+ * an object in the database. Emit a warning if the referred-to
+ * object does not exist.
+ */
+static int ref_resolves_to_object(struct ref_entry *entry)
+{
+ if (entry-flag REF_ISBROKEN)
+ return 0;
+ if (!has_sha1_file(entry-u.value.sha1)) {
+ error(%s does not point to a valid object!, entry-name);
+ return 0;
+ }
+ return 1;
+}
+
+/*
* current_ref is a performance hack: when iterating over references
* using the for_each_ref*() functions, current_ref is set to the
* current reference's entry before calling the callback function. If
@@ -549,14 +565,10 @@ static int do_one_ref(const char *base, each_ref_fn fn,
int trim,
if (prefixcmp(entry-name, base))
return 0;
- if (!(flags DO_FOR_EACH_INCLUDE_BROKEN)) {
- if (entry-flag REF_ISBROKEN)
- return 0; /* ignore broken refs e.g. dangling symref */
- if (!has_sha1_file(entry-u.value.sha1)) {
- error(%s does not point to a valid object!,
entry-name);
- return 0;
- }
- }
+ if (!(flags DO_FOR_EACH_INCLUDE_BROKEN)
+ !ref_resolves_to_object(entry))
+ return 0;
+
current_ref = entry;
retval = fn(entry-name + trim, entry-u.value.sha1, entry-flag,
cb_data);
current_ref = NULL;
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/33] t3210: test for spurious error messages for dangling packed refs
A packed reference can be overridden by a loose reference, in which case the packed reference is obsolete and is never used. The object pointed to by such a reference can be garbage collected. Since d66da478f2, this could lead to the emission of a spurious error message: error: refs/heads/master does not point to a valid object! The error is generated by repack_without_ref() if there is an obsolete dangling packed reference in packed-refs when the packed-refs file has to be rewritten due to the deletion of another packed reference. Add a failing test demonstrating this problem and some passing tests of related scenarios. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t3210-pack-refs.sh | 36 1 file changed, 36 insertions(+) diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh index cd04361..c032d88 100755 --- a/t/t3210-pack-refs.sh +++ b/t/t3210-pack-refs.sh @@ -118,4 +118,40 @@ test_expect_success 'pack, prune and repack' ' test_cmp all-of-them again ' +test_expect_success 'explicit pack-refs with dangling packed reference' ' + git commit --allow-empty -m soon to be garbage-collected + git pack-refs --all + git reset --hard HEAD^ + sleep 1 + git reflog expire --expire=now --all + git prune --expire=now + git pack-refs --all 2result + test_cmp /dev/null result +' + +test_expect_success 'delete ref with dangling packed version' ' + git checkout -b lamb + git commit --allow-empty -m future garbage + git pack-refs --all + git reset --hard HEAD^ + git checkout master + sleep 1 + git reflog expire --expire=now --all + git prune --expire=now + git branch -d lamb 2result + test_cmp /dev/null result +' + +test_expect_failure 'delete ref while another dangling packed ref' ' + git branch lamb + git commit --allow-empty -m future garbage + git pack-refs --all + git reset --hard HEAD^ + sleep 1 + git reflog expire --expire=now --all + git prune --expire=now + git branch -d lamb 2result + test_cmp /dev/null result +' + test_done -- 1.8.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 18/33] search_ref_dir(): return an index rather than a pointer
Change search_ref_dir() to return the index of the sought entry (or -1
on error) rather than a pointer to the entry. This will make it more
natural to use the function for removing an entry from the list.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 2957f6d..9fd49e8 100644
--- a/refs.c
+++ b/refs.c
@@ -366,18 +366,17 @@ static int ref_entry_cmp_sslice(const void *key_, const
void *ent_)
}
/*
- * Return the entry with the given refname from the ref_dir
- * (non-recursively), sorting dir if necessary. Return NULL if no
- * such entry is found. dir must already be complete.
+ * Return the index of the entry with the given refname from the
+ * ref_dir (non-recursively), sorting dir if necessary. Return -1 if
+ * no such entry is found. dir must already be complete.
*/
-static struct ref_entry *search_ref_dir(struct ref_dir *dir,
- const char *refname, size_t len)
+static int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len)
{
struct ref_entry **r;
struct string_slice key;
if (refname == NULL || !dir-nr)
- return NULL;
+ return -1;
sort_ref_dir(dir);
key.len = len;
@@ -386,9 +385,9 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir,
ref_entry_cmp_sslice);
if (r == NULL)
- return NULL;
+ return -1;
- return *r;
+ return r - dir-entries;
}
/*
@@ -402,8 +401,9 @@ static struct ref_dir *search_for_subdir(struct ref_dir
*dir,
const char *subdirname, size_t len,
int mkdir)
{
- struct ref_entry *entry = search_ref_dir(dir, subdirname, len);
- if (!entry) {
+ int entry_index = search_ref_dir(dir, subdirname, len);
+ struct ref_entry *entry;
+ if (entry_index == -1) {
if (!mkdir)
return NULL;
/*
@@ -414,6 +414,8 @@ static struct ref_dir *search_for_subdir(struct ref_dir
*dir,
*/
entry = create_dir_entry(dir-ref_cache, subdirname, len, 0);
add_entry_to_dir(dir, entry);
+ } else {
+ entry = dir-entries[entry_index];
}
return get_ref_dir(entry);
}
@@ -452,12 +454,16 @@ static struct ref_dir *find_containing_dir(struct ref_dir
*dir,
*/
static struct ref_entry *find_ref(struct ref_dir *dir, const char *refname)
{
+ int entry_index;
struct ref_entry *entry;
dir = find_containing_dir(dir, refname, 0);
if (!dir)
return NULL;
- entry = search_ref_dir(dir, refname, strlen(refname));
- return (entry !(entry-flag REF_DIR)) ? entry : NULL;
+ entry_index = search_ref_dir(dir, refname, strlen(refname));
+ if (entry_index == -1)
+ return NULL;
+ entry = dir-entries[entry_index];
+ return (entry-flag REF_DIR) ? NULL : entry;
}
/*
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 26/33] refs: use same lock_file object for both ref-packing functions
Use a single struct lock_file for both pack_refs() and
repack_without_ref().
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index b41dd5e..850df8e 100644
--- a/refs.c
+++ b/refs.c
@@ -2091,7 +2091,7 @@ static void prune_refs(struct ref_to_prune *r)
}
}
-static struct lock_file packed;
+static struct lock_file packlock;
int pack_refs(unsigned int flags)
{
@@ -2101,7 +2101,7 @@ int pack_refs(unsigned int flags)
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- fd = hold_lock_file_for_update(packed, git_path(packed-refs),
+ fd = hold_lock_file_for_update(packlock, git_path(packed-refs),
LOCK_DIE_ON_ERROR);
cbdata.refs_file = fdopen(fd, w);
if (!cbdata.refs_file)
@@ -2120,8 +2120,8 @@ int pack_refs(unsigned int flags)
* assign -1 to the lock file descriptor so that commit_lock_file()
* won't try to close() it.
*/
- packed.fd = -1;
- if (commit_lock_file(packed) 0)
+ packlock.fd = -1;
+ if (commit_lock_file(packlock) 0)
die_errno(unable to overwrite old ref-pack file);
prune_refs(cbdata.ref_to_prune);
return 0;
@@ -2175,8 +2175,6 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
return 0;
}
-static struct lock_file packlock;
-
static int repack_without_ref(const char *refname)
{
int fd;
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 24/33] pack-refs: merge code from pack-refs.{c,h} into refs.{c,h}
pack-refs.c doesn't contain much code, and the code it does contain is
closely related to reference handling. Moreover, there is some
duplication between pack_refs() and repack_without_ref(). Therefore,
merge pack-refs.c into refs.c and pack-refs.h into refs.h.
The code duplication will be addressed in future commits.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Makefile| 2 -
builtin/clone.c | 1 -
builtin/pack-refs.c | 2 +-
pack-refs.c | 148
pack-refs.h | 18 ---
refs.c | 144 ++
refs.h | 14 +
7 files changed, 159 insertions(+), 170 deletions(-)
delete mode 100644 pack-refs.c
delete mode 100644 pack-refs.h
diff --git a/Makefile b/Makefile
index 0f931a2..de38539 100644
--- a/Makefile
+++ b/Makefile
@@ -684,7 +684,6 @@ LIB_H += notes-cache.h
LIB_H += notes-merge.h
LIB_H += notes.h
LIB_H += object.h
-LIB_H += pack-refs.h
LIB_H += pack-revindex.h
LIB_H += pack.h
LIB_H += parse-options.h
@@ -817,7 +816,6 @@ LIB_OBJS += notes-cache.o
LIB_OBJS += notes-merge.o
LIB_OBJS += object.o
LIB_OBJS += pack-check.o
-LIB_OBJS += pack-refs.o
LIB_OBJS += pack-revindex.o
LIB_OBJS += pack-write.o
LIB_OBJS += pager.o
diff --git a/builtin/clone.c b/builtin/clone.c
index f9c380e..883cf2a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -18,7 +18,6 @@
#include transport.h
#include strbuf.h
#include dir.h
-#include pack-refs.h
#include sigchain.h
#include branch.h
#include remote.h
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b5a0f88..b20b1ec 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,6 +1,6 @@
#include builtin.h
#include parse-options.h
-#include pack-refs.h
+#include refs.h
static char const * const pack_refs_usage[] = {
N_(git pack-refs [options]),
diff --git a/pack-refs.c b/pack-refs.c
deleted file mode 100644
index d840055..000
--- a/pack-refs.c
+++ /dev/null
@@ -1,148 +0,0 @@
-#include cache.h
-#include refs.h
-#include tag.h
-#include pack-refs.h
-
-struct ref_to_prune {
- struct ref_to_prune *next;
- unsigned char sha1[20];
- char name[FLEX_ARRAY];
-};
-
-struct pack_refs_cb_data {
- unsigned int flags;
- struct ref_to_prune *ref_to_prune;
- FILE *refs_file;
-};
-
-static int do_not_prune(int flags)
-{
- /* If it is already packed or if it is a symref,
-* do not prune it.
-*/
- return (flags (REF_ISSYMREF|REF_ISPACKED));
-}
-
-static int pack_one_ref(const char *path, const unsigned char *sha1,
- int flags, void *cb_data)
-{
- struct pack_refs_cb_data *cb = cb_data;
- struct object *o;
- int is_tag_ref;
-
- /* Do not pack the symbolic refs */
- if ((flags REF_ISSYMREF))
- return 0;
- is_tag_ref = !prefixcmp(path, refs/tags/);
-
- /* ALWAYS pack refs that were already packed or are tags */
- if (!(cb-flags PACK_REFS_ALL) !is_tag_ref !(flags
REF_ISPACKED))
- return 0;
-
- fprintf(cb-refs_file, %s %s\n, sha1_to_hex(sha1), path);
-
- o = parse_object_or_die(sha1, path);
- if (o-type == OBJ_TAG) {
- o = deref_tag(o, path, 0);
- if (o)
- fprintf(cb-refs_file, ^%s\n,
- sha1_to_hex(o-sha1));
- }
-
- if ((cb-flags PACK_REFS_PRUNE) !do_not_prune(flags)) {
- int namelen = strlen(path) + 1;
- struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
- hashcpy(n-sha1, sha1);
- strcpy(n-name, path);
- n-next = cb-ref_to_prune;
- cb-ref_to_prune = n;
- }
- return 0;
-}
-
-/*
- * Remove empty parents, but spare refs/ and immediate subdirs.
- * Note: munges *name.
- */
-static void try_remove_empty_parents(char *name)
-{
- char *p, *q;
- int i;
- p = name;
- for (i = 0; i 2; i++) { /* refs/{heads,tags,...}/ */
- while (*p *p != '/')
- p++;
- /* tolerate duplicate slashes; see check_refname_format() */
- while (*p == '/')
- p++;
- }
- for (q = p; *q; q++)
- ;
- while (1) {
- while (q p *q != '/')
- q--;
- while (q p *(q-1) == '/')
- q--;
- if (q == p)
- break;
- *q = '\0';
- if (rmdir(git_path(%s, name)))
- break;
- }
-}
-
-/* make sure nobody touched the ref, and unlink */
-static void prune_ref(struct ref_to_prune *r)
-{
- struct ref_lock *lock = lock_ref_sha1(r-name + 5, r-sha1);
-
- if (lock) {
- unlink_or_warn(git_path(%s, r-name));
-
[PATCH v2 31/33] pack_one_ref(): do some cheap tests before a more expensive one
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index df670cb..b1cda1b 100644
--- a/refs.c
+++ b/refs.c
@@ -2007,18 +2007,17 @@ static int pack_one_ref(struct ref_entry *entry, void
*cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
enum peel_status peel_status;
- int is_tag_ref;
-
- /* Do not pack symbolic or broken refs: */
- if ((entry-flag REF_ISSYMREF) || !ref_resolves_to_object(entry))
- return 0;
- is_tag_ref = !prefixcmp(entry-name, refs/tags/);
+ int is_tag_ref = !prefixcmp(entry-name, refs/tags/);
/* ALWAYS pack refs that were already packed or are tags */
if (!(cb-flags PACK_REFS_ALL) !is_tag_ref
!(entry-flag REF_ISPACKED))
return 0;
+ /* Do not pack symbolic or broken refs: */
+ if ((entry-flag REF_ISSYMREF) || !ref_resolves_to_object(entry))
+ return 0;
+
peel_status = peel_entry(entry, 1);
if (peel_status != PEEL_PEELED peel_status != PEEL_NON_TAG)
die(internal error peeling reference %s (%s),
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 28/33] refs: inline function do_not_prune()
Function do_not_prune() was redundantly checking REF_ISSYMREF, which
was already tested at the top of pack_one_ref(), so remove that check.
And the rest was trivial, so inline the function.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 88875d1..f78e955 100644
--- a/refs.c
+++ b/refs.c
@@ -1995,14 +1995,6 @@ struct pack_refs_cb_data {
FILE *refs_file;
};
-static int do_not_prune(int flags)
-{
- /* If it is already packed or if it is a symref,
-* do not prune it.
-*/
- return (flags (REF_ISSYMREF|REF_ISPACKED));
-}
-
static int pack_one_ref(struct ref_entry *entry, void *cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
@@ -2030,7 +2022,8 @@ static int pack_one_ref(struct ref_entry *entry, void
*cb_data)
sha1_to_hex(o-sha1));
}
- if ((cb-flags PACK_REFS_PRUNE) !do_not_prune(entry-flag)) {
+ /* If the ref was already packed, there is no need to prune it. */
+ if ((cb-flags PACK_REFS_PRUNE) !(entry-flag REF_ISPACKED)) {
int namelen = strlen(entry-name) + 1;
struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
hashcpy(n-sha1, entry-u.value.sha1);
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 23/33] pack-refs: rename handle_one_ref() to pack_one_ref()
This code is about to be moved, so name the function more
distinctively.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
pack-refs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pack-refs.c b/pack-refs.c
index 4461f71..d840055 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -23,7 +23,7 @@ static int do_not_prune(int flags)
return (flags (REF_ISSYMREF|REF_ISPACKED));
}
-static int handle_one_ref(const char *path, const unsigned char *sha1,
+static int pack_one_ref(const char *path, const unsigned char *sha1,
int flags, void *cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
@@ -130,7 +130,7 @@ int pack_refs(unsigned int flags)
/* perhaps other traits later as well */
fprintf(cbdata.refs_file, # pack-refs with: peeled fully-peeled \n);
- for_each_ref(handle_one_ref, cbdata);
+ for_each_ref(pack_one_ref, cbdata);
if (ferror(cbdata.refs_file))
die(failed to write ref-pack file);
if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 27/33] pack_refs(): change to use do_for_each_entry()
pack_refs() was not using any of the extra features of for_each_ref(),
so change it to use do_for_each_entry(). This also gives it access to
the ref_entry and in particular its peeled field, which will be taken
advantage of in the next commit.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 850df8e..88875d1 100644
--- a/refs.c
+++ b/refs.c
@@ -2003,37 +2003,38 @@ static int do_not_prune(int flags)
return (flags (REF_ISSYMREF|REF_ISPACKED));
}
-static int pack_one_ref(const char *refname, const unsigned char *sha1,
- int flags, void *cb_data)
+static int pack_one_ref(struct ref_entry *entry, void *cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
struct object *o;
int is_tag_ref;
- /* Do not pack the symbolic refs */
- if ((flags REF_ISSYMREF))
+ /* Do not pack symbolic or broken refs: */
+ if ((entry-flag REF_ISSYMREF) || !ref_resolves_to_object(entry))
return 0;
- is_tag_ref = !prefixcmp(refname, refs/tags/);
+ is_tag_ref = !prefixcmp(entry-name, refs/tags/);
/* ALWAYS pack refs that were already packed or are tags */
- if (!(cb-flags PACK_REFS_ALL) !is_tag_ref !(flags
REF_ISPACKED))
+ if (!(cb-flags PACK_REFS_ALL) !is_tag_ref
+ !(entry-flag REF_ISPACKED))
return 0;
- fprintf(cb-refs_file, %s %s\n, sha1_to_hex(sha1), refname);
+ fprintf(cb-refs_file, %s %s\n, sha1_to_hex(entry-u.value.sha1),
+ entry-name);
- o = parse_object_or_die(sha1, refname);
+ o = parse_object_or_die(entry-u.value.sha1, entry-name);
if (o-type == OBJ_TAG) {
- o = deref_tag(o, refname, 0);
+ o = deref_tag(o, entry-name, 0);
if (o)
fprintf(cb-refs_file, ^%s\n,
sha1_to_hex(o-sha1));
}
- if ((cb-flags PACK_REFS_PRUNE) !do_not_prune(flags)) {
- int namelen = strlen(refname) + 1;
+ if ((cb-flags PACK_REFS_PRUNE) !do_not_prune(entry-flag)) {
+ int namelen = strlen(entry-name) + 1;
struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
- hashcpy(n-sha1, sha1);
- strcpy(n-name, refname);
+ hashcpy(n-sha1, entry-u.value.sha1);
+ strcpy(n-name, entry-name);
n-next = cb-ref_to_prune;
cb-ref_to_prune = n;
}
@@ -2110,7 +2111,7 @@ int pack_refs(unsigned int flags)
/* perhaps other traits later as well */
fprintf(cbdata.refs_file, # pack-refs with: peeled fully-peeled \n);
- for_each_ref(pack_one_ref, cbdata);
+ do_for_each_entry(NULL, , pack_one_ref, cbdata);
if (ferror(cbdata.refs_file))
die(failed to write ref-pack file);
if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 20/33] t3211: demonstrate loss of peeled refs if a packed ref is deleted
Add a test that demonstrates that the peeled values recorded in packed-refs are lost if a packed ref is deleted. (The code in repack_without_ref() doesn't even attempt to write peeled refs.) This will be fixed in a moment. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t3211-peel-ref.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t3211-peel-ref.sh b/t/t3211-peel-ref.sh index d4d7792..cca1acb 100755 --- a/t/t3211-peel-ref.sh +++ b/t/t3211-peel-ref.sh @@ -61,4 +61,13 @@ test_expect_success 'refs are peeled outside of refs/tags (old packed)' ' test_cmp expect actual ' +test_expect_failure 'peeled refs survive deletion of packed ref' ' + git pack-refs --all + cp .git/packed-refs fully-peeled + git branch yadda + git pack-refs --all + git branch -d yadda + test_cmp fully-peeled .git/packed-refs +' + test_done -- 1.8.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 25/33] pack_one_ref(): rename path parameter to refname
Make this function conform to the naming convention established in
65385ef7d4 for the rest of the refs.c file.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 268c1a0..b41dd5e 100644
--- a/refs.c
+++ b/refs.c
@@ -2003,7 +2003,7 @@ static int do_not_prune(int flags)
return (flags (REF_ISSYMREF|REF_ISPACKED));
}
-static int pack_one_ref(const char *path, const unsigned char *sha1,
+static int pack_one_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
@@ -2013,27 +2013,27 @@ static int pack_one_ref(const char *path, const
unsigned char *sha1,
/* Do not pack the symbolic refs */
if ((flags REF_ISSYMREF))
return 0;
- is_tag_ref = !prefixcmp(path, refs/tags/);
+ is_tag_ref = !prefixcmp(refname, refs/tags/);
/* ALWAYS pack refs that were already packed or are tags */
if (!(cb-flags PACK_REFS_ALL) !is_tag_ref !(flags
REF_ISPACKED))
return 0;
- fprintf(cb-refs_file, %s %s\n, sha1_to_hex(sha1), path);
+ fprintf(cb-refs_file, %s %s\n, sha1_to_hex(sha1), refname);
- o = parse_object_or_die(sha1, path);
+ o = parse_object_or_die(sha1, refname);
if (o-type == OBJ_TAG) {
- o = deref_tag(o, path, 0);
+ o = deref_tag(o, refname, 0);
if (o)
fprintf(cb-refs_file, ^%s\n,
sha1_to_hex(o-sha1));
}
if ((cb-flags PACK_REFS_PRUNE) !do_not_prune(flags)) {
- int namelen = strlen(path) + 1;
+ int namelen = strlen(refname) + 1;
struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
hashcpy(n-sha1, sha1);
- strcpy(n-name, path);
+ strcpy(n-name, refname);
n-next = cb-ref_to_prune;
cb-ref_to_prune = n;
}
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 33/33] refs: handle the main ref_cache specially
Hold the ref_cache instance for the main repository in a dedicated,
statically-allocated instance to avoid the need for a function call
and a linked-list traversal when it is needed.
Suggested by: Heiko Voigt hvo...@hvoigt.net
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 60 +++-
1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/refs.c b/refs.c
index f246b77..d17931a 100644
--- a/refs.c
+++ b/refs.c
@@ -810,9 +810,13 @@ static struct ref_cache {
struct ref_cache *next;
struct ref_entry *loose;
struct ref_entry *packed;
- /* The submodule name, or for the main repo. */
- char name[FLEX_ARRAY];
-} *ref_cache;
+ /*
+* The submodule name, or for the main repo. We allocate
+* length 1 rather than FLEX_ARRAY so that the main ref_cache
+* is initialized correctly.
+*/
+ char name[1];
+} ref_cache, *submodule_ref_caches;
static void clear_packed_ref_cache(struct ref_cache *refs)
{
@@ -850,18 +854,18 @@ static struct ref_cache *create_ref_cache(const char
*submodule)
*/
static struct ref_cache *get_ref_cache(const char *submodule)
{
- struct ref_cache *refs = ref_cache;
- if (!submodule)
- submodule = ;
- while (refs) {
+ struct ref_cache *refs;
+
+ if (!submodule || !*submodule)
+ return ref_cache;
+
+ for (refs = submodule_ref_caches; refs; refs = refs-next)
if (!strcmp(submodule, refs-name))
return refs;
- refs = refs-next;
- }
refs = create_ref_cache(submodule);
- refs-next = ref_cache;
- ref_cache = refs;
+ refs-next = submodule_ref_caches;
+ submodule_ref_caches = refs;
return refs;
}
@@ -1010,8 +1014,8 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
void add_packed_ref(const char *refname, const unsigned char *sha1)
{
- add_ref(get_packed_refs(get_ref_cache(NULL)),
- create_ref_entry(refname, sha1, REF_ISPACKED, 1));
+ add_ref(get_packed_refs(ref_cache),
+ create_ref_entry(refname, sha1, REF_ISPACKED, 1));
}
/*
@@ -1186,7 +1190,7 @@ int resolve_gitlink_ref(const char *path, const char
*refname, unsigned char *sh
*/
static struct ref_entry *get_packed_ref(const char *refname)
{
- return find_ref(get_packed_refs(get_ref_cache(NULL)), refname);
+ return find_ref(get_packed_refs(ref_cache), refname);
}
const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int
reading, int *flag)
@@ -1591,7 +1595,7 @@ int head_ref_submodule(const char *submodule, each_ref_fn
fn, void *cb_data)
int for_each_ref(each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(get_ref_cache(NULL), , fn, 0, 0, cb_data);
+ return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
}
int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
@@ -1601,7 +1605,7 @@ int for_each_ref_submodule(const char *submodule,
each_ref_fn fn, void *cb_data)
int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(get_ref_cache(NULL), prefix, fn, strlen(prefix),
0, cb_data);
+ return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0,
cb_data);
}
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
@@ -1642,7 +1646,7 @@ int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn fn, void *c
int for_each_replace_ref(each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(get_ref_cache(NULL), refs/replace/, fn, 13, 0,
cb_data);
+ return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
}
int head_ref_namespaced(each_ref_fn fn, void *cb_data)
@@ -1665,7 +1669,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
struct strbuf buf = STRBUF_INIT;
int ret;
strbuf_addf(buf, %srefs/, get_git_namespace());
- ret = do_for_each_ref(get_ref_cache(NULL), buf.buf, fn, 0, 0, cb_data);
+ ret = do_for_each_ref(ref_cache, buf.buf, fn, 0, 0, cb_data);
strbuf_release(buf);
return ret;
}
@@ -1707,7 +1711,7 @@ int for_each_glob_ref(each_ref_fn fn, const char
*pattern, void *cb_data)
int for_each_rawref(each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(get_ref_cache(NULL), , fn, 0,
+ return do_for_each_ref(ref_cache, , fn, 0,
DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
@@ -1913,7 +1917,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* name is a proper prefix of our refname.
*/
if (missing
-!is_refname_available(refname, NULL,
get_packed_refs(get_ref_cache(NULL {
+!is_refname_available(refname, NULL, get_packed_refs(ref_cache)))
{
[PATCH v2 32/33] refs: change do_for_each_*() functions to take ref_cache arguments
Change the callers convert submodule names into ref_cache pointers.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 29 ++---
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/refs.c b/refs.c
index b1cda1b..f246b77 100644
--- a/refs.c
+++ b/refs.c
@@ -1503,16 +1503,15 @@ void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refname)
}
/*
- * Call fn for each reference in the specified submodule, omitting
+ * Call fn for each reference in the specified ref_cache, omitting
* references not in the containing_dir of base. fn is called for all
* references, including broken ones. If fn ever returns a non-zero
* value, stop the iteration and return that value; otherwise, return
* 0.
*/
-static int do_for_each_entry(const char *submodule, const char *base,
+static int do_for_each_entry(struct ref_cache *refs, const char *base,
each_ref_entry_fn fn, void *cb_data)
{
- struct ref_cache *refs = get_ref_cache(submodule);
struct ref_dir *packed_dir = get_packed_refs(refs);
struct ref_dir *loose_dir = get_loose_refs(refs);
int retval = 0;
@@ -1541,7 +1540,7 @@ static int do_for_each_entry(const char *submodule, const
char *base,
}
/*
- * Call fn for each reference in the specified submodule for which the
+ * Call fn for each reference in the specified ref_cache for which the
* refname begins with base. If trim is non-zero, then trim that many
* characters off the beginning of each refname before passing the
* refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include
@@ -1549,8 +1548,8 @@ static int do_for_each_entry(const char *submodule, const
char *base,
* value, stop the iteration and return that value; otherwise, return
* 0.
*/
-static int do_for_each_ref(const char *submodule, const char *base,
each_ref_fn fn,
- int trim, int flags, void *cb_data)
+static int do_for_each_ref(struct ref_cache *refs, const char *base,
+ each_ref_fn fn, int trim, int flags, void *cb_data)
{
struct ref_entry_cb data;
data.base = base;
@@ -1559,7 +1558,7 @@ static int do_for_each_ref(const char *submodule, const
char *base, each_ref_fn
data.fn = fn;
data.cb_data = cb_data;
- return do_for_each_entry(submodule, base, do_one_ref, data);
+ return do_for_each_entry(refs, base, do_one_ref, data);
}
static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
@@ -1592,23 +1591,23 @@ int head_ref_submodule(const char *submodule,
each_ref_fn fn, void *cb_data)
int for_each_ref(each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(NULL, , fn, 0, 0, cb_data);
+ return do_for_each_ref(get_ref_cache(NULL), , fn, 0, 0, cb_data);
}
int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
{
- return do_for_each_ref(submodule, , fn, 0, 0, cb_data);
+ return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
}
int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(NULL, prefix, fn, strlen(prefix), 0, cb_data);
+ return do_for_each_ref(get_ref_cache(NULL), prefix, fn, strlen(prefix),
0, cb_data);
}
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(submodule, prefix, fn, strlen(prefix), 0,
cb_data);
+ return do_for_each_ref(get_ref_cache(submodule), prefix, fn,
strlen(prefix), 0, cb_data);
}
int for_each_tag_ref(each_ref_fn fn, void *cb_data)
@@ -1643,7 +1642,7 @@ int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn fn, void *c
int for_each_replace_ref(each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(NULL, refs/replace/, fn, 13, 0, cb_data);
+ return do_for_each_ref(get_ref_cache(NULL), refs/replace/, fn, 13, 0,
cb_data);
}
int head_ref_namespaced(each_ref_fn fn, void *cb_data)
@@ -1666,7 +1665,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
struct strbuf buf = STRBUF_INIT;
int ret;
strbuf_addf(buf, %srefs/, get_git_namespace());
- ret = do_for_each_ref(NULL, buf.buf, fn, 0, 0, cb_data);
+ ret = do_for_each_ref(get_ref_cache(NULL), buf.buf, fn, 0, 0, cb_data);
strbuf_release(buf);
return ret;
}
@@ -1708,7 +1707,7 @@ int for_each_glob_ref(each_ref_fn fn, const char
*pattern, void *cb_data)
int for_each_rawref(each_ref_fn fn, void *cb_data)
{
- return do_for_each_ref(NULL, , fn, 0,
+ return do_for_each_ref(get_ref_cache(NULL), , fn, 0,
DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
@@ -2103,7 +2102,7 @@ int pack_refs(unsigned int flags)
write_or_die(cbdata.fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
- do_for_each_entry(NULL, ,
What's cooking in git.git (Apr 2013, #07; Mon, 22)
What's cooking in git.git (Apr 2013, #07; Mon, 22) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Both of the two topics on preparing git add users for the behaviour change in Git 2.0 are in 'master'. We may want to tweak the wording of warning messages and such before the feature freeze. 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 -- [Graduated to master] * ap/strbuf-humanize (2013-04-10) 2 commits (merged to 'next' on 2013-04-14 at 66d7af5) + count-objects: add -H option to humanize sizes + strbuf: create strbuf_humanise_bytes() to show byte sizes Teach --human-readable aka -H option to git count-objects to show various large numbers in Ki/Mi/GiB scaled as necessary. I've decided to let this topic supersede mc/count-objects-kibibytes. Human users will get an even easier output with -H and by not changing the output without an explicit option we do not have to break third-party tools that may have been reading from the output of this command. * as/clone-reference-with-gitfile (2013-04-09) 2 commits (merged to 'next' on 2013-04-15 at ab0d128) + clone: Allow repo using gitfile as a reference + clone: Fix error message for reference repository git clone did not work if a repository pointed at by the --reference option is a gitfile that points at another place. * fc/branch-upstream-color (2013-04-15) 1 commit (merged to 'next' on 2013-04-15 at 2fc50fd) + branch: colour upstream branches Add more colors to git branch -vv output. * fc/remote-hg (2013-04-11) 21 commits (merged to 'next' on 2013-04-16 at cbeaf41) + remote-hg: activate graphlog extension for hg_log() + remote-hg: fix bad file paths + remote-hg: document location of stored hg repository + remote-hg: fix bad state issue + remote-hg: add 'insecure' option + remote-hg: add simple mail test + remote-hg: add basic author tests + remote-hg: show more proper errors + remote-hg: force remote push + remote-hg: push to the appropriate branch + remote-hg: update tags globally + remote-hg: update remote bookmarks + remote-hg: refactor export + remote-hg: split bookmark handling + remote-hg: redirect buggy mercurial output + remote-hg: trivial test cleanups + remote-hg: make sure fake bookmarks are updated + remote-hg: fix for files with spaces + remote-hg: properly report errors on bookmark pushes + remote-hg: add missing config variable in doc + remote-hg: trivial cleanups Updates remote-hg helper (in contrib/). * jk/a-thread-only-dies-once (2013-04-16) 2 commits (merged to 'next' on 2013-04-18 at 3208f44) + run-command: use thread-aware die_is_recursing routine + usage: allow pluggable die-recursion checks A regression fix for the logic to detect die() handler triggering itself recursively. * jk/chopped-ident (2013-04-17) 3 commits (merged to 'next' on 2013-04-19 at ecec2d4) + blame: handle broken commit headers gracefully + pretty: handle broken commit headers gracefully + cat-file: print tags raw for cat-file -p A commit object whose author or committer ident are malformed crashed some code that trusted that a name, an email and an timestamp can always be found in it. * jk/doc-http-backend (2013-04-13) 3 commits (merged to 'next' on 2013-04-19 at 7932840) + doc/http-backend: match query-string in apache half-auth example + doc/http-backend: give some lighttpd config examples + doc/http-backend: clarify half-auth repo configuration Improve documentation to illustrate push authenticated, fetch anonymous configuration for smart HTTP servers. * jx/i18n-branch-error-messages (2013-04-15) 1 commit (merged to 'next' on 2013-04-18 at 630c211) + i18n: branch: mark strings for translation * lf/read-blob-data-from-index (2013-04-17) 3 commits (merged to 'next' on 2013-04-17 at 611208f) + convert.c: remove duplicate code + read_blob_data_from_index(): optionally return the size of blob data + attr.c: extract read_index_data() as read_blob_data_from_index() Reduce duplicated code between convert.c and attr.c. * mv/sequencer-pick-error-diag (2013-04-11) 1 commit (merged to 'next' on 2013-04-16 at a2da926) + cherry-pick: make sure all input objects are commits git cherry-pick $blob $tree is diagnosed as a nonsense. * mv/ssl-ftp-curl (2013-04-12) 1 commit (merged to 'next' on 2013-04-15 at 7fdada6) + Support FTP-over-SSL/TLS for regular FTP Does anybody really use commit walkers over ftp??? * nd/checkout-keep-sparse (2013-04-15) 1 commit (merged to 'next' on 2013-04-19 at 803dabc) + checkout: add --ignore-skip-worktree-bits in sparse checkout mode Make the initial sparse selection of the paths more sticky across git checkout.
Re: [PATCH] t4202 (log): add failing test for log with subtree
Junio C Hamano wrote: (1) It uses the given pathname as single pathspec and drill down the same way without --follow until it notices the path disappears and until then there is no attempt to detect renames is made. And it only does -M variant of rename detection On this. It might be profitable to auto-follow at a low threshold (and change --follow to a number argument like -M, -C). I'm not asking you to do it for more than one-file at a time, but I often view one file's history: I was just going through the history of builtin/merge-trees.c (what I was looking for was mostly in the builtin-* variant), and it would've been nice if I didn't have to quit and come back with a --follow. -- 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 0/2] git add -A/--no-all finishing touches
Applying Jonathan's idea on top of the early part that has graduated to 'master', here is to add --ignore-removal (which is a more natural way to say --no-all) and use it in the warning message. Junio C Hamano (2): git add: --ignore-removal is a better named --no-all git add: rephrase -A/--no-all warning Documentation/git-add.txt | 10 ++ builtin/add.c | 23 +-- 2 files changed, 23 insertions(+), 10 deletions(-) -- 1.8.2.1-683-g39c426e -- 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/2] git add: --ignore-removal is a better named --no-all
In the historical context of git add --all . that tells the
command to pay attention to all kinds of changes (implying
without ignoring removals), the option --no-all to countermand
it may have made some sense, but because we will be making --all
the default when a pathspec is given, it makes more sense to rename
the option to a more explicit --ignore-removal. The -all option
naturally becomes its negation: --no-ignore-removal.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-add.txt | 10 ++
builtin/add.c | 11 +++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 5c501a2..48754cb 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,9 +9,9 @@ SYNOPSIS
[verse]
'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
- [--edit | -e] [--[no-]all | [--update | -u]] [--intent-to-add | -N]
- [--refresh] [--ignore-errors] [--ignore-missing] [--]
- [pathspec...]
+ [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
+ [--intent-to-add | -N] [--refresh] [--ignore-errors]
[--ignore-missing]
+ [--] [pathspec...]
DESCRIPTION
---
@@ -111,6 +111,7 @@ of Git, hence the form without pathspec should not be
used.
-A::
--all::
+--no-ignore-removal::
Update the index not only where the working tree has a file
matching pathspec but also where the index already has an
entry. This adds, modifies, and removes index entries to
@@ -122,6 +123,7 @@ and its subdirectories. This default will change in a
future version
of Git, hence the form without pathspec should not be used.
--no-all::
+--ignore-removal::
Update the index by adding new files that are unknown to the
index and files modified in the working tree, but ignore
files that have been removed from the working tree. This
@@ -130,7 +132,7 @@ of Git, hence the form without pathspec should not be
used.
This option is primarily to help the current users of Git, whose
git add pathspec... ignores removed files. In future versions
of Git, git add pathspec... will be a synonym to git add -A
-pathspec... and git add --no-all pathspec... will behave like
+pathspec... and git add --ignore-removal pathspec... will behave like
today's git add pathspec..., ignoring removed files.
-N::
diff --git a/builtin/add.c b/builtin/add.c
index 54cd2d4..aefbc45 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -382,6 +382,13 @@ static int ignore_add_errors, intent_to_add,
ignore_missing;
static int addremove = ADDREMOVE_DEFAULT;
static int addremove_explicit = -1; /* unspecified */
+static int ignore_removal_cb(const struct option *opt, const char *arg, int
unset)
+{
+ /* if we are told to ignore, we are not adding removals */
+ *(int *)opt-value = !unset ? 0 : 1;
+ return 0;
+}
+
static struct option builtin_add_options[] = {
OPT__DRY_RUN(show_only, N_(dry run)),
OPT__VERBOSE(verbose, N_(be verbose)),
@@ -393,6 +400,10 @@ static struct option builtin_add_options[] = {
OPT_BOOL('u', update, take_worktree_changes, N_(update tracked
files)),
OPT_BOOL('N', intent-to-add, intent_to_add, N_(record only the fact
that the path will be added later)),
OPT_BOOL('A', all, addremove_explicit, N_(add changes from all
tracked and untracked files)),
+ { OPTION_CALLBACK, 0, ignore-removal, addremove_explicit,
+ NULL /* takes no arguments */,
+ N_(ignore paths removed in the working tree (same as --no-all)),
+ PARSE_OPT_NOARG, ignore_removal_cb },
OPT_BOOL( 0 , refresh, refresh_only, N_(don't add, only refresh the
index)),
OPT_BOOL( 0 , ignore-errors, ignore_add_errors, N_(just skip files
which cannot be added because of errors)),
OPT_BOOL( 0 , ignore-missing, ignore_missing, N_(check if - even
missing - files are ignored in dry run)),
--
1.8.2.1-683-g39c426e
--
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/2] git add: rephrase -A/--no-all warning
With the synonym --ignore-removal for --no-all, we can rephrase the Git 2.0 transition warning message in a more natural way. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/add.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index aefbc45..c55615b 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -97,13 +97,13 @@ static int fix_unmerged_status(struct diff_filepair *p, } static const char *add_would_remove_warning = N_( - You ran 'git add' with neither '-A (--all)' or '--no-all', whose\n -behaviour will change in Git 2.0 with respect to paths you removed from\n -your working tree. Paths like '%s' that are\n -removed are ignored with this version of Git.\n + You ran 'git add' with neither '-A (--all)' or '--ignore-removal',\n +whose behaviour will change in Git 2.0 with respect to paths you removed.\n +Paths like '%s' that are\n +removed from your working tree are ignored with this version of Git.\n \n -* 'git add --no-all pathspec', which is the current default, ignores\n - paths you removed from your working tree.\n +* 'git add --ignore-removal pathspec', which is the current default,\n + ignores paths you removed from your working tree.\n \n * 'git add --all pathspec' will let you also record the removals.\n \n -- 1.8.2.1-683-g39c426e -- 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] t4202 (log): add failing test for log with subtree
From: Ramkumar Ramachandra artag...@gmail.com Sent: Monday, April 22, 2013 8:54 PM Thomas Rast wrote: There is a market for a rename detection that works at the tree level. Exactly. And making it auto-follow with a low threshold would be a great default. We'll have to deal with D/F conflicts that Junio was talking about in (2), every once in a while. We'll have a lot more incentive to build the D/F conflict resolution process a nice UI. git-core will actually start working with trees the way it works with blobs. -- Is this not similar to the previous attempts at bulk rename detection? Such as $gmane/160233. A practical bulk rename detection would be good. It doesn't have to be perfect. Philip -- 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] bisect: Store first bad commit as comment in log file
On Mon, Apr 15, 2013 at 11:53:39 +0200, Torstein Hegge wrote:
On Mon, Apr 15, 2013 at 06:38:09 +0200, Christian Couder wrote:
I wonder if we should also write something into the bisect log if for
example the bisection stopped because there are only 'skip'ped commits
left to test. But maybe this could go into another patch after this
one.
Yes, that would be useful, but I wasn't able to determine all the cases
that would be relevant to log. Only skipped commits left to test is one,
but bisect--helper also exits on various problems related to merge base
handling. The handling of problems related to inconsistent user input is
probably not relevant to log.
I took another look at this. I wasn't able to come up with anything
useful for the The merge base $rev is bad case, but for the only
skipped commits left to test case one could do something like this.
There has to be a better way to get the range of possible first bad
commits, similar to the output of 'git log --bisect --format=%H'.
--- 8 ---
Subject: [PATCH] bisect: Log possibly bad, skipped commits at bisection end
If the bisection completes with only skipped commits left to as possible
first bad commit, output the list of possible first bad commits to human
readers of the bisection log.
Signed-off-by: Torstein Hegge he...@resisty.net
---
git-bisect.sh | 10 ++
t/t6030-bisect-porcelain.sh | 20
2 files changed, 30 insertions(+)
diff --git a/git-bisect.sh b/git-bisect.sh
index c58eea7..d7518e9 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -317,6 +317,16 @@ bisect_next() {
bad_commit=$(git show-branch $bad_rev)
echo # first bad commit: $bad_commit $GIT_DIR/BISECT_LOG
exit 0
+ elif test $res -eq 2
+ then
+ echo # only skipped commits left to test
$GIT_DIR/BISECT_LOG
+ good_revs=$(git for-each-ref --format=--not %(objectname)
refs/bisect/good-*)
+ for skipped in $(git rev-list refs/bisect/bad $good_revs)
+ do
+ skipped_commit=$(git show-branch $skipped)
+ echo # possible first bad commit: $skipped_commit
$GIT_DIR/BISECT_LOG
+ done
+ exit $res
fi
# Check for an error in the bisection process
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 4d3074a..064f5ce 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -759,4 +759,24 @@ test_expect_success 'bisect log: successfull result' '
git bisect reset
'
+cat expected.bisect-skip-log EOF
+# bad: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add 4: Ciao for now into
hello.
+# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add 2: A new day for git
into hello.
+git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c'
'7b7f204a749c3125d5224ed61ea2ae1187ad046f'
+# skip: [3de952f2416b6084f557ec417709eac740c6818c] Add 3: Another new day for
git into hello.
+git bisect skip 3de952f2416b6084f557ec417709eac740c6818c
+# only skipped commits left to test
+# possible first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add
4: Ciao for now into hello.
+# possible first bad commit: [3de952f2416b6084f557ec417709eac740c6818c] Add
3: Another new day for git into hello.
+EOF
+
+test_expect_success 'bisect log: only skip commits left' '
+ git bisect reset
+ git bisect start $HASH4 $HASH2
+ test_must_fail git bisect skip
+ git bisect log bisect-skip-log.txt
+ test_cmp expected.bisect-skip-log bisect-skip-log.txt
+ git bisect reset
+'
+
test_done
--
1.7.10.4
--
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] t4202 (log): add failing test for log with subtree
Thomas Rast tr...@inf.ethz.ch writes: So how can we fix that? We could try to somehow figure out that M:sub/ refers to the same thing as M^2:/, by looking at them at the tree level. Let's provisionally call this --follow-tree-rename. There was a patch serie long ago that implemented directory rename detection: http://thread.gmane.org/gmane.comp.version-control.git/163328 I'm not sure why it hasn't been merged. -- 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] t4202 (log): add failing test for log with subtree
Philip Oakley wrote: Is this not similar to the previous attempts at bulk rename detection? Such as $gmane/160233. Yes. Does anyone know what happened to the series? ... and I wonder how git merge -s subtree works (still reading). -- 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] bisect: Store first bad commit as comment in log file
Torstein Hegge he...@resisty.net writes: I took another look at this. I wasn't able to come up with anything useful for the The merge base $rev is bad case, but for the only skipped commits left to test case one could do something like this. We skipped them because we can gain _no_ information from testing these commits. They are not even possibly bad, but are unknown. So it feels to me that by definition listing them would not be useful. What am I missing? -- 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] zlib: fix compilation failures with Sun C Compilaer
On Mon, Apr 22, 2013 at 12:18 PM, Stefano Lattarini stefano.lattar...@gmail.com wrote: zlib: fix compilation failures with Sun C Compilaer s/Compilaer/compiler/ Do this by removing a couple of useless return statements. Without this change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15 2010/08/11) fails with the following message: zlib.c, line 192: void function cannot return value zlib.c, line 201: void function cannot return value cc: acomp failed for zlib.c Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.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
[PATCH 00/16] remote-hg: second round of improvements
Hi, Now that the safter first round is merged to master, it's time for the second round which is a little tricker. Most of the patches should be safe, but the later ones might be not. All the tests pass, even gitifyhg ones, so everything seems to be fine, but we shall see. Among the important changes are: * Proper support for tags (they are global, and we can push them) * Improved e-mail sanitaztion (a bit similar to gitifyhg, but better) * Support for hg schemes (like bb://felipec/repo) * Support for tags, bookmarks and branches with spaces * Performance improvements Good stuff we should probably merge to master if there are no issues. Cheers. Dusty Phillips (1): remote-helpers: avoid has_key Felipe Contreras (15): remote-hg: safer bookmark pushing remote-hg: use python urlparse remote-hg: properly mark branches up-to-date remote-hg: add branch_tip() helper remote-hg: add support for tag objects remote-hg: custom method to write tags remote-hg: write tags in the appropriate branch remote-hg: add custom local tag write code remote-hg: improve email sanitation remote-hg: add support for schemes extension remote-hg: don't update bookmarks unnecessarily remote-hg: allow refs with spaces remote-hg: small performance improvement remote-hg: use marks instead of inlined files remote-hg: strip extra newline contrib/remote-helpers/git-remote-bzr | 2 +- contrib/remote-helpers/git-remote-hg | 167 +++--- contrib/remote-helpers/test-hg.sh | 8 +- 3 files changed, 141 insertions(+), 36 deletions(-) -- 1.8.2.1.790.g4588561 -- 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/16] remote-hg: safer bookmark pushing
It is possible that the remote has changed the bookmarks, so let's fetch
them before we make any assumptions, just the way mercurial does.
Probably doesn't make a difference, but better be safe than sorry.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-hg | 2 ++
1 file changed, 2 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 2cd1996..dcf6c98 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -782,6 +782,8 @@ def do_export(parser):
continue
if peer:
+rb = peer.listkeys('bookmarks')
+old = rb.get(bmark, '')
if not peer.pushkey('bookmarks', bmark, old, new):
print error %s % ref
continue
--
1.8.2.1.790.g4588561
--
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/16] remote-helpers: avoid has_key
From: Dusty Phillips du...@linux.ca It is deprecated. [fc: do the same in remote-bzr] Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-bzr | 2 +- contrib/remote-helpers/git-remote-hg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index aa7bc97..cc6609b 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -94,7 +94,7 @@ class Marks: return self.last_mark def is_marked(self, rev): -return self.marks.has_key(rev) +return str(rev) in self.marks def new_mark(self, rev, mark): self.marks[rev] = mark diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 5481331..2cd1996 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -129,7 +129,7 @@ class Marks: self.last_mark = mark def is_marked(self, rev): -return self.marks.has_key(str(rev)) +return str(rev) in self.marks def get_tip(self, branch): return self.tips.get(branch, 0) -- 1.8.2.1.790.g4588561 -- 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/16] remote-hg: use python urlparse
It's simpler, and we don't need to depend on certain Mercurial versions. Also, now we don't update the URL if 'file://' is not present. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index dcf6c98..b6589a3 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -22,6 +22,7 @@ import shutil import subprocess import urllib import atexit +import urlparse # # If you want to switch to hg-git compatibility mode: @@ -793,11 +794,11 @@ def do_export(parser): print def fix_path(alias, repo, orig_url): -repo_url = util.url(repo.url()) -url = util.url(orig_url) -if str(url) == str(repo_url): +url = urlparse.urlparse(orig_url, 'file') +if url.scheme != 'file' or os.path.isabs(url.path): return -cmd = ['git', 'config', 'remote.%s.url' % alias, hg::%s % repo_url] +abs_url = urlparse.urljoin(%s/ % os.getcwd(), orig_url) +cmd = ['git', 'config', 'remote.%s.url' % alias, hg::%s % abs_url] subprocess.call(cmd) def main(args): -- 1.8.2.1.790.g4588561 -- 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/16] remote-hg: add branch_tip() helper
Idea from gitifyhg, the backwards compatibility is how Mercurial used to do it. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index a4db5b0..bd93f82 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -457,6 +457,13 @@ def do_capabilities(parser): print +def branch_tip(repo, branch): +# older versions of mercurial don't have this +if hasattr(repo, 'branchtip'): +return repo.branchtip(branch) +else: +return repo.branchtags()[branch] + def get_branch_tip(repo, branch): global branches @@ -467,9 +474,7 @@ def get_branch_tip(repo, branch): # verify there's only one head if (len(heads) 1): warn(Branch '%s' has more than one head, consider merging % branch) -# older versions of mercurial don't have this -if hasattr(repo, branchtip): -return repo.branchtip(branch) +return branch_tip(repo, branch) return heads[0] -- 1.8.2.1.790.g4588561 -- 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
