git-status rename conflict with a few git-adds following a git-mv (git for windows 1.9.4)
Hello all, I noticed a weird rename conflict after doing the following steps: 1. Changing the extension of a file using 'git mv'. 2. Editing the file and staging the changes. 3. Creating a new file and staging it. Here is the Git Bash output showing the issue. The previously mentioned steps are highlighted with asterisks. $ git --version git version 1.9.4.msysgit.0 $ git status On branch master Your branch is up-to-date with 'origin/master'. nothing to commit, working directory clean STEP 1 $ git mv src/documents/contact.html.md src/documents/contact.html.md.eco $ git status On branch master Your branch is up-to-date with 'origin/master'. Changes to be committed: (use git reset HEAD file... to unstage) renamed: src/documents/contact.html.md - src/documents/contact.html.md.eco STEP 2 $ git status On branch master Your branch is up-to-date with 'origin/master'. Changes to be committed: (use git reset HEAD file... to unstage) renamed: src/documents/contact.html.md - src/documents/contact.html.md.eco Changes not staged for commit: (use git add file... to update what will be committed) (use git checkout -- file... to discard changes in working directory) modified: src/documents/contact.html.md.eco $ git add src/documents/contact.html.md.eco $ git status On branch master Your branch is up-to-date with 'origin/master'. Changes to be committed: (use git reset HEAD file... to unstage) deleted: src/documents/contact.html.md new file: src/documents/contact.html.md.eco STEP 3 $ git status On branch master Your branch is up-to-date with 'origin/master'. Changes to be committed: (use git reset HEAD file... to unstage) deleted: src/documents/contact.html.md new file: src/documents/contact.html.md.eco Untracked files: (use git add file... to include in what will be committed) src/partials/address.html.md $ git add src/partials/address.html.md $ git status On branch master Your branch is up-to-date with 'origin/master'. Changes to be committed: (use git reset HEAD file... to unstage) new file: src/documents/contact.html.md.eco renamed: src/documents/contact.html.md - src/partials/address.html.md Note here that the renamed object is wrong. Also note that I haven't commited the changes to the repo. The repo is cloned from https://github.com/prontog/elm;. Regards, Panos -- To unsubscribe from this list: send the line 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 6/8] mv: unindent one level for directory move code
On Tue, Aug 12, 2014 at 1:47 AM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/mv.c | 47 +--
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index dcfcb11..988945c 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -171,42 +171,37 @@ int cmd_mv(int argc, const char **argv, const char
*prefix)
lstat(dst, st) == 0)
bad = _(cannot move directory over file);
else if (src_is_dir) {
+ int first = cache_name_pos(src, length), last;
if (first = 0)
prepare_move_submodule(src, first,
submodule_gitfile + i);
+ else if (index_range_of_same_dir(src, length,
+ first, last) 1) {
The function returns (last - first), so (last - first) 1 holds
inside this block, right?
modes[i] = WORKING_DIRECTORY;
if (last - first 1)
bad = _(source directory is empty);
Then do you need this conditional, or it is always bad here?
If it is always bad, then modes[i] do not need to be assigned to,
either, I think.
Am I missing something?
No you're right.
+ } else { /* last - first = 1 */
+ int j, dst_len, n;
+ modes[i] = WORKING_DIRECTORY;
+ n = argc + last - first;
...
Otherwise, perhaps squash this in?
Sure. But I'm having second thought about this series.
mv.c is centered around files on worktree, which makes it pretty hard
if we want to make it more like rm.c where index and worktree entries
are treated more equally and pathspec is applied. Doing that in mv.c
would require a lot more reorganization that makes this series
obsolete. But on the other hand, I'm not even sure if I have time to
pursue that. So..
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-objects: turn off bitmaps when we see --shallow lines
On Tue, Aug 12, 2014 at 11:34 AM, Jeff King p...@peff.net wrote: Arguably is_repository_shallow should return 1 if anybody has registered a shallow graft, but that wouldn't be enough to fix this (we'd still need to check it again _after_ reading the --shallow lines). So I think this fix is fine here. I don't know if any other parts of the code would care, though. It's getting too subtle (is_repository_shallow fails to return 1). register_shallow() is used elsewhere too, luckily pack bitmap's use is still limited in pack-objects (I think).I prefer (in future) to teach is_repository_shallow about register_shallow and move it to right before get_object_list_from_bitmap() is called, and some sort of mechanism to say hey I'm all set, never change shallow repo status again from now on, or just die if you have to do it to protect us from similar bugs. But for now your fix is good (and simple). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] read-cache.c: Ensure unmerged entries are removed
A file in the index can be left as merged and unmerged at the same time by some tools as libgit2, this causes some undesiderable behaviours in git. I have seen, at least, these behaviours: - git reset --hard consuming 100% CPU and never ending - git reset --hard consuming all memory in git 2.0 - git add/git mergetool not resolving a conflict, even if they finish succesfully The state is something like this: $ git ls-files -s 100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 0 conflict 100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1 conflict 100644 5716ca5987cbf97d6bb54920bea6adde242d87e6 2 conflict 100644 f2e41136eac73c39554dede1fd7e67b12502d577 3 conflict This can be caused e.g. by libgit2 doing this: 1. Merge with conflicts, without solving them 2. Force checkout I see that this is not caused by git (I haven't been able to reproduce it only using git) but I think that git should be able to detect this situation and even handle it, specially to avoid the never-ending git resets. The proposed patch serves as protection and autoremediation for this kind of cases. Another option would be to detect the issue and tell the user to clean the index with git read-tree --empty, but I think this would be more intrussive than the patch. Regards, Jaime Soriano. Jaime Soriano Pastor (1): read-cache.c: Ensure unmerged entries are removed read-cache.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) -- 2.0.4.1.g8a38f21.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] read-cache.c: Ensure unmerged entries are removed
Wrong implementations of tools that modify the index can left
some files as merged and unmerged at the same time. Avoid undesiderable
behaviours by handling this situation.
Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com
---
read-cache.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 7f5645e..23e46e1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -935,6 +935,7 @@ static int add_index_entry_with_check(struct index_state
*istate, struct cache_e
int ok_to_replace = option ADD_CACHE_OK_TO_REPLACE;
int skip_df_check = option ADD_CACHE_SKIP_DFCHECK;
int new_only = option ADD_CACHE_NEW_ONLY;
+ int replaced = 0;
cache_tree_invalidate_path(istate-cache_tree, ce-name);
pos = index_name_stage_pos(istate, ce-name, ce_namelen(ce),
ce_stage(ce));
@@ -943,9 +944,10 @@ static int add_index_entry_with_check(struct index_state
*istate, struct cache_e
if (pos = 0) {
if (!new_only)
replace_index_entry(istate, pos, ce);
- return 0;
- }
- pos = -pos-1;
+ pos++;
+ replaced = 1;
+ } else
+ pos = -pos-1;
/*
* Inserting a merged entry (stage 0) into the index
@@ -959,6 +961,8 @@ static int add_index_entry_with_check(struct index_state
*istate, struct cache_e
}
}
+ if (replaced)
+ return 0;
if (!ok_to_add)
return -1;
if (!verify_path(ce-name))
--
2.0.4.1.g8a38f21.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: Error fatal: not a git repository after auto packing
On Tue, Aug 12, 2014 at 5:56 AM, Luke Campagnola luke.campagn...@gmail.com wrote: Greetings, I have been working happily with git for a couple of years, and ran into a mysterious issue today: after running a git-pull during which I saw the message Auto packing the repository for optimum performance. I now receive the error Fatal: not a git repository when running any git commands, and a little investigation revealed that my .git/refs directory has gone missing, presumably because the refs were all combined into .git/packed-refs. To restore access to the repository, all I needed was to `mkdir .git/refs`. Is this a known bug? Not to me. It seems like either git should tolerate the absence of a .git/refs directory, or the auto packer should not remove it. I tried to clone from vispy.git, running 'git gc' manually and 'pack-refs' (which should be in charge of removing empty dirs with v1.9.1. Things worked fine. Looked at refs.c:try_remove_empty_parents() as well. Still could not find any bug there.. So no good news. I don't suppose that if you run git gc again, it would remove $GIT_DIR/refs one more time? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error fatal: not a git repository after auto packing
On ma, 2014-08-11 at 18:56 -0400, Luke Campagnola wrote: raven:/home/luke/vispy (transform-cache)$ git checkout master Switched to branch 'master' Deleted 103 .pyc files Deleted 11 empty directories This looks like you have a local post-checkout hook that deletes empty directories. Fix that hook to not do anything in .git and it should be fine. -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error fatal: not a git repository after auto packing
On Tue, Aug 12, 2014 at 12:11 PM, Dennis Kaarsemaker den...@kaarsemaker.net wrote: On ma, 2014-08-11 at 18:56 -0400, Luke Campagnola wrote: raven:/home/luke/vispy (transform-cache)$ git checkout master Switched to branch 'master' Deleted 103 .pyc files Deleted 11 empty directories This looks like you have a local post-checkout hook that deletes empty directories. Fix that hook to not do anything in .git and it should be fine. Of course you are correct. Sorry for the noise. -- To unsubscribe from this list: send the line 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] git-gui: Make git-gui lib dir configurable at runtime
Pat Thoyts pattho...@users.sourceforge.net writes: Pat, do you want patches via the git mailing list, personal mail, or some other way? The standard method is both: personal to ensure I see it and mailing list to allow everyone to comment. I've applied this patch to git-gui master. Thanks, both. Is it a good time to pull the changes from you to be in the final 2.1 release? -- To unsubscribe from this list: send the line 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] mailsplit.c: remove dead code
Am 11.08.2014 um 23:11 schrieb Stefan Beller: This was found by coverity. (Id: 290001) the variable 'output' is only assigned to a value inequal to NUL, after all gotos to the corrupt label. Therefore we can conclude the two removed lines are actually dead code. After reading the above for the first time I thought it meant the opposite of what's actually going on. Perhaps it's the placement of only, the comma or a flawed understanding of grammar on my part? In any case, there is only one way to reach the label named corrupt, and the variable named output is always NULL if that branch is taken. That means the removed code was a no-op. With those two lines gone you also don't need to initialize output anymore, by the way. And since there is only a single goto, you could move the three remaining error handling lines up to the if statement. Keeping condition and dependent code together would be an improvement, I think. Signed-off-by: Stefan Beller stefanbel...@gmail.com --- builtin/mailsplit.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c index 06296d4..b499014 100644 --- a/builtin/mailsplit.c +++ b/builtin/mailsplit.c @@ -93,8 +93,6 @@ static int split_one(FILE *mbox, const char *name, int allow_bare) return status; corrupt: - if (output) - fclose(output); unlink(name); fprintf(stderr, corrupt mailbox\n); exit(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 1/2] blame.c: Add translation to warning about failed revision walk
Stefan Beller stefanbel...@gmail.com writes:
blame belonging to the group of
ancillaryinterrogators and not to plumbinginterrogators
should have localized error messages?
Unless running under --porcelain option to be driven by scripts, we
expect that we are talking to a human user, so using _(msg) is very
much appropriate for that case.
A possibly problematic script might do something like this:
git blame --porcelain $1 21 |
awk $awkScript
and the $awkScript may check the input lines that do not match the
expected pattern the output lines from the command follow and act on
them, though. _(msg) is unwelcome to such a script [*1*].
I suspect the above problem is likely to be theoretical. People
would be more sloppy and write this instead:
git blame --porcelain $1 |
awk $awkScript
and let the problem pass unnoticed, affecting the later parts of
their processing ;-). And _(msg), not msg, would help.
[Footnote]
*1* ... and with possible interleaving of output that came to the
standard output and the standard error, such parsing by $awkScript
would not be a reliable way to do this anyway. A truly careful one
has to be written along the lines of:
git blame --porcelain $1 $tmp
awk $awkScript $tmp
anyway.
Signed-off-by: Stefan Beller stefanbel...@gmail.com
---
builtin/blame.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 17d30d0..ca4ba6f 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2700,7 +2700,7 @@ parse_done:
* uninteresting.
*/
if (prepare_revision_walk(revs))
- die(revision walk setup failed);
+ die(_(revision walk setup failed));
if (is_null_sha1(sb.final-object.sha1)) {
o = sb.final-util;
--
To unsubscribe from this list: send the line 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 6/8] mv: unindent one level for directory move code
Duy Nguyen pclo...@gmail.com writes: Otherwise, perhaps squash this in? Sure. But I'm having second thought about this series. mv.c is centered around files on worktree, which makes it pretty hard if we want to make it more like rm.c where index and worktree entries are treated more equally and pathspec is applied. Doing that in mv.c would require a lot more reorganization that makes this series obsolete. Well, you may have started these as preparatory clean-up, but taken alone these are purely making the result more readable without changing any behaviour, so let's get them right first. 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] unpack-tree.c: remove dead code
Stefan Beller stefanbel...@gmail.com writes:
In line 1763 of unpack-tree.c we have a condition on the current tree
if (current) {
...
Within this block of code we can assume current to be non NULL, hence
the code after the statement in line 1796:
if (current)
return ...
cannot be reached.
The proposed patch here changes the order of the current tree and the
newtree part. I'm not sure if that's the right way to handle it.
If the existing code decides to reject the merge and falls into that
code path, src[0] aka current is not NULL at that point as you
noticed, and we would call reject_merge(current, o); we would keep
doing so after this remove dead code patch is applied.
If you remove the dead code, which are the inner check for current,
reject_merge() call with newtree and the final fallback of returning
-1, then it would be a faithful remove dead code without changing
anything else update.
Having said that, I think current/newtree/oldtree are used in the
call to reject_merge() *only* for their path aka ce-name, and they
all point at the same name (there is no rename funkies here); hence
all other failures code path should just rely on current always
being present and become something like this instead:
/* 20 or 21 */
...
} else if (o-gently) {
return -1;
} else {
return reject_merge(current, o);
}
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] read-cache.c: Ensure unmerged entries are removed
Jaime Soriano Pastor jsorianopas...@gmail.com writes: A file in the index can be left as merged and unmerged at the same time by some tools as libgit2, this causes some undesiderable behaviours in git. Well, doesn't it mean that libgit2 is broken? Have you filed a bug over there yet? Having said that, protecting ourselves from insanity left by other people is always a good idea, provided that it can be done without bending overly backwards. -- To unsubscribe from this list: send the line 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] read-cache.c: Ensure unmerged entries are removed
Jaime Soriano Pastor jsorianopas...@gmail.com writes: Wrong implementations of tools that modify the index can left some files as merged and unmerged at the same time. Avoid undesiderable behaviours by handling this situation. It is understandable that the way _you_ decided to handle the situation is so obvious to be spelled out to _you_, but that is the most important design decision that needs to be described here. Do you silently remove higher-stage entries when an entry at stage 0 exists? Do you silently remove stage 0 entry when higher-stage entries exist? Do you error out without doing neither? Silently removing these at runtime may not be something we would want to do; after all, we do not know if the broken tool actually wanted to have the higher stage entries, or the merged entry. Ideally, I think we should error out and let the users figure out how to proceed (we may of course need to make sure they have the necessary tools to do so, e.g. git cat-file blob 0:$path to resurrect the contents and git update-index --cacheinfo to stuff the contents into the stages). 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] Documentation/git-rebase.txt: fix -f description to match actual git behavior.
Sergey Organov sorga...@gmail.com writes: Previous description of -f option was wrong as git rebase does not require -f to perform rebase when current branch is a descendant of the commit you are rebasing onto, provided commit(s) to be rebased contain merge(s). Both the above and the updated documentation are a bit hard to read for me, so let me disect what you are and you are not saying to see if I understood them correctly: - The plain-vanilla git rebase that flattens the history will be a no-op *only* when the current tip is a linear descendant of the onto commit without any merge in between. - Merge-preserving form of git rebase -m/-p is a no-op when the current tip is a descendant of the onto commit. - rebase -f is a way to force rebase when it would otherwise be a no-op. - When you force a rebase that would otherwise be a no-op, only the timestamps would change. I think you are right that 'current branch is a descendant of the onto commit' is not necessarily equal to 'rebase that would otherwise be a no-op'. I am not sure if a 'rebase that would otherwise be a no-op' is equal to 'only timestamp would change', though. What happens if you do this, for example? $ GIT_COMMITTER_NAME='Somebody Else' git commit $ git rebase --force --onto HEAD^ HEAD^ HEAD^0 So I think the reasoning (i.e. is a descendant is not quite right) is correct, but the updated text is not quite right. Changing it further to only the committer timestamps and identities would change is probably not an improvement, either. Force the rebase that would otherwise be a no-op may be a better phrasing that does not risk going stale even if we update what are preserved and what are modified in the future. Also I notice the sentence Normally non-interactive...in such a situation is not helping the reader in this description very much. I wonder if we should keep it if we are rewriting this paragraph. Thanks. Signed-off-by: Sergey Organov sorga...@gmail.com --- Documentation/git-rebase.txt | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 2a93c64..62dac31 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -316,10 +316,9 @@ which makes little sense. -f:: --force-rebase:: - Force the rebase even if the current branch is a descendant - of the commit you are rebasing onto. Normally non-interactive rebase will - exit with the message Current branch is up to date in such a - situation. + Force the rebase even if the result will only change commit + timestamps. Normally non-interactive rebase will exit with the + message Current branch is up to date in such a situation. Incompatible with the --interactive option. + You may find this (or --no-ff with an interactive rebase) helpful after -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Not able to create feature branch Git
Hi, I am not able to push my feature branch using Git. I cloned using SSH and SSH key is also added. arup@linux-wzza:~/Ruby/yzz git status # On branch posward # Untracked files: # (use git add file... to include in what will be committed) # # lib/yzz/posward_side.rb nothing added to commit but untracked files present (use git add to track) arup@linux-wzza:~/Ruby/yzz git add lib/yzz/posward_side.rb arup@linux-wzza:~/Ruby/yzz git status # On branch posward # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: lib/yzz/posward_side.rb # arup@linux-wzza:~/Ruby/yzz git commit -m Yzz::Side is now refactord into 2 classes, one of which is Yzz::PoswardSide. [posward a31be0c] Yzz::Side is now refactord into 2 classes, one of which is Yzz::PoswardSide. 1 file changed, 41 insertions(+) create mode 100644 lib/yzz/posward_side.rb arup@linux-wzza:~/Ruby/yzz git push origin posward Warning: Permanently added the RSA host key for IP address '192.30.252.131' to the list of known hosts. ERROR: Permission to boris-s/yzz.git denied to aruprakshit. fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. arup@linux-wzza:~/Ruby/yzz ls -al ~/.ssh total 20 drwxr-xr-x 2 arup users 4096 May 27 00:17 . drwxr-xr-x 44 arup users 4096 Aug 13 01:07 .. -rw--- 1 arup users 1675 May 10 22:22 id_rsa -rw-r--r-- 1 arup users 408 May 10 22:22 id_rsa.pub -rw-r--r-- 1 arup users 2210 Aug 13 01:18 known_hosts arup@linux-wzza:~/Ruby/yzz xclip -sel clip ~/.ssh/id_rsa.pub arup@linux-wzza:~/Ruby/yzz git push origin posward ERROR: Permission to boris-s/yzz.git denied to aruprakshit. fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. arup@linux-wzza:~/Ruby/yzz Can anyone tell me what is the problem ? -- Regards, Arup Rakshit Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. --Brian Kernighan -- To unsubscribe from this list: send the line 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] Documentation/git-rebase.txt: fix -f description to match actual git behavior.
Junio C Hamano gits...@pobox.com writes: So I think the reasoning (i.e. is a descendant is not quite right) is correct, but the updated text is not quite right. Changing it further to only the committer timestamps and identities would change is probably not an improvement, either. Force the rebase that would otherwise be a no-op may be a better phrasing that does not risk going stale even if we update what are preserved and what are modified in the future. Also I notice the sentence Normally non-interactive...in such a situation is not helping the reader in this description very much. I wonder if we should keep it if we are rewriting this paragraph. How about doing it this way, perhaps? -- 8 -- From: Sergey Organov sorga...@gmail.com Date: Tue, 12 Aug 2014 00:22:48 +0400 Subject: [PATCH] Documentation/git-rebase.txt: -f forces a rebase that would otherwise be a no-op Current branch is a descendant of the commit you are rebasing onto does not necessarily mean rebase requires --force. For a plain vanilla history flattening rebase, the rebase can be done without forcing if there is a merge between the tip of the branch being rebased and the commit you are rebasing onto, even if the tip is descendant of the other. [jc: reworded both the text and the log description] Signed-off-by: Sergey Organov sorga...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-rebase.txt | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 2a93c64..f14100a 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -316,11 +316,8 @@ which makes little sense. -f:: --force-rebase:: - Force the rebase even if the current branch is a descendant - of the commit you are rebasing onto. Normally non-interactive rebase will - exit with the message Current branch is up to date in such a - situation. - Incompatible with the --interactive option. + Force a rebase even if the current branch is up-to-date and + the command without `--force` would return without doing anything. + You may find this (or --no-ff with an interactive rebase) helpful after reverting a topic branch merge, as this option recreates the topic branch with -- 2.1.0-rc2-238-g2566d2d -- To unsubscribe from this list: send the line 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 23/23] rebase -i: enable options --signoff, --reset-author for pick, reword
Hi Thomas,
Thomas Rast writes:
Fabian Ruch baf...@gmail.com writes:
@@ -634,21 +644,24 @@ do_replay () {
comment_for_reflog pick
mark_action_done
-do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
+eval do_pick $opts $sha1 \
+|| die_with_patch $sha1 Could not apply $sha1... $rest
You had me a little puzzled at the switch to 'eval' here. That is
necessary to match the quoting added in 20/23, not for any change in
this commit. This commit is simply the first one to trigger this.
This patch switches to 'eval' here because it is the first time
'opts' occurs. However, I agree that it might be confusing that
'opts' wasn't already added to the 'do_pick' lines by 20/23. By
trigger you mean that this commit is the first to actually fill
'opts' with contents? I will move these changes to 20/23 then.
Also, are you sure $sha1 does not require quoting through an eval?
At least if we can assume that it is really a SHA-1 object name. As
such it does not contain characters interpreted by the shell, like
backslashes, quotes or whitespace.
Please add tests to this patch.
The ones I had in mind are attached below the scissors line. The
current reroll fails the authorship checks and the 'git rebase
--continue' test cases. As the necessary changes would obfuscate this
sub-thread, they will be included in the next reroll.
Fabian
-- 8 --
diff --git a/t/t3427-rebase-line-options.sh b/t/t3427-rebase-line-options.sh
index 5881162..a5a9e66 100755
--- a/t/t3427-rebase-line-options.sh
+++ b/t/t3427-rebase-line-options.sh
@@ -6,10 +6,32 @@ test_description='git rebase -i with line options'
. $TEST_DIRECTORY/lib-rebase.sh
+commit_message () {
+ git cat-file commit $1 | sed '1,/^$/d'
+}
+
+commit_authorship () {
+ git cat-file commit $1 | sed -n '/^$/q;/^author /p'
+}
+
+authorship () {
+ echo author $GIT_AUTHOR_NAME $GIT_AUTHOR_EMAIL $GIT_AUTHOR_DATE
+}
+
+test_diff_file () {
+ if cmp $1 $2 /dev/null
+ then
+ echo '$1' and '$2' are the same
+ return 1
+ fi
+}
+
test_expect_success 'Set up repository' '
test_commit Initial
test_commit Commit1
- test_commit Commit2
+ test_commit Commit2
+ git checkout -b branch Commit1
+ test_commit Commit2_ Commit2.t
'
test_expect_success 'Unknown option' '
@@ -23,4 +45,137 @@ test_expect_success 'Unknown option' '
git rebase --continue
'
+test_msg_author () {
+ set_fake_editor
+ FAKE_LINES=1 $1 2 git rebase -i HEAD~2
+ commit_message HEAD actual.msg
+ commit_authorship HEAD actual.author
+ test_cmp expected.msg actual.msg
+ test_cmp expected.author actual.author
+}
+
+test_msg_author_misspelled () {
+ set_cat_todo_editor
+ test_must_fail git rebase -i HEAD^ todo
+ set_fake_editor
+ test_must_fail env FAKE_LINES=1 $1-misspelled 2 git rebase -i HEAD~2
+ set_fixed_todo_editor $(pwd)/todo
+ FAKE_LINES=$1 1 git rebase --edit-todo
+ git rebase --continue
+ commit_message HEAD actual.msg
+ commit_authorship HEAD actual.author
+ test_cmp expected.msg actual.msg
+ test_cmp expected.author actual.author
+}
+
+test_msg_author_conflicted () {
+ set_fake_editor
+ test_must_fail env FAKE_LINES=$1 1 git rebase -i master
+ git checkout --theirs Commit2.t
+ git add Commit2.t
+ git rebase --continue
+ commit_message HEAD actual.msg
+ commit_authorship HEAD actual.author
+ test_cmp expected.msg actual.msg
+ test_cmp expected.author actual.author
+}
+
+test_expect_success 'Misspelled pick --signoff' '
+ git checkout -b misspelled-pick--signoff master
+ cat expected.msg -EOF
+ $(commit_message HEAD)
+
+ Signed-off-by: C O Mitter commit...@example.com
+ EOF
+ commit_authorship HEAD expected.author
+ test_msg_author_misspelled pick_--signoff
+'
+
+test_expect_success 'Conflicted pick --signoff' '
+ git checkout -b conflicted-pick--signoff branch
+ cat expected.msg -EOF
+ $(commit_message HEAD)
+
+ Signed-off-by: C O Mitter commit...@example.com
+ EOF
+ commit_authorship HEAD expected.author
+ test_msg_author_conflicted pick_--signoff
+'
+
+test_expect_success 'pick --signoff' '
+ git checkout -b pick--signoff master
+ cat expected.msg -EOF
+ $(commit_message HEAD)
+
+ Signed-off-by: C O Mitter commit...@example.com
+ EOF
+ commit_authorship HEAD expected.author
+ test_msg_author pick_--signoff
+'
+
+test_expect_success 'reword --signoff' '
+ git checkout -b reword--signoff master
+ cat expected.msg -EOF
+ $(commit_message HEAD)
+
+ Signed-off-by: C O Mitter commit...@example.com
+ EOF
+ commit_authorship HEAD expected.author
+
[PATCH] unpack-tree.c: remove dead code
In line 1763 of unpack-tree.c we have a condition on the current tree
if (current) {
...
Within this block of code we can assume current to be non NULL, hence
the code after the statement in line 1796:
if (current)
return ...
cannot be reached.
current/newtree/oldtree are used in the
call to reject_merge() *only* for their path aka ce-name, and they
all point at the same name (there is no rename funkies here); hence
all other failures code path should just rely on current always
being present.
All referenced lines have been introduced in the same commit
076b0adc (2006-07-30, read-tree: move merge functions to the library),
which was just moving the code around.
The outer condition on the current tree (now in line 1763) was introduced
in c859600954df4c292e, June 2005, [PATCH] read-tree: save more user hassles
during fast-forward.
The inner condition on the current tree was introduced in
ee6566e8d70da682ac4926d, Sept. 2005, [PATCH] Rewrite read-tree
This issue was found by coverity, Id:290002
Signed-off-by: Stefan Beller stefanbel...@gmail.com
Helped-by: Junio C Hamano gits...@pobox.com
---
unpack-trees.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
Did I understand you right, when changing to this one?
diff --git a/unpack-trees.c b/unpack-trees.c
index c6aa8fb..42ee84e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1789,15 +1789,11 @@ int twoway_merge(const struct cache_entry * const *src,
/* 20 or 21 */
return merged_entry(newtree, current, o);
}
+ else if (o-gently) {
+ return -1 ;
+ }
else {
- /* all other failures */
- if (oldtree)
- return o-gently ? -1 : reject_merge(oldtree,
o);
- if (current)
- return o-gently ? -1 : reject_merge(current,
o);
- if (newtree)
- return o-gently ? -1 : reject_merge(newtree,
o);
- return -1;
+ reject_merge(current, o);
}
}
else if (newtree) {
--
2.1.0.rc2
--
To unsubscribe from this list: send the line 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] mailsplit.c: remove dead code
This was found by coverity. (Id: 290001)
The variable 'output' is assigned to a value
after all gotos to the corrupt label.
Remove the goto by moving the errorhandling code to the
condition, which detects the error.
Signed-off-by: Stefan Beller stefanbel...@gmail.com
Helped-by: René Scharfe l@web.de
---
builtin/mailsplit.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
If I understood correctly, this is what you had in mind?
Thanks,
Stefan
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 06296d4..763cda0 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -53,14 +53,16 @@ static int keep_cr;
*/
static int split_one(FILE *mbox, const char *name, int allow_bare)
{
- FILE *output = NULL;
+ FILE *output;
int fd;
int status = 0;
int is_bare = !is_from_line(buf.buf, buf.len);
- if (is_bare !allow_bare)
- goto corrupt;
-
+ if (is_bare !allow_bare) {
+ unlink(name);
+ fprintf(stderr, corrupt mailbox\n);
+ exit(1);
+ }
fd = open(name, O_WRONLY | O_CREAT | O_EXCL, 0666);
if (fd 0)
die_errno(cannot open output file '%s', name);
@@ -91,13 +93,6 @@ static int split_one(FILE *mbox, const char *name, int
allow_bare)
}
fclose(output);
return status;
-
- corrupt:
- if (output)
- fclose(output);
- unlink(name);
- fprintf(stderr, corrupt mailbox\n);
- exit(1);
}
static int populate_maildir_list(struct string_list *list, const char *path)
--
2.1.0.rc2
--
To unsubscribe from this list: send the line 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] mailsplit.c: remove dead code
Stefan Beller stefanbel...@gmail.com writes:
+ if (is_bare !allow_bare) {
+ unlink(name);
+ fprintf(stderr, corrupt mailbox\n);
+ exit(1);
+ }
Not directly related to your patch, but shouldn't this be using
die(_(corrupt mailbox));
instead?
--
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] mailsplit.c: remove dead code
Matthieu Moy matthieu@grenoble-inp.fr writes:
Stefan Beller stefanbel...@gmail.com writes:
+if (is_bare !allow_bare) {
+unlink(name);
+fprintf(stderr, corrupt mailbox\n);
+exit(1);
+}
Not directly related to your patch, but shouldn't this be using
die(_(corrupt mailbox));
instead?
Doesn't the exit status of mailsplit matter to its existing
invokers?
--
To unsubscribe from this list: send the line 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] mailsplit.c: remove dead code
... answering to myself, the only invoker does not seem to care,
so I do not mind if the fprintf/exit gets turned into die() in a
follow-up patch.
Thanks.
On Tue, Aug 12, 2014 at 2:38 PM, Junio C Hamano gits...@pobox.com wrote:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Stefan Beller stefanbel...@gmail.com writes:
+if (is_bare !allow_bare) {
+unlink(name);
+fprintf(stderr, corrupt mailbox\n);
+exit(1);
+}
Not directly related to your patch, but shouldn't this be using
die(_(corrupt mailbox));
instead?
Doesn't the exit status of mailsplit matter to its existing
invokers?
--
To unsubscribe from this list: send the line 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] mailsplit.c: remove dead code
Junio C Hamano gits...@pobox.com writes:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Stefan Beller stefanbel...@gmail.com writes:
+ if (is_bare !allow_bare) {
+ unlink(name);
+ fprintf(stderr, corrupt mailbox\n);
+ exit(1);
+ }
Not directly related to your patch, but shouldn't this be using
die(_(corrupt mailbox));
instead?
Doesn't the exit status of mailsplit matter to its existing
invokers?
Not within Git. I doubt anybody checks if the exit status is 1 and
relies on that for corrupt mailboxes, but OTOH, changing the code may
not be worth the trouble.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] git-imap-send: use libcurl for implementation
Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.
Signed-off-by: Bernhard Reiter ock...@raz.or.at
---
Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones reduces imap-send.c by some 1200 lines of code.
As I don't have access to that many IMAP servers, I haven't been able to
test a variety of parameter combinations. I did test both secure and
insecure (imaps:// and imap://) connections -- this e-mail was generated
that way -- but could e.g. neither test the authMethod nor the tunnel
parameter.
As git-imap-send is one of the two instances OpenSSL is currently used
by git -- the other one being SHA1 -- it might be worthwhile considering
dropping it altogether as there's already a SHA1 library built into git
available as an alternative.
Kind regards
Bernhard
PS: Please CC!
INSTALL | 14 +-
Makefile|4 +-
git.spec.in |5 +-
imap-send.c | 1288
+--
4 files changed, 111 insertions(+), 1200 deletions(-)
diff --git a/INSTALL b/INSTALL
index 6ec7a24..2cd3a42 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,16 @@ Issues of note:
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.
- - openssl library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
-
- By default, git uses OpenSSL for SHA1 but it will use its own
+ - By default, git uses OpenSSL for SHA1 but it will use its own
library (inspired by Mozilla's) with either NO_OPENSSL or
BLK_SHA1. Also included is a version optimized for PowerPC
(PPC_SHA1).
- - libcurl library is used by git-http-fetch and git-fetch. You
- might also want the curl executable for debugging purposes.
- If you do not use http:// or https:// repositories, you do not
- have to have them (use NO_CURL).
+ - libcurl library is used by git-http-fetch, git-fetch, and
+ git-impap-send. You might also want the curl executable for
+ debugging purposes. If you do not use http:// or https://
+ repositories, and do not want to put patches into an IMAP
+ mailbox, you do not have to have them (use NO_CURL).
- expat library; git-http-push uses it for remote lock
management over DAV. Similar to curl above, this is optional
diff --git a/Makefile b/Makefile
index 2320de5..7805603 100644
--- a/Makefile
+++ b/Makefile
@@ -2067,9 +2067,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(CURL_LIBCURL)
git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/git.spec.in b/git.spec.in
index d61d537..7d9230f 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: GPL
Group: Development/Tools
URL: http://kernel.org/pub/software/scm/git/
Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3}
+BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel = 7.30.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3}
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
Requires: perl-Git = %{version}-%{release}
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
# No files for you!
%changelog
+* Mon Aug 11 2014 Bernhard Reiter ock...@raz.or.at
+- Require version = 7.30.0 of curl-devel for IMAP functions.
+
* Sun Sep 18 2011 Jakub Narebski jna...@gmail.com
- Add gitweb manpages to 'gitweb' subpackage
diff --git a/imap-send.c b/imap-send.c
index 524fbab..0c4583f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,47 +22,13 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include cache.h
-#include credential.h
+#include http.h
#include exec_cmd.h
#include run-command.h
-#ifdef NO_OPENSSL
-typedef void *SSL;
-#endif
-static const char imap_send_usage[] = git imap-send mbox;
-
-#undef DRV_OK
-#define DRV_OK 0
-#define DRV_MSG_BAD -1
-#define DRV_BOX_BAD -2
-#define DRV_STORE_BAD -3
-
-static int Verbose, Quiet;
-
-__attribute__((format (printf, 1, 2)))
-static void imap_info(const char *, ...);
-__attribute__((format (printf, 1, 2)))
-static void imap_warn(const char *, ...);
-
-static char *next_arg(char **);
-
-__attribute__((format (printf, 3, 4)))
-static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
+#include curl/curl.h
[PATCH] send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
This is more of an RFH than a PATCH. I think we would want to
encapsulate the logic to set STATUS_REJECT_NODELETE in
set_ref_status_for_push() function, but I do not see a good code
structure to do so, given the constraints of the dataflow (i.e. the
protocol capability is only inspected here with server_supports()).
The motivation behind this patch is that I want to move the Among
refs on remote_refs list, these are not going to be updated on the
remote end logic into a separate helper function in the next step
and call that before we enter in this per-ref main loop.
-- 8 --
20e8b465 (refactor ref status logic for pushing, 2010-01-08)
restructured the code to set status for each ref to be pushed, but
did not quite go far enough. We inspect the status set earlier by
set_refs_status_for_push() and then perform yet another update to
the status of a ref with an otherwise OK status to be deleted to
mark it with REF_STATUS_REJECT_NODELETE when the protocol tells us
never to delete.
Split the latter into a separate loop that comes before we enter the
per-ref loop. This way we would have one less condition to check in
the main loop.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 6129b0f..51497c3 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -231,6 +231,14 @@ int send_pack(struct send_pack_args *args,
return 0;
}
+ /*
+* Why is delete-refs so specific to send-pack machinery
+* that set_ref_status_for_push() cannot set this bit for us???
+*/
+ for (ref = remote_refs; ref; ref = ref-next)
+ if (ref-deletion !allow_deleting_refs)
+ ref-status = REF_STATUS_REJECT_NODELETE;
+
if (!args-dry_run)
advertise_shallow_grafts_buf(req_buf);
@@ -249,17 +257,13 @@ int send_pack(struct send_pack_args *args,
case REF_STATUS_REJECT_FETCH_FIRST:
case REF_STATUS_REJECT_NEEDS_FORCE:
case REF_STATUS_REJECT_STALE:
+ case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_UPTODATE:
continue;
default:
; /* do nothing */
}
- if (ref-deletion !allow_deleting_refs) {
- ref-status = REF_STATUS_REJECT_NODELETE;
- continue;
- }
-
if (!ref-deletion)
new_refs++;
--
2.1.0-rc2-243-g9c8a734
--
To unsubscribe from this list: send the line 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] unpack-tree.c: remove dead code
Stefan Beller stefanbel...@gmail.com writes:
In line 1763 of unpack-tree.c we have a condition on the current tree
if (current) {
...
Within this block of code we can assume current to be non NULL, hence
the code after the statement in line 1796:
if (current)
return ...
cannot be reached.
current/newtree/oldtree are used in the
call to reject_merge() *only* for their path aka ce-name, and they
all point at the same name (there is no rename funkies here); hence
all other failures code path should just rely on current always
being present.
All referenced lines have been introduced in the same commit
076b0adc (2006-07-30, read-tree: move merge functions to the library),
which was just moving the code around.
The outer condition on the current tree (now in line 1763) was introduced
in c859600954df4c292e, June 2005, [PATCH] read-tree: save more user hassles
during fast-forward.
The inner condition on the current tree was introduced in
ee6566e8d70da682ac4926d, Sept. 2005, [PATCH] Rewrite read-tree
This issue was found by coverity, Id:290002
Signed-off-by: Stefan Beller stefanbel...@gmail.com
Helped-by: Junio C Hamano gits...@pobox.com
---
unpack-trees.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
Did I understand you right, when changing to this one?
Something like that. I've already pushed out the original with a
tentative SQUASH??? on top for today's integration; I'll try to
remember replacing them with this version.
Thanks.
diff --git a/unpack-trees.c b/unpack-trees.c
index c6aa8fb..42ee84e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1789,15 +1789,11 @@ int twoway_merge(const struct cache_entry * const
*src,
/* 20 or 21 */
return merged_entry(newtree, current, o);
}
+ else if (o-gently) {
+ return -1 ;
+ }
else {
- /* all other failures */
- if (oldtree)
- return o-gently ? -1 : reject_merge(oldtree,
o);
- if (current)
- return o-gently ? -1 : reject_merge(current,
o);
- if (newtree)
- return o-gently ? -1 : reject_merge(newtree,
o);
- return -1;
+ reject_merge(current, o);
}
}
else if (newtree) {
--
To unsubscribe from this list: send the line 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] mailsplit.c: remove dead code
Stefan Beller wrote: This was found by coverity. (Id: 290001) [...] Signed-off-by: Stefan Beller stefanbel...@gmail.com Helped-by: René Scharfe l@web.de --- builtin/mailsplit.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) The idea is to simplify error handling (removing cleanup of the 'output' var) by making it more obvious that there is only one kind of error, right? For what it's worth, it looks good to me (and much clearer than the last version). It's always possible to reintroduce goto-based error handling later if another kind of error is introduced, and in the meantime this is simpler and does not change behavior. Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] stash: default listing to --cc --simplify-combined-diff
On Wed, 2014-07-30 at 20:09 -0400, Jeff King wrote:
On Wed, Jul 30, 2014 at 12:43:09PM -0700, Junio C Hamano wrote:
git log --cc is one of the things I wanted for a long time to fix.
When the user explicitly asks --cc, we currently ignore it, but
because we know the user wants to view combined diff, we should turn
-p on automatically. And the change this patch introduces will be
broken when we fix log --cc (stash list will end up always
showing the patch, without a way to disable it).
Can you make this conditional? Do this only when options are
given to git stash list command and that includes -p or
something?
I'm definitely sympathetic. Using --cc with the diff family _does_
imply -p already, so it would be nice to do the same for the log
family.
Perhaps something along this line?
git-stash.sh | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index ae73ba4..0db1b19 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -297,8 +297,15 @@ have_stash () {
list_stash () {
have_stash || return 0
- git log --format=%gd: %gs -g --cc --simplify-combined-diff \
- $@ $ref_stash --
+ case $* in
+ *' --cc '*)
+ ;; # the user knows what she is doing
+ *' -p '* | *' -U'*)
+ set x --cc --simplify-combined-diff $@
+ shift
+ ;;
+ esac
+ git log --format=%gd: %gs -g $@ $ref_stash --
Ugh. I'd generally like to avoid this sort of ad-hoc command line
parsing, as it is easy to get confused by option arguments or
even non-options. At least we do not have to worry about pathspecs here,
as we already do an explicit -- (so we might be confused by them, but
they are broken anyway).
Still, I wonder if a cleaner solution is to provide alternate default
to this options for the revision.c option parser. We already have
--default to pick the default starting point. Could we do something
like --default-merge-handling=cc or something?
There's a similar ad-hoc parsing case in stash show, where we add
--stat if no arguments are given, but we have no clue if a diff format
was given (so git stash show --color accidentally turns on patch
format!). Something like --default-diff-format=stat would be more
robust.
I dunno. Maybe it is over-engineering. I just hate to see solutions that
work most of the time and crumble in weird corner cases, even if people
don't hit those corner cases very often.
-Peff
I agree with you on this one. Those corner cases tend to bite the worst.
Thanks,
Jake
--
To unsubscribe from this list: send the line 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/3] Re: [PATCH] unpack-tree.c: remove dead code
Stefan Beller wrote:
In line 1763 of unpack-tree.c we have a condition on the current tree
[...]
The description is describing why the patch is *correct* (i.e., not
going to introduce a bug), while what the reader wants to know is why
the change is *desirable*.
Is this about making the code more readable, or robust, or suppressing
a static analysis error, or something else? What did the user or
reader want to do that they couldn't do before and now can after this
patch?
[...]
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1789,15 +1789,11 @@ int twoway_merge(const struct cache_entry * const
*src,
/* 20 or 21 */
return merged_entry(newtree, current, o);
}
+ else if (o-gently) {
+ return -1 ;
+ }
(not about this patch) Elsewhere git uses the 'cuddled else':
if (foo) {
...
} else if (bar) {
...
} else {
...
}
That stylefix would be a topic for a different patch, though.
else {
- /* all other failures */
- if (oldtree)
- return o-gently ? -1 : reject_merge(oldtree,
o);
- if (current)
- return o-gently ? -1 : reject_merge(current,
o);
- if (newtree)
- return o-gently ? -1 : reject_merge(newtree,
o);
- return -1;
Does the static analysis tool support comments like
if (oldtree)
...
if (current)
...
...
/* not reached */
return -1;
? That might be the simplest minimally invasive fix for what coverity
pointed out.
Now that we're looking there, though, it's worth understanding why we
do the 'if oldtree exists, use it, else fall back to, etc' thing. Was
this meant as futureproofing in case commands like 'git checkout' want
to do rename detection some day?
Everywhere else in the file that reject_merge is used, it is as
return o-gently ? -1 : reject_merge(..., o);
The one exception is
!current
oldtree
newtree
oldtree != newtree
!initial_checkout
(#17), which seems like a bug (it should have the same check). Would
it make sense to inline the o-gently check into reject_merge so callers
don't have to care?
In that spirit, I suspect the simplest fix would be
else
return o-gently ? -1 : reject_merge(current, o);
and then all calls could be replaced in a followup patch.
Sensible?
Thanks,
Jonathan Nieder (2):
unpack-trees: use 'cuddled' style for if-else cascade
checkout -m: attempt merge when deletion of path was staged
Stefan Beller (1):
unpack-trees: simplify 'all other failures' case
unpack-trees.c | 31 ++-
1 file changed, 10 insertions(+), 21 deletions(-)
--
To unsubscribe from this list: send the line 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/3] unpack-trees: simplify 'all other failures' case
From: Stefan Beller stefanbel...@gmail.com
In the 'if (current)' block of twoway_merge, we handle the boring
errors by checking if the entry from the old tree, current index, and
new tree are present, to get a pathname for the error message from one
of them:
if (oldtree)
return o-gently ? -1 : reject_merge(oldtree, o);
if (current)
return o-gently ? -1 : reject_merge(current, o);
if (newtree)
return o-gently ? -1 : reject_merge(newtree, o);
return -1;
Since this is guarded by 'if (current)', the second test is guaranteed
to succeed. Moreover, any of the three entries, if present, would
have the same path because there is no rename detection in this code
path. Even if some day in the future the entries' paths differ, the
'current' path used in the index and worktree would presumably be the
most recognizable for the end user.
Simplify by just using 'current'.
Noticed by coverity, Id:290002
Signed-off-by: Stefan Beller stefanbel...@gmail.com
Improved-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
unpack-trees.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ad3e9a0..f4a9aa9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1791,16 +1791,8 @@ int twoway_merge(const struct cache_entry * const *src,
/* 20 or 21 */
return merged_entry(newtree, current, o);
}
- else {
- /* all other failures */
- if (oldtree)
- return o-gently ? -1 : reject_merge(oldtree,
o);
- if (current)
- return o-gently ? -1 : reject_merge(current,
o);
- if (newtree)
- return o-gently ? -1 : reject_merge(newtree,
o);
- return -1;
- }
+ else
+ return o-gently ? -1 : reject_merge(current, o);
}
else if (newtree) {
if (oldtree !o-initial_checkout) {
--
--
To unsubscribe from this list: send the line 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/3] unpack-trees: use 'cuddled' style for if-else cascade
Match the predominant style in git by following KR style for if/else
cascades. Documentation/CodingStyle from linux.git explains:
Note that the closing brace is empty on a line of its own, _except_ in
the cases where it is followed by a continuation of the same statement,
ie a while in a do-statement or an else in an if-statement, like
this:
if (x == y) {
..
} else if (x y) {
...
} else {
}
Rationale: KR.
Also, note that this brace-placement also minimizes the number of empty
(or almost empty) lines, without any loss of readability.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
unpack-trees.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index f4a9aa9..187b15b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1771,8 +1771,7 @@ int twoway_merge(const struct cache_entry * const *src,
return merged_entry(newtree, current,
o);
}
return o-gently ? -1 : reject_merge(current, o);
- }
- else if ((!oldtree !newtree) || /* 4 and 5 */
+ } else if ((!oldtree !newtree) || /* 4 and 5 */
(!oldtree newtree
same(current, newtree)) || /* 6 and 7 */
(oldtree newtree
@@ -1781,17 +1780,14 @@ int twoway_merge(const struct cache_entry * const *src,
!same(oldtree, newtree) /* 18 and 19 */
same(current, newtree))) {
return keep_entry(current, o);
- }
- else if (oldtree !newtree same(current, oldtree)) {
+ } else if (oldtree !newtree same(current, oldtree)) {
/* 10 or 11 */
return deleted_entry(oldtree, current, o);
- }
- else if (oldtree newtree
+ } else if (oldtree newtree
same(current, oldtree) !same(current, newtree)) {
/* 20 or 21 */
return merged_entry(newtree, current, o);
- }
- else
+ } else
return o-gently ? -1 : reject_merge(current, o);
}
else if (newtree) {
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] checkout -m: attempt merge when deletion of path was staged
twoway_merge() is missing an o-gently check in the case where a file
that needs to be modified is missing from the index but present in the
old and new trees. As a result, in this case 'git checkout -m' errors
out instead of trying to perform a merge.
Fix it by checking o-gently. While at it, inline the o-gently check
into reject_merge to prevent future call sites from making the same
mistake.
Noticed by code inspection. The motivating case hasn't been tested.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
This is the most iffy of the three patches, mostly because I was too
lazy to write a test. I believe it's safe as-is nonetheless.
Thanks for reading.
unpack-trees.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 187b15b..6c45af7 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1178,7 +1178,8 @@ return_failed:
static int reject_merge(const struct cache_entry *ce,
struct unpack_trees_options *o)
{
- return add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce-name);
+ return o-gently ? -1 :
+ add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce-name);
}
static int same(const struct cache_entry *a, const struct cache_entry *b)
@@ -1633,7 +1634,7 @@ int threeway_merge(const struct cache_entry * const
*stages,
/* #14, #14ALT, #2ALT */
if (remote !df_conflict_head head_match !remote_match) {
if (index !same(index, remote) !same(index, head))
- return o-gently ? -1 : reject_merge(index, o);
+ return reject_merge(index, o);
return merged_entry(remote, index, o);
}
/*
@@ -1641,7 +1642,7 @@ int threeway_merge(const struct cache_entry * const
*stages,
* make sure that it matches head.
*/
if (index !same(index, head))
- return o-gently ? -1 : reject_merge(index, o);
+ return reject_merge(index, o);
if (head) {
/* #5ALT, #15 */
@@ -1770,7 +1771,7 @@ int twoway_merge(const struct cache_entry * const *src,
else
return merged_entry(newtree, current,
o);
}
- return o-gently ? -1 : reject_merge(current, o);
+ return reject_merge(current, o);
} else if ((!oldtree !newtree) || /* 4 and 5 */
(!oldtree newtree
same(current, newtree)) || /* 6 and 7 */
@@ -1788,7 +1789,7 @@ int twoway_merge(const struct cache_entry * const *src,
/* 20 or 21 */
return merged_entry(newtree, current, o);
} else
- return o-gently ? -1 : reject_merge(current, o);
+ return reject_merge(current, o);
}
else if (newtree) {
if (oldtree !o-initial_checkout) {
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] checkout -m: attempt merge when deletion of path was staged
On Tue, Aug 12, 2014 at 5:03 PM, Jonathan Nieder jrnie...@gmail.com wrote:
twoway_merge() is missing an o-gently check in the case where a file
that needs to be modified is missing from the index but present in the
old and new trees. As a result, in this case 'git checkout -m' errors
out instead of trying to perform a merge.
Fix it by checking o-gently. While at it, inline the o-gently check
into reject_merge to prevent future call sites from making the same
mistake.
Noticed by code inspection. The motivating case hasn't been tested.
That sounds sloppy X-. I may comment more after figuring out
what _other_ reject_merge() caller that does not appear in the
patch would change its behaviour with this patch.
side note: of course, if this were two patches, one that adds the
same o-gently ? -1 : reject thing to places where they forget to
do so, and the other that moves the gently thing to reject helper,
then we can read all the necessary information to judge the change
in the patch ;-)
Thanks.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
This is the most iffy of the three patches, mostly because I was too
lazy to write a test. I believe it's safe as-is nonetheless.
Thanks for reading.
unpack-trees.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 187b15b..6c45af7 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1178,7 +1178,8 @@ return_failed:
static int reject_merge(const struct cache_entry *ce,
struct unpack_trees_options *o)
{
- return add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce-name);
+ return o-gently ? -1 :
+ add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce-name);
}
static int same(const struct cache_entry *a, const struct cache_entry *b)
@@ -1633,7 +1634,7 @@ int threeway_merge(const struct cache_entry * const
*stages,
/* #14, #14ALT, #2ALT */
if (remote !df_conflict_head head_match !remote_match) {
if (index !same(index, remote) !same(index, head))
- return o-gently ? -1 : reject_merge(index, o);
+ return reject_merge(index, o);
return merged_entry(remote, index, o);
}
/*
@@ -1641,7 +1642,7 @@ int threeway_merge(const struct cache_entry * const
*stages,
* make sure that it matches head.
*/
if (index !same(index, head))
- return o-gently ? -1 : reject_merge(index, o);
+ return reject_merge(index, o);
if (head) {
/* #5ALT, #15 */
@@ -1770,7 +1771,7 @@ int twoway_merge(const struct cache_entry * const *src,
else
return merged_entry(newtree, current,
o);
}
- return o-gently ? -1 : reject_merge(current, o);
+ return reject_merge(current, o);
} else if ((!oldtree !newtree) || /* 4 and 5 */
(!oldtree newtree
same(current, newtree)) || /* 6 and 7 */
@@ -1788,7 +1789,7 @@ int twoway_merge(const struct cache_entry * const *src,
/* 20 or 21 */
return merged_entry(newtree, current, o);
} else
- return o-gently ? -1 : reject_merge(current, o);
+ return reject_merge(current, o);
}
else if (newtree) {
if (oldtree !o-initial_checkout) {
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Possible bug v2.0.4: git reflog after a crash
This is about what happens when a Git process gets killed by someone,
or there is a system crash, and then we try to run Git again on the
repository.
When I run git-commit after adding a few files to the repository, git
updates the .git/refs/heads/master file using rename() to complete the
commit. Just before this, git appends some meta-information to the
.git/logs/HEAD file. If a crash happens such that one of these
operations has been issued but the other has not, then, running
git-reflog in the future seems to silently show wrong information.
Specifically, the HEAD@{} and the message of one commit (either the
in-transaction commit that was being performed during the crash, or
the previous commit) is shown against the commit number of the other
commit. (I am running git-2.0.4.)
The situation can be reproduced by killing a git process between the
append and the rename, using GDB. (Such a crash can also be caused
when a file system buffers only one of the operations for a longer
time and a kernel OOPS happens in-between. Typical re-ordering file
systems such as ext4-ordered buffer the append longer than the rename,
leading to about a 25 second window in-between).
A disclaimer: I am more involved in file system research than in using
Git expertly, and only noticed this issue while examining Git for a
research project. There is a chance that git-reflog is supposed to
output the information it currently outputs, and I am simply expecting
the wrong thing from it. Also, we have found a couple of places where
Git might act wrongly in the event of a system crash if a re-ordering
file system is used, apart from the usual fsync-before-rename
concerns; these, however, require unusual re-orderings that are not
done by most usual file systems. I have not reported them because I
get the sense that Git is written for an ordered file system; do let
me know if reporting these will be useful.
Example wrong outputs from git reflog:
1f9cd01 HEAD@{0}: commit: test2
1f9cd01 HEAD@{1}: commit (initial): test1
4550c4a HEAD@{0}: commit (initial): test1
Expected output:
4550c4a HEAD@{0}: commit: test2
1f9cd01 HEAD@{1}: commit (initial): test1
Thanks,
Thanu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
Bernhard Reiter wrote:
Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.
Wow! This sounds lovely. Thanks for working on this.
[...]
Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones reduces imap-send.c by some 1200 lines of code.
As I don't have access to that many IMAP servers, I haven't been able to
test a variety of parameter combinations. I did test both secure and
insecure (imaps:// and imap://) connections -- this e-mail was generated
that way -- but could e.g. neither test the authMethod nor the tunnel
parameter.
The above two paragraphs belong in the commit message, too, since
they'll be just as useful for someone looking back through the history
as for someone reading the patch on-list today.
[...]
--- a/INSTALL
+++ b/INSTALL
[...]
- - libcurl library is used by git-http-fetch and git-fetch. You
+ - libcurl library is used by git-http-fetch, git-fetch, and
+ git-impap-send. You might also want the curl executable for
Typo: s/impap-send/imap-send/
--- a/Makefile
+++ b/Makefile
@@ -2067,9 +2067,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^)
$(LIBS)
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(CURL_LIBCURL)
7.30.0 is only ~1 year old. Does this mean users would need to update
curl in order to build imap-send?
For example, Debian 7.6 ships curl 7.26.0 and CentOS 7 has curl 7.29.0.
Ideally this could be done as an optional feature:
1. Introduce a USE_CURL_FOR_IMAP_SEND makefile variable to take
advantage of the nice new API.
2. (optional) Use the curl_check makefile variable to turn on
USE_CURL_FOR_IMAP_SEND automatically when appropriate.
3. In a few years, when everyone has upgraded, we could simplify by
getting rid of the USE_OPENSSL_FOR_IMAP_SEND code path.
What do you think?
[...]
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,47 +22,13 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include cache.h
Usual style is to start with a #include of cache.h or
git-compat-util.h. http.h including cache.h for itself was an
old mistake. (I'll reply with a patch to fix that.)
[...]
+#include curl/curl.h
http.h already #includes this. Do you use other helpers from
http.h/http.c or do you use libcurl directly? (Just curious.)
Some style nits:
[...]
+static curl_socket_t opensocket(void *clientp, curlsocktype purpose,
+ struct
curl_sockaddr *address)
Long line. Do you have ts=4? (Git uses 8-space tabs. There's some
emacs magic in Documentation/CodingGuidelines. It should be possible
to add similar hints there for other editors if they don't do the
right thing by default.)
+{
+ curl_socket_t sockfd;
+ (void)purpose;
+ (void)address;
Elsewhere git lets unused parameters be. The unused parameter warning
is too noisy in callback-heavy code (e.g., for_each_ref) so we don't
turn it on.
+ sockfd = *(curl_socket_t *)clientp;
+ /* the actual externally set socket is passed in via the OPENSOCKETDATA
+option */
(style nit) Comments in git look like this:
/*
* The actual, externally set socket is passed in via the
* OPENSOCKETDATA option.
*/
return sockfd;
[...]
+static int sockopt_callback(void *clientp, curl_socket_t curlfd,
+ curlsocktype purpose)
+{
+ /* This return code was added in libcurl 7.21.5 */
+ return CURL_SOCKOPT_ALREADY_CONNECTED;
I'd drop the comment, unless there's some subtlety involved. (E.g.,
is there some other return code that would be more appropriate but was
introudced later or something?)
[...]
@@ -1368,12 +218,14 @@ static int git_imap_config(const char *key, const char
*val, void *cb)
int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
- struct strbuf msg = STRBUF_INIT;
+ struct buffer msg = { STRBUF_INIT, 0 };
Ah, ok --- we do use http.c stuff.
[...]
+ char path[8192];
+ int pathlen;
I realize the old code only had 8192 for the IMAP command buffer,
but could this be a strbuf now, or is there some underlying limit
somewhere else?
[...]
@@ -1417,31 +269,89 @@ int main(int argc, char **argv)
return 1;
}
+ curl_global_init(CURL_GLOBAL_ALL);
http.c seems to make the same mistake, but should the return value
from this be checked?
- /* write it to the imap server */
-
Re: [PATCH] pack-objects: turn off bitmaps when we see --shallow lines
On Tue, Aug 12, 2014 at 10:13:03PM +0700, Duy Nguyen wrote: On Tue, Aug 12, 2014 at 11:34 AM, Jeff King p...@peff.net wrote: Arguably is_repository_shallow should return 1 if anybody has registered a shallow graft, but that wouldn't be enough to fix this (we'd still need to check it again _after_ reading the --shallow lines). So I think this fix is fine here. I don't know if any other parts of the code would care, though. It's getting too subtle (is_repository_shallow fails to return 1). register_shallow() is used elsewhere too, luckily pack bitmap's use is still limited in pack-objects (I think). It is, though I have some patches in the works to use it in more places. I was tempted to make a check in prepare_bitmap_walk() to just return -1 (the same as if there are no bitmaps at all) if any commit grafts are in use. That would also catch new callers. But the graft (and replace) rules are not always the same. We should not respect those features when packing or pruning (though I think pruning _does_ currently respect grafts, which seems like an accident waiting to happen). I think this is a good minimal fix for now, but I'll revisit the replace/graft/shallow issues when I add more bitmap users outside of pack-objects. I prefer (in future) to teach is_repository_shallow about register_shallow and move it to right before get_object_list_from_bitmap() is called, and some sort of mechanism to say hey I'm all set, never change shallow repo status again from now on, or just die if you have to do it to protect us from similar bugs. But for now your fix is good (and simple). Yeah, that sounds like a good direction for the shallow part of it. -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
