Re: how to check for uncommitted/unstaged changes on remote side before pushing
On zo, 2015-11-08 at 22:23 +0100, Marc Haber wrote: > Hi, > > I am trying to abuse git as a code distribution channel and would > like > to be able to trigger redistribution just by git push. [insert obligatory remark about git not being a deployment tool] > The idea is to push to a remote to the branch that is currently > checked out followed by a git reset --hard in the post-receive hook. > I > have already figured out that I need to set receive.denyCurrentBranch > to ignore to be able to push to the currently checked out branch. You'll need a new enough git, so you can set it to updateInstead (and maybe use a push-to-checkout hook). > I am also aware that it is a good idea to git pull before git push > just in case there were local commits on the remote. No, hooks should never pull, merge or do anything that could be interactive. > git reset --hard will unconditionally throw away local uncommitted > changes. I would like to detect this situation on the remote and > abort > the receive progress. But my pre-receive hook does not work as > intended. Here is my code: > > [snip code] > > What is going wrong here? You mention a post-receive hook first, but have written a pre-receive hook. Not sure if that's what you intended (or even if that's what's going wrong). > If my entire approach is wrong, what is the recommended way to > prevent a repository with unstaged or uncommitted changes from being > pushed to? Push-to-checkout is a very simplistic way of deploying and while it works in simple cases, I'd not recommend it. Two safer/saner approaches are: - Have a separate non-bare repo, and make the post-receive hook in a bare repo trigger a fetch+reset in the non-bare one - Use git archive and symlink trickery for even better deploys Questions like this come up in #git all the time, so I wrote up a few more detailed recipes here, including working hooks and config for all three ways of deploying: http://git.seveas.net/simple-deployments-with-git.html -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: how to check for uncommitted/unstaged changes on remote side before pushing
Hi Dennis, thanks for your comments. I appreciate that. On Mon, Nov 09, 2015 at 09:25:00AM +0100, Dennis Kaarsemaker wrote: > On zo, 2015-11-08 at 22:23 +0100, Marc Haber wrote: > > Hi, > > > > I am trying to abuse git as a code distribution channel and would > > like > > to be able to trigger redistribution just by git push. > > [insert obligatory remark about git not being a deployment tool] That's why I said "abuse" ;-) Abusing git here has the advantage that one can save local changes and merge them, which is handy for the task at hand (which is deploying my dotfiles to "my" servers). > > The idea is to push to a remote to the branch that is currently > > checked out followed by a git reset --hard in the post-receive hook. > > I > > have already figured out that I need to set receive.denyCurrentBranch > > to ignore to be able to push to the currently checked out branch. > > You'll need a new enough git, so you can set it to updateInstead (and > maybe use a push-to-checkout hook). That's a nice new feature. Unfortunately even the git 2.1.4 which is in current Debian stable does not support that yet, but I think I can live with a backport, and in the past git has shown to be reasonably easy to backport. > > I am also aware that it is a good idea to git pull before git push > > just in case there were local commits on the remote. > > No, hooks should never pull, merge or do anything that could be > interactive. I mean that I would manually pull on the master before pushing to the remote, thus making sure that the push will go through fine. With the pull, I would get all committed remote changes merged locally on the master, leaving uncommitted and unstaged changes to worry about. > > git reset --hard will unconditionally throw away local uncommitted > > changes. I would like to detect this situation on the remote and > > abort > > the receive progress. But my pre-receive hook does not work as > > intended. Here is my code: > > > > [snip code] > > > > What is going wrong here? > > You mention a post-receive hook first, but have written a pre-receive > hook. Not sure if that's what you intended (or even if that's what's > going wrong). My intention was having a pre-receive hook check for uncommitted or unstaged changes and abort the process if there were any. If there were none, the push would go through and a post-receive hook would update the working copy. With updateInstead, I could probably go without the post-receive hook, but in my understanding, the check for uncommitted/unstaged changes would still be handy and helpful. > > If my entire approach is wrong, what is the recommended way to > > prevent a repository with unstaged or uncommitted changes from being > > pushed to? > > Push-to-checkout is a very simplistic way of deploying and while it > works in simple cases, I'd not recommend it. > > Two safer/saner approaches are: > - Have a separate non-bare repo, and make the post-receive hook in a > bare repo trigger a fetch+reset in the non-bare one > - Use git archive and symlink trickery for even better deploys Both these approaches would make it harder or even impossible to merge local changes done on the remotes. > Questions like this come up in #git all the time, so I wrote up a few > more detailed recipes here, including working hooks and config for all > three ways of deploying: > http://git.seveas.net/simple-deployments-with-git.html Thanks, that is appreciated. I refrained from coming to #git since my experience is that complex issues are hard to manage on IRC. I have, however just joined as Zugschlus, so if you want to talk to me directly feel free to do so. Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Leimen, Germany| lose things."Winona Ryder | Fon: *49 6224 1600402 Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600421 -- To unsubscribe from this list: send the line "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: how to check for uncommitted/unstaged changes on remote side before pushing
On ma, 2015-11-09 at 10:31 +0100, Marc Haber wrote: > Abusing git here has the advantage that one can save local changes > and merge them, which is handy for the task at hand (which is > deploying my dotfiles to "my" servers). For this I really like vcsh (https://github.com/RichiH/vcsh/) in combination with a .bashrc.d snippet that updates dotfiles upon login when possible ( https://github.com/seveas/dotfiles/blob/master/.bashrc.d/vcsh.sh) -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4] Add git-grep threads param
From: Jeff King [p...@peff.net] Sent: Tuesday, November 03, 2015 22:40 To: Junio C Hamano Cc: Victor Leschuk; git@vger.kernel.org; Victor Leschuk; torva...@linux-foundation.org; j...@keeping.me.uk Subject: Re: [PATCH v4] Add git-grep threads param On Tue, Nov 03, 2015 at 09:22:09AM -0800, Junio C Hamano wrote: > > +grep.threads:: > > + Number of grep worker threads, use it to tune up performance on > > + multicore machines. Default value is 8. Set to 0 to disable threading. > > + > > I am not enthused by this "Set to 0 to disable". As Zero is > magical, it would be more useful if 1 meant that threading is not > used (i.e. there is only 1 worker), and 0 meant that we would > automatically pick some reasonable parallelism for you (and we > promise that the our choice would not be outrageously wrong), or > something like that. > Not just useful, but consistent with other parts of git, like > pack.threads, where "0" already means "autodetect". Hello Peff and Junio, Yeah do also think it would be more reasonable to use "0" for "autodetect" default value. However chat this autodetect value should be? For index index-pack and pack-objects we use ncpus() for this, however according to my tests this wouldn't an Ideal for all cases. Maybe it should be something like ncpus()*2, anyway before it we even used hard-coded 8 for all systems... In this case we use "1" as "Do not use threads" and die on all negative numbers during parsing. Agreed? -- Victor-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] gitk: add -C commandline parameter to change path
This patch adds -C (change working directory) parameter to
gitk. With this parameter, instead of need to cd to directory
with .git folder, one can point the correct folder from
commandline.
Signed-off-by: Juha-Pekka Heikkila
---
v2: Adjusted the parameter as per Eric's suggestion. I think
it now work in similar manner as in many GNU tools as well
as git itself.
v3: catch error from cd and other small refinement.
Documentation/gitk.txt | 7 +++
gitk-git/gitk | 30 +-
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 6ade002..d194d9b 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -146,6 +146,13 @@ gitk-specific options
Select the specified commit after loading the graph.
Default behavior is equivalent to specifying '--select-commit=HEAD'.
+-C ::
+
+ Run as if gitk was started in '' instead of the current
+ working directory. When multiple `-C` options are given, each
+ subsequent non-absolute `-C ` is interpreted relative to
+ the preceding `-C `.
+
Examples
gitk v2.6.12.. include/scsi drivers/scsi::
diff --git a/gitk-git/gitk b/gitk-git/gitk
index fcc606e..a7076f8 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -12279,20 +12279,14 @@ setui $uicolor
setoptions
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-show_error {} . [mc "Cannot find a git repository here."]
-exit 1
-}
-
set selecthead {}
set selectheadid {}
set revtreeargs {}
set cmdline_files {}
-set i 0
set revtreeargscmd {}
-foreach arg $argv {
+for {set i 0} {$i < [llength $argv]} {incr i} {
+ set arg [lindex $argv [expr {$i}]]
switch -glob -- $arg {
"" { }
"--" {
@@ -12305,11 +12299,29 @@ foreach arg $argv {
"--argscmd=*" {
set revtreeargscmd [string range $arg 10 end]
}
+ "-C*" {
+ if {[string length $arg] < 3} {
+ incr i
+ set cwd_path [lindex $argv [expr {$i}]]
+ } else {
+ set cwd_path [string range $arg 2 end]
+ }
+
+ if {[catch {cd $cwd_path}]} {
+ show_error {} . [mc "Cannot change directory to '%s'"
$cwd_path]
+ exit 1
+ }
+ }
default {
lappend revtreeargs $arg
}
}
-incr i
+}
+
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+show_error {} . [mc "Cannot find a git repository here."]
+exit 1
}
if {$selecthead eq "HEAD"} {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Problem staging file
I'm using vss2git [1] which as the name suggests, converts a VSS source control database to git. It has had a couple of hiccups, but for the most part I've not had too much problems that I couldn't manually fix except for this one. I have a file that refuses to be staged. The commands are: git add -A git commit -f d:\tmp\tmp1233.tmp To override, I've tried to manually add the file and even use the -a switch on commit line, but no joy. The file in question stays firmly in the "Changes not staged for commit" section. Anyone know why this would be the case and how to fix it? Thanks, A [1] https://github.com/trevorr/vss2git -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/WIP PATCH] rev-list: list all heads with --all
HEAD is a worktree specific sysmref, so that a repository with multiple
worktrees can have multiple HEADs, or HEADs in a worktree different from
the current worktree.
So far, "rev-parse --all" adds only the HEAD from the current worktree
to the list of refs (besides branches etc.). So, a detached HEAD from a
different checkout would be missed unless a shared ref (or current HEAD)
points to it (or descents from it). As a consequence, "git prune" can
prune detached HEADs from worktrees and leave the repo in an
inconsistent state.
Make "rev-parse --all" add the HEADs from all worktrees. This results in
a non-worktree-specific ref list amd solves the pruning problem.
Signed-off-by: Michael J Gruber
---
This patch solves the pruning problem with worktrees, but I feel quite
uneasy about substituting the ref solving at the very heart of refs.c. So
please consider this a mere poc and a request for discussion/input
about how to do this the right way.
I'll cook up a test in proper form, but all you have to do is "worktree
add --detach new" and build a new commit on top of the detached head in
the new worktree. Compare "git prune -n" in "new" and the main worktree...
In essence, I feel the worktree interface still has to evolve a bit: I'd
rather for_each_worktree than loop myself, and if many call sites need to
be aware of multiple heads or worktrees than get_worktrees() should be
part of our init stuff, not here.
refs.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index bab92d7..c355171 100644
--- a/refs.c
+++ b/refs.c
@@ -5,6 +5,7 @@
#include "tag.h"
#include "dir.h"
#include "string-list.h"
+#include "worktree.h"
struct ref_lock {
char *ref_name;
@@ -2091,10 +2092,11 @@ static int do_for_each_ref(struct ref_cache *refs,
const char *base,
return do_for_each_entry(refs, base, do_one_ref, &data);
}
-static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int do_head_refs(const char *submodule, each_ref_fn fn, void *cb_data)
{
struct object_id oid;
- int flag;
+ struct worktree **worktrees;
+ int i, retval;
if (submodule) {
if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
@@ -2103,20 +2105,25 @@ static int do_head_ref(const char *submodule,
each_ref_fn fn, void *cb_data)
return 0;
}
- if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
- return fn("HEAD", &oid, flag, cb_data);
+ worktrees = get_worktrees();
+ retval = 0;
+ for (i=0; worktrees[i]; i++) {
+ hashcpy(oid.hash, worktrees[i]->head_sha1);
+ retval = retval || fn("HEAD", &oid, worktrees[i]->is_detached ?
0 : REF_ISSYMREF, cb_data);
+ }
- return 0;
+ free_worktrees(worktrees);
+ return retval;
}
int head_ref(each_ref_fn fn, void *cb_data)
{
- return do_head_ref(NULL, fn, cb_data);
+ return do_head_refs(NULL, fn, cb_data);
}
int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
{
- return do_head_ref(submodule, fn, cb_data);
+ return do_head_refs(submodule, fn, cb_data);
}
int for_each_ref(each_ref_fn fn, void *cb_data)
--
2.6.3.507.gca54332
--
To unsubscribe from this list: send the line "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] t5813: avoid creating urls that break on cygwin
On Sun, Nov 08, 2015 at 10:54:04AM +0100, Dennis Kaarsemaker wrote: > The fake ssh used by this test simply strips ssh://host from the url, > leaving paths behind that start with //, which cygwin interprets as UNC > paths, causing the test to fail. I found the first sentence a little misleading. It is git itself that strips the URL, isn't it? The problem is that we are feeding a URL with a bogus path, which the fake ssh then tries to access (but in a way that happens to work on Unix systems). -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: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?
On Sun, Nov 08, 2015 at 09:58:06PM -0500, Noam Postavsky wrote:
> > I am leaning more towards ignoring SIGHUP (configurably) being the only
> > really sane path forward. Do you want to try your hand at a patch?
>
> Something like this?
Yes, but with a proper commit message, and an update to
Documentation/config.txt. :)
Automated tests would be nice, but I suspect it may be too complicated
to be worth it.
> diff --git i/credential-cache--daemon.c w/credential-cache--daemon.c
> index eef6fce..e3f2612 100644
> --- i/credential-cache--daemon.c
> +++ w/credential-cache--daemon.c
> @@ -256,6 +256,9 @@ int main(int argc, const char **argv)
> OPT_END()
> };
>
> +int ignore_sighup = 0;
> +git_config_get_bool("credential.cache.ignoreSighup", &ignore_sighup);
I don't think we should use the credential.X.* namespace here. That is
already reserved for credential setup for URLs matching "X".
Probably "credentialCache.ignoreSIGHUP" would be better. Or maybe
"credential-cache". We usually avoid dashes in our config names, but
in this case it matches the program name.
Also, we usually spell config names as all-lowercase in the code. The
older callback-interface config code needed this (since we just strcmp'd
the keys against a normalized case). I think git_config_get_bool() will
normalize the key we feed it, but I'd rather stay consistent.
> @@ -264,6 +267,12 @@ int main(int argc, const char **argv)
>
> check_socket_directory(socket_path);
> register_tempfile(&socket_file, socket_path);
> +
> +if (ignore_sighup) {
> +sigchain_pop(SIGHUP);
> +signal(SIGHUP, SIG_IGN);
> +}
> +
I don't think you need to pop the tempfile handler here. You can simply
sigchain_push() the SIG_IGN, and since we won't ever pop and propagate
that, it doesn't matter what is under it.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Add git-grep threads param
On Mon, Nov 09, 2015 at 03:36:16AM -0800, Victor Leschuk wrote: > Yeah do also think it would be more reasonable to use "0" for > "autodetect" default value. However chat this autodetect value should > be? > > For index index-pack and pack-objects we use ncpus() for this, however > according to my tests this wouldn't an Ideal for all cases. Maybe it > should be something like ncpus()*2, > anyway before it we even used hard-coded 8 for all systems... Why don't we leave it at 8, then? That's the conservative choice, and once we have --threads, people can easily experiment with different values and we can follow-up with a change to the default if need be. -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 v4] Add git-grep threads param
> Why don't we leave it at 8, then? That's the conservative choice, and
> once we have --threads, people can easily experiment with different
> values and we can follow-up with a change to the default if need be.
I'd propose the following:
if (list.nr || cached) {
num_threads = 0; /* Can not multi-thread object lookup */
}
else if (num_threads < 0 && online_cpus() <= 1) {
num_threads = 0; /* User didn't set threading option and we have <= 1
of hardware cores */
}
else if (num_threads == 0) {
num_threads = GREP_NUM_THREADS_DEFAULT; /* User explicitly choose
default behavior */
}
else if (num_threads < 0) { /* Actually this one should be checked earlier
so no need to double check here */
die(_("Ivalid number of threads specified (%d)"), num_threads)
}
--
Victor
--
To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
On Mon, Nov 09, 2015 at 08:34:27AM -0800, Victor Leschuk wrote:
> > Why don't we leave it at 8, then? That's the conservative choice, and
> > once we have --threads, people can easily experiment with different
> > values and we can follow-up with a change to the default if need be.
>
> I'd propose the following:
>
> if (list.nr || cached) {
> num_threads = 0; /* Can not multi-thread object lookup */
> }
> else if (num_threads < 0 && online_cpus() <= 1) {
> num_threads = 0; /* User didn't set threading option and we have <= 1
> of hardware cores */
> }
OK, so you are presumably initializing:
static int num_threads = -1;
> else if (num_threads == 0) {
> num_threads = GREP_NUM_THREADS_DEFAULT; /* User explicitly choose
> default behavior */
> }
> else if (num_threads < 0) { /* Actually this one should be checked
> earlier so no need to double check here */
> die(_("Ivalid number of threads specified (%d)"), num_threads)
> }
What happens if the user has not specified a value (nr_threads == -1)?
Here you die, but shouldn't you take the default thread value?
I wonder if it would be simpler to just default to 0, and then treat
negative values the same as 0 (which is what pack.threads does). Like:
if (list.nr || cached)
num_threads = 1;
if (!num_threads)
num_threads = GREP_NUM_THREADS_DEFAULT;
and then later, instead of use_threads, do:
if (num_threads <= 1) {
... do single-threaded version ...
} else {
... do multi-threaded version ...
}
That matches the logic in builtin/pack-objects.c.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/11] refs: make is_branch public
From: David Turner is_branch was already non-static, but this patch declares it in the header. Signed-off-by: Ronnie Sahlberg Signed-off-by: David Turner Signed-off-by: Junio C Hamano Signed-off-by: Michael Haggerty --- refs.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/refs.h b/refs.h index 6d30c98..39b8edc 100644 --- a/refs.h +++ b/refs.h @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st */ int pack_refs(unsigned int flags); +int is_branch(const char *refname); + /* * Flags controlling ref_transaction_update(), ref_transaction_create(), etc. * REF_NODEREF: act on the ref directly, instead of dereferencing -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 00/11] refs backend pre-vtable
This is another reroll of the pre-vtable part of the refs-backend patch series dt/refs-backend-pre-vtable. v6 [1] proved cumbersome because it conflicted messily with lf/ref-is-hidden-namespace [2]. The conflicts were partly due to the motion of code across files but, even worse, due to the change of order of function definitions between old and new files. So I have heavily "optimized" this reroll for reviewability and to minimize conflicts with other work in the area. The only such work that I know of is lf/ref-is-hidden-namespace, which can now be merged with this series *without conflicts*. Changes since v6: * It doesn't move refs.c to the refs/ subdirectory, as v6 did. Instead, leave the shared code in the existing file, refs.c. * Don't rename refs.c to refs/files-backend.c and then selectively move content back to refs.c. Instead, move content only once and only in one direction, namely from refs.c -> refs/files-backend.c (in patch 08/11). This is a giant commit but *it makes no other changes* so it should be easy to review. (The "--patience" option of "git diff" is quite helpful here. It was turned on when this patch series was generated.) * Preserve the order of code during the move. Aside from a few lines of boilerplate, each of the following commands, when applied to commit 08, shows only lines being deleted: git diff --patience $commit^:refs.c $commit:refs.c git diff --patience $commit^:refs.c $commit:refs/files-backend.c * To make all of the above possible, patches 01 through 06 do a little bit of preparatory code untangling. These commits have themselves been split up a bit to make them as "obviously correct" as possible. Patch 07 creates the new header file, refs/refs-internal.h, thereby increasing the visibility of some declarations. * The final patches 09, 10, and 11 are not quite as "obviously correct" as the first eight, but I left them in to keep the logical contents of this patch series the same as v6. But these last three commits could just as well be postponed until the next tranche of patches if that helps speed the way of the first eight patches into master. * I also fixed a commit message and fixed the implementation of the new verify_refname_available() function to return a negative number on error as documented (previously it returned 1 on error). I've tried to attribute authorship of these changes as fairly as possible based on who initiated the corresponding changes. If anybody feels that I have appropriated his work or, conversely, put words into his mouth, just let me know and I would be happy to adjust the authorship. It would be great to get this patch series (at least the first eight patches) reviewed and merged as soon as possible. Even though it no longer conflicts with lf/ref-is-hidden-namespace, it is still very prone to conflicting with any other work in the references code. This patch series is also available on my GitHub fork [3] as branch "refs-backend-pre-vtable". Michael [1] http://thread.gmane.org/gmane.comp.version-control.git/280325/focus=280754 [2] http://article.gmane.org/gmane.comp.version-control.git/281004 [3] https://github.com/mhagger/git David Turner (5): refs: make is_branch public copy_msg(): rename to copy_reflog_msg() initdb: make safe_create_dir public files_log_ref_write: new function refs: break out ref conflict checks Michael Haggerty (4): pack_if_possible_fn(): use ref_type() instead of is_per_worktree_ref() refname_is_safe(): improve docstring refs/refs-internal.h: new header file refs: split filesystem-based refs code into a new file Ronnie Sahlberg (2): verify_refname_available(): rename function verify_refname_available(): new function Makefile |3 +- builtin/init-db.c | 12 - cache.h|8 + path.c | 12 + refs.c | 3709 +--- refs.h |2 + refs.c => refs/files-backend.c | 1287 +- refs/refs-internal.h | 202 +++ 8 files changed, 315 insertions(+), 4920 deletions(-) copy refs.c => refs/files-backend.c (75%) create mode 100644 refs/refs-internal.h -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 03/11] verify_refname_available(): new function
From: Ronnie Sahlberg
Add a new verify_refname_available() function, which checks whether the
refname is available for use, taking all references (both packed and
loose) into account. This function, unlike the old
verify_refname_available(), has semantics independent of the choice of
reference storage, and can therefore be implemented by alternative
reference backends.
Use the new function in a couple of places.
Signed-off-by: Ronnie Sahlberg
Signed-off-by: David Turner
Signed-off-by: Junio C Hamano
Signed-off-by: Michael Haggerty
---
refs.c | 66 --
1 file changed, 40 insertions(+), 26 deletions(-)
diff --git a/refs.c b/refs.c
index 0617e0c..ddcdf81 100644
--- a/refs.c
+++ b/refs.c
@@ -279,7 +279,7 @@ struct ref_dir {
* presence of an empty subdirectory does not block the creation of a
* similarly-named reference. (The fact that reference names with the
* same leading components can conflict *with each other* is a
- * separate issue that is regulated by verify_refname_available_dir().)
+ * separate issue that is regulated by verify_refname_available().)
*
* Please note that the name field contains the fully-qualified
* reference (or subdirectory) name. Space could be saved by only
@@ -897,19 +897,7 @@ static int nonmatching_ref_fn(struct ref_entry *entry,
void *vdata)
/*
* Return 0 if a reference named refname could be created without
* conflicting with the name of an existing reference in dir.
- * Otherwise, return a negative value and write an explanation to err.
- * If extras is non-NULL, it is a list of additional refnames with
- * which refname is not allowed to conflict. If skip is non-NULL,
- * ignore potential conflicts with refs in skip (e.g., because they
- * are scheduled for deletion in the same operation). Behavior is
- * undefined if the same name is listed in both extras and skip.
- *
- * Two reference names conflict if one of them exactly matches the
- * leading components of the other; e.g., "refs/foo/bar" conflicts
- * with both "refs/foo" and with "refs/foo/bar/baz" but not with
- * "refs/foo/bar" or "refs/foo/barbados".
- *
- * extras and skip must be sorted.
+ * See verify_refname_available for more information.
*/
static int verify_refname_available_dir(const char *refname,
const struct string_list *extras,
@@ -3120,6 +3108,40 @@ out:
return ret;
}
+/*
+ * Return 0 if a reference named refname could be created without
+ * conflicting with the name of an existing reference. Otherwise,
+ * return a negative value and write an explanation to err. If extras
+ * is non-NULL, it is a list of additional refnames with which refname
+ * is not allowed to conflict. If skip is non-NULL, ignore potential
+ * conflicts with refs in skip (e.g., because they are scheduled for
+ * deletion in the same operation). Behavior is undefined if the same
+ * name is listed in both extras and skip.
+ *
+ * Two reference names conflict if one of them exactly matches the
+ * leading components of the other; e.g., "foo/bar" conflicts with
+ * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
+ * "foo/barbados".
+ *
+ * extras and skip must be sorted.
+ */
+static int verify_refname_available(const char *newname,
+ struct string_list *extras,
+ struct string_list *skip,
+ struct strbuf *err)
+{
+ struct ref_dir *packed_refs = get_packed_refs(&ref_cache);
+ struct ref_dir *loose_refs = get_loose_refs(&ref_cache);
+
+ if (verify_refname_available_dir(newname, extras, skip,
+packed_refs, err) ||
+ verify_refname_available_dir(newname, extras, skip,
+loose_refs, err))
+ return -1;
+
+ return 0;
+}
+
static int rename_ref_available(const char *oldname, const char *newname)
{
struct string_list skip = STRING_LIST_INIT_NODUP;
@@ -3127,10 +3149,7 @@ static int rename_ref_available(const char *oldname,
const char *newname)
int ret;
string_list_insert(&skip, oldname);
- ret = !verify_refname_available_dir(newname, NULL, &skip,
- get_packed_refs(&ref_cache), &err)
- && !verify_refname_available_dir(newname, NULL, &skip,
-get_loose_refs(&ref_cache),
&err);
+ ret = !verify_refname_available(newname, NULL, &skip, &err);
if (!ret)
error("%s", err.buf);
@@ -4334,8 +4353,6 @@ static int ref_present(const char *refname,
int initial_ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
- struct ref_dir *loose_refs = get_loose_refs(&ref_cache);
- struct ref_dir *packed_refs = get_packed_refs(&ref_cache);
[PATCH v7 06/11] refname_is_safe(): improve docstring
Signed-off-by: Michael Haggerty
---
refs.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 82129f0..f48c58a 100644
--- a/refs.c
+++ b/refs.c
@@ -341,13 +341,17 @@ static struct ref_dir *get_ref_dir(struct ref_entry
*entry)
}
/*
- * Check if a refname is safe.
- * For refs that start with "refs/" we consider it safe as long they do
- * not try to resolve to outside of refs/.
+ * Return true iff refname is minimally safe. "Safe" here means that
+ * deleting a loose reference by this name will not do any damage, for
+ * example by causing a file that is not a reference to be deleted.
+ * This function does not check that the reference name is legal; for
+ * that, use check_refname_format().
*
- * For all other refs we only consider them safe iff they only contain
- * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not like
- * "config").
+ * We consider a refname that starts with "refs/" to be safe as long
+ * as any ".." components that it might contain do not escape "refs/".
+ * Names that do not start with "refs/" are considered safe iff they
+ * consist entirely of upper case characters and '_' (like "HEAD" and
+ * "MERGE_HEAD" but not "config" or "FOO/BAR").
*/
static int refname_is_safe(const char *refname)
{
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 11/11] refs: break out ref conflict checks
From: David Turner
Create new function find_descendant_ref, to hold one of the ref
conflict checks used in verify_refname_available. Multiple backends
will need this function, so move it to the common code.
Also move rename_ref_available to the common code, because alternate
backends might need it and it has no files-backend-specific code.
Signed-off-by: David Turner
Signed-off-by: Junio C Hamano
Signed-off-by: Michael Haggerty
---
refs.c | 44
refs/files-backend.c | 49 +++--
refs/refs-internal.h | 16
3 files changed, 67 insertions(+), 42 deletions(-)
diff --git a/refs.c b/refs.c
index 175b3e9..0e2eaf8 100644
--- a/refs.c
+++ b/refs.c
@@ -1027,3 +1027,47 @@ int ref_is_hidden(const char *refname)
}
return 0;
}
+
+const char *find_descendant_ref(const char *dirname,
+ const struct string_list *extras,
+ const struct string_list *skip)
+{
+ int pos;
+
+ if (!extras)
+ return NULL;
+
+ /*
+* Look at the place where dirname would be inserted into
+* extras. If there is an entry at that position that starts
+* with dirname (remember, dirname includes the trailing
+* slash) and is not in skip, then we have a conflict.
+*/
+ for (pos = string_list_find_insert_index(extras, dirname, 0);
+pos < extras->nr; pos++) {
+ const char *extra_refname = extras->items[pos].string;
+
+ if (!starts_with(extra_refname, dirname))
+ break;
+
+ if (!skip || !string_list_has_string(skip, extra_refname))
+ return extra_refname;
+ }
+ return NULL;
+}
+
+int rename_ref_available(const char *oldname, const char *newname)
+{
+ struct string_list skip = STRING_LIST_INIT_NODUP;
+ struct strbuf err = STRBUF_INIT;
+ int ret;
+
+ string_list_insert(&skip, oldname);
+ ret = !verify_refname_available(newname, NULL, &skip, &err);
+ if (!ret)
+ error("%s", err.buf);
+
+ string_list_clear(&skip, 0);
+ strbuf_release(&err);
+ return ret;
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1094348..8bb3728 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -729,6 +729,7 @@ static int verify_refname_available_dir(const char *refname,
struct strbuf *err)
{
const char *slash;
+ const char *extra_refname;
int pos;
struct strbuf dirname = STRBUF_INIT;
int ret = -1;
@@ -834,32 +835,12 @@ static int verify_refname_available_dir(const char
*refname,
}
}
- if (extras) {
- /*
-* Check for entries in extras that start with
-* "$refname/". We do that by looking for the place
-* where "$refname/" would be inserted in extras. If
-* there is an entry at that position that starts with
-* "$refname/" and is not in skip, then we have a
-* conflict.
-*/
- for (pos = string_list_find_insert_index(extras, dirname.buf,
0);
-pos < extras->nr; pos++) {
- const char *extra_refname = extras->items[pos].string;
-
- if (!starts_with(extra_refname, dirname.buf))
- break;
-
- if (!skip || !string_list_has_string(skip,
extra_refname)) {
- strbuf_addf(err, "cannot process '%s' and '%s'
at the same time",
- refname, extra_refname);
- goto cleanup;
- }
- }
- }
-
- /* No conflicts were found */
- ret = 0;
+ extra_refname = find_descendant_ref(dirname.buf, extras, skip);
+ if (extra_refname)
+ strbuf_addf(err, "cannot process '%s' and '%s' at the same
time",
+ refname, extra_refname);
+ else
+ ret = 0;
cleanup:
strbuf_release(&dirname);
@@ -2474,22 +2455,6 @@ int verify_refname_available(const char *newname,
return 0;
}
-static int rename_ref_available(const char *oldname, const char *newname)
-{
- struct string_list skip = STRING_LIST_INIT_NODUP;
- struct strbuf err = STRBUF_INIT;
- int ret;
-
- string_list_insert(&skip, oldname);
- ret = !verify_refname_available(newname, NULL, &skip, &err);
- if (!ret)
- error("%s", err.buf);
-
- string_list_clear(&skip, 0);
- strbuf_release(&err);
- return ret;
-}
-
static int write_ref_to_lockfile(struct ref_lock *lock,
const unsigned char *sha1, struct s
[PATCH v7 05/11] pack_if_possible_fn(): use ref_type() instead of is_per_worktree_ref()
is_per_worktree_ref() will soon be made private, so use the public
interface, ref_type(), in its place. And now that we're using
ref_type(), we can make it clear that we won't pack pseudorefs. This was
the case before, but due to the not-so-obvious reason that this function
is applied to references via the loose reference cache, which only
includes references that live inside "refs/".
Signed-off-by: Michael Haggerty
---
refs.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 480de9a..82129f0 100644
--- a/refs.c
+++ b/refs.c
@@ -2671,8 +2671,6 @@ struct pack_refs_cb_data {
struct ref_to_prune *ref_to_prune;
};
-static int is_per_worktree_ref(const char *refname);
-
/*
* An each_ref_entry_fn that is run over loose references only. If
* the loose reference can be packed, add an entry in the packed ref
@@ -2687,7 +2685,7 @@ static int pack_if_possible_fn(struct ref_entry *entry,
void *cb_data)
int is_tag_ref = starts_with(entry->name, "refs/tags/");
/* Do not pack per-worktree refs: */
- if (is_per_worktree_ref(entry->name))
+ if (ref_type(entry->name) != REF_TYPE_NORMAL)
return 0;
/* ALWAYS pack tags */
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/11] refs/refs-internal.h: new header file
There are a number of constants, structs, and static functions defined
in refs.c and treated as private to the references module. But we want
to support multiple reference backends within the reference module,
and those backends will need access to some heretofore private
declarations.
We don't want those declarations to be visible to non-refs code, so we
don't want to move them to refs.h. Instead, add a new header file,
refs/refs-internal.h, that is intended to be included only from within
the refs module. Make some functions non-static and move some
declarations (and their corresponding docstrings) from refs.c to this
file.
In a moment we will add more content to the "refs" subdirectory.
Signed-off-by: Michael Haggerty
---
refs.c | 175 +++--
refs/refs-internal.h | 182 +++
2 files changed, 191 insertions(+), 166 deletions(-)
create mode 100644 refs/refs-internal.h
diff --git a/refs.c b/refs.c
index f48c58a..9aff0c8 100644
--- a/refs.c
+++ b/refs.c
@@ -1,6 +1,6 @@
#include "cache.h"
+#include "refs/refs-internal.h"
#include "lockfile.h"
-#include "refs.h"
#include "object.h"
#include "tag.h"
#include "dir.h"
@@ -35,41 +35,6 @@ static unsigned char refname_disposition[256] = {
};
/*
- * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken
- * refs (i.e., because the reference is about to be deleted anyway).
- */
-#define REF_DELETING 0x02
-
-/*
- * Used as a flag in ref_update::flags when a loose ref is being
- * pruned.
- */
-#define REF_ISPRUNING 0x04
-
-/*
- * Used as a flag in ref_update::flags when the reference should be
- * updated to new_sha1.
- */
-#define REF_HAVE_NEW 0x08
-
-/*
- * Used as a flag in ref_update::flags when old_sha1 should be
- * checked.
- */
-#define REF_HAVE_OLD 0x10
-
-/*
- * Used as a flag in ref_update::flags when the lockfile needs to be
- * committed.
- */
-#define REF_NEEDS_COMMIT 0x20
-
-/*
- * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a
- * value to ref_update::flags
- */
-
-/*
* Try to read one refname component from the front of refname.
* Return the length of the component found, or -1 if the component is
* not legal. It is legal if it is something reasonable to have under
@@ -340,20 +305,7 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
return dir;
}
-/*
- * Return true iff refname is minimally safe. "Safe" here means that
- * deleting a loose reference by this name will not do any damage, for
- * example by causing a file that is not a reference to be deleted.
- * This function does not check that the reference name is legal; for
- * that, use check_refname_format().
- *
- * We consider a refname that starts with "refs/" to be safe as long
- * as any ".." components that it might contain do not escape "refs/".
- * Names that do not start with "refs/" are considered safe iff they
- * consist entirely of upper case characters and '_' (like "HEAD" and
- * "MERGE_HEAD" but not "config" or "FOO/BAR").
- */
-static int refname_is_safe(const char *refname)
+int refname_is_safe(const char *refname)
{
if (starts_with(refname, "refs/")) {
char *buf;
@@ -1823,39 +1775,7 @@ static int filter_refs(const char *refname, const struct
object_id *oid,
return filter->fn(refname, oid, flags, filter->cb_data);
}
-enum peel_status {
- /* object was peeled successfully: */
- PEEL_PEELED = 0,
-
- /*
-* object cannot be peeled because the named object (or an
-* object referred to by a tag in the peel chain), does not
-* exist.
-*/
- PEEL_INVALID = -1,
-
- /* object cannot be peeled because it is not a tag: */
- PEEL_NON_TAG = -2,
-
- /* ref_entry contains no peeled value because it is a symref: */
- PEEL_IS_SYMREF = -3,
-
- /*
-* ref_entry cannot be peeled because it is broken (i.e., the
-* symbolic reference cannot even be resolved to an object
-* name):
-*/
- PEEL_BROKEN = -4
-};
-
-/*
- * Peel the named object; i.e., if the object is a tag, resolve the
- * tag recursively until a non-tag is found. If successful, store the
- * result to sha1 and return PEEL_PEELED. If the object is not a tag
- * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
- * and leave sha1 unchanged.
- */
-static enum peel_status peel_object(const unsigned char *name, unsigned char
*sha1)
+enum peel_status peel_object(const unsigned char *name, unsigned char *sha1)
{
struct object *o = lookup_unknown_object(name);
@@ -3110,27 +3030,10 @@ out:
return ret;
}
-/*
- * Return 0 if a reference named refname could be created without
- * conflicting with the name of an existing reference. Otherwise,
- * return a negative value and write an explanation to err. If extras
- * is non-NULL, it is a list of additional refnames with whi
[PATCH v7 02/11] verify_refname_available(): rename function
From: Ronnie Sahlberg
Rename verify_refname_available() to verify_refname_available_dir() to
make the old name available for a more general purpose.
Signed-off-by: Ronnie Sahlberg
Signed-off-by: David Turner
Signed-off-by: Junio C Hamano
Signed-off-by: Michael Haggerty
---
refs.c | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/refs.c b/refs.c
index 132eff5..0617e0c 100644
--- a/refs.c
+++ b/refs.c
@@ -279,7 +279,7 @@ struct ref_dir {
* presence of an empty subdirectory does not block the creation of a
* similarly-named reference. (The fact that reference names with the
* same leading components can conflict *with each other* is a
- * separate issue that is regulated by verify_refname_available().)
+ * separate issue that is regulated by verify_refname_available_dir().)
*
* Please note that the name field contains the fully-qualified
* reference (or subdirectory) name. Space could be saved by only
@@ -911,11 +911,11 @@ static int nonmatching_ref_fn(struct ref_entry *entry,
void *vdata)
*
* extras and skip must be sorted.
*/
-static int verify_refname_available(const char *refname,
- const struct string_list *extras,
- const struct string_list *skip,
- struct ref_dir *dir,
- struct strbuf *err)
+static int verify_refname_available_dir(const char *refname,
+ const struct string_list *extras,
+ const struct string_list *skip,
+ struct ref_dir *dir,
+ struct strbuf *err)
{
const char *slash;
int pos;
@@ -2465,8 +2465,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
strbuf_git_path(&orig_ref_file, "%s", orig_refname);
if (remove_empty_directories(&orig_ref_file)) {
last_errno = errno;
- if (!verify_refname_available(orig_refname, extras,
skip,
-
get_loose_refs(&ref_cache), err))
+ if (!verify_refname_available_dir(orig_refname, extras,
skip,
+
get_loose_refs(&ref_cache), err))
strbuf_addf(err, "there are still refs under
'%s'",
orig_refname);
goto error_return;
@@ -2479,8 +2479,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
if (!refname) {
last_errno = errno;
if (last_errno != ENOTDIR ||
- !verify_refname_available(orig_refname, extras, skip,
- get_loose_refs(&ref_cache), err))
+ !verify_refname_available_dir(orig_refname, extras, skip,
+ get_loose_refs(&ref_cache),
err))
strbuf_addf(err, "unable to resolve reference %s: %s",
orig_refname, strerror(last_errno));
@@ -2493,8 +2493,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* our refname.
*/
if (is_null_oid(&lock->old_oid) &&
- verify_refname_available(refname, extras, skip,
-get_packed_refs(&ref_cache), err)) {
+ verify_refname_available_dir(refname, extras, skip,
+get_packed_refs(&ref_cache), err)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -3127,10 +3127,10 @@ static int rename_ref_available(const char *oldname,
const char *newname)
int ret;
string_list_insert(&skip, oldname);
- ret = !verify_refname_available(newname, NULL, &skip,
- get_packed_refs(&ref_cache), &err)
- && !verify_refname_available(newname, NULL, &skip,
-get_loose_refs(&ref_cache), &err);
+ ret = !verify_refname_available_dir(newname, NULL, &skip,
+ get_packed_refs(&ref_cache), &err)
+ && !verify_refname_available_dir(newname, NULL, &skip,
+get_loose_refs(&ref_cache),
&err);
if (!ret)
error("%s", err.buf);
@@ -4376,12 +4376,12 @@ int initial_ref_transaction_commit(struct
ref_transaction *transaction,
if ((update->flags & REF_HAVE_OLD) &&
!is_null_sha1(update->old_sha1))
die("BUG: initial ref transaction with old_sha1 set");
- if (verify_refname_available(update->refname,
-
[PATCH v7 04/11] copy_msg(): rename to copy_reflog_msg()
From: David Turner
We will soon increase the visibility of this function, so make its name
more distinctive.
Signed-off-by: David Turner
Signed-off-by: Junio C Hamano
Signed-off-by: Michael Haggerty
---
refs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index ddcdf81..480de9a 100644
--- a/refs.c
+++ b/refs.c
@@ -3287,7 +3287,7 @@ static int commit_ref(struct ref_lock *lock)
* large, while cleaning up the whitespaces. Especially, convert LF to space,
* because reflog file is one line per entry.
*/
-static int copy_msg(char *buf, const char *msg)
+static int copy_reflog_msg(char *buf, const char *msg)
{
char *cp = buf;
char c;
@@ -3391,7 +3391,7 @@ static int log_ref_write_fd(int fd, const unsigned char
*old_sha1,
sha1_to_hex(new_sha1),
committer);
if (msglen)
- len += copy_msg(logrec + len - 1, msg) - 1;
+ len += copy_reflog_msg(logrec + len - 1, msg) - 1;
written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
free(logrec);
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 09/11] initdb: make safe_create_dir public
From: David Turner
Soon we will want to create initdb functions for ref backends, and
code from initdb that calls this function needs to move into the files
backend. So this function needs to be public.
Signed-off-by: David Turner
Signed-off-by: Junio C Hamano
Signed-off-by: Michael Haggerty
---
builtin/init-db.c | 12
cache.h | 8
path.c| 12
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index f59f407..07229d6 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -24,18 +24,6 @@ static int init_shared_repository = -1;
static const char *init_db_template_dir;
static const char *git_link;
-static void safe_create_dir(const char *dir, int share)
-{
- if (mkdir(dir, 0777) < 0) {
- if (errno != EEXIST) {
- perror(dir);
- exit(1);
- }
- }
- else if (share && adjust_shared_perm(dir))
- die(_("Could not make %s writable by group"), dir);
-}
-
static void copy_templates_1(struct strbuf *path, struct strbuf *template,
DIR *dir)
{
diff --git a/cache.h b/cache.h
index 9a905a8..0f9808c 100644
--- a/cache.h
+++ b/cache.h
@@ -1737,4 +1737,12 @@ void stat_validity_update(struct stat_validity *sv, int
fd);
int versioncmp(const char *s1, const char *s2);
void sleep_millisec(int millisec);
+/*
+ * Create a directory and (if share is nonzero) adjust its permissions
+ * according to the shared_repository setting. Only use this for
+ * directories under $GIT_DIR. Don't use it for working tree
+ * directories.
+ */
+void safe_create_dir(const char *dir, int share);
+
#endif /* CACHE_H */
diff --git a/path.c b/path.c
index 212695a..9e0283c 100644
--- a/path.c
+++ b/path.c
@@ -723,6 +723,18 @@ int adjust_shared_perm(const char *path)
return 0;
}
+void safe_create_dir(const char *dir, int share)
+{
+ if (mkdir(dir, 0777) < 0) {
+ if (errno != EEXIST) {
+ perror(dir);
+ exit(1);
+ }
+ }
+ else if (share && adjust_shared_perm(dir))
+ die(_("Could not make %s writable by group"), dir);
+}
+
static int have_same_root(const char *path1, const char *path2)
{
int is_abs1, is_abs2;
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 10/11] files_log_ref_write: new function
From: David Turner
Because HEAD and stash are per-worktree, every refs backend needs to
go through the files backend to write these refs.
So create a new function, files_log_ref_write, and add it to
refs/refs-internal.h. Later, we will use this to handle reflog updates
for per-worktree symbolic refs (HEAD).
Signed-off-by: David Turner
Signed-off-by: Junio C Hamano
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 8
refs/refs-internal.h | 4
2 files changed, 12 insertions(+)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a2e5a56..1094348 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2735,6 +2735,14 @@ static int log_ref_write(const char *refname, const
unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg,
int flags, struct strbuf *err)
{
+ return files_log_ref_write(refname, old_sha1, new_sha1, msg, flags,
+ err);
+}
+
+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+ const unsigned char *new_sha1, const char *msg,
+ int flags, struct strbuf *err)
+{
struct strbuf sb = STRBUF_INIT;
int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb, flags,
err);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 8f5cfb4..88a5be0 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -179,4 +179,8 @@ struct ref_transaction {
enum ref_transaction_state state;
};
+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+ const unsigned char *new_sha1, const char *msg,
+ int flags, struct strbuf *err);
+
#endif /* REFS_REFS_INTERNAL_H */
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4] Add git-grep threads param
> if (list.nr || cached)
>num_threads = 1;
> if (!num_threads)
> num_threads = GREP_NUM_THREADS_DEFAULT;
> and then later, instead of use_threads, do:
> if (num_threads <= 1) {
... do single-threaded version ...
> } else {
... do multi-threaded version ...
> }
> That matches the logic in builtin/pack-objects.c.
Maybe use the simplest version (and keep num_numbers == 0 also as flag for all
other checks in code like if(num_flags) ):
if (list.nr || cached )
num_threads = 0; // do not use threads
else if (num_threads == 0)
num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT;
else if (num_threads < 0)
die(...)
// here we are num_threads > zero, so do nothing
--
To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
On Mon, Nov 09, 2015 at 09:28:12AM -0800, Victor Leschuk wrote: > Maybe use the simplest version (and keep num_numbers == 0 also as flag for > all other checks in code like if(num_flags) ): > > if (list.nr || cached ) > num_threads = 0; // do not use threads > else if (num_threads == 0) > num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT; That's OK. > else if (num_threads < 0) > die(...) Do we really want to die here? I think "threads < 0" works the same as "threads==0" in other git programs. It's also a weird place to die. It would make: git grep --cached --threads=-1 silently work, while: git grep --threads=-1 would die. If we do accept it, it may make sense to normalize it to 0 so that you can just check "!num_threads" elsewhere in the code. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t5813: avoid creating urls that break on cygwin
When passed an ssh:// url, git strips ssh://host from the url but does not remove leading slashes from the path. So when this test used ssh://remote//path/to/pwd, the path accessed by our fake SSH is //path/to/pwd, which cygwin interprets as a UNC path, causing the test to fail. We may want to actually fix this in git itself, making it remove extra slashes from urls before feeding them to transports or helpers, but that's for another topic as it could cause regressions. Signed-off-by: Dennis Kaarsemaker --- You're right of course. Somehow I remembered that the fake ssh was doing the stripping, but didn't check that when writing the commit message. How about this version? t/t5813-proto-disable-ssh.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5813-proto-disable-ssh.sh b/t/t5813-proto-disable-ssh.sh index ad877d7..a954ead 100755 --- a/t/t5813-proto-disable-ssh.sh +++ b/t/t5813-proto-disable-ssh.sh @@ -14,7 +14,7 @@ test_expect_success 'setup repository to clone' ' ' test_proto "host:path" ssh "remote:repo.git" -test_proto "ssh://" ssh "ssh://remote/$PWD/remote/repo.git" -test_proto "git+ssh://" ssh "git+ssh://remote/$PWD/remote/repo.git" +test_proto "ssh://" ssh "ssh://remote$PWD/remote/repo.git" +test_proto "git+ssh://" ssh "git+ssh://remote$PWD/remote/repo.git" test_done -- 2.6.3-495-gf0a7f49 -- Dennis Kaarsemaker http://twitter.com/seveas -- To unsubscribe from this list: send the line "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] t5813: avoid creating urls that break on cygwin
On Mon, Nov 09, 2015 at 06:49:35PM +0100, Dennis Kaarsemaker wrote: > When passed an ssh:// url, git strips ssh://host from the url but does > not remove leading slashes from the path. So when this test used > ssh://remote//path/to/pwd, the path accessed by our fake SSH is > //path/to/pwd, which cygwin interprets as a UNC path, causing the test > to fail. > > We may want to actually fix this in git itself, making it remove extra > slashes from urls before feeding them to transports or helpers, but > that's for another topic as it could cause regressions. > > Signed-off-by: Dennis Kaarsemaker > --- > > You're right of course. Somehow I remembered that the fake ssh was doing the > stripping, but didn't check that when writing the commit message. How about > this version? Looks good. Thanks. -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 v4] Add git-grep threads param
On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk wrote: > > Maybe use the simplest version (and keep num_numbers == 0 also as flag for > all other checks in code like if(num_flags) ): > > if (list.nr || cached ) > num_threads = 0; // do not use threads > else if (num_threads == 0) > num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT; I will say this AGAIN. The number of threads is *not* about the number of CPU's. Stop this craziness. It's wrong. The number of threads is about parallelism. Yes, CPU's is a small part of it. But as mentioned earlier, the *big* wins are for slow filesystems, NFS in particular. On NFS, even if you have things cached, the cache revalidation is going to cause network traffic almost every time. Being able to have several of those outstanding is a big deal. So stop with the "online_cpus()" stuff. And don't base your benchmarks purely on the CPU-bound case. Because the CPU-bound case is the case that is already generally so good that few people will care all *that* deeply. Many of the things git does are not for "best-case" behavior, but to avoid bad "worst-case" situations. Look at things like the index preloading (also threaded). The big win there is - again - when the stat() calls may need IO. Sure, it can help for CPU use too, but especially on Linux, cached "stat()" calls are really quite cheap. The big upside is, again, in situations like git repositories over NFS. In the CPU-intensive case, the threading might make things go from a couple of seconds to half a second. Big deal. You're not getting up to get a coffee in either case. In the network traffic case, the threading might make things go from one minute to ten seconds. And *that* is a big deal. That's huge. That's "annoyingly slow" to "oh, this is the fastest SCM I have ever worked with in my life". That can literally be something that changes how you work for a developer. You start doing things that you simply would never otherwise do. So *none* of the threading in git is about CPU's. Maybe we should add big honking comments about that. And that big honking comment should be in the documentation for the new flag too. Because it would be really really sad if people say "I have a laptop with just two cores, so I'll set the threading option to 2", when they then work mostly over a slow wireless network and their company wants minimal local installs. The biggest reason to NOT EVER add that configuration is literally the confusion about this being about CPU's. That was what got the whole thread started, that was what the original benchmark numbers were about, and that was WRONG. So I would strongly suggest that Junio ignore these patches unless they very clearly talk about the whole IO issue. Both in the source code, in the commit messages, and in the documentation for the config option. Linus -- To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
On Mon, Nov 09, 2015 at 09:55:34AM -0800, Linus Torvalds wrote: > > if (list.nr || cached ) > > num_threads = 0; // do not use threads > > else if (num_threads == 0) > > num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT; > > I will say this AGAIN. > > The number of threads is *not* about the number of CPU's. Stop this > craziness. It's wrong. I agree with you (and that's why we have a hard-coded default here, and not online_cpus()). This check is just whether to turn the threading on or not, based on whether we have multiple CPUs at all. And I agree that probably should _not_ be respecting online_cpus() either, because we are better off parallelizing even on a single CPU to avoid fs latency. But note that this is already what the code _currently_ does. I do not mind changing it, but that is a separate notion from Victor's topic, which is to make the number of threads configurable at all. What I'd really hope to see is: 1. Victor adds --threads support, and changes _nothing else_ about the defaults. 2. People experiment with --threads to see what works best in a variety of cases (especially things like cold-cache NFS). Then we get patches based on real world numbers to adjust: 2a. GREP_NUM_THREADS_DEFAULT (either bumping the hard-coded value, or using some dynamic heuristic) 2b. What to do for the single-CPU case. We're doing step 1 here. I don't mind jumping to 2b if we're pretty sure it's the right thing (and I think it probably is), but it should definitely be a separate patch. > So *none* of the threading in git is about CPU's. Maybe we should add > big honking comments about that. Minor nit: there definitely _are_ CPU-bound parallel tasks in git. pack-objects and index-pack come to mind. If we add any user-visible advice about tweaking thread configuration, we should be sure that it is scoped to the appropriate flags. > And that big honking comment should be in the documentation for the > new flag too. Because it would be really really sad if people say "I > have a laptop with just two cores, so I'll set the threading option to > 2", when they then work mostly over a slow wireless network and their > company wants minimal local installs. Agreed. It would probably make sense for the "--threads" option documentation for each command to discuss whether it is meant to parallelize work across CPUs, or avoid filesystem latency. -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 v4] Add git-grep threads param
On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk wrote: > > Maybe use the simplest version (and keep num_numbers == 0 also as flag for > all other checks in code like if(num_flags) ): > > if (list.nr || cached ) > num_threads = 0; // do not use threads > else if (num_threads == 0) > num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT; > I will say this AGAIN. > The number of threads is *not* about the number of CPU's. Stop this craziness. It's wrong. Actually I have never said the nCPUs played main role in it. The patch is intended to provide user ability to change this threads number according to their needs and to touch as small amount of other code as possible. > The number of threads is about parallelism. Yes, CPU's is a small part > of it. But as mentioned earlier, the *big* wins are for slow > filesystems, NFS in particular. On NFS, even if you have things > cached, the cache revalidation is going to cause network traffic > almost every time. Being able to have several of those outstanding is > a big deal. > So stop with the "online_cpus()" stuff. And don't base your benchmarks > purely on the CPU-bound case. Because the CPU-bound case is the case > that is already generally so good that few people will care all *that* > deeply. I have performed a cold-cached FS test (in previous thread to minimize the CPU part in the results) and it showed high correlation between speed and thread_num. Isn't it what you said? Even on systems with small number of cores we can gain profit of multithreading. That's I why I suggest this param to be customizable and not HARDCODED. We need to create a clear text for the documentation that this number should not based on number of CPU-s only. Currently do not mention anything on it. -- Victor -- To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
On Mon, Nov 9, 2015 at 9:55 AM, Linus Torvalds wrote: > > So stop with the "online_cpus()" stuff. And don't base your benchmarks > purely on the CPU-bound case. Because the CPU-bound case is the case > that is already generally so good that few people will care all *that* > deeply. > > Many of the things git does are not for "best-case" behavior, but to > avoid bad "worst-case" situations. Look at things like the index > preloading (also threaded). The big win there is - again - when the > stat() calls may need IO. Sure, it can help for CPU use too, but > especially on Linux, cached "stat()" calls are really quite cheap. The > big upside is, again, in situations like git repositories over NFS. > > In the CPU-intensive case, the threading might make things go from a > couple of seconds to half a second. Big deal. You're not getting up to > get a coffee in either case. Chiming in here as I have another series in flight doing parallelism. (Submodules done in parallel including fetching, cloning, checking out) online_cpus() seems to be one of the easiest ballpark estimates for the power of a system. So what I would have liked to use would be some kind of parallel_expect_bottleneck(enum kinds); with kinds being one of (FS, NETWORK, CPU, MEMORY?) to get an estimate 'good' number to use. -- To unsubscribe from this list: send the line "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 01/11] refs: make is_branch public
On 09/11/15 17:03, Michael Haggerty wrote: > From: David Turner > > is_branch was already non-static, but this patch declares it in the > header. > > Signed-off-by: Ronnie Sahlberg > Signed-off-by: David Turner > Signed-off-by: Junio C Hamano > Signed-off-by: Michael Haggerty > --- > refs.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/refs.h b/refs.h > index 6d30c98..39b8edc 100644 > --- a/refs.h > +++ b/refs.h > @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char > *msg_fmt, const struct st > */ > int pack_refs(unsigned int flags); > > +int is_branch(const char *refname); > + > /* > * Flags controlling ref_transaction_update(), ref_transaction_create(), etc. > * REF_NODEREF: act on the ref directly, instead of dereferencing > I don't understand, is_branch() is already declared in refs.h, see line 67. This is true in master, next and pu now appears to have two declarations. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
On Mon, Nov 9, 2015 at 10:32 AM, Victor Leschuk wrote: > On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk >> num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT; > > Actually I have never said the nCPUs played main role in it. T The pseudo-code you sent disagrees. Not that "online_cpus() <= 1" is likely to ever be really an issue on any development platform from the last decade. However, I do have to admit that that "online_cpus()" check goes back a long time, so I guess I can't really blame you. At least in the index preloading, I was very conscious of the IO issues. It doesn't actually make a big difference on traditional disks (seek times dominate, and concurrent IO often doesn't help at all), but the reason I keep on bringing up NFS is that back when I used CVS (oh, the horrors), I *also* worked at a company that did everything over NFS. CPU ended up almost never being the limiting factor for any SCM operation. So I don't have a very good idea of *what* we should use for automatic thread detection, but I'm pretty sure online_cpu's should not be it. Except, like Jeff mentioned, for pack formation (which does tend to be all about CPU). Sadly, detecting what kind of filesystem you are on and how well cached it is, is really pretty hard. Even when you have OS-specific knowledge, and can look up the *type* of the filesystem, what often matters more is things like "is the filesystem on a rotational media or using flash?" etc. In the meantime I'd argue for just getting rid of the online_cpu's check, because (a) I think it's actively misleading (b) the threaded grep probably doesn't hurt much even on a single CPU, and the _potential_ upside from IO could easily dwarf the cost. (c) do developers actually have single-core machines any more? But if somebody can come up with a smarter model, that would certainly be good too. The IO advantages really don't tend to be there for rotational media, but for both flash and network filesystems, threaded IO can be a huge deal. Linus -- To unsubscribe from this list: send the line "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: fetch unshallow fails to honor dry-run
[+cc Duy for shallow expertise] On Sun, Oct 25, 2015 at 10:16:00AM +0100, Tim Janik wrote: > git fetch --dry-run modifies the repository if --unshallow is passed: > > $ git --version > git version 2.1.4 > $ git fetch --dry-run --unshallow > remote: Counting objects: 30603, done. > remote: Compressing objects: 100% (6843/6843), done. > remote: Total 30603 (delta 24564), reused 29164 (delta 23386) > Receiving objects: 100% (30603/30603), 5.42 MiB | 0 bytes/s, done. > Resolving deltas: 100% (24564/24564), completed with 317 local objects. > remote: Counting objects: 7, done. > remote: Compressing objects: 100% (7/7), done. > remote: Total 7 (delta 0), reused 6 (delta 0) > Unpacking objects: 100% (7/7), done. Hmm. I think that is because --dry-run is effectively "transfer the objects, but do not update refs". So by fetching the objects, we've effectively deepened the repository, whether we update the refs or not. That being said, I suspect nobody has really thought hard about the interaction of these two flags. And while obviously we update the object database with a dry-run fetch, I can see the reasoning that we should not touch the .git/shallow file, even if we have the objects. Naively, something like this patch might help, but I have no idea if it causes other problems. diff --git a/builtin/fetch.c b/builtin/fetch.c index 4ce4fa0..24aa331 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -870,7 +870,7 @@ static struct transport *prepare_transport(struct remote *remote) set_option(transport, TRANS_OPT_KEEP, "yes"); if (depth) set_option(transport, TRANS_OPT_DEPTH, depth); - if (update_shallow) + if (update_shallow && !dry_run) set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes"); return transport; } > I actually tried --dry-run --unshallow in order to find a way to > detect in a script if the current git repository is shallow or not. > Better suggestions to find this out are very welcome. You can look for .git/shallow. I don't know if we've documented that anywhere as a public interface, but I think it should be safe to rely on. -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 v7 07/11] refs/refs-internal.h: new header file
On 09/11/15 17:03, Michael Haggerty wrote: > There are a number of constants, structs, and static functions defined > in refs.c and treated as private to the references module. But we want > to support multiple reference backends within the reference module, > and those backends will need access to some heretofore private > declarations. > > We don't want those declarations to be visible to non-refs code, so we > don't want to move them to refs.h. Instead, add a new header file, > refs/refs-internal.h, that is intended to be included only from within > the refs module. Make some functions non-static and move some > declarations (and their corresponding docstrings) from refs.c to this > file. > > In a moment we will add more content to the "refs" subdirectory. > > Signed-off-by: Michael Haggerty > --- > refs.c | 175 +++-- > refs/refs-internal.h | 182 > +++ > 2 files changed, 191 insertions(+), 166 deletions(-) > create mode 100644 refs/refs-internal.h > > diff --git a/refs.c b/refs.c > index f48c58a..9aff0c8 100644 > --- a/refs.c > +++ b/refs.c > @@ -1,6 +1,6 @@ > #include "cache.h" > +#include "refs/refs-internal.h" > #include "lockfile.h" > -#include "refs.h" This looked wrong to me, until I had read the remainder of the patch and noticed that the 'internal' header #included the 'public' interface header. Unfortunately, this still feels wrong to me! I would rather that the internal header _not_ include the public header (so, include them _both_ when necessary). Just my opinion, which you can simply ignore. :-D ATB, Ramsay Jones -- To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
. > In the meantime I'd argue for just getting rid of the online_cpu's > check, because > (a) I think it's actively misleading > (b) the threaded grep probably doesn't hurt much even on a single > CPU, and the _potential_ upside from IO could easily dwarf the cost. > (c) do developers actually have single-core machines any more? Linus So as far as I understood your point at current moment would be better to leave online_cpus() check behind, keep the default threads value (and do so for other threaded programs, in separate patches of course). After that we may focus (in future) on developing smarter heuristics for parallelity-relationed params. Also we should specify in documentation that number of your CPUs may not the optimal value and customer should find his own best values based on other circumstances. Correct? -- Victor-- To unsubscribe from this list: send the line "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/5] Use watchman to reduce index refresh time
On Tue, Nov 3, 2015 at 10:21 AM, Duy Nguyen wrote: > On Mon, Nov 2, 2015 at 8:23 PM, Duy Nguyen wrote: >> On Mon, Nov 2, 2015 at 3:54 PM, Paolo Ciarrocchi >> wrote: >>> On Sun, Nov 1, 2015 at 2:55 PM, Nguyễn Thái Ngọc Duy >>> wrote: >>> >>> Hi Duy, >>> This series builds on top of the index-helper series I just sent and uses watchman to keep track of file changes in order to avoid lstat() at refresh time. The series can also be found at [1] When I started this work, watchman did not support Windows yet. It does now, even if still experimental [2]. So Windows people, please try it out if you have time. To put all pieces so far together, we have split-index to reduce index write time, untracked cache to reduce I/O as well as computation for .gitignore, index-helper for index read time and this series for lstat() at refresh time. The remaining piece is killing lstat() from untracked cache, but right now it's just some idea and incomplete code. >>> >>> Did you manage to measure the speedup introduced by this series? >> >> It was from last year. I may have measured it but because I didn't >> save it in the commit message, it was lost anyway. Installing watchman >> and measuring with webkit.git soon.. > > Test repo: webkit.git with 104665 tracked files, 5615 untracked, > 3517 dirs. Best numbers out of a few tries. This is best case > scenario. Normal usage could have worse numbers. Thank you for the tests! I tried to replicate your results on webkit.git with 184672 tracked files, 5631 untracked, 9330 dirs. Also best numbers out of a few tries. > There is something strange about the "-uno" measurements. I don't > think watchman+untracked cache can beat -uno.. Maybe I did something > wrong. > > 0m0.383s index v2 > 0m0.351s index v4 > 0m0.352s v2 split-index > 0m0.309s v2 split index-helper > 0m0.159s v2 split helper untracked-cache > 0m0.123s v2 split helper "status -uno" > 0m0.098s v2 split helper untracked watchman > 0m0.071s v2 split helper watchman "status -uno" I got the following results from "time git status ...": 0m0.774s 0m0.799s split 0m0.766s split helper 0m0.335s split helper untracked 0m0.232s split helper untracked -uno 0m0.284s split helper untracked watchman 0m0.188s split helper untracked watchman -uno Using David's series I get worse results than all of the above but I guess it's because his series is based on an ancient git version (v2.0.0-rc0). > Note, the watchman series needs > s/free_watchman_shm/release_watchman_shm/ (I didn't do a good job > of testing after rebase). And there's a small bug in index-helper > --detach code writing incorrect PID.. I did the s/free_watchman_shm/release_watchman_shm/ change, but I didn't notice the index-helper bug. Thanks, Christian. -- To unsubscribe from this list: send the line "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 01/11] refs: make is_branch public
On Mon, 2015-11-09 at 18:54 +, Ramsay Jones wrote: > > On 09/11/15 17:03, Michael Haggerty wrote: > > From: David Turner > > > > is_branch was already non-static, but this patch declares it in the > > header. > > > > Signed-off-by: Ronnie Sahlberg > > Signed-off-by: David Turner > > Signed-off-by: Junio C Hamano > > Signed-off-by: Michael Haggerty > > --- > > refs.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/refs.h b/refs.h > > index 6d30c98..39b8edc 100644 > > --- a/refs.h > > +++ b/refs.h > > @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char > > *msg_fmt, const struct st > > */ > > int pack_refs(unsigned int flags); > > > > +int is_branch(const char *refname); > > + > > /* > > * Flags controlling ref_transaction_update(), ref_transaction_create(), > > etc. > > * REF_NODEREF: act on the ref directly, instead of dereferencing > > > > I don't understand, is_branch() is already declared in refs.h, see line 67. > > This is true in master, next and pu now appears to have two declarations. Agreed. This was necessary at the time I started on this work, but this patch apparently stuck around past the time it was needed. Please drop. -- To unsubscribe from this list: send the line "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 07/11] refs/refs-internal.h: new header file
On Mon, 2015-11-09 at 19:46 +, Ramsay Jones wrote: > > On 09/11/15 17:03, Michael Haggerty wrote: > > There are a number of constants, structs, and static functions defined > > in refs.c and treated as private to the references module. But we want > > to support multiple reference backends within the reference module, > > and those backends will need access to some heretofore private > > declarations. > > > > We don't want those declarations to be visible to non-refs code, so we > > don't want to move them to refs.h. Instead, add a new header file, > > refs/refs-internal.h, that is intended to be included only from within > > the refs module. Make some functions non-static and move some > > declarations (and their corresponding docstrings) from refs.c to this > > file. > > > > In a moment we will add more content to the "refs" subdirectory. > > > > Signed-off-by: Michael Haggerty > > --- > > refs.c | 175 > > +++-- > > refs/refs-internal.h | 182 > > +++ > > 2 files changed, 191 insertions(+), 166 deletions(-) > > create mode 100644 refs/refs-internal.h > > > > diff --git a/refs.c b/refs.c > > index f48c58a..9aff0c8 100644 > > --- a/refs.c > > +++ b/refs.c > > @@ -1,6 +1,6 @@ > > #include "cache.h" > > +#include "refs/refs-internal.h" > > #include "lockfile.h" > > -#include "refs.h" > > This looked wrong to me, until I had read the remainder of the > patch and noticed that the 'internal' header #included the > 'public' interface header. > > Unfortunately, this still feels wrong to me! I would rather that > the internal header _not_ include the public header (so, include > them _both_ when necessary). Just my opinion, which you can simply > ignore. :-D +1 on this. -- To unsubscribe from this list: send the line "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] refs backend pre-vtable
On Mon, 2015-11-09 at 18:03 +0100, Michael Haggerty wrote: > This is another reroll of the pre-vtable part of the refs-backend > patch series dt/refs-backend-pre-vtable. v6 [1] proved cumbersome > because it conflicted messily with lf/ref-is-hidden-namespace [2]. The > conflicts were partly due to the motion of code across files but, even > worse, due to the change of order of function definitions between old > and new files. > > So I have heavily "optimized" this reroll for reviewability and to > minimize conflicts with other work in the area. The only such work > that I know of is lf/ref-is-hidden-namespace, which can now be merged > with this series *without conflicts*. Modulo Ramsey's comments, this works for me. AIUI, Ronnie's original patchset was written the way it was to avoid merge conflicts. Because of the changes in the interim, it turns out not to quite work that way. So it's totally OK with me to write it this new way. -- To unsubscribe from this list: send the line "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 v4] Add git-grep threads param
Correct. I think adding the option (both to command line and to config file) is good, as long as the IO issues are documented. And default to just the fixed number of threads for now - and with the option, maybe people can then more easily try out different scenarios and maybe improve on the particular choice of fixed number of threads. > Or maybe the default should be something that isn't even described by any > particular fixed number. > For example, maybe the default that value could be something quite dynamic: > start off with a single thread (or a very low thread number) and just set a > timer. After 0.1s, if CPU usage is low, start more threads. After another > 0.1s, if that improved things, maybe > we could add still more threads... > Note that "CPU usage is low" can be hard to get portably, but we could > approximate it with "how much work did we actually get done". If we only > grepped a couple of files, that might be because of IO issues. And if speed > does not improve when we move from a > single thread to, say, four threads, then we should probably *not* increase > the thread number again at 0.2s. > So I think there are many possible avenues to explore that might be > interesting. I do *not* think that "online_cpus()" is one of them, > except > perhaps as a very rough measure of "is this a beefy system or not" (but even > that is questionable - 32 CPUs is definitely > likely "very beefy, so use lots of threads", but even 8 CPUs might still be > just a phone, and I'm not sure that tells you a lot, really. Linus Here is my version of note for Documentaion: Number of grep worker threads, use it to tune up performance on your machines. Leave it unset or set to "0" if you want to use default number (currently default number is 8 for all systems, however this behavior can be changed in future versions to better suite your hardware and circumstances). -- Victor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] remote-http(s): Support SOCKS proxies
Johannes Schindelin writes:
>Hi,
>
>On Tue, 27 Oct 2015, Johannes Schindelin wrote:
>
>> On Mon, 26 Oct 2015, Junio C Hamano wrote:
>>
>> > James McCoy writes:
>> >
>> > >> The code looks OK but the last paragraph makes _us_ worried. What
>> > >> is the licensing status of the original at SO?
>> > >
>> > > According to Stackoverflow[0],
>> > >
>> > > As noted in the Stack Exchange Terms of Service[1] and in the footer of
>> > > every page, all user contributions are licensed under Creative Commons
>> > > Attribution-Share Alike[2]. Proper attribution[3] is required if you
>> > > republish any Stack Exchange content.
>> > >
>> > > [0]: https://stackoverflow.com/help/licensing
>> >
>> > Yes, and (please correct me if I am wrong--this is one of the times
>> > I hope I am wrong!) I thought BY-SA does not mesh well with GPLv2,
>> > in which case we cannot use this patch (instead somebody has to
>> > reimplement the same without copying).
>>
>> Pat, could you please allow us to insert your SOB?
>
>On second thought... Junio, could you please sanity-check my claim that
>this patch:
>
>-- snip --
>@@ -465,6 +465,17 @@ static CURL *get_curl_handle(void)
>
>if (curl_http_proxy) {
>curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>+#if LIBCURL_VERSION_NUM >= 0x071800
>+ if (starts_with(curl_http_proxy, "socks5"))
>+ curl_easy_setopt(result,
>+ CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
>+ else if (starts_with(curl_http_proxy, "socks4a"))
>+ curl_easy_setopt(result,
>+ CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4A);
>+ else if (starts_with(curl_http_proxy, "socks"))
>+ curl_easy_setopt(result,
>+ CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
>+#endif
>}
> #if LIBCURL_VERSION_NUM >= 0x070a07
>curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>-- snap --
>
>cannot be copyrighted because it is pretty much the only way to implement
>said functionality?
>
>Still, Pat, if you find the time, could you please simply relicense your
>patch (I know that you are fine with it, but we need an explicit
>statement)?
>
>Ciao,
>Johannes
A bit late to the party but 'yes'. Frankly by posting something to SO I
rather consider it public domain but I hereby license this patch as
required for use by the Git project.
Signed-off-by: Pat Thoyts
--
Pat Thoytshttp://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
--
To unsubscribe from this list: send the line "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-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?
On Mon, Nov 9, 2015 at 10:53 AM, Jeff King wrote:
> Yes, but with a proper commit message, and an update to
> Documentation/config.txt. :)
Right, see attached.
>
> Automated tests would be nice, but I suspect it may be too complicated
> to be worth it.
I attempted
test_ignore_sighup ()
{
mkdir "$HOME/.git-credential-cache" &&
chmod 0700 "$HOME/.git-credential-cache" &&
git -c credentialCache.ignoreSIGHUP=true credential-cache--daemon
"$HOME/.git-credential-cache/socket" &
kill -SIGHUP $! &&
ps $!
}
test_expect_success 'credentialCache.ignoreSIGHUP works' 'test_ignore_sighup'
but it does't pass (testing manually by running
./git-credential-cache--daemon $HOME/.git-credential-cache/test-socket
& and then kill -HUP does work).
>
> I don't think we should use the credential.X.* namespace here. That is
> already reserved for credential setup for URLs matching "X".
>
> Probably "credentialCache.ignoreSIGHUP" would be better. Or maybe
> "credential-cache". We usually avoid dashes in our config names, but
> in this case it matches the program name.
I went with "credentialCache.ignoreSIGHUP".
>
> Also, we usually spell config names as all-lowercase in the code. The
> older callback-interface config code needed this (since we just strcmp'd
> the keys against a normalized case). I think git_config_get_bool() will
> normalize the key we feed it, but I'd rather stay consistent.
Oh, I didn't even realize git config names were case insensitive.
> I don't think you need to pop the tempfile handler here. You can simply
> sigchain_push() the SIG_IGN, and since we won't ever pop and propagate
> that, it doesn't matter what is under it.
Yup.
From 5fc95b6e2f956403da6845fc3ced83b21bee7bb0 Mon Sep 17 00:00:00 2001
From: Noam Postavsky
Date: Mon, 9 Nov 2015 19:26:29 -0500
Subject: [PATCH] credential-cache: new option to ignore sighup
Introduce new option "credentialCache.ignoreSIGHUP" which stops
git-credential-cache--daemon from quitting on SIGHUP. This is useful
when "git push" is started from Emacs, because all child
processes (including the daemon) will receive a SIGHUP when "git push"
exits.
---
Documentation/config.txt | 3 +++
credential-cache--daemon.c | 7 +++
2 files changed, 10 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4d3cb10..e5b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1122,6 +1122,9 @@ credential..*::
example.com. See linkgit:gitcredentials[7] for details on how URLs are
matched.
+credentialCache.ignoreSIGHUP::
+ Tell git-credential-cache--daemon to ignore SIGHUP, instead of quitting.
+
include::diff-config.txt[]
difftool..path::
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index eef6fce..6cda9c0 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -256,6 +256,9 @@ int main(int argc, const char **argv)
OPT_END()
};
+ int ignore_sighup = 0;
+ git_config_get_bool("credentialcache.ignoresighup", &ignore_sighup);
+
argc = parse_options(argc, argv, NULL, options, usage, 0);
socket_path = argv[0];
@@ -264,6 +267,10 @@ int main(int argc, const char **argv)
check_socket_directory(socket_path);
register_tempfile(&socket_file, socket_path);
+
+ if (ignore_sighup)
+ signal(SIGHUP, SIG_IGN);
+
serve_cache(socket_path, debug);
delete_tempfile(&socket_file);
--
2.6.1
Re: [PATCH] git-svn: improve rebase/mkdirs performance
Dair Grant wrote:
> This takes 10 minutes to process when using a single hash, vs
> 3 seconds with a hash of hashes.
>
> Signed-off-by: Dair Grant
Thanks! I've made some minor edits, will push the following out
to git://bogomips.org/git-svn.git for Junio to pull in a day
or so.
Btw, feel free to Cc: me on any git-(svn|Perl)-related stuff. Likewise,
Cc: any regular contributor/maintainer on any subsystem you work on. We
might not always answer, but appreciate being Cc:-ed anyways just in
case.
-- 8< ---
From: Dair Grant
Subject: [PATCH] git-svn: improve rebase/mkdirs performance
Processing empty_dir directives becomes extremely slow for svn
repositories with a large enough history.
This is due to using a single hash to store the list of empty
directories, with the expensive step being purging items from
that hash using grep+delete.
Storing directories in a hash of hashes improves the performance
of this purge step and removes a potentially lengthy delay after
every rebase/mkdirs command.
The svn repository with this behaviour has 110K commits with
unhandled.log containing 170K empty_dir directives.
This takes 10 minutes to process when using a single hash, vs
3 seconds with a hash of hashes.
[ew: wrap comments, shorten initializations, avoid unnecessary spaces]
Signed-off-by: Dair Grant
Signed-off-by: Eric Wong
---
perl/Git/SVN.pm | 84 +++--
1 file changed, 76 insertions(+), 8 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 152fb7e..b2c14e2 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1211,20 +1211,87 @@ sub do_fetch {
sub mkemptydirs {
my ($self, $r) = @_;
+ # add/remove/collect a paths table
+ #
+ # Paths are split into a tree of nodes, stored as a hash of hashes.
+ #
+ # Each node contains a 'path' entry for the path (if any) associated
+ # with that node and a 'children' entry for any nodes under that
+ # location.
+ #
+ # Removing a path requires a hash lookup for each component then
+ # dropping that node (and anything under it), which is substantially
+ # faster than a grep slice into a single hash of paths for large
+ # numbers of paths.
+ #
+ # For a large (200K) number of empty_dir directives this reduces
+ # scanning time to 3 seconds vs 10 minutes for grep+delete on a single
+ # hash of paths.
+ sub add_path {
+ my ($paths_table, $path) = @_;
+ my $node_ref;
+
+ foreach my $x (split('/', $path)) {
+ if (!exists($paths_table->{$x})) {
+ $paths_table->{$x} = { children => {} };
+ }
+
+ $node_ref = $paths_table->{$x};
+ $paths_table = $paths_table->{$x}->{children};
+ }
+
+ $node_ref->{path} = $path;
+ }
+
+ sub remove_path {
+ my ($paths_table, $path) = @_;
+ my $nodes_ref;
+ my $node_name;
+
+ foreach my $x (split('/', $path)) {
+ if (!exists($paths_table->{$x})) {
+ return;
+ }
+
+ $nodes_ref = $paths_table;
+ $node_name = $x;
+
+ $paths_table = $paths_table->{$x}->{children};
+ }
+
+ delete($nodes_ref->{$node_name});
+ }
+
+ sub collect_paths {
+ my ($paths_table, $paths_ref) = @_;
+
+ foreach my $v (values %$paths_table) {
+ my $p = $v->{path};
+ my $c = $v->{children};
+
+ collect_paths($c, $paths_ref);
+
+ if (defined($p)) {
+ push(@$paths_ref, $p);
+ }
+ }
+ }
+
sub scan {
- my ($r, $empty_dirs, $line) = @_;
+ my ($r, $paths_table, $line) = @_;
if (defined $r && $line =~ /^r(\d+)$/) {
return 0 if $1 > $r;
} elsif ($line =~ /^ \+empty_dir: (.+)$/) {
- $empty_dirs->{$1} = 1;
+ add_path($paths_table, $1);
} elsif ($line =~ /^ \-empty_dir: (.+)$/) {
- my @d = grep {m[^\Q$1\E(/|$)]} (keys %$empty_dirs);
- delete @$empty_dirs{@d};
+ remove_path($paths_table, $1);
}
1; # continue
};
- my %empty_dirs = ();
+ my @empty_dirs;
+ my %paths_table;
+
my $gz_file = "$self->{dir}/unhandled.log.gz";
if (-f $gz_file) {
if (!can_compress()) {
@@ -1235,7 +1302,7 @@ sub mkemptydirs {
die "Unable to open
[PATCH v4 00/12] object_id part 2
This is the second in a series of conversions to struct object_id. This series converts more of the refs code and struct object to use struct object_id. It introduces an additional helper function, has_object_file, which is the equivalent of has_sha1_file. The name was chosen to be slightly more logical than has_oid_file, although it can be changed if desired. This series splits out the conversion of struct object into the introduction of a helper macro, the use of that macro in several areas, the conversion of other struct object uses and update of the macro, and the removal of the macro. The final patch of that set (patch 11) was implemented entirely by the use of a Perl script, which will be included for inspection. Patches 1 through 7 remain unchanged except for the rebase and the necessary changes due to other topics. Necessary changes were minimal. I think this series is ready to be picked up, although I'm happy to make changes or refinements if people have comments. This series is also available in the object-id-part2 branch from https://github.com/bk2204/git.git https://git.crustytoothpaste.net/git/bmc/git.git Changes from v3: * Rebase on latest master. * Drop the first patch. * Re-run the script to expand get_object_hash. brian m. carlson (12): sha1_file: introduce has_object_file helper. Convert struct ref to use object_id. add_sought_entry_mem: convert to struct object_id parse_fetch: convert to use struct object_id get_remote_heads: convert to struct object_id push_refs_with_export: convert to struct object_id ref_newer: convert to use struct object_id object: introduce get_object_hash macro. Add several uses of get_object_hash. Convert struct object to object_id Remove get_object_hash. remote: convert functions to struct object_id archive.c| 6 +-- bisect.c | 10 ++-- branch.c | 2 +- builtin/am.c | 2 +- builtin/blame.c | 56 +++ builtin/branch.c | 2 +- builtin/checkout.c | 22 - builtin/clone.c | 18 builtin/commit-tree.c| 4 +- builtin/commit.c | 8 ++-- builtin/describe.c | 20 builtin/diff-tree.c | 12 ++--- builtin/diff.c | 12 ++--- builtin/fast-export.c| 34 +++--- builtin/fetch-pack.c | 14 +++--- builtin/fetch.c | 54 +++--- builtin/fmt-merge-msg.c | 6 +-- builtin/fsck.c | 36 +++ builtin/grep.c | 6 +-- builtin/index-pack.c | 10 ++-- builtin/log.c| 36 +++ builtin/ls-remote.c | 2 +- builtin/merge-base.c | 8 ++-- builtin/merge-tree.c | 6 +-- builtin/merge.c | 60 builtin/name-rev.c | 12 ++--- builtin/notes.c | 2 +- builtin/pack-objects.c | 16 +++ builtin/pull.c | 2 +- builtin/receive-pack.c | 2 +- builtin/reflog.c | 4 +- builtin/remote.c | 12 ++--- builtin/replace.c| 6 +-- builtin/reset.c | 30 ++-- builtin/rev-list.c | 18 builtin/rev-parse.c | 4 +- builtin/shortlog.c | 2 +- builtin/show-branch.c| 8 ++-- builtin/unpack-objects.c | 10 ++-- builtin/worktree.c | 2 +- bundle.c | 20 cache-tree.c | 2 +- cache.h | 3 ++ combine-diff.c | 4 +- commit.c | 32 ++--- connect.c| 22 + decorate.c | 2 +- diff-lib.c | 2 +- fetch-pack.c | 24 +- fsck.c | 14 +++--- http-backend.c | 2 +- http-push.c | 86 +-- http.c | 2 +- line-log.c | 6 +-- list-objects.c | 4 +- log-tree.c | 40 merge-blobs.c| 4 +- merge-recursive.c| 26 +-- merge.c | 2 +- notes-merge.c| 24 +- object.c | 8 ++-- object.h | 2 +- pack-bitmap-write.c | 16 +++ pack-bitmap.c| 34 +++--- patch-ids.c | 6 +-- pretty.c | 18 ref-filter.c | 14 +++--- refs.c | 2 +- remote-curl.c| 20 remote.c | 116 +++ remote.h | 8 ++-- revision.c | 48 ++-- send-pack.c | 16 +++ sequencer.c | 40 server-info.c| 2 +- sha1_file.c | 5 ++ sha1_name.c | 20 shallow.c| 6 +-- submodule.c | 8 ++-- tag.c
[PATCH v4 01/12] sha1_file: introduce has_object_file helper.
Add has_object_file, which is a wrapper around has_sha1_file, but for
struct object_id.
Signed-off-by: brian m. carlson
---
cache.h | 3 +++
sha1_file.c | 5 +
2 files changed, 8 insertions(+)
diff --git a/cache.h b/cache.h
index 3ba0b8f3..27bc121b 100644
--- a/cache.h
+++ b/cache.h
@@ -1030,6 +1030,9 @@ static inline int has_sha1_file(const unsigned char *sha1)
return has_sha1_file_with_flags(sha1, 0);
}
+/* Same as the above, except for struct object_id. */
+extern int has_object_file(const struct object_id *oid);
+
/*
* Return true iff an alternate object database has a loose object
* with the specified name. This function does not respect replace
diff --git a/sha1_file.c b/sha1_file.c
index c5b31de9..59515b77 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3227,6 +3227,11 @@ int has_sha1_file_with_flags(const unsigned char *sha1,
int flags)
return find_pack_entry(sha1, &e);
}
+int has_object_file(const struct object_id *oid)
+{
+ return has_sha1_file(oid->hash);
+}
+
static void check_tree(const void *buf, size_t size)
{
struct tree_desc desc;
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 v4 03/12] add_sought_entry_mem: convert to struct object_id
Convert this function to use struct object_id. Express several
hardcoded constants in terms of GIT_SHA1_HEXSZ.
Signed-off-by: brian m. carlson
---
builtin/fetch-pack.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 19215b33..cf3019e0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -14,12 +14,14 @@ static void add_sought_entry_mem(struct ref ***sought, int
*nr, int *alloc,
const char *name, int namelen)
{
struct ref *ref = xcalloc(1, sizeof(*ref) + namelen + 1);
- unsigned char sha1[20];
+ struct object_id oid;
+ const int chunksz = GIT_SHA1_HEXSZ + 1;
- if (namelen > 41 && name[40] == ' ' && !get_sha1_hex(name, sha1)) {
- hashcpy(ref->old_oid.hash, sha1);
- name += 41;
- namelen -= 41;
+ if (namelen > chunksz && name[chunksz - 1] == ' ' &&
+ !get_oid_hex(name, &oid)) {
+ oidcpy(&ref->old_oid, &oid);
+ name += chunksz;
+ namelen -= chunksz;
}
memcpy(ref->name, name, namelen);
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 v4 10/12] Convert struct object to object_id
struct object is one of the major data structures dealing with object
IDs. Convert it to use struct object_id instead of an unsigned char
array. Convert get_object_hash to refer to the new member as well.
Signed-off-by: brian m. carlson
---
bisect.c | 4 ++--
builtin/am.c | 2 +-
builtin/blame.c | 32
builtin/checkout.c | 12 ++--
builtin/commit-tree.c| 4 ++--
builtin/describe.c | 14 +++---
builtin/diff-tree.c | 4 ++--
builtin/fast-export.c| 22 +++---
builtin/fmt-merge-msg.c | 2 +-
builtin/fsck.c | 32
builtin/grep.c | 4 ++--
builtin/index-pack.c | 8
builtin/log.c| 18 +-
builtin/merge-base.c | 8
builtin/merge-tree.c | 2 +-
builtin/merge.c | 10 +-
builtin/name-rev.c | 8
builtin/pack-objects.c | 2 +-
builtin/pull.c | 2 +-
builtin/replace.c| 6 +++---
builtin/reset.c | 28 ++--
builtin/rev-list.c | 14 +++---
builtin/shortlog.c | 2 +-
builtin/show-branch.c| 4 ++--
builtin/unpack-objects.c | 8
builtin/worktree.c | 2 +-
bundle.c | 18 +-
commit.c | 20 ++--
fetch-pack.c | 2 +-
fsck.c | 8
http-backend.c | 2 +-
http-push.c | 22 +++---
list-objects.c | 4 ++--
log-tree.c | 20 ++--
merge-recursive.c| 14 +++---
merge.c | 2 +-
notes-merge.c| 4 ++--
object.c | 2 +-
object.h | 4 ++--
pack-bitmap-write.c | 2 +-
pack-bitmap.c| 8
pretty.c | 10 +-
ref-filter.c | 8
remote.c | 4 ++--
revision.c | 32
sequencer.c | 22 +++---
server-info.c| 2 +-
sha1_name.c | 4 ++--
shallow.c| 4 ++--
submodule.c | 6 +++---
tag.c| 4 ++--
tree.c | 6 +++---
upload-pack.c| 16
walker.c | 8
54 files changed, 256 insertions(+), 256 deletions(-)
diff --git a/bisect.c b/bisect.c
index 59e86369..54166f00 100644
--- a/bisect.c
+++ b/bisect.c
@@ -193,7 +193,7 @@ static int compare_commit_dist(const void *a_, const void
*b_)
b = (struct commit_dist *)b_;
if (a->distance != b->distance)
return b->distance - a->distance; /* desc sort */
- return hashcmp(a->commit->object.sha1, b->commit->object.sha1);
+ return oidcmp(&a->commit->object.oid, &b->commit->object.oid);
}
static struct commit_list *best_bisection_sorted(struct commit_list *list, int
nr)
@@ -575,7 +575,7 @@ static struct commit_list *skip_away(struct commit_list
*list, int count)
for (i = 0; cur; cur = cur->next, i++) {
if (i == index) {
- if (hashcmp(cur->item->object.sha1,
current_bad_oid->hash))
+ if (oidcmp(&cur->item->object.oid, current_bad_oid))
return cur;
if (previous)
return previous;
diff --git a/builtin/am.c b/builtin/am.c
index f1a25ab6..9fb42fdd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1441,7 +1441,7 @@ static void get_commit_info(struct am_state *state,
struct commit *commit)
assert(!state->msg);
msg = strstr(buffer, "\n\n");
if (!msg)
- die(_("unable to parse commit %s"),
sha1_to_hex(commit->object.sha1));
+ die(_("unable to parse commit %s"),
oid_to_hex(&commit->object.oid));
state->msg = xstrdup(msg + 2);
state->msg_len = strlen(state->msg);
}
diff --git a/builtin/blame.c b/builtin/blame.c
index c6ea9774..28c48bd4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -557,7 +557,7 @@ static struct origin *find_origin(struct scoreboard *sb,
PATHSPEC_LITERAL_PATH, "", paths);
diff_setup_done(&diff_opts);
- if (is_null_sha1(origin->commit->object.sha1))
+ if (is_null_oid(&origin->commit->object.oid))
do_diff_cache(get_object_hash(parent->tree->object),
&diff_opts);
else
diff_tree_sha1(get_object_hash(parent->tree->object),
@@ -627,7 +627,7 @@ static struct origin *find_rename(struct scoreboard *sb,
diff_opts.single_follow = origin->path;
diff_setup_done(&diff_opts);
- if (is_null_sha1(origin->commit->object.sha1))
+ if (is_null_oid(&origin->commit->object.oid))
do_diff_cach
[PATCH v4 02/12] Convert struct ref to use object_id.
Use struct object_id in three fields in struct ref and convert all the
necessary places that use it.
Signed-off-by: brian m. carlson
---
builtin/clone.c| 16 +++---
builtin/fetch-pack.c | 4 ++--
builtin/fetch.c| 50 +--
builtin/ls-remote.c| 2 +-
builtin/receive-pack.c | 2 +-
builtin/remote.c | 12 +--
connect.c | 2 +-
fetch-pack.c | 18
http-push.c| 44 +++---
http.c | 2 +-
remote-curl.c | 10 -
remote.c | 58 +-
remote.h | 6 +++---
send-pack.c| 16 +++---
transport-helper.c | 18
transport.c| 32 ++--
transport.h| 8 +++
walker.c | 2 +-
18 files changed, 151 insertions(+), 151 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index caae43e7..215d015f 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -559,7 +559,7 @@ static void write_remote_refs(const struct ref *local_refs)
for (r = local_refs; r; r = r->next) {
if (!r->peer_ref)
continue;
- if (ref_transaction_create(t, r->peer_ref->name, r->old_sha1,
+ if (ref_transaction_create(t, r->peer_ref->name,
r->old_oid.hash,
0, NULL, &err))
die("%s", err.buf);
}
@@ -579,9 +579,9 @@ static void write_followtags(const struct ref *refs, const
char *msg)
continue;
if (ends_with(ref->name, "^{}"))
continue;
- if (!has_sha1_file(ref->old_sha1))
+ if (!has_object_file(&ref->old_oid))
continue;
- update_ref(msg, ref->name, ref->old_sha1,
+ update_ref(msg, ref->name, ref->old_oid.hash,
NULL, 0, UPDATE_REFS_DIE_ON_ERR);
}
}
@@ -601,7 +601,7 @@ static int iterate_ref_map(void *cb_data, unsigned char
sha1[20])
if (!ref)
return -1;
- hashcpy(sha1, ref->old_sha1);
+ hashcpy(sha1, ref->old_oid.hash);
*rm = ref->next;
return 0;
}
@@ -650,12 +650,12 @@ static void update_head(const struct ref *our, const
struct ref *remote,
/* Local default branch link */
create_symref("HEAD", our->name, NULL);
if (!option_bare) {
- update_ref(msg, "HEAD", our->old_sha1, NULL, 0,
+ update_ref(msg, "HEAD", our->old_oid.hash, 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);
+ struct commit *c = lookup_commit_reference(our->old_oid.hash);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
update_ref(msg, "HEAD", c->object.sha1,
NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
@@ -665,7 +665,7 @@ static void update_head(const struct ref *our, const struct
ref *remote,
* HEAD points to a branch but we don't know which one.
* Detach HEAD in all these cases.
*/
- update_ref(msg, "HEAD", remote->old_sha1,
+ update_ref(msg, "HEAD", remote->old_oid.hash,
NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
}
}
@@ -1016,7 +1016,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
* remote HEAD check.
*/
for (ref = refs; ref; ref = ref->next)
- if (is_null_sha1(ref->old_sha1)) {
+ if (is_null_oid(&ref->old_oid)) {
complete_refs_before_fetch = 0;
break;
}
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4a6b340a..19215b33 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -17,7 +17,7 @@ static void add_sought_entry_mem(struct ref ***sought, int
*nr, int *alloc,
unsigned char sha1[20];
if (namelen > 41 && name[40] == ' ' && !get_sha1_hex(name, sha1)) {
- hashcpy(ref->old_sha1, sha1);
+ hashcpy(ref->old_oid.hash, sha1);
name += 41;
namelen -= 41;
}
@@ -210,7 +210,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char
*prefix)
while (ref) {
printf("%s %s\n",
- sha1_to_hex(ref->old_sha1), ref->name);
+ oid_to_hex(&ref->old_oid), ref->name);
[PATCH v4 09/12] Add several uses of get_object_hash.
Convert most instances where the sha1 member of struct object is
dereferenced to use get_object_hash. Most instances that are passed to
functions that have versions taking struct object_id, such as
get_sha1_hex/get_oid_hex, or instances that can be trivially converted
to use struct object_id instead, are not converted.
Signed-off-by: brian m. carlson
---
archive.c| 6 +++---
bisect.c | 6 +++---
branch.c | 2 +-
builtin/blame.c | 24 ++---
builtin/branch.c | 2 +-
builtin/checkout.c | 10 -
builtin/clone.c | 2 +-
builtin/commit.c | 8 +++
builtin/describe.c | 6 +++---
builtin/diff-tree.c | 8 +++
builtin/diff.c | 12 +--
builtin/fast-export.c| 12 +--
builtin/fetch.c | 4 ++--
builtin/fmt-merge-msg.c | 4 ++--
builtin/fsck.c | 4 ++--
builtin/grep.c | 2 +-
builtin/index-pack.c | 2 +-
builtin/log.c| 18
builtin/merge-tree.c | 4 ++--
builtin/merge.c | 54
builtin/name-rev.c | 6 +++---
builtin/notes.c | 2 +-
builtin/pack-objects.c | 14 ++---
builtin/reflog.c | 4 ++--
builtin/reset.c | 2 +-
builtin/rev-list.c | 4 ++--
builtin/rev-parse.c | 4 ++--
builtin/show-branch.c| 4 ++--
builtin/unpack-objects.c | 2 +-
bundle.c | 2 +-
cache-tree.c | 2 +-
combine-diff.c | 4 ++--
commit.c | 12 +--
decorate.c | 2 +-
diff-lib.c | 2 +-
fetch-pack.c | 4 ++--
fsck.c | 6 +++---
http-push.c | 20 +-
line-log.c | 6 +++---
log-tree.c | 20 +-
merge-blobs.c| 4 ++--
merge-recursive.c| 16 +++---
notes-merge.c| 20 +-
object.c | 6 +++---
pack-bitmap-write.c | 14 ++---
pack-bitmap.c| 26 +++
patch-ids.c | 6 +++---
pretty.c | 8 +++
ref-filter.c | 6 +++---
refs.c | 2 +-
revision.c | 16 +++---
sequencer.c | 18
sha1_name.c | 16 +++---
shallow.c| 2 +-
submodule.c | 2 +-
tag.c| 6 +++---
test-match-trees.c | 2 +-
tree.c | 4 ++--
upload-pack.c| 10 -
walker.c | 8 +++
wt-status.c | 2 +-
61 files changed, 253 insertions(+), 253 deletions(-)
diff --git a/archive.c b/archive.c
index 4ac86c83..af23001b 100644
--- a/archive.c
+++ b/archive.c
@@ -241,7 +241,7 @@ int write_archive_entries(struct archiver_args *args,
len--;
if (args->verbose)
fprintf(stderr, "%.*s\n", (int)len, args->base);
- err = write_entry(args, args->tree->object.sha1, args->base,
+ err = write_entry(args, get_object_hash(args->tree->object),
args->base,
len, 040777);
if (err)
return err;
@@ -374,7 +374,7 @@ static void parse_treeish_arg(const char **argv,
commit = lookup_commit_reference_gently(oid.hash, 1);
if (commit) {
- commit_sha1 = commit->object.sha1;
+ commit_sha1 = get_object_hash(commit->object);
archive_time = commit->date;
} else {
commit_sha1 = NULL;
@@ -390,7 +390,7 @@ static void parse_treeish_arg(const char **argv,
unsigned int mode;
int err;
- err = get_tree_entry(tree->object.sha1, prefix,
+ err = get_tree_entry(get_object_hash(tree->object), prefix,
tree_oid.hash, &mode);
if (err || !S_ISDIR(mode))
die("current working directory is untracked");
diff --git a/bisect.c b/bisect.c
index 053d1a2a..59e86369 100644
--- a/bisect.c
+++ b/bisect.c
@@ -500,7 +500,7 @@ struct commit_list *filter_skipped(struct commit_list *list,
struct commit_list *next = list->next;
list->next = NULL;
if (0 <= sha1_array_lookup(&skipped_revs,
- list->item->object.sha1)) {
+
get_object_hash(list->item->object))) {
if (skipped_first && !*skipped_first)
*skipped_first = 1;
/* Move current to tried list */
@@ -784,7 +784,7 @@ static void check_merge_bases(int no_checkout)
result = get_merge_bases_many(rev[0], r
[PATCH v4 05/12] get_remote_heads: convert to struct object_id
Replace an unsigned char array with struct object_id and express several
hard-coded constants in terms of GIT_SHA1_HEXSZ.
Signed-off-by: brian m. carlson
---
connect.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/connect.c b/connect.c
index d13a10a9..fd7ffe18 100644
--- a/connect.c
+++ b/connect.c
@@ -120,7 +120,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t
src_len,
*list = NULL;
for (;;) {
struct ref *ref;
- unsigned char old_sha1[20];
+ struct object_id old_oid;
char *name;
int len, name_len;
char *buffer = packet_buffer;
@@ -139,34 +139,36 @@ struct ref **get_remote_heads(int in, char *src_buf,
size_t src_len,
if (len > 4 && skip_prefix(buffer, "ERR ", &arg))
die("remote error: %s", arg);
- if (len == 48 && skip_prefix(buffer, "shallow ", &arg)) {
- if (get_sha1_hex(arg, old_sha1))
+ if (len == GIT_SHA1_HEXSZ + strlen("shallow ") &&
+ skip_prefix(buffer, "shallow ", &arg)) {
+ if (get_oid_hex(arg, &old_oid))
die("protocol error: expected shallow sha-1,
got '%s'", arg);
if (!shallow_points)
die("repository on the other end cannot be
shallow");
- sha1_array_append(shallow_points, old_sha1);
+ sha1_array_append(shallow_points, old_oid.hash);
continue;
}
- if (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] !=
' ')
+ if (len < GIT_SHA1_HEXSZ + 2 || get_oid_hex(buffer, &old_oid) ||
+ buffer[GIT_SHA1_HEXSZ] != ' ')
die("protocol error: expected sha/ref, got '%s'",
buffer);
- name = buffer + 41;
+ name = buffer + GIT_SHA1_HEXSZ + 1;
name_len = strlen(name);
- if (len != name_len + 41) {
+ if (len != name_len + GIT_SHA1_HEXSZ + 1) {
free(server_capabilities);
server_capabilities = xstrdup(name + name_len + 1);
}
if (extra_have && !strcmp(name, ".have")) {
- sha1_array_append(extra_have, old_sha1);
+ sha1_array_append(extra_have, old_oid.hash);
continue;
}
if (!check_ref(name, flags))
continue;
- ref = alloc_ref(buffer + 41);
- hashcpy(ref->old_oid.hash, old_sha1);
+ ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);
+ oidcpy(&ref->old_oid, &old_oid);
*list = ref;
list = &ref->next;
got_at_least_one_head = 1;
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 v4 06/12] push_refs_with_export: convert to struct object_id
Signed-off-by: brian m. carlson
---
transport-helper.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 91cb0e72..0eb3cf01 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -878,13 +878,13 @@ static int push_refs_with_export(struct transport
*transport,
for (ref = remote_refs; ref; ref = ref->next) {
char *private;
- unsigned char sha1[20];
+ struct object_id oid;
private = apply_refspecs(data->refspecs, data->refspec_nr,
ref->name);
- if (private && !get_sha1(private, sha1)) {
+ if (private && !get_sha1(private, oid.hash)) {
strbuf_addf(&buf, "^%s", private);
string_list_append(&revlist_args, strbuf_detach(&buf,
NULL));
- hashcpy(ref->old_oid.hash, sha1);
+ oidcpy(&ref->old_oid, &oid);
}
free(private);
@@ -898,7 +898,7 @@ static int push_refs_with_export(struct transport
*transport,
name = resolve_ref_unsafe(
ref->peer_ref->name,
RESOLVE_REF_READING,
-sha1, &flag);
+oid.hash, &flag);
if (!name || !(flag & REF_ISSYMREF))
name = ref->peer_ref->name;
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 v4 07/12] ref_newer: convert to use struct object_id
Convert ref_newer and its caller to use struct object_id instead of
unsigned char *.
Signed-off-by: brian m. carlson
---
builtin/remote.c | 2 +-
http-push.c | 4 ++--
remote.c | 8
remote.h | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 68dec162..6694cf20 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -417,7 +417,7 @@ static int get_push_ref_states(const struct ref
*remote_refs,
else if (is_null_oid(&ref->old_oid))
info->status = PUSH_STATUS_CREATE;
else if (has_object_file(&ref->old_oid) &&
-ref_newer(ref->new_oid.hash, ref->old_oid.hash))
+ref_newer(&ref->new_oid, &ref->old_oid))
info->status = PUSH_STATUS_FASTFORWARD;
else
info->status = PUSH_STATUS_OUTOFDATE;
diff --git a/http-push.c b/http-push.c
index f2db3a62..68ce3405 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1886,8 +1886,8 @@ int main(int argc, char **argv)
!is_null_oid(&ref->old_oid) &&
!ref->force) {
if (!has_object_file(&ref->old_oid) ||
- !ref_newer(ref->peer_ref->new_oid.hash,
- ref->old_oid.hash)) {
+ !ref_newer(&ref->peer_ref->new_oid,
+ &ref->old_oid)) {
/*
* We do not have the remote ref, or
* we know that the remote ref is not
diff --git a/remote.c b/remote.c
index 4beebadc..bdae86c6 100644
--- a/remote.c
+++ b/remote.c
@@ -1590,7 +1590,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int
send_mirror,
else if
(!lookup_commit_reference_gently(ref->old_oid.hash, 1) ||
!lookup_commit_reference_gently(ref->new_oid.hash, 1))
reject_reason = REF_STATUS_REJECT_NEEDS_FORCE;
- else if (!ref_newer(ref->new_oid.hash,
ref->old_oid.hash))
+ else if (!ref_newer(&ref->new_oid, &ref->old_oid))
reject_reason =
REF_STATUS_REJECT_NONFASTFORWARD;
}
@@ -1944,7 +1944,7 @@ static void unmark_and_free(struct commit_list *list,
unsigned int mark)
}
}
-int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1)
+int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
{
struct object *o;
struct commit *old, *new;
@@ -1955,12 +1955,12 @@ int ref_newer(const unsigned char *new_sha1, const
unsigned char *old_sha1)
* Both new and old must be commit-ish and new is descendant of
* old. Otherwise we require --force.
*/
- o = deref_tag(parse_object(old_sha1), NULL, 0);
+ o = deref_tag(parse_object(old_oid->hash), NULL, 0);
if (!o || o->type != OBJ_COMMIT)
return 0;
old = (struct commit *) o;
- o = deref_tag(parse_object(new_sha1), NULL, 0);
+ o = deref_tag(parse_object(new_oid->hash), NULL, 0);
if (!o || o->type != OBJ_COMMIT)
return 0;
new = (struct commit *) o;
diff --git a/remote.h b/remote.h
index 163ea5e8..4a039bae 100644
--- a/remote.h
+++ b/remote.h
@@ -150,7 +150,7 @@ extern struct ref **get_remote_heads(int in, char *src_buf,
size_t src_len,
struct sha1_array *shallow);
int resolve_remote_symref(struct ref *ref, struct ref *list);
-int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);
+int ref_newer(const struct object_id *new_oid, const struct object_id
*old_oid);
/*
* Remove and free all but the first of any entries in the input list
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 v4 12/12] remote: convert functions to struct object_id
Convert several unsigned char arrays to use struct object_id instead,
and change hard-coded 40-based constants to use GIT_SHA1_HEXSZ as well.
Signed-off-by: brian m. carlson
---
remote.c | 58 +-
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/remote.c b/remote.c
index 1abe1de2..9d34b5a5 100644
--- a/remote.c
+++ b/remote.c
@@ -456,7 +456,7 @@ static void alias_all_urls(void)
static void read_config(void)
{
static int loaded;
- unsigned char sha1[20];
+ struct object_id oid;
const char *head_ref;
int flag;
@@ -465,7 +465,7 @@ static void read_config(void)
loaded = 1;
current_branch = NULL;
- head_ref = resolve_ref_unsafe("HEAD", 0, sha1, &flag);
+ head_ref = resolve_ref_unsafe("HEAD", 0, oid.hash, &flag);
if (head_ref && (flag & REF_ISSYMREF) &&
skip_prefix(head_ref, "refs/heads/", &head_ref)) {
current_branch = make_branch(head_ref, 0);
@@ -544,12 +544,12 @@ static struct refspec *parse_refspec_internal(int
nr_refspec, const char **refsp
flags = REFNAME_ALLOW_ONELEVEL | (is_glob ?
REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
- unsigned char unused[40];
+ struct object_id unused;
/* LHS */
if (!*rs[i].src)
; /* empty is ok; it means "HEAD" */
- else if (llen == 40 && !get_sha1_hex(rs[i].src, unused))
+ else if (llen == GIT_SHA1_HEXSZ &&
!get_oid_hex(rs[i].src, &unused))
rs[i].exact_sha1 = 1; /* ok */
else if (!check_refname_format(rs[i].src, flags))
; /* valid looking ref is ok */
@@ -1082,7 +1082,7 @@ static struct ref *alloc_delete_ref(void)
static int try_explicit_object_name(const char *name,
struct ref **match)
{
- unsigned char sha1[20];
+ struct object_id oid;
if (!*name) {
if (match)
@@ -1090,12 +1090,12 @@ static int try_explicit_object_name(const char *name,
return 0;
}
- if (get_sha1(name, sha1))
+ if (get_sha1(name, oid.hash))
return -1;
if (match) {
*match = alloc_ref(name);
- hashcpy((*match)->new_oid.hash, sha1);
+ oidcpy(&(*match)->new_oid, &oid);
}
return 0;
}
@@ -1110,10 +1110,10 @@ static struct ref *make_linked_ref(const char *name,
struct ref ***tail)
static char *guess_ref(const char *name, struct ref *peer)
{
struct strbuf buf = STRBUF_INIT;
- unsigned char sha1[20];
+ struct object_id oid;
const char *r = resolve_ref_unsafe(peer->name, RESOLVE_REF_READING,
- sha1, NULL);
+ oid.hash, NULL);
if (!r)
return NULL;
@@ -1171,12 +1171,12 @@ static int match_explicit(struct ref *src, struct ref
*dst,
return -1;
if (!dst_value) {
- unsigned char sha1[20];
+ struct object_id oid;
int flag;
dst_value = resolve_ref_unsafe(matched_src->name,
RESOLVE_REF_READING,
- sha1, &flag);
+ oid.hash, &flag);
if (!dst_value ||
((flag & REF_ISSYMREF) &&
!starts_with(dst_value, "refs/heads/")))
@@ -1292,13 +1292,13 @@ struct tips {
int nr, alloc;
};
-static void add_to_tips(struct tips *tips, const unsigned char *sha1)
+static void add_to_tips(struct tips *tips, const struct object_id *oid)
{
struct commit *commit;
- if (is_null_sha1(sha1))
+ if (is_null_oid(oid))
return;
- commit = lookup_commit_reference_gently(sha1, 1);
+ commit = lookup_commit_reference_gently(oid->hash, 1);
if (!commit || (commit->object.flags & TMP_MARK))
return;
commit->object.flags |= TMP_MARK;
@@ -1322,9 +1322,9 @@ static void add_missing_tags(struct ref *src, struct ref
**dst, struct ref ***ds
for (ref = *dst; ref; ref = ref->next) {
if (ref->peer_ref &&
!is_null_oid(&ref->peer_ref->new_oid))
- add_to_tips(&sent_tips, ref->peer_ref->new_oid.hash);
+ add_to_tips(&sent_tips, &ref->peer_ref->new_oid);
else
- add_to_tips(&sent_tips, ref->old_oid.hash);
+ add_to_tips(&sent_tips, &ref->old_oid);
if (starts_with(ref->name, "refs/tags/"))
string_list_append(&dst_tag, ref->name);
[PATCH v4 04/12] parse_fetch: convert to use struct object_id
Convert the parse_fetch function to use struct object_id. Remove the
strlen check as get_oid_hex will fail safely on receiving a too-short
NUL-terminated string.
Signed-off-by: brian m. carlson
---
remote-curl.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index bc3af79e..f404faf0 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -803,19 +803,19 @@ static void parse_fetch(struct strbuf *buf)
if (skip_prefix(buf->buf, "fetch ", &p)) {
const char *name;
struct ref *ref;
- unsigned char old_sha1[20];
+ struct object_id old_oid;
- if (strlen(p) < 40 || get_sha1_hex(p, old_sha1))
+ if (get_oid_hex(p, &old_oid))
die("protocol error: expected sha/ref, got
%s'", p);
- if (p[40] == ' ')
- name = p + 41;
- else if (!p[40])
+ if (p[GIT_SHA1_HEXSZ] == ' ')
+ name = p + GIT_SHA1_HEXSZ + 1;
+ else if (!p[GIT_SHA1_HEXSZ])
name = "";
else
die("protocol error: expected sha/ref, got
%s'", p);
ref = alloc_ref(name);
- hashcpy(ref->old_oid.hash, old_sha1);
+ oidcpy(&ref->old_oid, &old_oid);
*list = ref;
list = &ref->next;
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 v4 11/12] Remove get_object_hash.
Convert all instances of get_object_hash to use an appropriate reference
to the hash member of the oid member of struct object. This provides no
functional change, as it is essentially a macro substitution.
Signed-off-by: brian m. carlson
---
The script used to do this conversion (which hasn't changed) follows.
#!/usr/bin/perl
use strict;
use warnings;
local $^I = '';
my $skip = 0;
while (<>) {
if (/#define get_object_hash/) {
$skip = 1;
next;
}
# Clean up the whitespace following the definition.
if ($skip) {
$skip = 0;
next;
}
s/get_object_hash\(\*(\(\(struct \S+\s*\*\)\S+\)[^)]+)\)/$1->oid.hash/g;
s/get_object_hash\(\*([^)]+)\)/$1->oid.hash/g;
s/get_object_hash\(([^)]+)\)/$1.oid.hash/g;
print;
}
archive.c| 6 +++---
bisect.c | 6 +++---
branch.c | 2 +-
builtin/blame.c | 24 +++
builtin/branch.c | 2 +-
builtin/checkout.c | 10 +-
builtin/clone.c | 2 +-
builtin/commit.c | 8
builtin/describe.c | 6 +++---
builtin/diff-tree.c | 8
builtin/diff.c | 12 ++--
builtin/fast-export.c| 12 ++--
builtin/fetch.c | 4 ++--
builtin/fmt-merge-msg.c | 4 ++--
builtin/fsck.c | 4 ++--
builtin/grep.c | 2 +-
builtin/index-pack.c | 2 +-
builtin/log.c| 18 -
builtin/merge-tree.c | 4 ++--
builtin/merge.c | 50
builtin/name-rev.c | 4 ++--
builtin/notes.c | 2 +-
builtin/pack-objects.c | 14 +++---
builtin/pull.c | 2 +-
builtin/reflog.c | 4 ++--
builtin/reset.c | 2 +-
builtin/rev-list.c | 4 ++--
builtin/rev-parse.c | 4 ++--
builtin/show-branch.c| 4 ++--
builtin/unpack-objects.c | 2 +-
bundle.c | 2 +-
cache-tree.c | 2 +-
combine-diff.c | 4 ++--
commit.c | 12 ++--
decorate.c | 2 +-
diff-lib.c | 2 +-
fetch-pack.c | 4 ++--
fsck.c | 6 +++---
http-push.c | 20 +--
line-log.c | 6 +++---
log-tree.c | 20 +--
merge-blobs.c| 4 ++--
merge-recursive.c| 16
notes-merge.c| 20 +--
object.c | 6 +++---
object.h | 2 --
pack-bitmap-write.c | 14 +++---
pack-bitmap.c| 26 -
patch-ids.c | 6 +++---
pretty.c | 8
ref-filter.c | 6 +++---
refs.c | 2 +-
revision.c | 16
sequencer.c | 18 -
sha1_name.c | 16
shallow.c| 2 +-
submodule.c | 2 +-
tag.c| 6 +++---
test-match-trees.c | 2 +-
tree.c | 4 ++--
upload-pack.c| 10 +-
walker.c | 8
wt-status.c | 2 +-
63 files changed, 251 insertions(+), 253 deletions(-)
diff --git a/archive.c b/archive.c
index af23001b..0687afae 100644
--- a/archive.c
+++ b/archive.c
@@ -241,7 +241,7 @@ int write_archive_entries(struct archiver_args *args,
len--;
if (args->verbose)
fprintf(stderr, "%.*s\n", (int)len, args->base);
- err = write_entry(args, get_object_hash(args->tree->object),
args->base,
+ err = write_entry(args, args->tree->object.oid.hash, args->base,
len, 040777);
if (err)
return err;
@@ -374,7 +374,7 @@ static void parse_treeish_arg(const char **argv,
commit = lookup_commit_reference_gently(oid.hash, 1);
if (commit) {
- commit_sha1 = get_object_hash(commit->object);
+ commit_sha1 = commit->object.oid.hash;
archive_time = commit->date;
} else {
commit_sha1 = NULL;
@@ -390,7 +390,7 @@ static void parse_treeish_arg(const char **argv,
unsigned int mode;
int err;
- err = get_tree_entry(get_object_hash(tree->object), prefix,
+ err = get_tree_entry(tree->object.oid.hash, prefix,
tree_oid.hash, &mode);
if (err || !S_ISDIR(mode))
die("current working directory is untracked");
diff --git a/bisect.c b/bisect.c
index 54166f00..42aa7aa6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -500,7 +500,7 @@ struct commit_list *filter_skipped(struct commit_list
[PATCH v4 08/12] object: introduce get_object_hash macro.
This macro is a temporary change to ease the transition of struct object
to use struct object_id. It takes an argument of struct object and
returns the object's hash. Provide this hash next to struct object for
easier conversion.
Signed-off-by: brian m. carlson
---
object.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/object.h b/object.h
index 6416247d..7c098d03 100644
--- a/object.h
+++ b/object.h
@@ -52,6 +52,8 @@ struct object {
unsigned char sha1[20];
};
+#define get_object_hash(x) ((x).sha1)
+
extern const char *typename(unsigned int type);
extern int type_from_string_gently(const char *str, ssize_t, int gentle);
#define type_from_string(str) type_from_string_gently(str, -1, 0)
--
2.6.2.409.gb049f0a
--
To unsubscribe from this list: send the line "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 01/11] refs: make is_branch public
On 11/09/2015 07:54 PM, Ramsay Jones wrote: > On 09/11/15 17:03, Michael Haggerty wrote: >> [...] >> diff --git a/refs.h b/refs.h >> index 6d30c98..39b8edc 100644 >> --- a/refs.h >> +++ b/refs.h >> @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char >> *msg_fmt, const struct st >> */ >> int pack_refs(unsigned int flags); >> >> +int is_branch(const char *refname); >> + >> /* >> * Flags controlling ref_transaction_update(), ref_transaction_create(), >> etc. >> * REF_NODEREF: act on the ref directly, instead of dereferencing >> > > I don't understand, is_branch() is already declared in refs.h, see line 67. > > This is true in master, next and pu now appears to have two declarations. Thanks for noticing! I'll drop that patch from the next round. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "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 07/11] refs/refs-internal.h: new header file
On 11/09/2015 08:46 PM, Ramsay Jones wrote: > On 09/11/15 17:03, Michael Haggerty wrote: >> [...] >> diff --git a/refs.c b/refs.c >> index f48c58a..9aff0c8 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -1,6 +1,6 @@ >> #include "cache.h" >> +#include "refs/refs-internal.h" >> #include "lockfile.h" >> -#include "refs.h" > > This looked wrong to me, until I had read the remainder of the > patch and noticed that the 'internal' header #included the > 'public' interface header. > > Unfortunately, this still feels wrong to me! I would rather that > the internal header _not_ include the public header (so, include > them _both_ when necessary). Just my opinion, which you can simply > ignore. :-D Yeah, I was of two minds about this. I will change this in the next round. Thanks for your review! Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
