[PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Did some simple changing to the chain of if() statements in function install_branch_config() of branch.c This was a micro-project for GSOC 2014 From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001 From: Nemina Amarasinghe nemi...@gmail.com Date: Mon, 10 Mar 2014 13:02:55 +0530 Subject: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c --- branch.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/branch.c b/branch.c index d3b9d49..0304a7a 100644 --- a/branch.c +++ b/branch.c @@ -87,7 +87,12 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(Branch %s set up to track local branch %s by rebasing.) : _(Branch %s set up to track local branch %s.), local, shortname); - else if (!remote_is_branch (origin || !origin)) + else if (!remote_is_branch origin) + printf_ln(rebasing ? + _(Branch %s set up to track remote ref %s by rebasing.) : + _(Branch %s set up to track remote ref %s.), + local, remote); + else if (!remote_is_branch !origin) printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), -- 1.9.0.152.g6ab4ae2 -- To unsubscribe from this list: send the line 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] Documentation: Say that submodule clones use a separate gitdirs.
Le dimanche 09 mars 2014 à 19:24 -0400, Andrew Keller a écrit : On Mar 7, 2014, at 7:50 PM, Henri GEIST wrote: Le vendredi 07 mars 2014 à 15:37 -0800, Junio C Hamano a écrit : Henri GEIST geist.he...@laposte.net writes: This information is technical in nature but has some importance for general users. As this kind of clone have a separate gitdir, you will have a surprise if you copy past the worktree as the gitdir will not come together. I am not sure if I understand exactly what you are trying to say. Are you saying that you had a submodule at sub/dir in your working tree, and then mkdir ../another cp -R sub/dir ../another did not result in a usable Git working tree in ../another directory? It is almost like complaining that mkdir ../newone cp -R * ../newone/ did not result in a usable git repository in ../newone directory and honestly speaking, that sounds borderline insane, I'd have to say. Yes, if a user knows what she is doing, she should be able to make something like that work, without running git clone (which is probably the way most users would do it). And yes, it would be good to let the user learn from the documentation enough so that she knows what she is doing. But no, I do not think end-user facing documentation for git-submodule subcommand is the way to do that. That is why I suggested repository-layout as potentially a better alternative location. But perhaps I am mis-reading your rationale. Let me rephrase my example : To give one of my project to someone else I have copied it on a USB key. By a simple drag and drop with the mouse. And I am quite sure I am not alone doing this way. I have done those kind of things lot of time without any problem. But that day 'the_project' happened to be a submodule cloned by 'git submodule update' then on the USB key the $GIT_DIR of 'the_project' was missing. If 'man git-submodule' have made me aware of the particularities of submodules clone I had write in a terminal: git clone the_project /media/usb/the_project Or at least I had understand what happened quicker. I have nothing against also adding something in repository-layout but I am pretty sure normal users never read repository-layout as it is not a command they use. And it is not mentioned in most tutorials. How about something like this: The git directory of a submodule lives inside the git directory of the parent repository instead of within the working directory. I'm not sure where to put it, though. - Andrew Keller 'git directory' seems ambiguous to me. Maybe we could use 'git metadata'. signature.asc Description: This is a digitally signed message part
Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001 From: Nemina Amarasinghe nemi...@gmail.com Date: Mon, 10 Mar 2014 13:02:55 +0530 Subject: [PATCH] simplified the chain if() statements of install_brach_config() function in branch.c --- branch.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/branch.c b/branch.c index d3b9d49..0304a7a 100644 --- a/branch.c +++ b/branch.c @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons _(Branch %s set up to track local branch %s by rebasing.) : _(Branch %s set up to track local branch %s.), local, shortname); - else if (!remote_is_branch origin) - printf_ln(rebasing ? - _(Branch %s set up to track remote ref %s by rebasing.) : - _(Branch %s set up to track remote ref %s.), - local, remote); - else if (!remote_is_branch !origin) + else if (!remote_is_branch (origin || !origin)) printf_ln(rebasing ? _(Branch %s set up to track remote ref %s by rebasing.) : _(Branch %s set up to track remote ref %s.), -- 1.9.0.152.g6ab4ae2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2014, #01; Tue, 4)
Am 3/5/2014 1:10, schrieb Junio C Hamano: * nd/gitignore-trailing-whitespace (2014-02-10) 2 commits - dir: ignore trailing spaces in exclude patterns - dir: warn about trailing spaces in exclude patterns Warn and then ignore trailing whitespaces in .gitignore files, unless they are quoted for fnmatch(3), e.g. path\ . Will merge to 'next'. The new test does not pass on Windows. I'll attempt to prepare patches that amount to work around using FUNNYNAMES, but I'm running out of time now. -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe nemi...@gmail.com writes: Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one Please, re-read Documentation/SubmittingPatches. In short, don't inline patch headers and don't forget the sign-off. Subject: [PATCH] simplified the chain if() statements of install_brach_config() function in branch.c Keep the subject line short (ideally 50 characters), and avoid past tense. We usually use imperative (the patch asks the codebase to refactor itself = simplify if statements We usually prefix the subject line with the place/subsystem where the change is done = branch.c: simplify if - else if (!remote_is_branch origin) - printf_ln(rebasing ? - _(Branch %s set up to track remote ref %s by rebasing.) : - _(Branch %s set up to track remote ref %s.), - local, remote); - else if (!remote_is_branch !origin) + else if (!remote_is_branch (origin || !origin)) Is it me, or is (origin || !origin) a tautology? -- 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/RFC] rebase: new convenient option to edit/reword/delete a single commit
On 03/09/2014 03:49 AM, Nguyễn Thái Ngọc Duy wrote: Prepare the todo list for you to edit/reword/delete the given commit. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Allowing multiple actions is a bit too much for my shell skills. I don't really need it so I won't push it, but if somebody gives me a sketch, I'll try to polish it. --squash and --fixup would require two commits, making it a bit awkward in option handling. Or is the fixup/squash always HEAD? These commands always squash/fixup the indicated commit with the previous one. I think the same approach that you use below should work for these commands, too. Documentation/git-rebase.txt | 11 +++ git-rebase--interactive.sh | 17 ++--- git-rebase.sh| 22 +- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 2a93c64..becb749 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -13,6 +13,7 @@ SYNOPSIS 'git rebase' [-i | --interactive] [options] [--exec cmd] [--onto newbase] --root [branch] 'git rebase' --continue | --skip | --abort | --edit-todo +'git rebase' [--edit | -E | --reword | -R | --delete | -D ] commit-ish DESCRIPTION --- @@ -356,6 +357,16 @@ unless the `--fork-point` option is specified. user edit that list before rebasing. This mode can also be used to split commits (see SPLITTING COMMITS below). +-E=commit:: +--edit=commit:: +-R=commit:: +--reword=commit:: +-D=commit:: +--delete=commit:: + Prepare the todo list to edit or reword or delete the + specified commit. Configuration variable `rebase.autostash` is + ignored. If I understand correctly, when one of these options is used, the editor is not presented to the user at all. If so, then it is probably confusing to emphasize the todo list, because the user will never see it. How about Edit, reword, or delete the specified commit, replaying subsequent commits on top of it (like running `git rebase --interactive commit^` and then changing the command on the line containing commit). If conflicts arise when replaying the later commits, resolve them and run git rebase --continue, as usual. The configuration variable `rebase.autosquash` is ignored when these options are used. + -p:: --preserve-merges:: Instead of ignoring merges, try to recreate them. diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index a1adae8..3ded4c7 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1040,9 +1040,20 @@ fi has_action $todo || die_abort Nothing to do -cp $todo $todo.backup -git_sequence_editor $todo || - die_abort Could not execute editor +if test -n $one_action +then + commit=`git rev-parse --short $one_commit` + sed 1s/pick $commit /$one_action $commit / $todo $todo.new || It wouldn't hurt to anchor this pattern at the beginning of the line. I understand that it wouldn't help, either (assuming everything else is working right), but it makes the intention clearer. + die_abort failed to update todo list + grep ^$one_action $commit $todo.new /dev/null || + die_abort expected to find '$one_action $commit' line but did not The die_aborts above is really an internal consistency check, right? If so, maybe it should start with internal error: so that the user doesn't think that he has done something wrong. + mv $todo.new $todo || + die_abort failed to update todo list +else + cp $todo $todo.backup + git_sequence_editor $todo || + die_abort Could not execute editor +fi has_action $todo || die_abort Nothing to do diff --git a/git-rebase.sh b/git-rebase.sh index 5f6732b..2acffb4 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -32,6 +32,9 @@ verify allow pre-rebase hook to run rerere-autoupdate allow rerere to update index with resolved conflicts root! rebase all reachable commits up to the root(s) autosquash move commits that begin with squash!/fixup! under -i +E,edit=! generate todo list to edit this commit +R,reword=! generate todo list to reword this commit's message +D,delete=! generate todo list to delete this commit committer-date-is-author-date! passed to 'git am' ignore-date! passed to 'git am' whitespace=! passed to 'git apply' @@ -228,6 +231,7 @@ then fi test -n $type in_progress=t +one_action= total_argc=$# while test $# != 0 do @@ -290,6 +294,7 @@ do ;; --autostash) autostash=true + explicit_autosquash=t Should that be explicit_autostash? ;; --verbose) verbose=t @@ -335,6 +340,13 @@ do --gpg-sign=*)
Re: Potential GSOC microproject idea
On 03/09/2014 02:54 AM, Kyle J. McKay wrote: On Mar 3, 2014, at 23:58, Michael Haggerty wrote: list regulars should FEEL ENCOURAGED to submit microprojects to add to the list. (Either submit them as a pull request to the GitHub repository that contains the text [1] or to the mailing list with CC to me.) Potential idea for a microproject: Add a new config setting: user.allowImplicitIdentity Defaults to true. If user.name and GIT_COMMITTER_NAME are unset or user.email and GIT_COMMITTER_EMAIL and EMAIL are unset, an implicit value is substituted for one or both of user.name and user.email. If an automatically generated value is used for both name and email a warning Your name and email address were configured automatically... is displayed. If set to false, no or never, instead of a warning, an error is generated and the operation fails: *** Please tell me who you are. Run git config --global user.email y...@example.com git config --global user.name Your Name to set your account's default identity. Omit --global to set the identity only in this repository. fatal: implicit user identity not allowed the advice portion can be suppressed by setting advice.implicitIdentity to false, but not the fatal: implicit user identity not allowed part. Note that if git config --system --bool user.allowImplicitIdentity false is in effect, it should still be possible to clone (ref logs may be updated, but they should be allowed to use an implicit identity). In other words user.allowImplicitIdentity=false should only inhibit writing any new commit/tag objects that need the current user's name and email when it has not been explicitly provided. I'm not sure how micro this is. :) I do think the amount of code involved is rather small though. Support for something like this has popped up on the list before. Perhaps user.allowAutomaticIdentity and advice.automaticIdentity would be better config names. This would be much more work than the other microprojects. The ones that I wrote were mostly one- or few-line changes that didn't require the student to learn a lot of context. (And most students struggled mightily with even those.) So I'm not sure this task is suitable. Other feedback welcome. We're pretty much out of microprojects, which is possibly even worse than having difficult ones. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
Matthieu Moy matthieu@grenoble-inp.fr writes: Nemina Amarasinghe nemi...@gmail.com writes: Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one Please, re-read Documentation/SubmittingPatches. In short, don't inline patch headers and don't forget the sign-off. Subject: [PATCH] simplified the chain if() statements of install_brach_config() function in branch.c Keep the subject line short (ideally 50 characters), and avoid past tense. We usually use imperative (the patch asks the codebase to refactor itself = simplify if statements We usually prefix the subject line with the place/subsystem where the change is done = branch.c: simplify if -else if (!remote_is_branch origin) -printf_ln(rebasing ? - _(Branch %s set up to track remote ref %s by rebasing.) : - _(Branch %s set up to track remote ref %s.), - local, remote); -else if (!remote_is_branch !origin) +else if (!remote_is_branch (origin || !origin)) Is it me, or is (origin || !origin) a tautology? Since it _is_ you, that question is a tautology because of its first part already. But the second one would be just as good. -- David Kastrup -- To unsubscribe from this list: send the line 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] rebase: new convenient option to edit/reword/delete a single commit
Michael Haggerty mhag...@alum.mit.edu writes: @@ -290,6 +294,7 @@ do ;; --autostash) autostash=true +explicit_autosquash=t Should that be explicit_autostash? My guess is: no, but it should be below the --autoquash case, not this one. -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe neminaa at gmail.com writes: Is it me, or is (origin || !origin) a tautology? Thanks for the advices Matthieu. I will go through the documentations again. Is there anything wrong with my logic? What I wanted to express is ((!remote_is_branch origin) || (!remote_is_branch || !origin)) -- To unsubscribe from this list: send the line 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe nemi...@gmail.com writes: Nemina Amarasinghe neminaa at gmail.com writes: Is it me, or is (origin || !origin) a tautology? Thanks for the advices Matthieu. I will go through the documentations again. Is there anything wrong with my logic? What I wanted to express is ((!remote_is_branch origin) || (!remote_is_branch || !origin)) (The second || should be a in this sentence) I'm not saying your rewrite is incorrect, but there's no reason to keep (origin || !origin) in a logical formula. -- 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe nemi...@gmail.com writes: Nemina Amarasinghe neminaa at gmail.com writes: Is it me, or is (origin || !origin) a tautology? Thanks for the advices Matthieu. I will go through the documentations again. Is there anything wrong with my logic? What I wanted to express is ((!remote_is_branch origin) || (!remote_is_branch || !origin)) Is it? The above is the same as (!remote_is_branch || !origin). What you wrote before is the same as (!remote_is_branch). Maybe you should try copypaste from the expressions you are trying to combine to make sure that what you start with makes sense. -- David Kastrup -- To unsubscribe from this list: send the line 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] submodule : Add --no-separate-git-dir option to add and update command.
Le samedi 08 mars 2014 à 00:00 +0100, Jens Lehmann a écrit : Am 06.03.2014 23:20, schrieb Henri GEIST: Le jeudi 06 mars 2014 à 21:51 +0100, Jens Lehmann a écrit : Am 06.03.2014 21:15, schrieb Henri GEIST: Le jeudi 06 mars 2014 à 20:48 +0100, Jens Lehmann a écrit : Am 06.03.2014 02:25, schrieb Henri GEIST: Wow, that shouldn't even work (as everything inside submodule shouldn't be part of the superproject but must be contained in the submodule itself). Do the git submodule script and other git commands like git status work for you in such setups? As I stated above it is the purpose of the other patch that I have not already send to implement this behavior. And that is why it work. Ok. Everything including 'git submodule' and 'git status' work perfectly. The intent of this patch is only to permit this for gitlinks. Not for regular files. But I still believe that this shouldn't be permitted at all, no matter if files or submodules are concerned. The pitfalls files face in such a scenario are pretty much the same for submodules too. May be you have a good argument for this belief ? Sure, I stated it further down: The problem you're creating with your future patch is related to the work tree, not the GIT_DIR: subsubmodule could also be added to and tracked by submodule (as that is completely unaware of subsubmodule already being tracked by the superproject). Then you would end up in real trouble, as superproject and submodule could have differing SHA-1s recorded for subsubmodule. Don't go there, for just the same reasons we do not allow that for files. As for the difference between submodules and regular files the only difference is in the meaning. Technically directory are just a special kind of file. But there day to day use is drastically different of the use of files which are not directories. I am not against enabling it for files as well. I am just unable to imagine a case where it make sens. It doesn't make sense for both files and submodules. A possible solution when someone try to do it is to issue a warning. We are not able to see any good reason to do this are sure (y/n) ? No, the only possible solution I see is not to allow that at all. In this case where should the separate gitdir of subsubmodule be placed ? - In superproject/modules/submodule/subsubmodule ? - In superproject/submodule/modules/subsubmodule ? - Depending on the 'git submodule update' command order ? - Or both ? It should be placed in .git/modules/submodule/modules/subsubmodule of the superproject (assuming the subsubmodule is part of the first level submodule). But in your example that would live in .git/modules/submodule/subsubmodule (but as mentioned above, I do not consider this a valid setup because then two repositories would be responsible for the data inside subsubmodule, which will lead to lots of trouble). That is why a had proposed an option '--no-separate-git-dir' for 'git submodule add|update' then no repository is responsible for the data in subsubmodule except subsubmodule itself. As I mentioned in my other email, it doesn't matter at all for the setup you're describing if the git directory lives under .git/modules of the superproject or in a .git directory in the submodule. The problem you're creating with your future patch is related to the work tree, not the GIT_DIR: subsubmodule could also be added to and tracked by submodule (as that is completely unaware of subsubmodule already being tracked by the superproject). Then you would end up in real trouble, as superproject and submodule could have differing SHA-1s recorded for subsubmodule. Don't go there, for just the same reasons we do not allow that for files. In fact it mater. Because multiples checkout of superproject and submodules in versions where subsubmodule is present and not. subsubmodule could have been clone one time by submodule and one time by superproject. And each would have a different .git directory. Where is the problem with that? Size? Different refs? The problem is having two gitdir for one worktree. with the .git file in the worktree pointing sometime on one and sometime on the other. And then if they are cloned with --separate-gitdir subsubmodule can have two gitdirs in superproject/modules/submodule/subsubmodule and in superproject/submodule/modules/subsubmodule. Only one is active at a given time but they are two and not synchronized. But the synchronization is done via the superproject, no? Only lot of careful manual command by the user could keep them synchronize. But it is big wast of time. For no added value. It is quicker to make subsubmodule a regular clone without a separate gitdir then there is nothing needing a synchronization. What is the use case you are trying to solve and why can that not be handled by adding
Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c
((!remote_is_branch origin) || (!remote_is_branch || !origin)) Is it? The above is the same as (!remote_is_branch || !origin). What you wrote before is the same as (!remote_is_branch). Maybe you should try copypaste from the expressions you are trying to combine to make sure that what you start with makes sense. OMG.. Really sorry for that... that was a silly mistake. This is the one.. ((!remote_is_branch origin) || (!remote_is_branch !origin)) -- To unsubscribe from this list: send the line 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] simplified the chain if() statements of install_branch_config() function in branch.c
Nemina Amarasinghe nemi...@gmail.com writes: ((!remote_is_branch origin) || (!remote_is_branch || !origin)) Is it? The above is the same as (!remote_is_branch || !origin). What you wrote before is the same as (!remote_is_branch). Maybe you should try copypaste from the expressions you are trying to combine to make sure that what you start with makes sense. OMG.. Really sorry for that... that was a silly mistake. This is the one.. ((!remote_is_branch origin) || (!remote_is_branch !origin)) That is, indeed, perfectly equivalent to (!remote_is_branch). If you write (!remote_is_branch (origin || !origin)) then you will have people (and possibly also the compiler) loudly wondering about what you are trying to say here. The suspicion would be that either this is a result of a typo or is supposed to be an annoyingly obtuse replacement for a /* TODO: treat origin and !origin differently */ kind of comment. -- David Kastrup -- To unsubscribe from this list: send the line 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: howto to run git without a master branch
Ilya Bobyr ilya.bo...@gmail.com writes: There is a git remote set-head to manipulate HEAD in a remote repository. This is misleading. The command does nothing on the remote side, it only changes the refs/remote namespace in your repository. The purpose is to change what branch the ref remote/name resolves to, ie. without explicit branch name. (The only command that manipulates the remote repository is git push, and the plumbing beneath that. To change HEAD in a remote repository you need filesystem access to it.) Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
question about: Facebook makes Mercurial faster than Git
according to these blog posts http://www.infoq.com/news/2014/01/facebook-scaling-hg https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/ mercurial can be faster then git but i don't found any reply from the git community if it is a real problem or if there a ongoing (maybe git 2.0) changes to compete better in this 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: question about: Facebook makes Mercurial faster than Git
On Mon, 10 Mar 2014, Dennis Luehring wrote: according to these blog posts http://www.infoq.com/news/2014/01/facebook-scaling-hg https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/ mercurial can be faster then git but i don't found any reply from the git community if it is a real problem or if there a ongoing (maybe git 2.0) changes to compete better in this case As I understand this, the biggest part of what happened is that Facebook made a tweak to mercurial so that when it needs to know what files have changed in their massive tree, their version asks their special storage array, while git would have to look at it through the filesystem interface (by doing stat calls on the directories and files to see if anything has changed) In other words, unless you have a very high end storage system that can keep track of such things for you, the Facebook 'fix' won't help you. And even if it does have such a capability, unless you use the same storage system that Facebook uses, you would have to port it to your class of device. Now, in addition to this, they did some other tweaks and changes, but compared to this status change, everything else is minor. David Lang -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Accidentially deleted directory, bug in git clean -d?
I accidentially deleted a directory using git clean. I would think this is a bug, but I'm not sure. Was using 1.8.1, but upgraded to 1.9.0 just to see if it was still reproducable, and it was. Here's a minimal way to reproduce: $ git init $ mkdir foo foobar $ git clean -df foobar Removing foo/ Removing foobar/ $ ls $ I expected only foobar to be deleted, but foo was also deleted. The same thing happens in the opposite case: $ git init $ mkdir foo foobar $ git clean -df foo Removing foo/ Removing foobar/ $ ls $ However, it only happens when there is a common prefix in the names: $ git init $ mkdir foo bar $ git clean -df foo Removing foo/ $ ls bar $ In this case, bar was not deleted. -- Best regards, Robin Pedersen Software Engineer SnapTV AS Jordmor Magdalenes vei 17 N-9519 Kviby. Norway rob...@snap.tv http://www.snap.tv -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
I have started working on pluggable ref backends. In this email I
would like to share my plans and solicit feedback.
(This morning I removed this project from the GSoC ideas page, because
it is unfair to ask a student to shoot at a moving target.)
Why?
Currently, the reference- and reflog-handling code in Git is too
coupled to the rest of the system. There are too many places that
know, for example, the difference between loose and packed refs, or
that loose references are stored as files directly under
$GIT_DIR/refs/heads/, or the locking protocols that have to be adhered
to when managing references. This tight coupling, in turn, makes it
nearly impossible to experiment with alternate reference storage
schemes.
But there is a lot of potential to use alternate reference storage
schemes to fix some currently-unfixable problems, and to implement
some cool new features.
Unfixable problems
--
The on-disk format that we currently use to store references makes
some problems impossible to fix:
* It is impossible to get a self-consistent snapshot of all references
at a given moment in time. This makes it impossible, even in
principle, to do object pruning in a 100% race-free way. (Our
current workaround of not deleting objects that are less than two
weeks works in most cases but, aside from being ugly, has holes.
* There are awkward filesystem-imposed constraints on reference
naming, for example:
* D/F conflicts (I): it is not possible to have branches named
my-feature and my-feature/base at the same time.
* D/F conflicts (II): it is not possible to have reflogs for
branches named my-feature and my-feature/base at the same
time. This leads to the problem that it is not, in general,
possible to retain reflogs for branches that have been deleted.
* There are additional constraints on reference names depending on
the filesystem used to store them. For example, a Git repository
on a case-insensitive filesystem fails in confusing ways if there
are two loose references whose names differ only in case; however,
packed references differing in case might work for a while. Also,
reference names that include Unicode characters can have their
normalization form changed if they are written on Mac OS.
* The packed-refs file has to be rewritten whenever a packed reference
is deleted. It might be nice to write 0{40} to a loose reference
file to indicate that the reference has been deleted, but that would
open the way for more D/F conflicts.)
Wild new ideas
--
So, I would like to reorganize the Git code to allow pluggable
reference backends. If we had this, we could try out ideas like
* Retain the idea of loose/packed references, but encode loose
reference names using a portable naming scheme before storing them
to the filesystem; maybe something like
refs/heads/Foo.42 - refs.dir/heads.dir/%46oo%2e42
logs/refs/heads/Foo.42 - refs.dir/heads.dir/%46oo%2e42.log
Yes, it looks uglier. But users shouldn't be looking in these
directories anyway. This single change would prevent D/F conflicts,
allow a reference to be deleted by writing 0{40} to its loose
reference file, allow reflogs to be kept for deleted refs, and
remove the problem of filesystem-dependent naming constraints.
* Store references in a SQLite database, to get correct transaction
handling.
* Store references directly in the Git object database.
* Implement repository groups that share a common object database
and also a common reference store. Each repository in a group would
get a sub-namespace in the shared database, and store its references
in names like refs/member/$MEMBERID/refs/heads/ The member
repos would act like restricted views of the shared database. This
would be like a combination between alternates (with lowered risk of
corruption) and gitnamespaces(7) (but usable for all git commands).
* Reference transactions that can be used across multiple Git
commands. Imagine,
export GIT_TRANSACTION=$(git transaction begin)
trap 'git transaction rollback' ERR
git foo ...
git bar ...
git baz ...
if ! git transaction commit
then
# Transaction failed; all references rolled back
else
# Transaction succeeded; all references updated atomically
fi
trap '' ERR
unset GIT_TRANSACTION
The GIT_TRANSACTION environment variable would tell git to read
from the usual references, overridden with any reference changes
that have occurred during the transaction, but write any changes
(including both old and new values) to the transaction. The command
git transaction commit would verify that the old values listed in
the transaction still agree with the current values, and then make
all of the changes atomically.
Such transactions could also be broadcast to mirrors when they are
committed to keep multiple Git
Re: question about: Facebook makes Mercurial faster than Git
On 10 March 2014 11:07, Dennis Luehring dl.so...@gmx.net wrote: according to these blog posts http://www.infoq.com/news/2014/01/facebook-scaling-hg https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/ mercurial can be faster then git but i don't found any reply from the git community if it is a real problem or if there a ongoing (maybe git 2.0) changes to compete better in this case They mailed the list about performance issues in git. From what I saw there was relatively little feedback. I had the impression, and I would not be surprised if they had the impression that the git development community is relatively unconcerned about performance issues on larger repositories. There have been other reports, which are difficult to keep track of without a bug tracking system, but the ones I know of are: Poor performance of git status with large number of excluded files and large repositories. Poor performance, and breakage, on repositories with very large numbers of files in them. (Rebase for instance will break if you rebase a commit that contains a *lot* of files.) Poor performance in protocol layer (and other places) with repos with large numbers of refs. (Maybe this is fixed, not sure.) cheers, Yves -- perl -Mre=debug -e /just|another|perl|hacker/ -- To unsubscribe from this list: send the line 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: question about: Facebook makes Mercurial faster than Git
Am 10.03.2014 12:28, schrieb demerphq: I had the impression, and I would not be surprised if they had the impression that the git development community is relatively unconcerned about performance issues on larger repositories. so the question is if the git community is interested in beeing competive in such large scale scenarios - something what mercurial seems to be now out of the box -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
On Mon, Mar 10, 2014 at 12:00 PM, Michael Haggerty mhag...@alum.mit.edu wrote: I have started working on pluggable ref backends. In this email I would like to share my plans and solicit feedback. No comments or useful feedback yet, except that I enthusiastically approve of the objective and the plan you have for how to get there. ...Johan -- Johan Herland, jo...@herland.net www.herland.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: question about: Facebook makes Mercurial faster than Git
On Mon, Mar 10, 2014 at 12:42 PM, Dennis Luehring dl.so...@gmx.net wrote: Am 10.03.2014 12:28, schrieb demerphq: I had the impression, and I would not be surprised if they had the impression that the git development community is relatively unconcerned about performance issues on larger repositories. so the question is if the git community is interested in beeing competive in such large scale scenarios - something what mercurial seems to be now out of the box AFAIK, David Lang's comment is not far off the mark. Facebook has made a tool called Watchman (https://github.com/facebook/watchman) that watches your work tree (i.e. wrapping inotify on Linux) and triggers various commands when files within are changed (e.g. do an auto-build whenever a file in your project changes). Since this tool will discover when files change, they have adjusted Mercurial to discover changes by querying Watchman instead of stat-ing the entire work tree. AFAICS, this is basically a tradeoff between the time it takes to stat your work tree and the overhead/administrivia of running a daemon to monitor the work tree. It seems Facebook has organized their code and infrastructure in a way that makes the latter approach worthwhile for them, and has contributed their solution back to Mercurial. It should be possible to teach Git to do similar things, and IINM there are (and have previously been) several attempts to do similar things in Git, e.g.: - http://thread.gmane.org/gmane.comp.version-control.git/240339 - http://thread.gmane.org/gmane.comp.version-control.git/217817 I haven't looked closely at these attempts (it is not my scratch to itch), and I don't know if/how they would work on top of Watchman, but in principle I don't see why Git shouldn't be able to leverage Watchman the same way Mercurial does. ...Johan -- Johan Herland, jo...@herland.net www.herland.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
[PATCH 06/26] refs.h: Rename the action_on_err constants
Given that these constants are only being used when updating
references, it is inappropriate to give them such generic names as
DIE_ON_ERR. So prefix their names with UPDATE_REFS_.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/checkout.c | 2 +-
builtin/clone.c| 9 +
builtin/merge.c| 6 +++---
builtin/notes.c| 6 +++---
builtin/reset.c| 6 --
builtin/update-ref.c | 5 +++--
contrib/examples/builtin-fetch--tool.c | 3 ++-
notes-cache.c | 2 +-
notes-utils.c | 3 ++-
refs.c | 18 +-
refs.h | 9 +++--
11 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ada51fa..f79b222 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -624,7 +624,7 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
/* Nothing to do. */
} else if (opts-force_detach || !new-path) { /* No longer on any
branch. */
update_ref(msg.buf, HEAD, new-commit-object.sha1, NULL,
- REF_NODEREF, DIE_ON_ERR);
+ REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
if (!opts-quiet) {
if (old-path advice_detached_head)
detach_advice(new-name);
diff --git a/builtin/clone.c b/builtin/clone.c
index 43e772c..af3b86f 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -521,7 +521,7 @@ static void write_followtags(const struct ref *refs, const
char *msg)
if (!has_sha1_file(ref-old_sha1))
continue;
update_ref(msg, ref-name, ref-old_sha1,
- NULL, 0, DIE_ON_ERR);
+ NULL, 0, UPDATE_REFS_DIE_ON_ERR);
}
}
@@ -589,14 +589,15 @@ static void update_head(const struct ref *our, const
struct ref *remote,
create_symref(HEAD, our-name, NULL);
if (!option_bare) {
const char *head = skip_prefix(our-name,
refs/heads/);
- update_ref(msg, HEAD, our-old_sha1, NULL, 0,
DIE_ON_ERR);
+ update_ref(msg, HEAD, our-old_sha1, NULL, 0,
+ UPDATE_REFS_DIE_ON_ERR);
install_branch_config(0, head, option_origin,
our-name);
}
} else if (our) {
struct commit *c = lookup_commit_reference(our-old_sha1);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
update_ref(msg, HEAD, c-object.sha1,
- NULL, REF_NODEREF, DIE_ON_ERR);
+ NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
} else if (remote) {
/*
* We know remote HEAD points to a non-branch, or
@@ -604,7 +605,7 @@ static void update_head(const struct ref *our, const struct
ref *remote,
* Detach HEAD in all these cases.
*/
update_ref(msg, HEAD, remote-old_sha1,
- NULL, REF_NODEREF, DIE_ON_ERR);
+ NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
}
}
diff --git a/builtin/merge.c b/builtin/merge.c
index f0cf120..a4c3b17 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -398,7 +398,7 @@ static void finish(struct commit *head_commit,
const char *argv_gc_auto[] = { gc, --auto, NULL };
update_ref(reflog_message.buf, HEAD,
new_head, head, 0,
- DIE_ON_ERR);
+ UPDATE_REFS_DIE_ON_ERR);
/*
* We ignore errors in 'gc --auto', since the
* user should see them.
@@ -1222,7 +1222,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
die(_(%s - not something we can merge), argv[0]);
read_empty(remote_head-object.sha1, 0);
update_ref(initial pull, HEAD, remote_head-object.sha1,
- NULL, 0, DIE_ON_ERR);
+ NULL, 0, UPDATE_REFS_DIE_ON_ERR);
goto done;
} else {
struct strbuf merge_names = STRBUF_INIT;
@@ -1339,7 +1339,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
}
update_ref(updating ORIG_HEAD, ORIG_HEAD, head_commit-object.sha1,
- NULL, 0, DIE_ON_ERR);
+ NULL, 0, UPDATE_REFS_DIE_ON_ERR);
if (remoteheads !common)
; /* No common ancestors found. We need a real merge. */
diff --git a/builtin/notes.c b/builtin/notes.c
[PATCH 04/26] parse_arg(): Really test that argument is properly terminated
Add a docstring to the function incorporating the comments that were
formerly within the function plus some added information. Test that
the argument is properly terminated by either whitespace or a NUL
character, even if it is quoted, to be consistent with the non-quoted
case. Adjust the tests to expect the new error message.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 20 +++-
t/t1400-update-ref.sh | 4 ++--
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 1292cfe..02b5f95 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -62,16 +62,26 @@ static void update_store_old_sha1(struct ref_update *update,
update-have_old = *oldvalue || line_termination;
}
+/*
+ * Parse one whitespace- or NUL-terminated, possibly C-quoted argument
+ * and append the result to arg. Return a pointer to the terminator.
+ * Die if there is an error in how the argument is C-quoted. This
+ * function is only used if not -z.
+ */
static const char *parse_arg(const char *next, struct strbuf *arg)
{
- /* Parse SP-terminated, possibly C-quoted argument */
- if (*next != '')
+ if (*next == '') {
+ const char *orig = next;
+
+ if (unquote_c_style(arg, next, next))
+ die(badly quoted argument: %s, orig);
+ if (*next !isspace(*next))
+ die(unexpected character after quoted argument: %s,
orig);
+ } else {
while (*next !isspace(*next))
strbuf_addch(arg, *next++);
- else if (unquote_c_style(arg, next, next))
- die(badly quoted argument: %s, next);
+ }
- /* Return position after the argument */
return next;
}
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e2f1dfa..5836842 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -356,10 +356,10 @@ test_expect_success 'stdin fails on badly quoted input' '
grep fatal: badly quoted argument: \\\master err
'
-test_expect_success 'stdin fails on arguments not separated by space' '
+test_expect_success 'stdin fails on junk after quoted argument' '
echo create \$a\master stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: expected SP but got: master err
+ grep fatal: unexpected character after quoted argument:
\\\$a\\\master err
'
test_expect_success 'stdin fails create with no ref' '
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/26] t1400: Fix name and expected result of one test
The test stdin -z create ref fails with zero new value actually passes an empty new value, not a zero new value. So rename the test s/zero/empty/, and change the expected error from fatal: create $c given zero new value to fatal: create $c missing newvalue Of course, this makes the test fail now, so mark it test_expect_failure. The failure will be fixed later in this patch series. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t1400-update-ref.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 6ffd82f..fa927d2 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -827,10 +827,10 @@ test_expect_success 'stdin -z create ref fails with bad new value' ' test_must_fail git rev-parse --verify -q $c ' -test_expect_success 'stdin -z create ref fails with zero new value' ' +test_expect_failure 'stdin -z create ref fails with empty new value' ' printf $F create $c stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: create $c given zero new value err + grep fatal: create $c missing newvalue err test_must_fail git rev-parse --verify -q $c ' -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/26] Clean up update-refs --stdin and implement ref_transaction
I just sent an email to the list [1] describing how I want to decouple reference-handling code from the rest of Git, and implement pluggable reference storage backends. This patch series is the first movement in that direction. update_refs() and update-ref --stdin implement the beginning of transactions for git references, by allowing a group of reference changes to be done in an all-or-nothing fashion. The main point of this patch series is to increase the abstraction level of the API for dealing with reference transactions, by moving the handling of the transaction to refs.c. The new API for dealing with reference transactions is ref_transaction *transaction = create_ref_transaction(); queue_create_ref(transaction, refname, new_sha1, ...); queue_update_ref(transaction, refname, new_sha1, old_sha1, ...); queue_delete_ref(transaction, refname, old_sha1, ...); ... if (commit_ref_transaction(transaction, msg, ...)) die(...); When implementing this I found a number of minor problems in the implementation of git update-ref --stdin, not to mention that it used struct ref_update all the way up and down its parser call stack. So most of the commits in this series are actually cleanups in builtin/update-ref.c. I also spend some time making the error messages emitted by that command more uniform. Then, in just a couple of commits, the ref_transaction abstraction is introduced, update-ref is changed to use it, and update_refs() is removed from the refs API (it was only used by this one caller). Finally, now that refs.c owns the data structures for dealing with transactions, it is possible to make a few simplifications. More changes in this neighborhood will be coming in future patches. [1] http://article.gmane.org/gmane.comp.version-control.git/243726 Michael Haggerty (26): t1400: Fix name and expected result of one test t1400: Provide sensible input to the command t1400: Pass a legitimate newvalue to update command parse_arg(): Really test that argument is properly terminated t1400: Add some more tests involving quoted arguments refs.h: Rename the action_on_err constants update_refs(): Fix constness update-ref --stdin: Read the whole input at once parse_cmd_verify(): Copy old_sha1 instead of evaluating oldvalue twice update-ref.c: Extract a new function, parse_refname() update-ref --stdin: Improve error messages for invalid values update-ref --stdin: Make error messages more consistent update-ref --stdin: Simplify error messages for missing oldvalues update-ref.c: Extract a new function, parse_next_sha1() update-ref --stdin: Improve the error message for unexpected EOF update-ref --stdin: Harmonize error messages refs: Add a concept of a reference transaction update-ref --stdin: Reimplement using reference transactions refs: Remove API function update_refs() struct ref_update: Rename field ref_name to refname struct ref_update: Store refname as a FLEX_ARRAY. commit_ref_transaction(): Introduce temporary variables struct ref_update: Add a lock member struct ref_update: Add type field commit_ref_transaction(): Also free the ref_transaction commit_ref_transaction(): Work with transaction-updates in place builtin/checkout.c | 2 +- builtin/clone.c| 9 +- builtin/merge.c| 6 +- builtin/notes.c| 6 +- builtin/reset.c| 6 +- builtin/update-ref.c | 402 +++-- contrib/examples/builtin-fetch--tool.c | 3 +- notes-cache.c | 2 +- notes-utils.c | 3 +- refs.c | 184 +++ refs.h | 93 ++-- t/t1400-update-ref.sh | 86 --- 12 files changed, 524 insertions(+), 278 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/26] refs: Remove API function update_refs()
This should be done via reference transactions now. This also means
that struct ref_update can become private.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 31 ---
refs.h | 20
2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/refs.c b/refs.c
index 54260ce..91af0a0 100644
--- a/refs.c
+++ b/refs.c
@@ -3267,6 +3267,20 @@ static int update_ref_write(const char *action, const
char *refname,
return 0;
}
+/**
+ * Information needed for a single ref update. Set new_sha1 to the
+ * new value or to zero to delete the ref. To check the old value
+ * while locking the ref, set have_old to 1 and set old_sha1 to the
+ * value or to zero to ensure the ref does not exist before update.
+ */
+struct ref_update {
+ const char *ref_name;
+ unsigned char new_sha1[20];
+ unsigned char old_sha1[20];
+ int flags; /* REF_NODEREF? */
+ int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
+};
+
/*
* Data structure for holding a reference transaction, which can
* consist of checks and updates to multiple references, carried out
@@ -3385,16 +3399,17 @@ static int ref_update_reject_duplicates(struct
ref_update **updates, int n,
return 0;
}
-int update_refs(const char *action, struct ref_update * const *updates_orig,
- int n, enum action_on_err onerr)
+int commit_ref_transaction(struct ref_transaction *transaction,
+ const char *msg, enum action_on_err onerr)
{
int ret = 0, delnum = 0, i;
struct ref_update **updates;
int *types;
struct ref_lock **locks;
const char **delnames;
+ int n = transaction-nr;
- if (!updates_orig || !n)
+ if (!n)
return 0;
/* Allocate work space */
@@ -3404,7 +3419,7 @@ int update_refs(const char *action, struct ref_update *
const *updates_orig,
delnames = xmalloc(sizeof(*delnames) * n);
/* Copy, sort, and reject duplicate refs */
- memcpy(updates, updates_orig, sizeof(*updates) * n);
+ memcpy(updates, transaction-updates, sizeof(*updates) * n);
qsort(updates, n, sizeof(*updates), ref_update_compare);
ret = ref_update_reject_duplicates(updates, n, onerr);
if (ret)
@@ -3426,7 +3441,7 @@ int update_refs(const char *action, struct ref_update *
const *updates_orig,
/* Perform updates first so live commits remain referenced */
for (i = 0; i n; i++)
if (!is_null_sha1(updates[i]-new_sha1)) {
- ret = update_ref_write(action,
+ ret = update_ref_write(msg,
updates[i]-ref_name,
updates[i]-new_sha1,
locks[i], onerr);
@@ -3457,12 +3472,6 @@ cleanup:
return ret;
}
-int commit_ref_transaction(struct ref_transaction *transaction,
- const char *msg, enum action_on_err onerr)
-{
- return update_refs(msg, transaction-updates, transaction-nr, onerr);
-}
-
char *shorten_unambiguous_ref(const char *refname, int strict)
{
int i;
diff --git a/refs.h b/refs.h
index 2848fb7..b1f8b74 100644
--- a/refs.h
+++ b/refs.h
@@ -10,20 +10,6 @@ struct ref_lock {
int force_write;
};
-/**
- * Information needed for a single ref update. Set new_sha1 to the
- * new value or to zero to delete the ref. To check the old value
- * while locking the ref, set have_old to 1 and set old_sha1 to the
- * value or to zero to ensure the ref does not exist before update.
- */
-struct ref_update {
- const char *ref_name;
- unsigned char new_sha1[20];
- unsigned char old_sha1[20];
- int flags; /* REF_NODEREF? */
- int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
-};
-
struct ref_transaction;
/*
@@ -288,12 +274,6 @@ int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr);
-/**
- * Lock all refs and then perform all modifications.
- */
-int update_refs(const char *action, struct ref_update * const *updates,
- int n, enum action_on_err onerr);
-
extern int parse_hide_refs_config(const char *var, const char *value, const
char *);
extern int ref_is_hidden(const char *);
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/26] update-ref --stdin: Reimplement using reference transactions
This change is mostly clerical: the parse_cmd_*() functions need to
use local variables rather than a struct ref_update to collect the
arguments needed for each update, and then call queue_*_ref() to queue
the change rather than building up the list of changes at the caller
side.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 142 +++
1 file changed, 76 insertions(+), 66 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index ac41635..ffed061 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -12,29 +12,11 @@ static const char * const git_update_ref_usage[] = {
NULL
};
-static int updates_alloc;
-static int updates_count;
-static struct ref_update **updates;
+static struct ref_transaction *transaction;
static char line_termination = '\n';
static int update_flags;
-static struct ref_update *update_alloc(void)
-{
- struct ref_update *update;
-
- /* Allocate and zero-init a struct ref_update */
- update = xcalloc(1, sizeof(*update));
- ALLOC_GROW(updates, updates_count + 1, updates_alloc);
- updates[updates_count++] = update;
-
- /* Store and reset accumulated options */
- update-flags = update_flags;
- update_flags = 0;
-
- return update;
-}
-
/*
* Parse one whitespace- or NUL-terminated, possibly C-quoted argument
* and append the result to arg. Return a pointer to the terminator.
@@ -174,95 +156,118 @@ static int parse_next_sha1(struct strbuf *input, const
char **next,
static const char *parse_cmd_update(struct strbuf *input, const char *next)
{
- struct ref_update *update;
-
- update = update_alloc();
+ char *refname;
+ unsigned char new_sha1[20];
+ unsigned char old_sha1[20];
+ int have_old;
- update-ref_name = parse_refname(input, next);
- if (!update-ref_name)
+ refname = parse_refname(input, next);
+ if (!refname)
die(update: missing ref);
- if (parse_next_sha1(input, next, update-new_sha1,
- update, update-ref_name, 0))
- die(update %s: missing newvalue, update-ref_name);
+ if (parse_next_sha1(input, next, new_sha1,
+ update, refname, 0))
+ die(update %s: missing newvalue, refname);
- update-have_old = !parse_next_sha1(input, next, update-old_sha1,
- update, update-ref_name, 1);
+ have_old = !parse_next_sha1(input, next, old_sha1,
+ update, refname, 1);
if (*next != line_termination)
- die(update %s: extra input: %s, update-ref_name, next);
+ die(update %s: extra input: %s, refname, next);
+
+ queue_update_ref(transaction, refname, new_sha1, old_sha1,
+update_flags, have_old);
+
+ update_flags = 0;
+ free(refname);
return next;
}
static const char *parse_cmd_create(struct strbuf *input, const char *next)
{
- struct ref_update *update;
-
- update = update_alloc();
+ char *refname;
+ unsigned char new_sha1[20];
- update-ref_name = parse_refname(input, next);
- if (!update-ref_name)
+ refname = parse_refname(input, next);
+ if (!refname)
die(create: missing ref);
- if (parse_next_sha1(input, next, update-new_sha1,
- create, update-ref_name, 0))
- die(create %s: missing newvalue, update-ref_name);
+ if (parse_next_sha1(input, next, new_sha1,
+ create, refname, 0))
+ die(create %s: missing newvalue, refname);
- if (is_null_sha1(update-new_sha1))
- die(create %s: zero newvalue, update-ref_name);
+ if (is_null_sha1(new_sha1))
+ die(create %s: zero newvalue, refname);
if (*next != line_termination)
- die(create %s: extra input: %s, update-ref_name, next);
+ die(create %s: extra input: %s, refname, next);
+
+ queue_create_ref(transaction, refname, new_sha1, update_flags);
+
+ update_flags = 0;
+ free(refname);
return next;
}
static const char *parse_cmd_delete(struct strbuf *input, const char *next)
{
- struct ref_update *update;
+ char *refname;
+ unsigned char old_sha1[20];
+ int have_old;
- update = update_alloc();
-
- update-ref_name = parse_refname(input, next);
- if (!update-ref_name)
+ refname = parse_refname(input, next);
+ if (!refname)
die(delete: missing ref);
- if (parse_next_sha1(input, next, update-old_sha1,
- delete, update-ref_name, 1)) {
- update-have_old = 0;
+ if (parse_next_sha1(input, next, old_sha1, delete, refname, 1)) {
+ have_old =
[PATCH 11/26] update-ref --stdin: Improve error messages for invalid values
If an invalid value is passed to update-ref --stdin as oldvalue or
newvalue, include the command and the name of the reference at the
beginning of the error message. Update the tests accordingly.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 24 +---
t/t1400-update-ref.sh | 8
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 0dc2061..13a884a 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -35,20 +35,22 @@ static struct ref_update *update_alloc(void)
return update;
}
-static void update_store_new_sha1(struct ref_update *update,
+static void update_store_new_sha1(const char *command,
+ struct ref_update *update,
const char *newvalue)
{
if (*newvalue get_sha1(newvalue, update-new_sha1))
- die(invalid new value for ref %s: %s,
- update-ref_name, newvalue);
+ die(%s %s: invalid new value: %s,
+ command, update-ref_name, newvalue);
}
-static void update_store_old_sha1(struct ref_update *update,
+static void update_store_old_sha1(const char *command,
+ struct ref_update *update,
const char *oldvalue)
{
if (*oldvalue get_sha1(oldvalue, update-old_sha1))
- die(invalid old value for ref %s: %s,
- update-ref_name, oldvalue);
+ die(%s %s: invalid old value: %s,
+ command, update-ref_name, oldvalue);
/* We have an old value if non-empty, or if empty without -z */
update-have_old = *oldvalue || line_termination;
@@ -165,12 +167,12 @@ static const char *parse_cmd_update(struct strbuf *input,
const char *next)
die(update line missing ref);
if (!parse_next_arg(input, next, newvalue))
- update_store_new_sha1(update, newvalue.buf);
+ update_store_new_sha1(update, update, newvalue.buf);
else
die(update %s missing newvalue, update-ref_name);
if (!parse_next_arg(input, next, oldvalue)) {
- update_store_old_sha1(update, oldvalue.buf);
+ update_store_old_sha1(update, update, oldvalue.buf);
if (*next != line_termination)
die(update %s has extra input: %s, update-ref_name,
next);
} else if (!line_termination)
@@ -191,7 +193,7 @@ static const char *parse_cmd_create(struct strbuf *input,
const char *next)
die(create line missing ref);
if (!parse_next_arg(input, next, newvalue))
- update_store_new_sha1(update, newvalue.buf);
+ update_store_new_sha1(create, update, newvalue.buf);
else
die(create %s missing newvalue, update-ref_name);
@@ -216,7 +218,7 @@ static const char *parse_cmd_delete(struct strbuf *input,
const char *next)
die(delete line missing ref);
if (!parse_next_arg(input, next, oldvalue)) {
- update_store_old_sha1(update, oldvalue.buf);
+ update_store_old_sha1(delete, update, oldvalue.buf);
if (update-have_old is_null_sha1(update-old_sha1))
die(delete %s given zero old value, update-ref_name);
} else if (!line_termination)
@@ -240,7 +242,7 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
die(verify line missing ref);
if (!parse_next_arg(input, next, value)) {
- update_store_old_sha1(update, value.buf);
+ update_store_old_sha1(verify, update, value.buf);
hashcpy(update-new_sha1, update-old_sha1);
} else if (!line_termination)
die(verify %s missing [oldvalue] NUL, update-ref_name);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 627aaaf..c5be870 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -518,14 +518,14 @@ test_expect_success 'stdin update ref fails with wrong
old value' '
test_expect_success 'stdin update ref fails with bad old value' '
echo update $c $m does-not-exist stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: invalid old value for ref $c: does-not-exist err
+ grep fatal: update $c: invalid old value: does-not-exist err
test_must_fail git rev-parse --verify -q $c
'
test_expect_success 'stdin create ref fails with bad new value' '
echo create $c does-not-exist stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: invalid new value for ref $c: does-not-exist err
+ grep fatal: create $c: invalid new value: does-not-exist err
test_must_fail git rev-parse --verify -q $c
'
@@ -840,14 +840,14 @@ test_expect_success 'stdin -z update ref fails
[PATCH 08/26] update-ref --stdin: Read the whole input at once
Decouple the parsing code from the input source (the old parsing code
had to read new data even in the middle of commands). This might also
be a tad faster, but that is inconsequential. Add docstrings for the
parsing functions.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 170 ---
1 file changed, 108 insertions(+), 62 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index a8a68e8..5f197fe 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -85,44 +85,70 @@ static const char *parse_arg(const char *next, struct
strbuf *arg)
return next;
}
-static const char *parse_first_arg(const char *next, struct strbuf *arg)
+/*
+ * Parse the argument immediately after command SP. If not -z, then
+ * handle C-quoting. Write the argument to arg. Set *next to point
+ * at the character that terminates the argument. Die if C-quoting is
+ * malformed.
+ */
+static void parse_first_arg(struct strbuf *input, const char **next,
+ struct strbuf *arg)
{
- /* Parse argument immediately after command SP */
strbuf_reset(arg);
if (line_termination) {
/* Without -z, use the next argument */
- next = parse_arg(next, arg);
+ *next = parse_arg(*next, arg);
} else {
- /* With -z, use rest of first NUL-terminated line */
- strbuf_addstr(arg, next);
- next = next + arg-len;
+ /* With -z, use everything up to the next NUL */
+ strbuf_addstr(arg, *next);
+ *next += arg-len;
}
- return next;
}
-static const char *parse_next_arg(const char *next, struct strbuf *arg)
+/*
+ * Parse a SP/NUL separator followed by the next SP- or NUL-terminated
+ * argument, if any. If there is an argument, write it to arg, set
+ * *next to point at the character terminating the argument, and
+ * return 0. If there is no argument at all (not even the empty
+ * string), return a non-zero result and leave *next unchanged.
+ */
+static int parse_next_arg(struct strbuf *input, const char **next,
+ struct strbuf *arg)
{
- /* Parse next SP-terminated or NUL-terminated argument, if any */
strbuf_reset(arg);
if (line_termination) {
/* Without -z, consume SP and use next argument */
- if (!*next)
- return NULL;
- if (*next != ' ')
- die(expected SP but got: %s, next);
- next = parse_arg(next + 1, arg);
+ if (!**next || **next == line_termination)
+ return -1;
+ if (**next != ' ')
+ die(expected SP but got: %s, *next);
+ (*next)++;
+ *next = parse_arg(*next, arg);
} else {
/* With -z, read the next NUL-terminated line */
- if (*next)
- die(expected NUL but got: %s, next);
- if (strbuf_getline(arg, stdin, '\0') == EOF)
- return NULL;
- next = arg-buf + arg-len;
+ if (**next)
+ die(expected NUL but got: %s, *next);
+ (*next)++;
+ if (*next == input-buf + input-len)
+ return -1;
+ strbuf_addstr(arg, *next);
+ *next += arg-len;
}
- return next;
+ return 0;
}
-static void parse_cmd_update(const char *next)
+
+/*
+ * The following five parse_cmd_*() functions parse the corresponding
+ * command. In each case, next points at the character following the
+ * command name and the following space. They each return a pointer
+ * to the character terminating the command, and die with an
+ * explanatory message if there are any parsing problems. All of
+ * these functions handle either text or binary format input,
+ * depending on how line_termination is set.
+ */
+
+static const char *parse_cmd_update(struct strbuf *input, const char *next)
{
struct strbuf ref = STRBUF_INIT;
struct strbuf newvalue = STRBUF_INIT;
@@ -131,26 +157,28 @@ static void parse_cmd_update(const char *next)
update = update_alloc();
- if ((next = parse_first_arg(next, ref)) != NULL ref.buf[0])
+ parse_first_arg(input, next, ref);
+ if (ref.buf[0])
update_store_ref_name(update, ref.buf);
else
die(update line missing ref);
- if ((next = parse_next_arg(next, newvalue)) != NULL)
+ if (!parse_next_arg(input, next, newvalue))
update_store_new_sha1(update, newvalue.buf);
else
die(update %s missing newvalue, ref.buf);
- if ((next = parse_next_arg(next, oldvalue)) != NULL)
+ if (!parse_next_arg(input, next, oldvalue)) {
[PATCH 15/26] update-ref --stdin: Improve the error message for unexpected EOF
Distinguish this error from the error that an argument is missing for another reason. Update the tests accordingly. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/update-ref.c | 4 ++-- t/t1400-update-ref.sh | 10 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 5937291..0a81a11 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -156,8 +156,8 @@ static int parse_next_sha1(struct strbuf *input, const char **next, eof: die(old ? - %s %s missing oldvalue : - %s %s missing newvalue, + %s %s: unexpected end of input when reading oldvalue : + %s %s: unexpected end of input when reading newvalue, command, refname); } diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 7332753..e9a0103 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -709,7 +709,7 @@ test_expect_success 'stdin -z fails create with bad ref name' ' test_expect_success 'stdin -z fails create with no new value' ' printf $F create $a stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: create $a missing newvalue err + grep fatal: create $a: unexpected end of input when reading newvalue err ' test_expect_success 'stdin -z fails create with too many arguments' ' @@ -733,13 +733,13 @@ test_expect_success 'stdin -z fails update with bad ref name' ' test_expect_success 'stdin -z fails update with no new value' ' printf $F update $a stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: update $a missing newvalue err + grep fatal: update $a: unexpected end of input when reading newvalue err ' test_expect_success 'stdin -z fails update with no old value' ' printf $F update $a $m stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: update $a missing oldvalue err + grep fatal: update $a: unexpected end of input when reading oldvalue err ' test_expect_success 'stdin -z fails update with too many arguments' ' @@ -763,7 +763,7 @@ test_expect_success 'stdin -z fails delete with bad ref name' ' test_expect_success 'stdin -z fails delete with no old value' ' printf $F delete $a stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: delete $a missing oldvalue err + grep fatal: delete $a: unexpected end of input when reading oldvalue err ' test_expect_success 'stdin -z fails delete with too many arguments' ' @@ -781,7 +781,7 @@ test_expect_success 'stdin -z fails verify with too many arguments' ' test_expect_success 'stdin -z fails verify with no old value' ' printf $F verify $a stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: verify $a missing oldvalue err + grep fatal: verify $a: unexpected end of input when reading oldvalue err ' test_expect_success 'stdin -z fails option with unknown name' ' -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/26] update-ref --stdin: Simplify error messages for missing oldvalues
Instead of, for example, fatal: update refs/heads/master missing [oldvalue] NUL emit fatal: update refs/heads/master missing oldvalue Update the tests accordingly. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/update-ref.c | 6 +++--- t/t1400-update-ref.sh | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index e4c0854..a9eb5fe 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -176,7 +176,7 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next) if (*next != line_termination) die(update %s has extra input: %s, update-ref_name, next); } else if (!line_termination) - die(update %s missing [oldvalue] NUL, update-ref_name); + die(update %s missing oldvalue, update-ref_name); return next; } @@ -222,7 +222,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next) if (update-have_old is_null_sha1(update-old_sha1)) die(delete %s given zero oldvalue, update-ref_name); } else if (!line_termination) - die(delete %s missing [oldvalue] NUL, update-ref_name); + die(delete %s missing oldvalue, update-ref_name); if (*next != line_termination) die(delete %s has extra input: %s, update-ref_name, next); @@ -245,7 +245,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next) update_store_old_sha1(verify, update, value.buf); hashcpy(update-new_sha1, update-old_sha1); } else if (!line_termination) - die(verify %s missing [oldvalue] NUL, update-ref_name); + die(verify %s missing oldvalue, update-ref_name); if (*next != line_termination) die(verify %s has extra input: %s, update-ref_name, next); diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 3045ae7..42fec4e 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -739,7 +739,7 @@ test_expect_success 'stdin -z fails update with no new value' ' test_expect_success 'stdin -z fails update with no old value' ' printf $F update $a $m stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: update $a missing \\[oldvalue\\] NUL err + grep fatal: update $a missing oldvalue err ' test_expect_success 'stdin -z fails update with too many arguments' ' @@ -763,7 +763,7 @@ test_expect_success 'stdin -z fails delete with bad ref name' ' test_expect_success 'stdin -z fails delete with no old value' ' printf $F delete $a stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: delete $a missing \\[oldvalue\\] NUL err + grep fatal: delete $a missing oldvalue err ' test_expect_success 'stdin -z fails delete with too many arguments' ' @@ -781,7 +781,7 @@ test_expect_success 'stdin -z fails verify with too many arguments' ' test_expect_success 'stdin -z fails verify with no old value' ' printf $F verify $a stdin test_must_fail git update-ref -z --stdin stdin 2err - grep fatal: verify $a missing \\[oldvalue\\] NUL err + grep fatal: verify $a missing oldvalue err ' test_expect_success 'stdin -z fails option with unknown name' ' -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/26] t1400: Provide sensible input to the command
The old version was passing (among other things)
update SP refs/heads/c NUL NUL 0{40} NUL
to git update-ref -z --stdin to test whether the old-value check for
c is working. But the newvalue is empty, which is not allowed for
the update command.
So, to be sure that we are testing what we want to test, provide a
legitimate newvalue on the update line.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
t/t1400-update-ref.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index fa927d2..29391c6 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -912,7 +912,7 @@ test_expect_success 'stdin -z update refs works with
identity updates' '
test_expect_success 'stdin -z update refs fails with wrong old value' '
git update-ref $c $m
- printf $F update $a $m $m update $b $m $m update $c
$Z stdin
+ printf $F update $a $m $m update $b $m $m update $c $m
$Z stdin
test_must_fail git update-ref -z --stdin stdin 2err
grep fatal: Cannot lock the ref '''$c''' err
git rev-parse $m expect
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/26] update-ref --stdin: Make error messages more consistent
The old error messages emitted for invalid input sometimes said
oldvalue/newvalue and sometimes said old value/new value.
Convert them all to the former. Update the tests accordingly.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 8
t/t1400-update-ref.sh | 14 +++---
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 13a884a..e4c0854 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -40,7 +40,7 @@ static void update_store_new_sha1(const char *command,
const char *newvalue)
{
if (*newvalue get_sha1(newvalue, update-new_sha1))
- die(%s %s: invalid new value: %s,
+ die(%s %s: invalid newvalue: %s,
command, update-ref_name, newvalue);
}
@@ -49,7 +49,7 @@ static void update_store_old_sha1(const char *command,
const char *oldvalue)
{
if (*oldvalue get_sha1(oldvalue, update-old_sha1))
- die(%s %s: invalid old value: %s,
+ die(%s %s: invalid oldvalue: %s,
command, update-ref_name, oldvalue);
/* We have an old value if non-empty, or if empty without -z */
@@ -198,7 +198,7 @@ static const char *parse_cmd_create(struct strbuf *input,
const char *next)
die(create %s missing newvalue, update-ref_name);
if (is_null_sha1(update-new_sha1))
- die(create %s given zero new value, update-ref_name);
+ die(create %s given zero newvalue, update-ref_name);
if (*next != line_termination)
die(create %s has extra input: %s, update-ref_name, next);
@@ -220,7 +220,7 @@ static const char *parse_cmd_delete(struct strbuf *input,
const char *next)
if (!parse_next_arg(input, next, oldvalue)) {
update_store_old_sha1(delete, update, oldvalue.buf);
if (update-have_old is_null_sha1(update-old_sha1))
- die(delete %s given zero old value, update-ref_name);
+ die(delete %s given zero oldvalue,
update-ref_name);
} else if (!line_termination)
die(delete %s missing [oldvalue] NUL, update-ref_name);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index c5be870..3045ae7 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -518,21 +518,21 @@ test_expect_success 'stdin update ref fails with wrong
old value' '
test_expect_success 'stdin update ref fails with bad old value' '
echo update $c $m does-not-exist stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: update $c: invalid old value: does-not-exist err
+ grep fatal: update $c: invalid oldvalue: does-not-exist err
test_must_fail git rev-parse --verify -q $c
'
test_expect_success 'stdin create ref fails with bad new value' '
echo create $c does-not-exist stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: create $c: invalid new value: does-not-exist err
+ grep fatal: create $c: invalid newvalue: does-not-exist err
test_must_fail git rev-parse --verify -q $c
'
test_expect_success 'stdin create ref fails with zero new value' '
echo create $c stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: create $c given zero new value err
+ grep fatal: create $c given zero newvalue err
test_must_fail git rev-parse --verify -q $c
'
@@ -556,7 +556,7 @@ test_expect_success 'stdin delete ref fails with wrong old
value' '
test_expect_success 'stdin delete ref fails with zero old value' '
echo delete $a stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: delete $a given zero old value err
+ grep fatal: delete $a given zero oldvalue err
git rev-parse $m expect
git rev-parse $a actual
test_cmp expect actual
@@ -840,14 +840,14 @@ test_expect_success 'stdin -z update ref fails with wrong
old value' '
test_expect_success 'stdin -z update ref fails with bad old value' '
printf $F update $c $m does-not-exist stdin
test_must_fail git update-ref -z --stdin stdin 2err
- grep fatal: update $c: invalid old value: does-not-exist err
+ grep fatal: update $c: invalid oldvalue: does-not-exist err
test_must_fail git rev-parse --verify -q $c
'
test_expect_success 'stdin -z create ref fails with bad new value' '
printf $F create $c does-not-exist stdin
test_must_fail git update-ref -z --stdin stdin 2err
- grep fatal: create $c: invalid new value: does-not-exist err
+ grep fatal: create $c: invalid newvalue: does-not-exist err
test_must_fail git rev-parse --verify -q $c
'
@@ -878,7 +878,7 @@ test_expect_success 'stdin -z delete ref fails with wrong
old
[PATCH 17/26] refs: Add a concept of a reference transaction
Build out the API for dealing with a bunch of reference checks and
changes within a transaction. Define an opaque ref_transaction type
that is managed entirely within refs.c. Introduce functions for
starting a transaction, adding updates to a transaction, and
committing a transaction.
This API will soon replace update_refs().
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 85 ++
refs.h | 63 +
2 files changed, 148 insertions(+)
diff --git a/refs.c b/refs.c
index 0963077..54260ce 100644
--- a/refs.c
+++ b/refs.c
@@ -3267,6 +3267,85 @@ static int update_ref_write(const char *action, const
char *refname,
return 0;
}
+/*
+ * Data structure for holding a reference transaction, which can
+ * consist of checks and updates to multiple references, carried out
+ * as atomically as possible. This structure is opaque to callers.
+ */
+struct ref_transaction {
+ struct ref_update **updates;
+ size_t alloc;
+ size_t nr;
+};
+
+struct ref_transaction *create_ref_transaction(void)
+{
+ return xcalloc(1, sizeof(struct ref_transaction));
+}
+
+void free_ref_transaction(struct ref_transaction *transaction)
+{
+ int i;
+
+ for (i = 0; i transaction-nr; i++) {
+ struct ref_update *update = transaction-updates[i];
+
+ free((char *)update-ref_name);
+ free(update);
+ }
+
+ free(transaction-updates);
+ free(transaction);
+}
+
+static struct ref_update *add_update(struct ref_transaction *transaction,
+const char *refname)
+{
+ struct ref_update *update = xcalloc(1, sizeof(*update));
+
+ update-ref_name = xstrdup(refname);
+ ALLOC_GROW(transaction-updates, transaction-nr + 1,
transaction-alloc);
+ transaction-updates[transaction-nr++] = update;
+ return update;
+}
+
+void queue_update_ref(struct ref_transaction *transaction, const char *refname,
+ unsigned char *new_sha1, unsigned char *old_sha1,
+ int flags, int have_old)
+{
+ struct ref_update *update = add_update(transaction, refname);
+
+ hashcpy(update-new_sha1, new_sha1);
+ update-flags = flags;
+ update-have_old = have_old;
+ if (have_old)
+ hashcpy(update-old_sha1, old_sha1);
+}
+
+void queue_create_ref(struct ref_transaction *transaction, const char *refname,
+ unsigned char *new_sha1,
+ int flags)
+{
+ struct ref_update *update = add_update(transaction, refname);
+
+ hashcpy(update-new_sha1, new_sha1);
+ hashclr(update-old_sha1);
+ update-flags = flags;
+ update-have_old = 1;
+}
+
+void queue_delete_ref(struct ref_transaction *transaction, const char *refname,
+ unsigned char *old_sha1,
+ int flags, int have_old)
+{
+ struct ref_update *update = add_update(transaction, refname);
+
+ update-flags = flags;
+ update-have_old = have_old;
+ if (have_old)
+ hashcpy(update-old_sha1, old_sha1);
+}
+
int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr)
@@ -3378,6 +3457,12 @@ cleanup:
return ret;
}
+int commit_ref_transaction(struct ref_transaction *transaction,
+ const char *msg, enum action_on_err onerr)
+{
+ return update_refs(msg, transaction-updates, transaction-nr, onerr);
+}
+
char *shorten_unambiguous_ref(const char *refname, int strict)
{
int i;
diff --git a/refs.h b/refs.h
index 08e60ac..2848fb7 100644
--- a/refs.h
+++ b/refs.h
@@ -24,6 +24,8 @@ struct ref_update {
int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
};
+struct ref_transaction;
+
/*
* Bit values set in the flags argument passed to each_ref_fn():
*/
@@ -220,6 +222,67 @@ enum action_on_err {
UPDATE_REFS_QUIET_ON_ERR
};
+/*
+ * Allocate and initialize a ref_transaction object. The object must
+ * be freed by calling free_ref_transaction().
+ */
+struct ref_transaction *create_ref_transaction(void);
+
+/*
+ * Free a ref_transaction and all associated data. This function does
+ * not commit the transaction; that must be done first (if desired) by
+ * calling commit_ref_transaction().
+ */
+void free_ref_transaction(struct ref_transaction *transaction);
+
+
+/*
+ * The following functions add a reference check or update to a
+ * ref_transaction. In all of them, refname is the name of the
+ * reference to be affected. The functions make internal copies of
+ * refname, so the caller retains ownership of the parameter. flags
+ * can be REF_NODEREF; it is passed to update_ref_lock().
+ */
+
+
+/*
+ * Add a reference update to transaction. new_sha1 is the value that
+
[PATCH 14/26] update-ref.c: Extract a new function, parse_next_sha1()
Replace three functions, update_store_new_sha1(),
update_store_old_sha1(), and parse_next_arg(), with a single function,
parse_next_sha1(). The new function takes care of a whole argument,
including checking whether it is there, converting it to an SHA-1, and
emitting errors on EOF or for invalid values. The return value
indicates whether the argument was present or absent, which requires
a bit of intelligence because absent values are represented
differently depending on whether -z was used.
The new interface means that the calling functions, parse_cmd_*(),
don't have to interpret the result differently based on the
line_termination mode that is in effect. It also means that
parse_cmd_create() can distinguish unambiguously between an empty new
value and a zeros new value, which fixes a failure in t1400.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 138 +++---
t/t1400-update-ref.sh | 2 +-
2 files changed, 77 insertions(+), 63 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index a9eb5fe..5937291 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -35,27 +35,6 @@ static struct ref_update *update_alloc(void)
return update;
}
-static void update_store_new_sha1(const char *command,
- struct ref_update *update,
- const char *newvalue)
-{
- if (*newvalue get_sha1(newvalue, update-new_sha1))
- die(%s %s: invalid newvalue: %s,
- command, update-ref_name, newvalue);
-}
-
-static void update_store_old_sha1(const char *command,
- struct ref_update *update,
- const char *oldvalue)
-{
- if (*oldvalue get_sha1(oldvalue, update-old_sha1))
- die(%s %s: invalid oldvalue: %s,
- command, update-ref_name, oldvalue);
-
- /* We have an old value if non-empty, or if empty without -z */
- update-have_old = *oldvalue || line_termination;
-}
-
/*
* Parse one whitespace- or NUL-terminated, possibly C-quoted argument
* and append the result to arg. Return a pointer to the terminator.
@@ -112,35 +91,74 @@ static char *parse_refname(struct strbuf *input, const
char **next)
}
/*
- * Parse a SP/NUL separator followed by the next SP- or NUL-terminated
- * argument, if any. If there is an argument, write it to arg, set
- * *next to point at the character terminating the argument, and
+ * Parse an argument separator followed by the next argument, if any.
+ * If there is an argument, convert it to a SHA-1, write it to sha1,
+ * set *next to point at the character terminating the argument, and
* return 0. If there is no argument at all (not even the empty
- * string), return a non-zero result and leave *next unchanged.
+ * string), return 1 and leave *next unchanged. If the value is
+ * provided but cannot be converted to a SHA-1, die.
*/
-static int parse_next_arg(struct strbuf *input, const char **next,
- struct strbuf *arg)
+static int parse_next_sha1(struct strbuf *input, const char **next,
+ unsigned char *sha1,
+ const char *command, const char *refname, int old)
{
- strbuf_reset(arg);
+ struct strbuf arg = STRBUF_INIT;
+ int ret = 0;
+
+ if (*next == input-buf + input-len)
+ goto eof;
+
if (line_termination) {
/* Without -z, consume SP and use next argument */
if (!**next || **next == line_termination)
- return -1;
+ return 1;
if (**next != ' ')
- die(expected SP but got: %s, *next);
+ die(%s %s: expected SP but got: %s,
+ command, refname, *next);
(*next)++;
- *next = parse_arg(*next, arg);
+ *next = parse_arg(*next, arg);
+ if (arg.len) {
+ if (get_sha1(arg.buf, sha1))
+ goto invalid;
+ } else {
+ /* Without -z, an empty value means all zeros: */
+ hashclr(sha1);
+ }
} else {
/* With -z, read the next NUL-terminated line */
if (**next)
- die(expected NUL but got: %s, *next);
+ die(%s %s: expected NUL but got: %s,
+ command, refname, *next);
(*next)++;
if (*next == input-buf + input-len)
- return -1;
- strbuf_addstr(arg, *next);
- *next += arg-len;
+ goto eof;
+ strbuf_addstr(arg, *next);
+ *next += arg.len;
+
+ if (arg.len) {
+ if
[PATCH 22/26] commit_ref_transaction(): Introduce temporary variables
Use temporary variables in the for-loop blocks to simplify expressions
in the rest of the loop.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 335d0e2..ec638e9 100644
--- a/refs.c
+++ b/refs.c
@@ -3424,10 +3424,12 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
/* Acquire all locks while verifying old values */
for (i = 0; i n; i++) {
- locks[i] = update_ref_lock(updates[i]-refname,
- (updates[i]-have_old ?
- updates[i]-old_sha1 : NULL),
- updates[i]-flags,
+ struct ref_update *update = updates[i];
+
+ locks[i] = update_ref_lock(update-refname,
+ (update-have_old ?
+ update-old_sha1 : NULL),
+ update-flags,
types[i], onerr);
if (!locks[i]) {
ret = 1;
@@ -3436,23 +3438,28 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
}
/* Perform updates first so live commits remain referenced */
- for (i = 0; i n; i++)
- if (!is_null_sha1(updates[i]-new_sha1)) {
+ for (i = 0; i n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (!is_null_sha1(update-new_sha1)) {
ret = update_ref_write(msg,
- updates[i]-refname,
- updates[i]-new_sha1,
+ update-refname,
+ update-new_sha1,
locks[i], onerr);
locks[i] = NULL; /* freed by update_ref_write */
if (ret)
goto cleanup;
}
+ }
/* Perform deletes now that updates are safely completed */
- for (i = 0; i n; i++)
+ for (i = 0; i n; i++) {
if (locks[i]) {
delnames[delnum++] = locks[i]-ref_name;
ret |= delete_ref_loose(locks[i], types[i]);
}
+ }
+
ret |= repack_without_refs(delnames, delnum);
for (i = 0; i delnum; i++)
unlink_or_warn(git_path(logs/%s, delnames[i]));
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/26] commit_ref_transaction(): Work with transaction-updates in place
Now that we free the transaction when we are done, there is no need to
make a copy of transaction-updates before working with it.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index d83fc7b..ea33adc 100644
--- a/refs.c
+++ b/refs.c
@@ -3402,19 +3402,17 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
const char *msg, enum action_on_err onerr)
{
int ret = 0, delnum = 0, i;
- struct ref_update **updates;
const char **delnames;
int n = transaction-nr;
+ struct ref_update **updates = transaction-updates;
if (!n)
return 0;
/* Allocate work space */
- updates = xmalloc(sizeof(*updates) * n);
delnames = xmalloc(sizeof(*delnames) * n);
/* Copy, sort, and reject duplicate refs */
- memcpy(updates, transaction-updates, sizeof(*updates) * n);
qsort(updates, n, sizeof(*updates), ref_update_compare);
ret = ref_update_reject_duplicates(updates, n, onerr);
if (ret)
@@ -3469,7 +3467,6 @@ cleanup:
for (i = 0; i n; i++)
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
- free(updates);
free(delnames);
free_ref_transaction(transaction);
return ret;
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/26] t1400: Add some more tests involving quoted arguments
Previously there were no good tests of C-quoted arguments. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t1400-update-ref.sh | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 5836842..627aaaf 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -350,12 +350,18 @@ test_expect_success 'stdin fails on unknown command' ' grep fatal: unknown command: unknown $a err ' -test_expect_success 'stdin fails on badly quoted input' ' +test_expect_success 'stdin fails on unbalanced quotes' ' echo create $a \master stdin test_must_fail git update-ref --stdin stdin 2err grep fatal: badly quoted argument: \\\master err ' +test_expect_success 'stdin fails on invalid escape' ' + echo create $a \ma\zter\ stdin + test_must_fail git update-ref --stdin stdin 2err + grep fatal: badly quoted argument: \\\mazter\\\ err +' + test_expect_success 'stdin fails on junk after quoted argument' ' echo create \$a\master stdin test_must_fail git update-ref --stdin stdin 2err @@ -458,6 +464,24 @@ test_expect_success 'stdin create ref works' ' test_cmp expect actual ' +test_expect_success 'stdin succeeds with quoted argument' ' + git update-ref -d $a + echo create $a \$m\ stdin + git update-ref --stdin stdin + git rev-parse $m expect + git rev-parse $a actual + test_cmp expect actual +' + +test_expect_success 'stdin succeeds with escaped character' ' + git update-ref -d $a + echo create $a \ma\\163ter\ stdin + git update-ref --stdin stdin + git rev-parse $m expect + git rev-parse $a actual + test_cmp expect actual +' + test_expect_success 'stdin update ref creates with zero old value' ' echo update $b $m $Z stdin git update-ref --stdin stdin -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/26] update-ref.c: Extract a new function, parse_refname()
There is no reason to obscure the fact that parse_first_arg() always
parses refnames. Form the new function by combining parse_first_arg()
and update_store_ref_name().
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 90
1 file changed, 41 insertions(+), 49 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 51adf2d..0dc2061 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -35,14 +35,6 @@ static struct ref_update *update_alloc(void)
return update;
}
-static void update_store_ref_name(struct ref_update *update,
- const char *ref_name)
-{
- if (check_refname_format(ref_name, REFNAME_ALLOW_ONELEVEL))
- die(invalid ref format: %s, ref_name);
- update-ref_name = xstrdup(ref_name);
-}
-
static void update_store_new_sha1(struct ref_update *update,
const char *newvalue)
{
@@ -86,23 +78,35 @@ static const char *parse_arg(const char *next, struct
strbuf *arg)
}
/*
- * Parse the argument immediately after command SP. If not -z, then
- * handle C-quoting. Write the argument to arg. Set *next to point
- * at the character that terminates the argument. Die if C-quoting is
- * malformed.
+ * Parse the reference name immediately after command SP. If not
+ * -z, then handle C-quoting. Return a pointer to a newly allocated
+ * string containing the name of the reference, or NULL if there was
+ * an error. Update *next to point at the character that terminates
+ * the argument. Die if C-quoting is malformed or the reference name
+ * is invalid.
*/
-static void parse_first_arg(struct strbuf *input, const char **next,
- struct strbuf *arg)
+static char *parse_refname(struct strbuf *input, const char **next)
{
- strbuf_reset(arg);
+ struct strbuf ref = STRBUF_INIT;
+
if (line_termination) {
/* Without -z, use the next argument */
- *next = parse_arg(*next, arg);
+ *next = parse_arg(*next, ref);
} else {
/* With -z, use everything up to the next NUL */
- strbuf_addstr(arg, *next);
- *next += arg-len;
+ strbuf_addstr(ref, *next);
+ *next += ref.len;
+ }
+
+ if (!ref.len) {
+ strbuf_release(ref);
+ return NULL;
}
+
+ if (check_refname_format(ref.buf, REFNAME_ALLOW_ONELEVEL))
+ die(invalid ref format: %s, ref.buf);
+
+ return strbuf_detach(ref, NULL);
}
/*
@@ -150,111 +154,99 @@ static int parse_next_arg(struct strbuf *input, const
char **next,
static const char *parse_cmd_update(struct strbuf *input, const char *next)
{
- struct strbuf ref = STRBUF_INIT;
struct strbuf newvalue = STRBUF_INIT;
struct strbuf oldvalue = STRBUF_INIT;
struct ref_update *update;
update = update_alloc();
- parse_first_arg(input, next, ref);
- if (ref.buf[0])
- update_store_ref_name(update, ref.buf);
- else
+ update-ref_name = parse_refname(input, next);
+ if (!update-ref_name)
die(update line missing ref);
if (!parse_next_arg(input, next, newvalue))
update_store_new_sha1(update, newvalue.buf);
else
- die(update %s missing newvalue, ref.buf);
+ die(update %s missing newvalue, update-ref_name);
if (!parse_next_arg(input, next, oldvalue)) {
update_store_old_sha1(update, oldvalue.buf);
if (*next != line_termination)
- die(update %s has extra input: %s, ref.buf, next);
+ die(update %s has extra input: %s, update-ref_name,
next);
} else if (!line_termination)
- die(update %s missing [oldvalue] NUL, ref.buf);
+ die(update %s missing [oldvalue] NUL, update-ref_name);
return next;
}
static const char *parse_cmd_create(struct strbuf *input, const char *next)
{
- struct strbuf ref = STRBUF_INIT;
struct strbuf newvalue = STRBUF_INIT;
struct ref_update *update;
update = update_alloc();
- parse_first_arg(input, next, ref);
- if (ref.buf[0])
- update_store_ref_name(update, ref.buf);
- else
+ update-ref_name = parse_refname(input, next);
+ if (!update-ref_name)
die(create line missing ref);
if (!parse_next_arg(input, next, newvalue))
update_store_new_sha1(update, newvalue.buf);
else
- die(create %s missing newvalue, ref.buf);
+ die(create %s missing newvalue, update-ref_name);
if (is_null_sha1(update-new_sha1))
- die(create %s given zero new value, ref.buf);
+ die(create %s given zero new
[PATCH 07/26] update_refs(): Fix constness
Since full const correctness is beyond the ability of C's type system,
just put the const where it doesn't do any harm. A (struct ref_update
**) can be passed to a (struct ref_update * const *) argument, but not
to a (const struct ref_update **) argument.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 2 +-
refs.c | 2 +-
refs.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index f6345e5..a8a68e8 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -14,7 +14,7 @@ static const char * const git_update_ref_usage[] = {
static int updates_alloc;
static int updates_count;
-static const struct ref_update **updates;
+static struct ref_update **updates;
static char line_termination = '\n';
static int update_flags;
diff --git a/refs.c b/refs.c
index 58faf95..0963077 100644
--- a/refs.c
+++ b/refs.c
@@ -3306,7 +3306,7 @@ static int ref_update_reject_duplicates(struct ref_update
**updates, int n,
return 0;
}
-int update_refs(const char *action, const struct ref_update **updates_orig,
+int update_refs(const char *action, struct ref_update * const *updates_orig,
int n, enum action_on_err onerr)
{
int ret = 0, delnum = 0, i;
diff --git a/refs.h b/refs.h
index a713b34..08e60ac 100644
--- a/refs.h
+++ b/refs.h
@@ -228,7 +228,7 @@ int update_ref(const char *action, const char *refname,
/**
* Lock all refs and then perform all modifications.
*/
-int update_refs(const char *action, const struct ref_update **updates,
+int update_refs(const char *action, struct ref_update * const *updates,
int n, enum action_on_err onerr);
extern int parse_hide_refs_config(const char *var, const char *value, const
char *);
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/26] struct ref_update: Rename field ref_name to refname
This is consistent with the usual nomenclature.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 18 +-
refs.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 91af0a0..5d08cdf 100644
--- a/refs.c
+++ b/refs.c
@@ -3274,7 +3274,7 @@ static int update_ref_write(const char *action, const
char *refname,
* value or to zero to ensure the ref does not exist before update.
*/
struct ref_update {
- const char *ref_name;
+ const char *refname;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
int flags; /* REF_NODEREF? */
@@ -3304,7 +3304,7 @@ void free_ref_transaction(struct ref_transaction
*transaction)
for (i = 0; i transaction-nr; i++) {
struct ref_update *update = transaction-updates[i];
- free((char *)update-ref_name);
+ free((char *)update-refname);
free(update);
}
@@ -3317,7 +3317,7 @@ static struct ref_update *add_update(struct
ref_transaction *transaction,
{
struct ref_update *update = xcalloc(1, sizeof(*update));
- update-ref_name = xstrdup(refname);
+ update-refname = xstrdup(refname);
ALLOC_GROW(transaction-updates, transaction-nr + 1,
transaction-alloc);
transaction-updates[transaction-nr++] = update;
return update;
@@ -3375,7 +3375,7 @@ static int ref_update_compare(const void *r1, const void
*r2)
{
const struct ref_update * const *u1 = r1;
const struct ref_update * const *u2 = r2;
- return strcmp((*u1)-ref_name, (*u2)-ref_name);
+ return strcmp((*u1)-refname, (*u2)-refname);
}
static int ref_update_reject_duplicates(struct ref_update **updates, int n,
@@ -3383,14 +3383,14 @@ static int ref_update_reject_duplicates(struct
ref_update **updates, int n,
{
int i;
for (i = 1; i n; i++)
- if (!strcmp(updates[i - 1]-ref_name, updates[i]-ref_name)) {
+ if (!strcmp(updates[i - 1]-refname, updates[i]-refname)) {
const char *str =
Multiple updates for ref '%s' not allowed.;
switch (onerr) {
case UPDATE_REFS_MSG_ON_ERR:
- error(str, updates[i]-ref_name); break;
+ error(str, updates[i]-refname); break;
case UPDATE_REFS_DIE_ON_ERR:
- die(str, updates[i]-ref_name); break;
+ die(str, updates[i]-refname); break;
case UPDATE_REFS_QUIET_ON_ERR:
break;
}
@@ -3427,7 +3427,7 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
/* Acquire all locks while verifying old values */
for (i = 0; i n; i++) {
- locks[i] = update_ref_lock(updates[i]-ref_name,
+ locks[i] = update_ref_lock(updates[i]-refname,
(updates[i]-have_old ?
updates[i]-old_sha1 : NULL),
updates[i]-flags,
@@ -3442,7 +3442,7 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
for (i = 0; i n; i++)
if (!is_null_sha1(updates[i]-new_sha1)) {
ret = update_ref_write(msg,
- updates[i]-ref_name,
+ updates[i]-refname,
updates[i]-new_sha1,
locks[i], onerr);
locks[i] = NULL; /* freed by update_ref_write */
diff --git a/refs.h b/refs.h
index b1f8b74..cc24213 100644
--- a/refs.h
+++ b/refs.h
@@ -154,7 +154,7 @@ extern void unlock_ref(struct ref_lock *lock);
extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
const char *msg);
/** Setup reflog before using. **/
-int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
+int log_ref_setup(const char *refname, char *logfile, int bufsize);
/** Reads log for the value of ref during at_time. **/
extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/26] parse_cmd_verify(): Copy old_sha1 instead of evaluating oldvalue twice
Aside from avoiding work, this makes it transparently obvious that
old_sha1 and new_sha1 are identical.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 5f197fe..51adf2d 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -249,7 +249,7 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
if (!parse_next_arg(input, next, value)) {
update_store_old_sha1(update, value.buf);
- update_store_new_sha1(update, value.buf);
+ hashcpy(update-new_sha1, update-old_sha1);
} else if (!line_termination)
die(verify %s missing [oldvalue] NUL, ref.buf);
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/26] update-ref --stdin: Harmonize error messages
Make (most of) the error messages for invalid input have the same
format [1]:
$COMMAND [SP $REFNAME]: $MESSAGE
Update the tests accordingly.
[1] A few error messages still have their old form, because $COMMAND
and $REFNAME aren't passed all the way down the call stack.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Making more error messages conform to the new pattern is an exercise
left to the reader (or maybe the writer if I find time to get back to
it).
builtin/update-ref.c | 24
t/t1400-update-ref.sh | 32
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 0a81a11..ac41635 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -180,17 +180,17 @@ static const char *parse_cmd_update(struct strbuf *input,
const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(update line missing ref);
+ die(update: missing ref);
if (parse_next_sha1(input, next, update-new_sha1,
update, update-ref_name, 0))
- die(update %s missing newvalue, update-ref_name);
+ die(update %s: missing newvalue, update-ref_name);
update-have_old = !parse_next_sha1(input, next, update-old_sha1,
update, update-ref_name, 1);
if (*next != line_termination)
- die(update %s has extra input: %s, update-ref_name, next);
+ die(update %s: extra input: %s, update-ref_name, next);
return next;
}
@@ -203,17 +203,17 @@ static const char *parse_cmd_create(struct strbuf *input,
const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(create line missing ref);
+ die(create: missing ref);
if (parse_next_sha1(input, next, update-new_sha1,
create, update-ref_name, 0))
- die(create %s missing newvalue, update-ref_name);
+ die(create %s: missing newvalue, update-ref_name);
if (is_null_sha1(update-new_sha1))
- die(create %s given zero newvalue, update-ref_name);
+ die(create %s: zero newvalue, update-ref_name);
if (*next != line_termination)
- die(create %s has extra input: %s, update-ref_name, next);
+ die(create %s: extra input: %s, update-ref_name, next);
return next;
}
@@ -226,19 +226,19 @@ static const char *parse_cmd_delete(struct strbuf *input,
const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(delete line missing ref);
+ die(delete: missing ref);
if (parse_next_sha1(input, next, update-old_sha1,
delete, update-ref_name, 1)) {
update-have_old = 0;
} else {
if (is_null_sha1(update-old_sha1))
- die(delete %s given zero oldvalue,
update-ref_name);
+ die(delete %s: zero oldvalue, update-ref_name);
update-have_old = 1;
}
if (*next != line_termination)
- die(delete %s has extra input: %s, update-ref_name, next);
+ die(delete %s: extra input: %s, update-ref_name, next);
return next;
}
@@ -251,7 +251,7 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(verify line missing ref);
+ die(verify: missing ref);
if (parse_next_sha1(input, next, update-old_sha1,
verify, update-ref_name, 1)) {
@@ -262,7 +262,7 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
}
if (*next != line_termination)
- die(verify %s has extra input: %s, update-ref_name, next);
+ die(verify %s: extra input: %s, update-ref_name, next);
return next;
}
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e9a0103..3cc5c66 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -371,7 +371,7 @@ test_expect_success 'stdin fails on junk after quoted
argument' '
test_expect_success 'stdin fails create with no ref' '
echo create stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: create line missing ref err
+ grep fatal: create: missing ref err
'
test_expect_success 'stdin fails create with bad ref name' '
@@ -383,19 +383,19 @@ test_expect_success 'stdin fails create with bad ref
name' '
test_expect_success 'stdin fails create with no new value' '
echo create $a stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal:
[PATCH 25/26] commit_ref_transaction(): Also free the ref_transaction
Change commit_ref_transaction() to also free the associated data, to
absolve the caller from having to do it.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/update-ref.c | 1 -
refs.c | 1 +
refs.h | 11 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index ffed061..b33288c 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -346,7 +346,6 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
update_refs_stdin();
ret = commit_ref_transaction(transaction, msg,
UPDATE_REFS_DIE_ON_ERR);
- free_ref_transaction(transaction);
return ret;
}
diff --git a/refs.c b/refs.c
index 1fd38b0..d83fc7b 100644
--- a/refs.c
+++ b/refs.c
@@ -3471,6 +3471,7 @@ cleanup:
unlock_ref(updates[i]-lock);
free(updates);
free(delnames);
+ free_ref_transaction(transaction);
return ret;
}
diff --git a/refs.h b/refs.h
index cc24213..754894b 100644
--- a/refs.h
+++ b/refs.h
@@ -210,14 +210,15 @@ enum action_on_err {
/*
* Allocate and initialize a ref_transaction object. The object must
- * be freed by calling free_ref_transaction().
+ * be freed by calling commit_ref_transaction() or
+ * free_ref_transaction().
*/
struct ref_transaction *create_ref_transaction(void);
/*
- * Free a ref_transaction and all associated data. This function does
- * not commit the transaction; that must be done first (if desired) by
- * calling commit_ref_transaction().
+ * Free a ref_transaction and all associated data. This function
+ * should be called to free a ref_transaction that will not be
+ * committed.
*/
void free_ref_transaction(struct ref_transaction *transaction);
@@ -264,7 +265,7 @@ void queue_delete_ref(struct ref_transaction *transaction,
const char *refname,
/*
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
- * problem. The transaction is unmodified by this function.
+ * problem. The transaction is freed by this function.
*/
int commit_ref_transaction(struct ref_transaction *transaction,
const char *msg, enum action_on_err onerr);
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 24/26] struct ref_update: Add type field
This is temporary space for commit_ref_transaction()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 73aec88..1fd38b0 100644
--- a/refs.c
+++ b/refs.c
@@ -3279,6 +3279,7 @@ struct ref_update {
int flags; /* REF_NODEREF? */
int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
struct ref_lock *lock;
+ int type;
const char refname[FLEX_ARRAY];
};
@@ -3402,7 +3403,6 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
{
int ret = 0, delnum = 0, i;
struct ref_update **updates;
- int *types;
const char **delnames;
int n = transaction-nr;
@@ -3411,7 +3411,6 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
/* Allocate work space */
updates = xmalloc(sizeof(*updates) * n);
- types = xmalloc(sizeof(*types) * n);
delnames = xmalloc(sizeof(*delnames) * n);
/* Copy, sort, and reject duplicate refs */
@@ -3429,7 +3428,7 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
(update-have_old ?
update-old_sha1 : NULL),
update-flags,
- types[i], onerr);
+ update-type, onerr);
if (!update-lock) {
ret = 1;
goto cleanup;
@@ -3457,7 +3456,7 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
if (update-lock) {
delnames[delnum++] = update-lock-ref_name;
- ret |= delete_ref_loose(update-lock, types[i]);
+ ret |= delete_ref_loose(update-lock, update-type);
}
}
@@ -3471,7 +3470,6 @@ cleanup:
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
free(updates);
- free(types);
free(delnames);
return ret;
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/26] t1400: Pass a legitimate newvalue to update command
This test is trying to test a few ways to delete references using git update-ref -z --stdin. The third line passed in is update SP /refs/heads/c NUL NUL sha1 NUL , which is not a correct way to delete a reference according to the documentation (the new value should be zeros, not empty). Pass zeros instead as the new value to test the code correctly. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t1400-update-ref.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 29391c6..e2f1dfa 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -927,7 +927,7 @@ test_expect_success 'stdin -z update refs fails with wrong old value' ' test_expect_success 'stdin -z delete refs works with packed and loose refs' ' git pack-refs --all git update-ref $c $m~1 - printf $F delete $a $m update $b $Z $m update $c $m~1 stdin + printf $F delete $a $m update $b $Z $m update $c $Z $m~1 stdin git update-ref -z --stdin stdin test_must_fail git rev-parse --verify -q $a test_must_fail git rev-parse --verify -q $b -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] i18n: assure command not corrupted by _() process
Is there any update on this patch?
Le 2014-03-03 09:55, Sandy Carter a écrit :
Separate message from command examples to avoid translation issues
such as a dash being omitted in a translation.
Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
builtin/branch.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index b4d7716..b304da8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -1022,11 +1022,13 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
*/
if (argc == 1 track == BRANCH_TRACK_OVERRIDE
!branch_existed remote_tracking) {
- fprintf(stderr, _(\nIf you wanted to make '%s' track '%s', do
this:\n\n), head, branch-name);
- fprintf(stderr, _(git branch -d %s\n),
branch-name);
- fprintf(stderr, _(git branch --set-upstream-to
%s\n), branch-name);
+ fprintf(stderr, \n);
+ fprintf(stderr, _(If you wanted to make '%s' track '%s', do
this:), head, branch-name);
+ fprintf(stderr, \n\n);
+ fprintf(stderr, git branch -d %s\n, branch-name);
+ fprintf(stderr, git branch --set-upstream-to %s\n,
branch-name);
+ fprintf(stderr, \n);
}
-
} else
usage_with_options(builtin_branch_usage, options);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git merge --date --author
Hi, Le vendredi 07 mars 2014 à 11:43 -0800, Junio C Hamano a écrit : Andreas Schwab sch...@linux-m68k.org writes: Yann Droneaud ydrone...@opteya.com writes: But I'd like to know if there's a specific reason for git merge to not support --date and --author ? It's rather unusual that a merge is performed on behalf of a different author. Yes. Michael's Nobody bothered to implement it is also correct but the reason why nobody bothered to most likely is due to why would you want to lie?. When was Git changed in some kind of TSA agent one has to bribe to get allowed to cross^Wcommit ? Why git lawyer is not implemented ? I want a fair trial ! And before adding to Git a perfect lie detector (how will it able to make the difference between truth and lie ? then, will it be able to make the difference between good and bad ?, oh god, no !), I would prefer to have it detect bugs before one could commit instead. You seems to think I'm lying, but I'm not a liar: I just need to make some arrangements with the history under another identity, as I could be legally bound to. So it may sound like a lie for you, but ultimately, it's the plain truth. So as the tool is not in position to distinguish lie from truth, I'd prefer to not see this concept brought here. If the use case is to rebuild history, you would need to be able to also lie about the committer, so git merge \ --date 2013-12-31 23:59:59 + \ --author Happy New Year happy.new-year@gregorian.calendar \ current-year in such a history-rebuild script would not be sufficient. The script can set necessary environment variables to lie about both author and commiter, though, of course. Thanks for reminding this: I have to use GIT_COMMITER_DATE, GIT_COMMITER_NAME and GIT_COMMITER_EMAIL. As I'm not calling for adding --date and --author, I will continue to use the environment variables: they're good enough for the job. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line 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 21/26] struct ref_update: Store refname as a FLEX_ARRAY.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 5d08cdf..335d0e2 100644
--- a/refs.c
+++ b/refs.c
@@ -3274,11 +3274,11 @@ static int update_ref_write(const char *action, const
char *refname,
* value or to zero to ensure the ref does not exist before update.
*/
struct ref_update {
- const char *refname;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
int flags; /* REF_NODEREF? */
int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
+ const char refname[FLEX_ARRAY];
};
/*
@@ -3301,12 +3301,8 @@ void free_ref_transaction(struct ref_transaction
*transaction)
{
int i;
- for (i = 0; i transaction-nr; i++) {
- struct ref_update *update = transaction-updates[i];
-
- free((char *)update-refname);
- free(update);
- }
+ for (i = 0; i transaction-nr; i++)
+ free(transaction-updates[i]);
free(transaction-updates);
free(transaction);
@@ -3315,9 +3311,10 @@ void free_ref_transaction(struct ref_transaction
*transaction)
static struct ref_update *add_update(struct ref_transaction *transaction,
const char *refname)
{
- struct ref_update *update = xcalloc(1, sizeof(*update));
+ size_t len = strlen(refname);
+ struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
- update-refname = xstrdup(refname);
+ strcpy((char *)update-refname, refname);
ALLOC_GROW(transaction-updates, transaction-nr + 1,
transaction-alloc);
transaction-updates[transaction-nr++] = update;
return update;
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 23/26] struct ref_update: Add a lock member
Now that we manage ref_update objects internally, we can use them to
hold some of the scratch space we need when actually carrying out the
updates. Store the (struct ref_lock *) there.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/refs.c b/refs.c
index ec638e9..73aec88 100644
--- a/refs.c
+++ b/refs.c
@@ -3278,6 +3278,7 @@ struct ref_update {
unsigned char old_sha1[20];
int flags; /* REF_NODEREF? */
int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
+ struct ref_lock *lock;
const char refname[FLEX_ARRAY];
};
@@ -3402,7 +3403,6 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
int ret = 0, delnum = 0, i;
struct ref_update **updates;
int *types;
- struct ref_lock **locks;
const char **delnames;
int n = transaction-nr;
@@ -3412,7 +3412,6 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
/* Allocate work space */
updates = xmalloc(sizeof(*updates) * n);
types = xmalloc(sizeof(*types) * n);
- locks = xcalloc(n, sizeof(*locks));
delnames = xmalloc(sizeof(*delnames) * n);
/* Copy, sort, and reject duplicate refs */
@@ -3426,12 +3425,12 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
- locks[i] = update_ref_lock(update-refname,
- (update-have_old ?
- update-old_sha1 : NULL),
- update-flags,
- types[i], onerr);
- if (!locks[i]) {
+ update-lock = update_ref_lock(update-refname,
+ (update-have_old ?
+ update-old_sha1 : NULL),
+ update-flags,
+ types[i], onerr);
+ if (!update-lock) {
ret = 1;
goto cleanup;
}
@@ -3445,8 +3444,8 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
ret = update_ref_write(msg,
update-refname,
update-new_sha1,
- locks[i], onerr);
- locks[i] = NULL; /* freed by update_ref_write */
+ update-lock, onerr);
+ update-lock = NULL; /* freed by update_ref_write */
if (ret)
goto cleanup;
}
@@ -3454,9 +3453,11 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
/* Perform deletes now that updates are safely completed */
for (i = 0; i n; i++) {
- if (locks[i]) {
- delnames[delnum++] = locks[i]-ref_name;
- ret |= delete_ref_loose(locks[i], types[i]);
+ struct ref_update *update = updates[i];
+
+ if (update-lock) {
+ delnames[delnum++] = update-lock-ref_name;
+ ret |= delete_ref_loose(update-lock, types[i]);
}
}
@@ -3467,11 +3468,10 @@ int commit_ref_transaction(struct ref_transaction
*transaction,
cleanup:
for (i = 0; i n; i++)
- if (locks[i])
- unlock_ref(locks[i]);
+ if (updates[i]-lock)
+ unlock_ref(updates[i]-lock);
free(updates);
free(types);
- free(locks);
free(delnames);
return ret;
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/26] t1400: Add some more tests involving quoted arguments
On Mon, Mar 10, 2014 at 1:46 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Previously there were no good tests of C-quoted arguments. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu FWIW, the first 5 patches seem trivially correct to me. Feel free to add: Reviewed-by: Johan Herland jo...@herland.net -- Johan Herland, jo...@herland.net www.herland.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: question about: Facebook makes Mercurial faster than Git
Am 10.03.2014 12:42, schrieb Dennis Luehring: Am 10.03.2014 12:28, schrieb demerphq: I had the impression, and I would not be surprised if they had the impression that the git development community is relatively unconcerned about performance issues on larger repositories. so the question is if the git community is interested in beeing competive in such large scale scenarios - something what mercurial seems to be now out of the box The hgwatchman site claims (https://bitbucket.org/facebook/hgwatchman) On a real-world repository with over 200,000 files, hg status normally takes over 3 seconds. With hgwatchman it takes under 0.6 seconds. There have been a few performance improvements in git status to support such large repositories. I just re-checked git status performance with the WebKit repo (~200k files): Linux (with core.preloadIndex) git status -uall: 0.620s git status -uno : 0.255s Windows (with core.preloadIndex and core.fscache) git status -uall: 1.006s git status -uno : 0.695s Of course, for more reliable benchmark data, you'd have to compare the same repo on the same platform. But on first glance, it seems that mercurial with hgwatchman extension may be as fast as git is out of the box, not the other way around. This comes at the cost of running a background daemon, which may slow down the entire system. E.g. if the daemon activates whenever the compiler creates a .o file, it will probably slow down build performance. Note that hgwatchman doesn't support Windows, so git is probably much faster there. -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
On Mon, Mar 10, 2014 at 4:00 AM, Michael Haggerty mhag...@alum.mit.edu wrote: I have started working on pluggable ref backends. In this email I would like to share my plans and solicit feedback. Yay! JGit already has pluggable ref backends, so it is good to see this starting in git-core. FWIW the Gerrit Code Review community is interested in this project. * Store references in a SQLite database, to get correct transaction handling. No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. * Reference transactions that can be used across multiple Git commands. Imagine, export GIT_TRANSACTION=$(git transaction begin) trap 'git transaction rollback' ERR git foo ... git bar ... git baz ... if ! git transaction commit then # Transaction failed; all references rolled back else # Transaction succeeded; all references updated atomically fi trap '' ERR unset GIT_TRANSACTION The GIT_TRANSACTION environment variable would tell git to read from the usual references, overridden with any reference changes that have occurred during the transaction, but write any changes (including both old and new values) to the transaction. The command git transaction commit would verify that the old values listed in the transaction still agree with the current values, and then make all of the changes atomically. Yay! Gerrit Code Review really wants to get transactions implemented. So I am very much in favor of trying to improve the situation in git-core. We want not only a transaction over 2+ references in the same repository, but we also want to perform transactions across repositories. Consider a git submodule child and parent being updated at the same time. We really want to update refs/heads/master in both repositories atomically at the central server. Such transactions could also be broadcast to mirrors when they are committed to keep multiple Git repositories in sync. Ooh, this would be very interesting. Git hosters [1] will be likely to take advantage of alternate reference backends pretty easily, because they know which tools touch their repositories and need only update those tools. It is expected that alternate reference backends will be useful for hosters even if they don't become practical for end-users. Alternate reference backends are absolutely useful to large hosters. The loose reference format isn't very scalable. The packed-refs helps, but you can do better. IIRC our android.googlesource.com reference backend uses only 79 bytes per reference on average, including both the name string and the value. This super compact format is easy to hold in RAM for hundreds of busy repositories. For end-users it is important that their repository be readable by all of the tools that they use. So if we want to make a new format a viable option for normal Git users (let alone make it the new default format), some coordination will be needed between all of the commonly-used Git implementations (git-core, libgit2, JGit, and maybe Dulwich, Grit, ...). Whether or not this happens in real life depends on how advantageous the hypothetical new format is to Git users and is beyond the scope of this proposal. It is sad we have this many implementations, but as one of the authors (JGit) I am happy to at least see you are worrying about compatibility with them. -- To unsubscribe from this list: send the line 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: question about: Facebook makes Mercurial faster than Git
On 03/10/2014 01:10 PM, Johan Herland wrote: It should be possible to teach Git to do similar things, and IINM there are (and have previously been) several attempts to do similar things in Git, e.g.: - http://thread.gmane.org/gmane.comp.version-control.git/240339 - http://thread.gmane.org/gmane.comp.version-control.git/217817 I haven't looked closely at these attempts (it is not my scratch to itch), and I don't know if/how they would work on top of Watchman, but in principle I don't see why Git shouldn't be able to leverage Watchman the same way Mercurial does. This touches on the most important thing that we should take to heart from this episode: Of course Facebook could have modified either Git or Mercurial to do what they want. Why did they pick Mercurial? The article seems to claim that they were initially biased towards Git, but they chose Mercurial because its code base is easier to modify. This is a claim that I can easily believe. The two projects are almost exactly the same age. The number of commits in the two projects is similar. Mercurial has had fewer contributors active at any given time over its project lifetime. But let's see how much code is in the main part of Mercurial vs. Git: $ find mercurial hgext \( -name '*.c' -o -name '*.py' \) -print | xargs cat | wc -l 46164 $ cat *.c *.h *.sh *.perl builtin/*.c | wc -l 188530 These are just crude estimates and I hope I got the right directories for Mercurial. But, by these numbers, Git has 4 times as much code as Mercurial. That alone will go a long way to making Git harder to modify. I don't think that Git has anywhere near 4 times the features of Mercurial. Probably most of the difference can be explained by the choice of implementation languages; 94% of the code in these hg directories is Python, whereas 88% of Git's core code is C. How can we make Git easier to hack (short of switching languages)? Here are my suggestions: * Better function docstrings -- don't make developers have to read the whole call stack to find out what a function does, or who owns the memory that is passed around. * More modularity -- more coherent and abstract APIs between different parts of the system, and less pawing around in your neighbor's data structures. * Higher-level abstractions -- make more use of APIs like strbuf and string_list as opposed to handling every malloc() and realloc() by hand. I personally wish that we as a project would be more willing to spend a few extra CPU microseconds to make our code easier to read and modify and more robust. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trust issues with hooks and config files
Julian Brost jul...@0x4a42.net writes: On 07.03.2014 22:04, Jeff King wrote: If you want to work on it, I think it's an interesting area. But any development would need to think about the transition plan for existing sites that will be broken. I can understand the problem with backward compatibility but in my opinion the default behavior should definitely be to ignore untrusted config files and hooks as it would otherwise only protect users that are already aware of the issue anyways and manually enable this option. Are there any plans for some major release in the future that would allow introducing backward incompatible changes? Git 2.0 has been in the planning for quite some months, and I am inclined to merge these topics prepared for that release to 'master' during this cycle. Anything new like this one is way too late for it, but that does not mean we can never do 3.0 in the future. Perhaps going this way might be possible: * Introduce a configuration that is read only from $HOME/.gitconfig (or its xdg equivalent) to enable or disable the unsafe behaviour. By default (i.e. when the above configuration is not set), allow unsafe read; when instructed by the above configuration to forbid unsafe read, ignore configuration files that are not owned by the owner of the process. People can toggle the unsafe read to experiment with the above (~gitdaemon/.gitconfig can perhaps be used to restrict the daemon access) Keep it that way for a few releases. * After a few releases, start warning people who do not have the unsafe option in their $HOME/.gitconfig about a future default change, to force them to explicitly set it. Keep it that way for a few releases. * Flip the default, perhaps still keeping the warning on the flipped default to help people who have not been following along. Keep it that way for a few releases. * Then finally remove the warning. A release cycle usually last 10-12 weeks on average. -- To unsubscribe from this list: send the line 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 v3] upload-pack: send shallow info over stdin to pack-objects
Duy Nguyen pclo...@gmail.com writes: On Sat, Mar 8, 2014 at 1:27 AM, Junio C Hamano gits...@pobox.com wrote: On the receive-pack side, the comment at the bottom of preprare_shallow_update() makes it clear that, if we wanted to use hooks, we cannot avoid having the proposed new shallow-file in a temporary file, which is unfortunate. Do we have a similar issue on hooks on the upload-pack side? No. I don't think we have hooks on upload-pack. The question was not only about do we happen to work OK with the current code? but about are we closing the door for the future? If we ever need to add hooks to upload-pack, with the updated approach, its operation will not be affected by the temporary shallow file tailored for this specific customer. Which I think is a good thing in general. But at the same time, it means that its operation cannot be customized for the specific customer, taking into account what they lack (which can be gleaned by looking at the temporary shallow information). I do think that it is not a big downside, but that is merely my knee-jerk reaction. When upload-pack learns about hooks, I think we'll need to go back with --shallow-file, perhaps we a secure temporary place to write in. I don't see another way out. Not really sure why upload-pack needs customization though. The only case I can think of is to prevent most users from fetching certain objects, but that does not sound realistic.. I was more thinking along the lines of logging. But you are right that we can easily revisit --shallow-file interface later. Of course.. So what should we do with this? Go with this no temp file approach and deal with hooks when they come, or prepare now and introduce a secure temp. area? I was rather hoping that we did not have to worry about temporary files. This patch solves the issue for the service people would expect to be read-only the most, and it is a good step in the right direction. So let's follow through with the approach and not worry about secure temporary for 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 v7 00/11] Add interpret-trailers builtin
Øystein Walle oys...@gmail.com writes: Junio C Hamano gitster at pobox.com writes: ... is easier to read and maintain if written like so (with using HT properly---our MUAs may damage it and turn the indentation into spaces): ... sed -e s/ Z$/ / expect -\EOF Fixes: Z Acked-by= Z Reviewed-by: Z Signed-off-by: Z EOF ... How about: printf '%s: \n' Fixes Acked-by Reviewed-by Signed-off-by expect Not really. This solution scores high marks in both readability and maintainability in my mind. I actually considered that approach while I was writing the message you responded to, but discarded it because it forces us to commit to the view that we forsee no need to test an output that does not end with a trailing whitespace. And I do not think that is a limitation we want to place on this test. -- To unsubscribe from this list: send the line 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] Documentation: Say that submodule clones use a separate gitdirs.
Andrew Keller and...@kellerfarm.com writes: On Mar 7, 2014, at 7:50 PM, Henri GEIST wrote: ... To give one of my project to someone else I have copied it on a USB key. By a simple drag and drop with the mouse. And I am quite sure I am not alone doing this way. I have done those kind of things lot of time without any problem. But that day 'the_project' happened to be a submodule cloned by 'git submodule update' then on the USB key the $GIT_DIR of 'the_project' was missing. If 'man git-submodule' have made me aware of the particularities of submodules clone I had write in a terminal: git clone the_project /media/usb/the_project Or at least I had understand what happened quicker. I have nothing against also adding something in repository-layout but I am pretty sure normal users never read repository-layout as it is not a command they use. And it is not mentioned in most tutorials. How about something like this: The git directory of a submodule lives inside the git directory of the parent repository instead of within the working directory. I'm not sure where to put it, though. This is not limited to submodules. There are multiple lower-level mechanisms for a $path/.git to borrow the repository data from elsewhere outside of $path and a cloned submodule uses only one of them. For any such $path, cp -R $path $otherplace will result in an $otherplace that does not work as a Git repository in exactly the same way, whether it happens to be a submodule checkout or not. That is why I suggested to enhance description on a more general part of the documentation that covers what a Git repository is. -- To unsubscribe from this list: send the line 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: [GSoC][PATCH v2] use strchrnul() in place of strchr() and strlen()
Rohit Mani rohit.m...@outlook.com writes: Avoid scanning strings twice, once with strchr() and then with strlen(), by using strchrnul(). Thanks. The patch looks good. -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
On 10.03.2014, at 15:30, Shawn Pearce spea...@spearce.org wrote: On Mon, Mar 10, 2014 at 4:00 AM, Michael Haggerty mhag...@alum.mit.edu wrote: I have started working on pluggable ref backends. In this email I would like to share my plans and solicit feedback. Yay! Yay, too! JGit already has pluggable ref backends, so it is good to see this starting in git-core. FWIW the Gerrit Code Review community is interested in this project. * Store references in a SQLite database, to get correct transaction handling. No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. I understood this as an example (indeed, it is listed under Wile new ideas), not a proposal to put this into the git core. It might be an interesting experiment in any case, and if the proposed modularity is truly achieved, it could (if there was any interest in it, that is) be implemented in an external 3rd party project. Anyway, I am quite excited about this project. Usually, I am quite skeptical about such large scope ideas (Yeah, cool idea, but who will pull it off, and with which resources?). But this one seems to have a good chance of being implemented gradually and inside the main repository, with the help of feature flags. Thus, I am looking forward to Michael's announced initial patch series. I feel that I don't know enough yet about git overall to be of much help on my own at this point. But perhaps over time some mini- or micro-projects pop up were others can help (e.g. adapt these 50 tests to work with the 'quagga' ref); if they are pointed out (assuming that doing so isn't more work than just addressing them yourself ;-), I am willing to help out. Cheers, Max signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [RFC/WIP] Pluggable reference backends
On Mon, Mar 10, 2014 at 07:30:45AM -0700, Shawn Pearce wrote: * Store references in a SQLite database, to get correct transaction handling. No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. That seems like a poor reason not to implement a pluggable feature for git-core. If we implement it, then a site using only git-core can take advantage of it. Sites with JGit cannot, and would use a different pluggable storage mechanism that's supported by both. But if we don't implement, it hurts people using only git-core, and it does not help sites using JGit at all. That's assuming that attention spent on implementing the feature does not take away from implementing some other parallel scheme that does the same thing but does not use SQLite. I don't know what that would be offhand; mapping the ref and reflog into a relational database is pretty simple, and we get a lot of robustness and efficiency benefits for free. We could perhaps have some kind of relational backend could use an ODBC-like abstraction to point to a database. I have no idea if people would want to ever store refs in a real server-backend RDBMS, but I suspect Java has native support for such things. Certainly I think we should aim for compatibility where we can, but if there's not a compatible way to do something, I don't think the limitations of one platform should drag other ones down. And that goes both ways; we had to reimplement disk-compatible EWAH from scratch in C for git-core to have bitmaps, whereas JGit just got to use a ready-made library. I don't think that was a bad thing. People in mixed-implementation environments couldn't use it, but people with JGit-only environments were free to take advantage of it. At any rate, the repository needs to advertise this is the ref storage mechanism I use in the config. We're going to need to bump core.repositoryformatversion for such cases (because an old version of git should not blindly lock and write to a refs/ directory that nobody else is ever going to look at). And I'd suggest with that bump adding in something like core.refstorage, so that an implementation can say foobar ref storage? Never heard of it and barf. Whether it's because that implementation doesn't support foobar, because it's an old version that doesn't understand foobar yet, or because it was simply built without foobar support. -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: [RFC/WIP] Pluggable reference backends
Jeff King p...@peff.net writes: On Mon, Mar 10, 2014 at 07:30:45AM -0700, Shawn Pearce wrote: * Store references in a SQLite database, to get correct transaction handling. No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. That seems like a poor reason not to implement a pluggable feature for git-core. If we implement it, then a site using only git-core can take advantage of it. Sites with JGit cannot, and would use a different pluggable storage mechanism that's supported by both. But if we don't implement, it hurts people using only git-core, and it does not help sites using JGit at all. Of course, the basic premise for this feature is let's assume that our file and/or operating system suck at providing file system functionality at file name granularity. There have been two historically approaches to that problem that are not independent: a) use Linux b) kick Linus. Option b) has been fairly successful over quite a bit of time, but at the current point of time, it has become harder to aim that kick on a single person and/or where it counts. The database approach is an alternative approach based on kicking an alternate set of people, namely database rather than operating system providers, based on the assumption that the former have softer behinds (the backend-based approach) making them more sensitive to kicking. So the database approach is most promising on the what are we going to do if our operating system vendor won't bother with sensible file system performance angle. Which isn't doing total system architecture a favor. Personally, I have little sympathy for helping subpar systems, keeping them on life support while they are in turn trying to squish the better systems. But then it is not me doing the actual work, so this is no more than an idle reflection. -- David Kastrup -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
On Mon, 10 Mar 2014, David Kastrup wrote: Jeff King p...@peff.net writes: On Mon, Mar 10, 2014 at 07:30:45AM -0700, Shawn Pearce wrote: * Store references in a SQLite database, to get correct transaction handling. No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. That seems like a poor reason not to implement a pluggable feature for git-core. If we implement it, then a site using only git-core can take advantage of it. Sites with JGit cannot, and would use a different pluggable storage mechanism that's supported by both. But if we don't implement, it hurts people using only git-core, and it does not help sites using JGit at all. Of course, the basic premise for this feature is let's assume that our file and/or operating system suck at providing file system functionality at file name granularity. There have been two historically approaches to that problem that are not independent: a) use Linux b) kick Linus. As a note, if this is done properly, it could allow for plugins that connect to the underlying storage system (similar to the Facebook Mecurial change) Even for those who don't have the $ storage arrays, there may be other storage specific hacks that can be done to detect that files haven't changed. For example, with btrfs and you compile into a different directory thatn your source, you may be able to detect that things didn't change by the fact that the filesystem didn't have to do a rewrite of the parent node. David Lang -- To unsubscribe from this list: send the line 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 03/26] t1400: Pass a legitimate newvalue to update command
On 03/10/2014 08:46 AM, Michael Haggerty wrote: This test is trying to test a few ways to delete references using git update-ref -z --stdin. The third line passed in is update SP /refs/heads/c NUL NUL sha1 NUL , which is not a correct way to delete a reference according to the documentation (the new value should be zeros, not empty). Pass zeros instead as the new value to test the code correctly. In my original work on this feature, an empty newvalue is allowed. Since newvalue is not optional an empty value can be treated as zero. The relevant documentation is: update:: Set ref to newvalue after verifying oldvalue, if given. Specify a zero newvalue to ensure the ref does not exist ... Use 40 0 or the empty string to specify a zero value, except that with `-z` an empty oldvalue is considered missing. The two together say that newvalue can be the empty string instead of a literal zero. -Brad -- To unsubscribe from this list: send the line 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 13/26] update-ref --stdin: Simplify error messages for missing oldvalues
On 03/10/2014 01:08 PM, Brad King wrote: -die(update %s missing [oldvalue] NUL, update-ref_name); +die(update %s missing oldvalue, update-ref_name); The reason for the original wording is that the oldvalue is indeed optional. This can only occur at end-of-input, and it is actually the *NUL* that is missing because an empty old value can be specified to mean that it it intentionally missing. I see a following patch makes the wording even clearer about unexpected end of input, so ignore my previous review. -Brad -- To unsubscribe from this list: send the line 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 13/26] update-ref --stdin: Simplify error messages for missing oldvalues
On 03/10/2014 08:46 AM, Michael Haggerty wrote: Instead of, for example, fatal: update refs/heads/master missing [oldvalue] NUL emit fatal: update refs/heads/master missing oldvalue [snip] - die(update %s missing [oldvalue] NUL, update-ref_name); + die(update %s missing oldvalue, update-ref_name); The reason for the original wording is that the oldvalue is indeed optional. This can only occur at end-of-input, and it is actually the *NUL* that is missing because an empty old value can be specified to mean that it it intentionally missing. -Brad -- To unsubscribe from this list: send the line 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] clean: respect pathspecs with -d
git-clean uses read_directory to fill in a `struct dir` with
potential hits. However, read_directory does not actually
check against our pathspec. It uses a simplified version
that may turn up false positives. As a result, we need to
check that any hits match our pathspec. We do so reliably
for non-directories. For directories, if -d is not given
we check that the pathspec matched exactly (i.e., we are
even stricter, and require an explicit git clean foo to
clean foo/). But if -d is given, rather than relaxing
the exact match to allow a recursive match, we do not check
the pathspec at all.
This regression was introduced in 113f10f (Make git-clean a
builtin, 2007-11-11).
Signed-off-by: Jeff King p...@peff.net
---
On Mon, Mar 10, 2014 at 11:31:37AM +0100, Robin Pedersen wrote:
I accidentially deleted a directory using git clean. I would think
this is a bug, but I'm not sure. Was using 1.8.1, but upgraded to
1.9.0 just to see if it was still reproducable, and it was.
Definitely a bug, and it dates back quite a while. Thanks for a very
clear bug report.
-- 8 --
builtin/clean.c | 5 +++--
t/t7300-clean.sh | 8
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 114d7bf..31c1488 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -947,14 +947,15 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (pathspec.nr)
matches = dir_path_match(ent, pathspec, 0, NULL);
+ if (pathspec.nr !matches)
+ continue;
+
if (S_ISDIR(st.st_mode)) {
if (remove_directories || (matches == MATCHED_EXACTLY))
{
rel = relative_path(ent-name, prefix, buf);
string_list_append(del_list, rel);
}
} else {
- if (pathspec.nr !matches)
- continue;
rel = relative_path(ent-name, prefix, buf);
string_list_append(del_list, rel);
}
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 710be90..0c602de 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -511,4 +511,12 @@ test_expect_success SANITY 'git clean -d with an
unreadable empty directory' '
! test -d foo
'
+test_expect_success 'git clean -d respects pathspecs' '
+ mkdir foo
+ mkdir foobar
+ git clean -df foobar
+ test_path_is_dir foo
+ test_path_is_missing foobar
+'
+
test_done
--
1.9.0.403.g7a2f4b0
--
To unsubscribe from this list: send the line 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] clean: respect pathspecs with -d
On Mon, Mar 10, 2014 at 01:20:02PM -0400, Jeff King wrote:
On Mon, Mar 10, 2014 at 11:31:37AM +0100, Robin Pedersen wrote:
I accidentially deleted a directory using git clean. I would think
this is a bug, but I'm not sure. Was using 1.8.1, but upgraded to
1.9.0 just to see if it was still reproducable, and it was.
Definitely a bug, and it dates back quite a while. Thanks for a very
clear bug report.
-- 8 --
Whoops, accidentally included a scissors line here that will break
people using git am --scissors to pick up the patch. Here it is
correctly formatted.
-- 8 --
Subject: clean: respect pathspecs with -d
git-clean uses read_directory to fill in a `struct dir` with
potential hits. However, read_directory does not actually
check against our pathspec. It uses a simplified version
that may turn up false positives. As a result, we need to
check that any hits match our pathspec. We do so reliably
for non-directories. For directories, if -d is not given
we check that the pathspec matched exactly (i.e., we are
even stricter, and require an explicit git clean foo to
clean foo/). But if -d is given, rather than relaxing
the exact match to allow a recursive match, we do not check
the pathspec at all.
This regression was introduced in 113f10f (Make git-clean a
builtin, 2007-11-11).
Signed-off-by: Jeff King p...@peff.net
---
builtin/clean.c | 5 +++--
t/t7300-clean.sh | 8
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 114d7bf..31c1488 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -947,14 +947,15 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (pathspec.nr)
matches = dir_path_match(ent, pathspec, 0, NULL);
+ if (pathspec.nr !matches)
+ continue;
+
if (S_ISDIR(st.st_mode)) {
if (remove_directories || (matches == MATCHED_EXACTLY))
{
rel = relative_path(ent-name, prefix, buf);
string_list_append(del_list, rel);
}
} else {
- if (pathspec.nr !matches)
- continue;
rel = relative_path(ent-name, prefix, buf);
string_list_append(del_list, rel);
}
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 710be90..0c602de 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -511,4 +511,12 @@ test_expect_success SANITY 'git clean -d with an
unreadable empty directory' '
! test -d foo
'
+test_expect_success 'git clean -d respects pathspecs' '
+ mkdir foo
+ mkdir foobar
+ git clean -df foobar
+ test_path_is_dir foo
+ test_path_is_missing foobar
+'
+
test_done
--
1.9.0.403.g7a2f4b0
--
To unsubscribe from this list: send the line 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] clean: simplify dir/not-dir logic
On Mon, Mar 10, 2014 at 01:20:02PM -0400, Jeff King wrote:
git-clean uses read_directory to fill in a `struct dir` with
potential hits. However, read_directory does not actually
check against our pathspec. It uses a simplified version
that may turn up false positives. As a result, we need to
check that any hits match our pathspec. We do so reliably
for non-directories. For directories, if -d is not given
we check that the pathspec matched exactly (i.e., we are
even stricter, and require an explicit git clean foo to
clean foo/). But if -d is given, rather than relaxing
the exact match to allow a recursive match, we do not check
the pathspec at all.
This regression was introduced in 113f10f (Make git-clean a
builtin, 2007-11-11).
The code has been cleaned up quite a bit from that original version, and
it was pretty easy to see the discrepancy between the two code paths.
However, if the code were structured like the cleanup patch below, I
think it would have been even easier.
This comes on top of my other patch. So the bug is already fixed, but I
think the end result is more readable.
-- 8 --
When we get a list of paths from read_directory, we further
prune it to create the final list of items to remove. The
code paths for directories and non-directories repeat the
same add to list code.
This patch restructures the code so that we don't repeat
ourselves. Also, by following a if (condition) continue
pattern like the pathspec check above, it makes it more
obvious that the conditional is about excluding directories
under certain circumstances.
Signed-off-by: Jeff King p...@peff.net
---
builtin/clean.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 31c1488..cf76b1f 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -950,15 +950,12 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (pathspec.nr !matches)
continue;
- if (S_ISDIR(st.st_mode)) {
- if (remove_directories || (matches == MATCHED_EXACTLY))
{
- rel = relative_path(ent-name, prefix, buf);
- string_list_append(del_list, rel);
- }
- } else {
- rel = relative_path(ent-name, prefix, buf);
- string_list_append(del_list, rel);
- }
+ if (S_ISDIR(st.st_mode) !remove_directories
+ matches != MATCHED_EXACTLY)
+ continue;
+
+ rel = relative_path(ent-name, prefix, buf);
+ string_list_append(del_list, rel);
}
if (interactive del_list.nr 0)
--
1.9.0.403.g7a2f4b0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Clean up update-refs --stdin and implement ref_transaction
Hi Michael, This is excellent work. I haven't reviewed every line of logic in detail but the changes look correct at a high level. The only exception is that the empty newvalue is supposed to be accepted and treated as zero even in --stdin -z mode. See my response to that individual change. On 03/10/2014 08:46 AM, Michael Haggerty wrote: The new API for dealing with reference transactions is ref_transaction *transaction = create_ref_transaction(); queue_create_ref(transaction, refname, new_sha1, ...); queue_update_ref(transaction, refname, new_sha1, old_sha1, ...); queue_delete_ref(transaction, refname, old_sha1, ...); ... if (commit_ref_transaction(transaction, msg, ...)) die(...); The layout of this API looks good. The name queue is not fully representative of the current behavior. It implies that the order is meaningful but we currently allow at most one update to a ref and sort them by refname. Does your follow-up work define behavior for multiple updates to one ref? Can it collapse them into a single update after checking internal consistency of the sequence? So most of the commits in this series are actually cleanups in builtin/update-ref.c. I also spend some time making the error messages emitted by that command more uniform. All good cleanups, thanks. Finally, now that refs.c owns the data structures for dealing with transactions, it is possible to make a few simplifications. Yes, it is much nicer to keep the data structures private, especially as it avoids the copy of the transaction made before sorting. Thanks, -Brad -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
Jeff King p...@peff.net writes: On Mon, Mar 10, 2014 at 07:30:45AM -0700, Shawn Pearce wrote: * Store references in a SQLite database, to get correct transaction handling. No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. That seems like a poor reason not to implement a pluggable feature for git-core. If we implement it, then a site using only git-core can take advantage of it. Sites with JGit cannot, and would use a different pluggable storage mechanism that's supported by both. But if we don't implement, it hurts people using only git-core, and it does not help sites using JGit at all. We would need to eventually have at least one backend that we know will play well with different Git implementations that matter (namely, git-core, Jgit and libgit2) before the feature can be widely adopted. The first backend that is used while the plugging-interface is in development can be anything and does not have to be one that eventual ubiquitous one, however; as long as it is something that we do not mind carrying it forever, along with that final reference backend. I take the objection from Shawn only as against making the sqlite that final one. -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
On Mon, Mar 10, 2014 at 10:46:01AM -0700, Junio C Hamano wrote: No to SQLLite in git-core. Using it from JGit requires building SQLLite and a JNI wrapper, which makes JGit significantly less portable. I know SQLLite is pretty amazing, but implementing compatibility with it from JGit will be a big nightmare for us. That seems like a poor reason not to implement a pluggable feature for git-core. If we implement it, then a site using only git-core can take advantage of it. Sites with JGit cannot, and would use a different pluggable storage mechanism that's supported by both. But if we don't implement, it hurts people using only git-core, and it does not help sites using JGit at all. We would need to eventually have at least one backend that we know will play well with different Git implementations that matter (namely, git-core, Jgit and libgit2) before the feature can be widely adopted. I assumed that the current refs/ and logs/ code, massaged into pluggable backend form, would be the first such. And I wouldn't be surprised to see some iteration on that once it is easier to move from scheme to scheme (e.g., to use some encoding of the names on the filesystem to avoid D/F conflicts, and thus allow reflogs for deleted refs). The first backend that is used while the plugging-interface is in development can be anything and does not have to be one that eventual ubiquitous one, however; as long as it is something that we do not mind carrying it forever, along with that final reference backend. I take the objection from Shawn only as against making the sqlite that final one. Sure, I'd agree with that. I'd think something like an sqlite interface would be mainly of interest to people running busy servers. I don't know that it would make a good default. -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: question about: Facebook makes Mercurial faster than Git
On Mon, 10 Mar 2014, Ondřej Bílka wrote: On Mon, Mar 10, 2014 at 03:13:45AM -0700, David Lang wrote: On Mon, 10 Mar 2014, Dennis Luehring wrote: according to these blog posts http://www.infoq.com/news/2014/01/facebook-scaling-hg https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/ mercurial can be faster then git but i don't found any reply from the git community if it is a real problem or if there a ongoing (maybe git 2.0) changes to compete better in this case As I understand this, the biggest part of what happened is that Facebook made a tweak to mercurial so that when it needs to know what files have changed in their massive tree, their version asks their special storage array, while git would have to look at it through the filesystem interface (by doing stat calls on the directories and files to see if anything has changed) That is mostly a kernel problem. Long ago there was proposed patch to add a recursive mtime so you could check what subtrees changed. If somebody ressurected that patch it would gave similar boost. btrfs could actually implement this efficiently, but for a lot of other filesysems this could be very expensive. The question is if it could be enough of a win to make it a good choice for people who are doing a heavy git workload as opposed to more generic uses. there's also the issue of managed vs generated files, if you update the mtime all the way up the tree because a source file was compiled and a binary created, that will quickly defeat the value of the recursive mtime. David Lang There are two issues that need to be handled, first if you are concerned about one mtime change doing lot of updates a application needs to mark all directories it is interested on, when we do update we unmark directory and by that we update each directory at most once per application run. Second problem were hard links where probably a best course is keep list of these and stat them separately.
Re: question about: Facebook makes Mercurial faster than Git
On Mon, Mar 10, 2014 at 03:13:45AM -0700, David Lang wrote: On Mon, 10 Mar 2014, Dennis Luehring wrote: according to these blog posts http://www.infoq.com/news/2014/01/facebook-scaling-hg https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/ mercurial can be faster then git but i don't found any reply from the git community if it is a real problem or if there a ongoing (maybe git 2.0) changes to compete better in this case As I understand this, the biggest part of what happened is that Facebook made a tweak to mercurial so that when it needs to know what files have changed in their massive tree, their version asks their special storage array, while git would have to look at it through the filesystem interface (by doing stat calls on the directories and files to see if anything has changed) That is mostly a kernel problem. Long ago there was proposed patch to add a recursive mtime so you could check what subtrees changed. If somebody ressurected that patch it would gave similar boost. There are two issues that need to be handled, first if you are concerned about one mtime change doing lot of updates a application needs to mark all directories it is interested on, when we do update we unmark directory and by that we update each directory at most once per application run. Second problem were hard links where probably a best course is keep list of these and stat them separately. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] merge: fix GIT_EDITOR override for commit hook
Don't set GIT_EDITOR to : when calling prepare-commit-msg hook if the
editor is going to be called (e.g. with merge -e).
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
builtin/merge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 67f312d..b11a528 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -821,7 +821,7 @@ static void prepare_to_commit(struct commit_list
*remoteheads)
if (0 option_edit)
strbuf_commented_addf(msg, _(merge_editor_comment),
comment_line_char);
write_merge_msg(msg);
- if (run_commit_hook(1, get_index_file(), prepare-commit-msg,
+ if (run_commit_hook(0 option_edit, get_index_file(),
prepare-commit-msg,
git_path(MERGE_MSG), merge, NULL))
abort_commit(remoteheads, NULL);
if (0 option_edit) {
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] run-command: mark run_hook_with_custom_index as deprecated
--- run-command.h | 1 + 1 file changed, 1 insertion(+) diff --git a/run-command.h b/run-command.h index 88460f9..3653bfa 100644 --- a/run-command.h +++ b/run-command.h @@ -51,6 +51,7 @@ extern int run_hook_le(const char *const *env, const char *name, ...); extern int run_hook_ve(const char *const *env, const char *name, va_list args); LAST_ARG_MUST_BE_NULL +__attribute__((deprecated)) extern int run_hook_with_custom_index(const char *index_file, const char *name, ...); #define RUN_COMMAND_NO_STDIN 1 -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] merge hook tests: use 'test_must_fail' instead of '!'
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7505-prepare-commit-msg-hook.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index 1c95652..5531abb 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -154,7 +154,7 @@ test_expect_success 'with failing hook' ' head=`git rev-parse HEAD` echo more file git add file - ! GIT_EDITOR=\\$FAKE_EDITOR\ git commit -c $head + test_must_fail env GIT_EDITOR=\\$FAKE_EDITOR\ git commit -c $head ' @@ -163,7 +163,7 @@ test_expect_success 'with failing hook (--no-verify)' ' head=`git rev-parse HEAD` echo more file git add file - ! GIT_EDITOR=\\$FAKE_EDITOR\ git commit --no-verify -c $head + test_must_fail env GIT_EDITOR=\\$FAKE_EDITOR\ git commit --no-verify -c $head ' -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 0/7] fix hunk editing with 'commit -p -m'
This patch fixes the fact that hunk editing with 'commit -p -m' does not work: GIT_EDITOR is set to ':' to indicate to hooks that no editor will be launched, which result in the 'hunk edit' option not launching the editor (and selecting the whole hunk). The fix consists in deferring the GIT_EDITOR override to the hook subprocess, like it's already done for GIT_INDEX_FILE: - rename 'run_hook' to 'run_hook_le' and change the first parameter to the environment to set - add a 'run_hook_ve' variant that take a va_list - add a new 'run_commit_hook' helper (to set both GIT_EDITOR and GIT_INDEX_FILE) - the old 'run_hook' functionality is available as 'run_hook_with_custom_index' (and marked as deprecated in the last optional patch of this series) N.B.: the merge builtin 'prepare-commit-msg' hook handling has also been updated to be consistent; i.e. GIT_EDITOR will not be set to ':' if the '--edit' option is used. Benoit Pierre (7): merge hook tests: fix missing '' in test merge hook tests: use 'test_must_fail' instead of '!' test patch hunk editing with commit -p -m commit: fix patch hunk editing with commit -p -m merge: fix GIT_EDITOR override for commit hook merge hook tests: fix and update tests run-command: mark run_hook_with_custom_index as deprecated builtin/checkout.c | 8 +++ builtin/clone.c| 4 ++-- builtin/commit.c | 35 -- builtin/gc.c | 2 +- builtin/merge.c| 6 +++--- commit.h | 3 +++ run-command.c | 44 +++--- run-command.h | 7 +- t/t7505-prepare-commit-msg-hook.sh | 33 t/t7513-commit_-p_-m_hunk_edit.sh | 34 + 10 files changed, 137 insertions(+), 39 deletions(-) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] merge hook tests: fix and update tests
- update 'no editor' hook test and add 'editor' hook test
- make sure the tree is reset to a clean state after running a test
(using test_when_finished) so later tests are not impacted
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
t/t7505-prepare-commit-msg-hook.sh | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/t/t7505-prepare-commit-msg-hook.sh
b/t/t7505-prepare-commit-msg-hook.sh
index 5531abb..03dce09 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -134,14 +134,26 @@ test_expect_success 'with hook (-c)' '
test_expect_success 'with hook (merge)' '
- head=`git rev-parse HEAD`
- git checkout -b other HEAD@{1}
- echo more file
+ test_when_finished git checkout -f master
+ git checkout -B other HEAD@{1}
+ echo more file
+ git add file
+ git commit -m other
+ git checkout -
+ git merge --no-ff other
+ test `git log -1 --pretty=format:%s` = merge (no editor)
+'
+
+test_expect_success 'with hook and editor (merge)' '
+
+ test_when_finished git checkout -f master
+ git checkout -B other HEAD@{1}
+ echo more file
git add file
git commit -m other
git checkout -
- git merge other
- test `git log -1 --pretty=format:%s` = merge
+ env GIT_EDITOR=\\$FAKE_EDITOR\ git merge --no-ff -e other
+ test `git log -1 --pretty=format:%s` = merge
'
cat $HOOK 'EOF'
@@ -151,6 +163,7 @@ EOF
test_expect_success 'with failing hook' '
+ test_when_finished git checkout -f master
head=`git rev-parse HEAD`
echo more file
git add file
@@ -160,6 +173,7 @@ test_expect_success 'with failing hook' '
test_expect_success 'with failing hook (--no-verify)' '
+ test_when_finished git checkout -f master
head=`git rev-parse HEAD`
echo more file
git add file
@@ -169,6 +183,7 @@ test_expect_success 'with failing hook (--no-verify)' '
test_expect_success 'with failing hook (merge)' '
+ test_when_finished git checkout -f master
git checkout -B other HEAD@{1}
echo more file
git add file
@@ -178,7 +193,7 @@ test_expect_success 'with failing hook (merge)' '
exit 1
EOF
git checkout -
- test_must_fail git merge other
+ test_must_fail git merge --no-ff other
'
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] test patch hunk editing with commit -p -m
Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh new file mode 100755 index 000..994939a --- /dev/null +++ b/t/t7513-commit_-p_-m_hunk_edit.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 + echo line3 file + cat expect -\EOF + diff --git a/file b/file + index a29bdeb..c0d0fb4 100644 + --- a/file + +++ b/file + @@ -1 +1,2 @@ +line1 + +line2 + EOF +' + +test_expect_failure 'edit hunk commit -p -m message' ' + echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 file + git diff HEAD^ HEAD actual + test_cmp expect actual +' + +test_done -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] commit: fix patch hunk editing with commit -p -m
Don't change git environment: move the GIT_EDITOR=: override to the
hook command subprocess, like it's already done for GIT_INDEX_FILE.
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
builtin/checkout.c| 8 +++
builtin/clone.c | 4 ++--
builtin/commit.c | 35 ---
builtin/gc.c | 2 +-
builtin/merge.c | 6 +++---
commit.h | 3 +++
run-command.c | 44 ---
run-command.h | 6 +-
t/t7513-commit_-p_-m_hunk_edit.sh | 2 +-
9 files changed, 79 insertions(+), 31 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837..da423b2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -53,10 +53,10 @@ struct checkout_opts {
static int post_checkout_hook(struct commit *old, struct commit *new,
int changed)
{
- return run_hook(NULL, post-checkout,
- sha1_to_hex(old ? old-object.sha1 : null_sha1),
- sha1_to_hex(new ? new-object.sha1 : null_sha1),
- changed ? 1 : 0, NULL);
+return run_hook_le(NULL, post-checkout,
+ sha1_to_hex(old ? old-object.sha1 : null_sha1),
+ sha1_to_hex(new ? new-object.sha1 : null_sha1),
+ changed ? 1 : 0, NULL);
/* new can be NULL when checking out from the index before
a commit exists. */
diff --git a/builtin/clone.c b/builtin/clone.c
index 43e772c..9b3c04d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -660,8 +660,8 @@ static int checkout(void)
commit_locked_index(lock_file))
die(_(unable to write new index file));
- err |= run_hook(NULL, post-checkout, sha1_to_hex(null_sha1),
- sha1_to_hex(sha1), 1, NULL);
+ err |= run_hook_le(NULL, post-checkout, sha1_to_hex(null_sha1),
+ sha1_to_hex(sha1), 1, NULL);
if (!err option_recursive)
err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..baf1fc0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -612,7 +612,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
- if (!no_verify run_hook(index_file, pre-commit, NULL))
+ if (!no_verify run_commit_hook(use_editor, index_file, pre-commit,
NULL))
return 0;
if (squash_message) {
@@ -866,8 +866,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
return 0;
}
- if (run_hook(index_file, prepare-commit-msg,
-git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+ if (run_commit_hook(use_editor, index_file, prepare-commit-msg,
+ git_path(commit_editmsg), hook_arg1, hook_arg2,
NULL))
return 0;
if (use_editor) {
@@ -883,7 +883,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
}
if (!no_verify
- run_hook(index_file, commit-msg, git_path(commit_editmsg), NULL))
{
+ run_commit_hook(use_editor, index_file, commit-msg,
git_path(commit_editmsg), NULL)) {
return 0;
}
@@ -1067,8 +1067,6 @@ static int parse_and_validate_options(int argc, const
char *argv[],
use_editor = 0;
if (0 = edit_flag)
use_editor = edit_flag;
- if (!use_editor)
- setenv(GIT_EDITOR, :, 1);
/* Sanity check options */
if (amend !current_head)
@@ -1445,6 +1443,29 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
return finish_command(proc);
}
+int run_commit_hook(int editor_is_used, const char *index_file, const char
*name, ...)
+{
+ const char *hook_env[3] = { NULL };
+ char index[PATH_MAX];
+ va_list args;
+ int ret;
+
+ snprintf(index, sizeof(index), GIT_INDEX_FILE=%s, index_file);
+ hook_env[0] = index;
+
+ /*
+* Let the hook know that no editor will be launched.
+*/
+ if (!editor_is_used)
+ hook_env[1] = GIT_EDITOR=:;
+
+ va_start(args, name);
+ ret = run_hook_ve(hook_env, name, args);
+ va_end(args);
+
+ return ret;
+}
+
int cmd_commit(int argc, const char **argv, const char *prefix)
{
static struct wt_status s;
@@ -1669,7 +1690,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
not exceeded, and then \git reset HEAD\ to recover.));
rerere(0);
- run_hook(get_index_file(), post-commit, NULL);
+ run_commit_hook(use_editor,
[PATCH 1/7] merge hook tests: fix missing '' in test
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7505-prepare-commit-msg-hook.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index 3573751..1c95652 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -174,7 +174,7 @@ test_expect_success 'with failing hook (merge)' ' git add file rm -f $HOOK git commit -m other - write_script $HOOK -EOF + write_script $HOOK -EOF exit 1 EOF git checkout - -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Documentation: Say that submodule clones use a separate gitdirs.
Le lundi 10 mars 2014 à 08:31 -0700, Junio C Hamano a écrit : Andrew Keller and...@kellerfarm.com writes: On Mar 7, 2014, at 7:50 PM, Henri GEIST wrote: ... To give one of my project to someone else I have copied it on a USB key. By a simple drag and drop with the mouse. And I am quite sure I am not alone doing this way. I have done those kind of things lot of time without any problem. But that day 'the_project' happened to be a submodule cloned by 'git submodule update' then on the USB key the $GIT_DIR of 'the_project' was missing. If 'man git-submodule' have made me aware of the particularities of submodules clone I had write in a terminal: git clone the_project /media/usb/the_project Or at least I had understand what happened quicker. I have nothing against also adding something in repository-layout but I am pretty sure normal users never read repository-layout as it is not a command they use. And it is not mentioned in most tutorials. How about something like this: The git directory of a submodule lives inside the git directory of the parent repository instead of within the working directory. I'm not sure where to put it, though. This is not limited to submodules. There are multiple lower-level mechanisms for a $path/.git to borrow the repository data from elsewhere outside of $path and a cloned submodule uses only one of them. For any such $path, cp -R $path $otherplace will result in an $otherplace that does not work as a Git repository in exactly the same way, whether it happens to be a submodule checkout or not. That is why I suggested to enhance description on a more general part of the documentation that covers what a Git repository is. You are entirely right. My view of the situation was biased by the fact that submodule cloning is the only case I am aware of except when the user manually type '--separate-git-dir' where this problem could occur. If there is some other situation where this can occur as a side effect of a git command it can be good to have the user aware of the list or at least inform them in general case a git repository cannot be copied in a place every body will see. Or place a note in the manpage of every git command which can have this side effect. signature.asc Description: This is a digitally signed message part
[PATCH][GSOC2014] changed logical chain in branch.c to lookup tables
Signed-off-by: TamerTas tamer...@outlook.com
---
branch.c | 31 ---
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..397edd3 100644
--- a/branch.c
+++ b/branch.c
@@ -50,6 +50,9 @@ static int should_setup_rebase(const char *origin)
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
{
const char *shortname = remote + 11;
+ const char *location[] = {local, remote};
+ const char *type[] = {branch, ref};
+
int remote_is_branch = starts_with(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
@@ -77,29 +80,11 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
if (flag BRANCH_CONFIG_VERBOSE) {
- if (remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote branch %s
from %s by rebasing.) :
- _(Branch %s set up to track remote branch %s
from %s.),
- local, shortname, origin);
- else if (remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local branch %s
by rebasing.) :
- _(Branch %s set up to track local branch
%s.),
- local, shortname);
- else if (!remote_is_branch origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track remote ref %s by
rebasing.) :
- _(Branch %s set up to track remote ref %s.),
- local, remote);
- else if (!remote_is_branch !origin)
- printf_ln(rebasing ?
- _(Branch %s set up to track local ref %s by
rebasing.) :
- _(Branch %s set up to track local ref %s.),
- local, remote);
- else
- die(BUG: impossible combination of %d and %p,
- remote_is_branch, origin);
+
+printf_ln(rebasing ?
+ _(Branch %s set up to track %s %s %s by rebasing.) :
+ _(Branch %s set up to track %s %s %s.),
+ local, location[!origin], type[remote_is_branch], remote);
}
}
--
1.7.9.5
--
To unsubscribe from this list: send the line 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][GSOC2014]
I inspected branch.c:install_branch_config() and since the conditionals cover all the possibilities, I decided that making the code table-driven would be much cleaner and shorter. I've ran the tests and they all passed. Reimplementation using git am also didn't have any problems. Please let me know if there is any problems that I missed. I would also love to learn if you think that the code was better before and why it was better. TamerTas (1): changed logical chain in branch.c to lookup tables branch.c | 31 --- 1 file changed, 8 insertions(+), 23 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line 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] status: allow NULL fmt for status_printf/status_vprintf_ln
Useful for calling status_printf only to change/reset the color (and
output an additional '\n' with status_vprintf_ln).
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
wt-status.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/wt-status.c b/wt-status.c
index 4e55810..17f63a4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -49,7 +49,8 @@ static void status_vprintf(struct wt_status *s, int at_bol,
const char *color,
struct strbuf linebuf = STRBUF_INIT;
const char *line, *eol;
- strbuf_vaddf(sb, fmt, ap);
+ if (NULL != fmt)
+ strbuf_vaddf(sb, fmt, ap);
if (!sb.len) {
if (s-display_comment_prefix) {
strbuf_addch(sb, comment_line_char);
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] fix status_printf_ln calls zero-length format warnings
Those happens with gcc -Wformat-zero-length. Since passing NULL does not generate a warning (as __attribute__((printf())) does not imply nonull), modify status_printf/status_printf_ln to allow a NULL format and update calls with an empty string. Benoit Pierre (2): status: allow NULL fmt for status_printf/status_vprintf_ln fix status_printf_ln calls zero-length format warnings builtin/commit.c | 2 +- wt-status.c | 23 --- 2 files changed, 13 insertions(+), 12 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] fix status_printf_ln calls zero-length format warnings
Those happens with gcc -Wformat-zero-length.
Change empty strings to NULL now that it's allowed.
Signed-off-by: Benoit Pierre benoit.pie...@gmail.com
---
builtin/commit.c | 2 +-
wt-status.c | 20 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..6d19439 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -805,7 +805,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
committer_ident.buf);
if (ident_shown)
- status_printf_ln(s, GIT_COLOR_NORMAL, );
+ status_printf_ln(s, GIT_COLOR_NORMAL, NULL);
saved_color_setting = s-use_color;
s-use_color = 0;
diff --git a/wt-status.c b/wt-status.c
index 17f63a4..d9b1d00 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -189,7 +189,7 @@ static void wt_status_print_unmerged_header(struct
wt_status *s)
} else {
status_printf_ln(s, c, _( (use \git add/rm file...\ as
appropriate to mark resolution)));
}
- status_printf_ln(s, c, );
+ status_printf_ln(s, c, NULL);
}
static void wt_status_print_cached_header(struct wt_status *s)
@@ -205,7 +205,7 @@ static void wt_status_print_cached_header(struct wt_status
*s)
status_printf_ln(s, c, _( (use \git reset %s file...\ to
unstage)), s-reference);
else
status_printf_ln(s, c, _( (use \git rm --cached file...\
to unstage)));
- status_printf_ln(s, c, );
+ status_printf_ln(s, c, NULL);
}
static void wt_status_print_dirty_header(struct wt_status *s,
@@ -224,7 +224,7 @@ static void wt_status_print_dirty_header(struct wt_status
*s,
status_printf_ln(s, c, _( (use \git checkout -- file...\ to
discard changes in working directory)));
if (has_dirty_submodules)
status_printf_ln(s, c, _( (commit or discard the untracked or
modified content in submodules)));
- status_printf_ln(s, c, );
+ status_printf_ln(s, c, NULL);
}
static void wt_status_print_other_header(struct wt_status *s,
@@ -236,12 +236,12 @@ static void wt_status_print_other_header(struct wt_status
*s,
if (!s-hints)
return;
status_printf_ln(s, c, _( (use \git %s file...\ to include in
what will be committed)), how);
- status_printf_ln(s, c, );
+ status_printf_ln(s, c, NULL);
}
static void wt_status_print_trailer(struct wt_status *s)
{
- status_printf_ln(s, color(WT_STATUS_HEADER, s), );
+ status_printf_ln(s, color(WT_STATUS_HEADER, s), NULL);
}
#define quote_path quote_path_relative
@@ -794,7 +794,7 @@ static void wt_status_print_other(struct wt_status *s,
string_list_clear(output, 0);
strbuf_release(buf);
conclude:
- status_printf_ln(s, GIT_COLOR_NORMAL, );
+ status_printf_ln(s, GIT_COLOR_NORMAL, NULL);
}
void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
@@ -1292,7 +1292,7 @@ void wt_status_print(struct wt_status *s)
on_what = _(Not currently on any branch.);
}
}
- status_printf(s, color(WT_STATUS_HEADER, s), );
+ status_printf(s, color(WT_STATUS_HEADER, s), NULL);
status_printf_more(s, branch_status_color, %s, on_what);
status_printf_more(s, branch_color, %s\n, branch_name);
if (!s-is_initial)
@@ -1305,9 +1305,9 @@ void wt_status_print(struct wt_status *s)
free(state.detached_from);
if (s-is_initial) {
- status_printf_ln(s, color(WT_STATUS_HEADER, s), );
+ status_printf_ln(s, color(WT_STATUS_HEADER, s), NULL);
status_printf_ln(s, color(WT_STATUS_HEADER, s), _(Initial
commit));
- status_printf_ln(s, color(WT_STATUS_HEADER, s), );
+ status_printf_ln(s, color(WT_STATUS_HEADER, s), NULL);
}
wt_status_print_updated(s);
@@ -1324,7 +1324,7 @@ void wt_status_print(struct wt_status *s)
if (s-show_ignored_files)
wt_status_print_other(s, s-ignored, _(Ignored
files), add -f);
if (advice_status_u_option 2000 s-untracked_in_ms) {
- status_printf_ln(s, GIT_COLOR_NORMAL, );
+ status_printf_ln(s, GIT_COLOR_NORMAL, NULL);
status_printf_ln(s, GIT_COLOR_NORMAL,
_(It took %.2f seconds to enumerate
untracked files. 'status -uno'\n
may speed it up, but you have to be
careful not to forget to add\n
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Documentation: Say that submodule clones use a separate gitdirs.
Henri GEIST geist.he...@laposte.net writes: Le lundi 10 mars 2014 à 08:31 -0700, Junio C Hamano a écrit : ... This is not limited to submodules. There are multiple lower-level mechanisms for a $path/.git to borrow the repository data from elsewhere outside of $path and a cloned submodule uses only one of them. For any such $path, cp -R $path $otherplace will result in an $otherplace that does not work as a Git repository in exactly the same way, whether it happens to be a submodule checkout or not. That is why I suggested to enhance description on a more general part of the documentation that covers what a Git repository is. ... If there is some other situation where this can occur as a side effect of a git command it can be good to have the user aware of the list or at least inform them in general case a git repository cannot be copied in a place every body will see. Or place a note in the manpage of every git command which can have this side effect. I think we should do two things: - In the repository format document, state that there are two lower-level mechanisms and a half that lets a repository borrow from somewhere else, and cp -R of the former will not result in a complete, usable repository, and who employs these mechanisms. * Redirecting the entire .git via the textual gitfile mechanism, which is used by clone --separate-git-dir and submodule; * Borrowing .git/objects read-only from elsewhere, overlaying our own, via .git/objects/info/alternates, which is used by clone --reference; * Redirecting some paths in .git to another, via git workdir; soon to be replaced with .git/commondir mechansim. - In each of the documentation page on an end-user facing command that will be mentioned above, add See also reference to the above description in the repository format document. We could elaborate the See also as something like If you use this feature, do not think you can cp -R the repository to elsewhere and expect the copy to function; see also ..., if we wanted to. -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
On Mon, Mar 10, 2014 at 05:14:02PM +0100, David Kastrup wrote: [storing refs in sqlite] Of course, the basic premise for this feature is let's assume that our file and/or operating system suck at providing file system functionality at file name granularity. There have been two historically approaches to that problem that are not independent: a) use Linux b) kick Linus. You didn't define suck here, but there are a number of issues with the current ref storage system. Here is a sampling: 1. The filesystem does not present an atomic view of the data (e.g., you read a, then while you are reading b, somebody else updates a; your view is one that never existed at any point in time). 2. Using the filesystem creates D/F conflicts between branches foo and foo/bar. Because this name is a primary key even for the reflogs, we cannot easily persist reflogs after the ref is removed. 3. We use packed-refs in conjunction with loose ones to achieve reasonable performance when there are a large number of refs. The scheme for determining the current value of a ref is complicated and error-prone (we had several race conditions that caused real data loss). Those things can be solved through better support from the filesystem. But they were also solved decades ago by relational databases. I generally avoid databases where possible. They lock your data up in a binary format that you can't easily touch with standard unix tools. And they introduce complexity and opportunity for bugs. But they are also a proven technology for solving exactly the sorts of problems that some people are having with git. I do not see a reason not to consider them as an option for a pluggable refs system. But I also do not see a reason to inflict their costs on people who do not have those problems. And that is why Michael's email is about _pluggable_ ref backends, and not let's convert git to sqlite. I do not even know if sqlite is going to end up as an interesting option. But it will be nice to be able to experiment with it easily due to git's ref code becoming more modular. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase: new option to post edit a squashed or fixed up commit
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
After squashing or fixing up, you may want to have a final look at the
commit, edit some more if needed or even do some testing. --postedit
enables that. This is (to me) a paranoid mode so either I enable it
for all squashes and fixups, or none. Hence a new option, not new todo
commands that give finer selection.
If we were to adopt Michael's (?) idea of allowing flags to each
insn in the insn sheet, would this restriction be easily lifted?
That is, instead of saying squash, you say squash --stop or
something.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a1adae8..42061fc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -571,6 +571,11 @@ do_next () {
;;
esac
record_in_rewritten $sha1
+ if test -n $postedit
+ then
+ warn Stopped at $sha1... $rest
+ exit_with_patch $sha1 0
+ fi
;;
I would have expected that any new code would stop only at the last
squash (or fixup) in a series of squashes, but this appears to stop
even at an intermediate squashed result, which will not appear in
the final history. Am I misreading the patch (or misunderstanding
the intent of the patch)?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] fix status_printf_ln calls zero-length format warnings
On Mon, Mar 10, 2014 at 08:27:25PM +0100, Benoit Pierre wrote: Those happens with gcc -Wformat-zero-length. Since passing NULL does not generate a warning (as __attribute__((printf())) does not imply nonull), modify status_printf/status_printf_ln to allow a NULL format and update calls with an empty string. Most of us who compile with -Wall decided a while ago to use -Wno-format-zero-length, because it really is a silly complaint (it assumes there are no side effects of the function besides printing the format string, which is obviously not true in this case). It would be nice to compile out of the box with -Wall -Werror, and I think your solution looks relatively clean. But I am slightly concerned about the assumption that it is OK to pass a NULL fmt parameter to a function marked with __attribute__((format)). It certainly seems to be the case now, and I do not know of any plans for that to change, but it seems like a potentially obvious thing for the compiler to check. I dunno; perhaps it has already been considered and rejected by gcc folks to allow the exact escape hatch we are using here. -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] rev-parse --parseopt: option argument name hints
Ilya Bobyr ilya.bo...@gmail.com writes: On 3/4/2014 11:22 AM, Junio C Hamano wrote: Ilya Bobyr ilya.bo...@gmail.com writes: @@ -333,6 +339,7 @@ h,helpshow the help foo some nifty option --foo bar= some cool option --bar with an argument +baz=arg another cool option --baz with an argument named arg It probably is better not to have named arg at the end here, as that gives an apparent-but-false contradiction with the Angle brackets are added *automatically* and confuse readers. At least, it confused _this_ reader. I am not sure I understand what is confusing here. But I removed the named arg part. After reading Angle brackets are automatically given, seeing that the argument description has manually spelled arg gave me Huh?. Without named arg there is no such confusion. If there would be an example, I think, it is easy to understand how it works. Of course. That is why I suggested to do without named arg part---I didn't mean to suggest not to add the example. I also think that you can demonstrate something other than '=' (whose usage is already shown with bar= above) here as well, but I think we can go either way. After the eval in the existing example to parse the $@ argument list in this part of the documentation, it may be a good idea to say something like: The above command, when $@ is --help, produces the following help output: ... sample output here ... to show the actual output. That way, we can illustrate how input baz?arg description of baz is turned into --baz[=arg] output clearly (yes, I am suggesting to use '?' in the new example, not '=' whose usage is already shown in the existing example). Documentation on the whole argument parsing is quite short, so, I though, adding an example just to show how usage is generated would look like I am trying to make this feature look important than it is :) You already are by saying the Angle brackets are automatic, aren't you? At the same time the target structure that holds the option description calls this string argh. OK, that is fine, then (I'd prefer a field name not to sound like arrrgh, but that is an entirely different topic). I've renamed it to end. It is used to remember possible end of the argument name in just one paragraph of code. Sounds good. -- To unsubscribe from this list: send the line 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/WIP] Pluggable reference backends
Jeff King p...@peff.net writes: On Mon, Mar 10, 2014 at 05:14:02PM +0100, David Kastrup wrote: [storing refs in sqlite] Of course, the basic premise for this feature is let's assume that our file and/or operating system suck at providing file system functionality at file name granularity. There have been two historically approaches to that problem that are not independent: a) use Linux b) kick Linus. You didn't define suck here, but there are a number of issues with the current ref storage system. Here is a sampling: 1. The filesystem does not present an atomic view of the data (e.g., you read a, then while you are reading b, somebody else updates a; your view is one that never existed at any point in time). If there are no system calls suitable for addressing this problem that fundamentally concerns the use of the file system as a file-name addressed data store, I don't see why kick Linus would not apply here. 2. Using the filesystem creates D/F conflicts between branches foo and foo/bar. Because this name is a primary key even for the reflogs, we cannot easily persist reflogs after the ref is removed. That actually sounds more like kick Junio territory (the wonderful times when kick Linus could achieve almost anything are over). To wit: this sounds like a design shortcoming in Git's use of filesystems, not something that is actually inherent in the use of files. 3. We use packed-refs in conjunction with loose ones to achieve reasonable performance when there are a large number of refs. The scheme for determining the current value of a ref is complicated and error-prone (we had several race conditions that caused real data loss). Again, that sounds like we are talking about a scenario that is not a problem of files inherently but rather of Git's ways of managing them. Those things can be solved through better support from the filesystem. But they were also solved decades ago by relational databases. Relational databases that are not implemented on raw storage managed by database servers will still map their operations to file operations. But they are also a proven technology for solving exactly the sorts of problems that some people are having with git. I do not see a reason not to consider them as an option for a pluggable refs system. But I think it would be wrong to try solving 2. above at the database level when its actual problem lies with the reference-filename mapping scheme. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2014, #01; Tue, 4)
Johannes Sixt j.s...@viscovery.net writes: Am 3/5/2014 1:10, schrieb Junio C Hamano: * nd/gitignore-trailing-whitespace (2014-02-10) 2 commits - dir: ignore trailing spaces in exclude patterns - dir: warn about trailing spaces in exclude patterns Warn and then ignore trailing whitespaces in .gitignore files, unless they are quoted for fnmatch(3), e.g. path\ . Will merge to 'next'. The new test does not pass on Windows. Thanks, will mark not to merge to 'master' yet. -- To unsubscribe from this list: send the line 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] clean: respect pathspecs with -d
On Mon, Mar 10, 2014 at 01:22:15PM -0400, Jeff King wrote: +test_expect_success 'git clean -d respects pathspecs' ' + mkdir foo + mkdir foobar + git clean -df foobar + test_path_is_dir foo + test_path_is_missing foobar +' + test_done I think we should also test removing foo, which was also in the original report, to make sure we don't match prefixes, e.g.: test_expect_success 'git clean -d respects pathspecs' ' mkdir foo mkdir foobar git clean -df foo test_path_is_missing foo test_path_is_dir foobar ' Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- To unsubscribe from this list: send the line 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/7] commit: fix patch hunk editing with commit -p -m
On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote: Don't change git environment: move the GIT_EDITOR=: override to the hook command subprocess, like it's already done for GIT_INDEX_FILE. Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- builtin/checkout.c| 8 +++ builtin/clone.c | 4 ++-- builtin/commit.c | 35 --- builtin/gc.c | 2 +- builtin/merge.c | 6 +++--- commit.h | 3 +++ run-command.c | 44 --- run-command.h | 6 +- t/t7513-commit_-p_-m_hunk_edit.sh | 2 +- 9 files changed, 79 insertions(+), 31 deletions(-) This is a lot of change, and in some ways I think it is making things better overall. However, the simplest fix for this is basically to move the setting of GIT_EDITOR down to after we prepare the index. Jun Hao (cc'd) has been preparing a series for this based on the Bloomberg git hackday a few weeks ago, but it hasn't been sent to the list yet. Commits are here: https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing if you care to look. I'm not sure which solution is technically superior, but it's worth considering both. I regret not encouraging Jun to post to the list sooner, as we might have avoided some duplicated effort. But that's a sunk cost, and we should pick up whichever is the best for the project. -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
