* [PATCH v3 0/3] Add support for GIT_ONE_FILESYSTEM @ 2010-03-17 19:55 Lars R. Damerow 2010-03-17 19:55 ` [PATCH 1/3] config.c: remove static keyword from git_env_bool() Lars R. Damerow ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Lars R. Damerow @ 2010-03-17 19:55 UTC (permalink / raw) To: git Here's another try for GIT_ONE_FILESYSTEM. I took Duy's recommendation not to die() in gentle mode as well as to just use cwd in my error messages (after truncating them). I also fixed the problem with the existing error message's use of an untruncated cwd. Thanks for all of the suggestions! Lars R. Damerow (3): config.c: remove static keyword from git_env_bool() truncate cwd string before printing error message Add support for GIT_ONE_FILESYSTEM Documentation/git.txt | 3 +++ cache.h | 1 + config.c | 2 +- setup.c | 28 +++++++++++++++++++++++++++- 4 files changed, 32 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] config.c: remove static keyword from git_env_bool() 2010-03-17 19:55 [PATCH v3 0/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow @ 2010-03-17 19:55 ` Lars R. Damerow 2010-03-17 19:55 ` [PATCH 2/3] truncate cwd string before printing error message Lars R. Damerow 2010-03-17 19:55 ` [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow 2 siblings, 0 replies; 20+ messages in thread From: Lars R. Damerow @ 2010-03-17 19:55 UTC (permalink / raw) To: git Since this function is the preferred way to handle boolean environment variables it's useful to have it available to other files. Signed-off-by: Lars R. Damerow <lars@pixar.com> --- cache.h | 1 + config.c | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/cache.h b/cache.h index 89f6a40..c29030f 100644 --- a/cache.h +++ b/cache.h @@ -945,6 +945,7 @@ extern int git_config_set_multivar(const char *, const char *, const char *, int extern int git_config_rename_section(const char *, const char *); extern const char *git_etc_gitconfig(void); extern int check_repository_format_version(const char *var, const char *value, void *cb); +extern int git_env_bool(const char *, int); extern int git_config_system(void); extern int git_config_global(void); extern int config_error_nonbool(const char *); diff --git a/config.c b/config.c index 6963fbe..70e4600 100644 --- a/config.c +++ b/config.c @@ -683,7 +683,7 @@ const char *git_etc_gitconfig(void) return system_wide; } -static int git_env_bool(const char *k, int def) +int git_env_bool(const char *k, int def) { const char *v = getenv(k); return v ? git_config_bool(k, v) : def; -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] truncate cwd string before printing error message 2010-03-17 19:55 [PATCH v3 0/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow 2010-03-17 19:55 ` [PATCH 1/3] config.c: remove static keyword from git_env_bool() Lars R. Damerow @ 2010-03-17 19:55 ` Lars R. Damerow 2010-03-17 19:55 ` [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow 2 siblings, 0 replies; 20+ messages in thread From: Lars R. Damerow @ 2010-03-17 19:55 UTC (permalink / raw) To: git Without this truncation the error message printed only shows the cwd from the start of the search, not where it failed. Signed-off-by: Lars R. Damerow <lars@pixar.com> --- setup.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/setup.c b/setup.c index 5716d90..f0b56b9 100644 --- a/setup.c +++ b/setup.c @@ -422,8 +422,10 @@ const char *setup_git_directory_gently(int *nongit_ok) } die("Not a git repository (or any of the parent directories): %s", DEFAULT_GIT_DIR_ENVIRONMENT); } - if (chdir("..")) + if (chdir("..")) { + cwd[offset] = '\0'; die_errno("Cannot change to '%s/..'", cwd); + } } inside_git_dir = 0; -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-17 19:55 [PATCH v3 0/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow 2010-03-17 19:55 ` [PATCH 1/3] config.c: remove static keyword from git_env_bool() Lars R. Damerow 2010-03-17 19:55 ` [PATCH 2/3] truncate cwd string before printing error message Lars R. Damerow @ 2010-03-17 19:55 ` Lars R. Damerow 2010-03-28 9:22 ` Jeff King 2 siblings, 1 reply; 20+ messages in thread From: Lars R. Damerow @ 2010-03-17 19:55 UTC (permalink / raw) To: git This patch makes git pay attention to the GIT_ONE_FILESYSTEM environment variable. When that variable is set, git will stop searching for a GIT_DIR when it attempts to cross a filesystem boundary. When working in an environment with too many automount points to make maintaining a GIT_CEILING_DIRECTORIES list enjoyable, GIT_ONE_FILESYSTEM gives the option of turning all such attempts off with one setting. Signed-off-by: Lars R. Damerow <lars@pixar.com> --- Documentation/git.txt | 3 +++ setup.c | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 0 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 35c0c79..dbb590f 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -529,6 +529,9 @@ git so take care if using Cogito etc. a GIT_DIR set on the command line or in the environment. (Useful for excluding slow-loading network directories.) +'GIT_ONE_FILESYSTEM':: + Stop at filesystem boundaries when looking for .git or objects. + git Commits ~~~~~~~~~~~ 'GIT_AUTHOR_NAME':: diff --git a/setup.c b/setup.c index f0b56b9..ef23e79 100644 --- a/setup.c +++ b/setup.c @@ -323,6 +323,8 @@ const char *setup_git_directory_gently(int *nongit_ok) const char *gitdirenv; const char *gitfile_dir; int len, offset, ceil_offset, root_len; + int current_device = 0, one_filesystem = 0; + struct stat buf; /* * Let's assume that we are in a git repository. @@ -390,6 +392,11 @@ const char *setup_git_directory_gently(int *nongit_ok) * etc. */ offset = len = strlen(cwd); + if ((one_filesystem = git_env_bool("GIT_ONE_FILESYSTEM", 0))) { + if (stat(".", &buf)) + die_errno("failed to stat '.'"); + current_device = buf.st_dev; + } for (;;) { gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT); if (gitfile_dir) { @@ -422,6 +429,23 @@ const char *setup_git_directory_gently(int *nongit_ok) } die("Not a git repository (or any of the parent directories): %s", DEFAULT_GIT_DIR_ENVIRONMENT); } + if (one_filesystem) { + if (stat("..", &buf)) { + cwd[offset] = '\0'; + die_errno("failed to stat '%s/..'", cwd); + } + if (buf.st_dev != current_device) { + if (nongit_ok) { + if (chdir(cwd)) + die_errno("Cannot come back to cwd"); + *nongit_ok = 1; + return NULL; + } + cwd[offset] = '\0'; + die("Not a git repository (or any parent up to mount parent %s)\n" + "Stopping at filesystem boundary since GIT_ONE_FILESYSTEM is set.", cwd); + } + } if (chdir("..")) { cwd[offset] = '\0'; die_errno("Cannot change to '%s/..'", cwd); -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-17 19:55 ` [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow @ 2010-03-28 9:22 ` Jeff King 2010-03-28 12:05 ` Jeff King 2010-03-28 16:44 ` Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: Jeff King @ 2010-03-28 9:22 UTC (permalink / raw) To: Lars R. Damerow; +Cc: git On Wed, Mar 17, 2010 at 12:55:53PM -0700, Lars R. Damerow wrote: > This patch makes git pay attention to the GIT_ONE_FILESYSTEM environment > variable. When that variable is set, git will stop searching for a > GIT_DIR when it attempts to cross a filesystem boundary. > > When working in an environment with too many automount points to make > maintaining a GIT_CEILING_DIRECTORIES list enjoyable, GIT_ONE_FILESYSTEM > gives the option of turning all such attempts off with one setting. Thanks, this version (and the whole series) looks good to me, with two minor nits below. As far as a command-line option or a config option, I don't see the point. It seems to me something you would want enabled all the time, not a one-shot option, so a command-line option doesn't make much sense (after all, you could just use --git-dir at that point, which would be even faster). A config option might be useful, but I think it might be tricky to implement due to the startup sequence. If anybody disagrees, they can always build on top of your series very easily. > diff --git a/Documentation/git.txt b/Documentation/git.txt > index 35c0c79..dbb590f 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -529,6 +529,9 @@ git so take care if using Cogito etc. > a GIT_DIR set on the command line or in the environment. > (Useful for excluding slow-loading network directories.) > > +'GIT_ONE_FILESYSTEM':: > + Stop at filesystem boundaries when looking for .git or objects. > + We should give the user a little more information than that. Specifically: - what form should the value take? - how does it interact with GIT_DIR? Also, we use the phrase "repository directory" in GIT_CEILING_DIRECTORIES, so we should probably do the same here. So how about this squashed into your patch? diff --git a/Documentation/git.txt b/Documentation/git.txt index e7ebaaa..bf1b45e 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -531,7 +531,10 @@ git so take care if using Cogito etc. (Useful for excluding slow-loading network directories.) 'GIT_ONE_FILESYSTEM':: - Stop at filesystem boundaries when looking for .git or objects. + If set to a true value ("true" or a non-zero integer), stop at + filesystem boundaries when looking for a repository directory. + Like 'GIT_CEILING_DIRECTORIES', it will not affect an explicit + respository directory set via 'GIT_DIR' or on the command line. git Commits ~~~~~~~~~~~ > --- a/setup.c > +++ b/setup.c > @@ -390,6 +392,11 @@ const char *setup_git_directory_gently(int *nongit_ok) > * etc. > */ > offset = len = strlen(cwd); > + if ((one_filesystem = git_env_bool("GIT_ONE_FILESYSTEM", 0))) { > + if (stat(".", &buf)) > + die_errno("failed to stat '.'"); > + current_device = buf.st_dev; > + } Style nit. I know you did the proper () to silence the compiler warning, but we usually avoid assignment-inside-conditional altogether. Personally I don't really care either way. With those fixes, I think it should be ready for 'next'. -Peff ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-28 9:22 ` Jeff King @ 2010-03-28 12:05 ` Jeff King 2010-03-28 16:44 ` Junio C Hamano 1 sibling, 0 replies; 20+ messages in thread From: Jeff King @ 2010-03-28 12:05 UTC (permalink / raw) To: Lars R. Damerow; +Cc: git On Sun, Mar 28, 2010 at 05:22:54AM -0400, Jeff King wrote: > > --- a/setup.c > > +++ b/setup.c > > @@ -390,6 +392,11 @@ const char *setup_git_directory_gently(int *nongit_ok) > > * etc. > > */ > > offset = len = strlen(cwd); > > + if ((one_filesystem = git_env_bool("GIT_ONE_FILESYSTEM", 0))) { > > + if (stat(".", &buf)) > > + die_errno("failed to stat '.'"); > > + current_device = buf.st_dev; > > + } > > Style nit. I know you did the proper () to silence the compiler warning, > but we usually avoid assignment-inside-conditional altogether. > Personally I don't really care either way. Actually I take this back. It is in our CodingStyle guide, but we violate it all over the place (my grep counted 522). Perhaps it should be removed from CodingStyle. -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-28 9:22 ` Jeff King 2010-03-28 12:05 ` Jeff King @ 2010-03-28 16:44 ` Junio C Hamano 2010-03-28 17:32 ` Lars Damerow 2010-03-30 22:43 ` Linus Torvalds 1 sibling, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2010-03-28 16:44 UTC (permalink / raw) To: Jeff King; +Cc: Lars R. Damerow, git Jeff King <peff@peff.net> writes: > With those fixes, I think it should be ready for 'next'. Yeah, looks nice; thanks both. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-28 16:44 ` Junio C Hamano @ 2010-03-28 17:32 ` Lars Damerow 2010-03-30 15:58 ` Jeff King 2010-03-30 22:43 ` Linus Torvalds 1 sibling, 1 reply; 20+ messages in thread From: Lars Damerow @ 2010-03-28 17:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git >From Junio C Hamano <gitster@pobox.com>, Sun, Mar 28, 2010 at 09:44:09AM -0700: > Jeff King <peff@peff.net> writes: > > > With those fixes, I think it should be ready for 'next'. > > Yeah, looks nice; thanks both. Thanks all for the suggestions! Jeff, shall I just squash in the doc patch you sent and resubmit the patches to the list? -lars -- lars r. damerow :: button pusher :: pixar animation studios ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-28 17:32 ` Lars Damerow @ 2010-03-30 15:58 ` Jeff King 0 siblings, 0 replies; 20+ messages in thread From: Jeff King @ 2010-03-30 15:58 UTC (permalink / raw) To: Lars Damerow; +Cc: Junio C Hamano, git On Sun, Mar 28, 2010 at 10:32:25AM -0700, Lars Damerow wrote: > > > With those fixes, I think it should be ready for 'next'. > > > > Yeah, looks nice; thanks both. > > Thanks all for the suggestions! Jeff, shall I just squash in the doc > patch you sent and resubmit the patches to the list? No need. Usually Junio's "thanks both" means he picked up the patches and squashed it himself, and indeed, it looks like he has your patches (with my doc change) in his 'pu' branch. -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-28 16:44 ` Junio C Hamano 2010-03-28 17:32 ` Lars Damerow @ 2010-03-30 22:43 ` Linus Torvalds 2010-03-30 22:59 ` Thomas Rast 2010-03-30 23:02 ` Jeff King 1 sibling, 2 replies; 20+ messages in thread From: Linus Torvalds @ 2010-03-30 22:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Lars R. Damerow, git On Sun, 28 Mar 2010, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > With those fixes, I think it should be ready for 'next'. > > Yeah, looks nice; thanks both. I realize that I'm late to the party, but I do wonder if the "one filesystem" mode shouldn't be the default, rather than be enabled by a config option? IOW, just switch the meaning of the config option the other way. I suspect that it is _very_ unusual to have a source repo that crosses multiple filesystems, and the original reason for this patch-series seems to me to be likely to be more common than that multi-fs case. So having the logic go the other way would seem to match the common case, no? Linus ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-30 22:43 ` Linus Torvalds @ 2010-03-30 22:59 ` Thomas Rast 2010-03-30 23:04 ` Jeff King 2010-03-30 23:02 ` Jeff King 1 sibling, 1 reply; 20+ messages in thread From: Thomas Rast @ 2010-03-30 22:59 UTC (permalink / raw) To: Linus Torvalds; +Cc: Junio C Hamano, Jeff King, Lars R. Damerow, git Linus Torvalds wrote: > > I suspect that it is _very_ unusual to have a source repo that crosses > multiple filesystems, and the original reason for this patch-series seems > to me to be likely to be more common than that multi-fs case. So having > the logic go the other way would seem to match the common case, no? Not sure if I'm the only one, but I noticed at some point that mounting the t/ directory of git.git on tmpfs gives a huge speed boost to the test suite... -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-30 22:59 ` Thomas Rast @ 2010-03-30 23:04 ` Jeff King 0 siblings, 0 replies; 20+ messages in thread From: Jeff King @ 2010-03-30 23:04 UTC (permalink / raw) To: Thomas Rast; +Cc: Linus Torvalds, Junio C Hamano, Lars R. Damerow, git On Wed, Mar 31, 2010 at 12:59:33AM +0200, Thomas Rast wrote: > Linus Torvalds wrote: > > > > I suspect that it is _very_ unusual to have a source repo that crosses > > multiple filesystems, and the original reason for this patch-series seems > > to me to be likely to be more common than that multi-fs case. So having > > the logic go the other way would seem to match the common case, no? > > Not sure if I'm the only one, but I noticed at some point that > mounting the t/ directory of git.git on tmpfs gives a huge speed boost > to the test suite... I noticed it, too, but my solution was a little different: $ git show f423ef5f tests: allow user to specify trash directory location The tests generate a large amount of I/O activity creating and destroying repositories and files. We can improve the time it takes to run the test suite by creating trash directories on filesystems with better performance characteristic, even though we may not want the rest of the git repository on those filesystems (e.g., because they are not network connected, or because they are temporary ramdisks). For example, on a dual processor system: $ cd t && time make -j32 real 1m51.562s user 0m59.260s sys 1m20.933s # /dev/shm is tmpfs $ cd t && time make -j32 GIT_TEST_OPTS="--root=/dev/shm" real 1m1.484s user 0m53.555s sys 1m5.264s -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-30 22:43 ` Linus Torvalds 2010-03-30 22:59 ` Thomas Rast @ 2010-03-30 23:02 ` Jeff King 2010-03-30 23:10 ` Junio C Hamano 2010-03-30 23:12 ` Linus Torvalds 1 sibling, 2 replies; 20+ messages in thread From: Jeff King @ 2010-03-30 23:02 UTC (permalink / raw) To: Linus Torvalds; +Cc: Junio C Hamano, Lars R. Damerow, git On Tue, Mar 30, 2010 at 03:43:01PM -0700, Linus Torvalds wrote: > I realize that I'm late to the party, but I do wonder if the "one > filesystem" mode shouldn't be the default, rather than be enabled by a > config option? IOW, just switch the meaning of the config option the other > way. Fashionably late, of course. I agree with your reasoning that it is a more sane default. The only thing that would make me hesitate on it now is that it is a behavior change. I suspect the group we would be breaking is small or even zero, though. -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-30 23:02 ` Jeff King @ 2010-03-30 23:10 ` Junio C Hamano 2010-03-30 23:12 ` Linus Torvalds 1 sibling, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2010-03-30 23:10 UTC (permalink / raw) To: Jeff King; +Cc: Linus Torvalds, Junio C Hamano, Lars R. Damerow, git Jeff King <peff@peff.net> writes: > On Tue, Mar 30, 2010 at 03:43:01PM -0700, Linus Torvalds wrote: > >> I realize that I'm late to the party, but I do wonder if the "one >> filesystem" mode shouldn't be the default, rather than be enabled by a >> config option? IOW, just switch the meaning of the config option the other >> way. > > Fashionably late, of course. I agree with your reasoning that it is > a more sane default. The only thing that would make me hesitate on it > now is that it is a behavior change. I suspect the group we would be > breaking is small or even zero, though. Yeah, I think I agree with the reasoning, and it is not even _late_, as the feature hasn't escaped the lab yet. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-30 23:02 ` Jeff King 2010-03-30 23:10 ` Junio C Hamano @ 2010-03-30 23:12 ` Linus Torvalds 2010-03-31 0:54 ` Erik Faye-Lund 1 sibling, 1 reply; 20+ messages in thread From: Linus Torvalds @ 2010-03-30 23:12 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Lars R. Damerow, git On Tue, 30 Mar 2010, Jeff King wrote: > > Fashionably late, of course. I agree with your reasoning that it is > a more sane default. The only thing that would make me hesitate on it > now is that it is a behavior change. I suspect the group we would be > breaking is small or even zero, though. Well, I have to admit that I'm a _tiny_ bit nervous that some odd OS or filesystem has special magic st_dev rules so that it changes randomly even within a filesystem, but that would be very non-posix (think of the confusion it would cause standard UNIX tools like 'find -xdev' etc), so it's more a worry of "I have no idea what st_dev means on Windows" than anything really solid. Linus ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM 2010-03-30 23:12 ` Linus Torvalds @ 2010-03-31 0:54 ` Erik Faye-Lund 2010-04-04 18:00 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Erik Faye-Lund @ 2010-03-31 0:54 UTC (permalink / raw) To: Linus Torvalds; +Cc: Jeff King, Junio C Hamano, Lars R. Damerow, git On Wed, Mar 31, 2010 at 1:12 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > On Tue, 30 Mar 2010, Jeff King wrote: >> >> Fashionably late, of course. I agree with your reasoning that it is >> a more sane default. The only thing that would make me hesitate on it >> now is that it is a behavior change. I suspect the group we would be >> breaking is small or even zero, though. > > Well, I have to admit that I'm a _tiny_ bit nervous that some odd OS or > filesystem has special magic st_dev rules so that it changes randomly even > within a filesystem, but that would be very non-posix (think of the > confusion it would cause standard UNIX tools like 'find -xdev' etc), so > it's more a worry of "I have no idea what st_dev means on Windows" than > anything really solid. > st_dev means "Drive number of the disk containing the file (same as st_rdev)."[1], and mounting file systems as subdirs of other file systems is rare (but possible) on Windows. However, in our (f)stat implementation, st_dev means 0 -- but this might be possible to improve on. [1]: http://msdn.microsoft.com/en-us/library/14h5k7ff(VS.71).aspx -- Erik "kusma" Faye-Lund ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries 2010-03-31 0:54 ` Erik Faye-Lund @ 2010-04-04 18:00 ` Junio C Hamano 2010-04-04 22:30 ` [PATCH] Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM Junio C Hamano 2010-04-04 22:39 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2010-04-04 18:00 UTC (permalink / raw) To: Lars R. Damerow; +Cc: Linus Torvalds, Jeff King, Erik Faye-Lund, git Regarding the new environment variable, Linus Torvalds <torvalds@linux-foundation.org> writes on Tue, 30 Mar 2010 in <alpine.LFD.2.00.1003301537150.3707@i5.linux-foundation.org>: I suspect that it is _very_ unusual to have a source repo that crosses multiple filesystems, and the original reason for this patch-series seems to me to be likely to be more common than that multi-fs case. So having the logic go the other way would seem to match the common case, no? The "crossing filesystem boundary" condition is checked by comparing st_dev field in the result from stat(2). This is slightly worrysome if non-POSIX ports return different values in the field even for directories in the same work tree extracted to the same "filesystem". Erik Faye-Lund confirms that in the msysgit port st_dev is 0, so this should be safe, as "even Windows is safe" ;-) This will affect those who use /.git to cram /etc and /home/me in the same repostiory, /home is mounted from non-root filesystem, and a git operation is done from inside /home/me/src. But that is such a corner case we don't want to give preference over helping people who will benefit from having this default so that they do not have to suffer from slow automounters. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * I agree with Linus that it make sense to flip the default, but this should probably have to wait for at least two release cycles for the usual backward-compatibility rules. I wonder if "git add" and friends should also notice it and warn. If you have more than one values of ce->ce_dev in the index, it means that the working tree spans more than one filesystem and from a subdirectory with an entry that has a ce->ce_dev different from the value for a path at the top of the work tree, you will not be able to discover the top of the tree without GIT_ONE_FILESYSTEM set to true. A likely scenario for this to happen would be: (1) You have a tarball of some sort; you extract it $there; $ mkdir $there && cd $there $ tar xf /var/tmp/tarball.tar (2) You notice the filesystem lacks enough free space, and move some part (say "images/") to a separate filesystem, and bind-mount; $ mv images $another/. && rm -fr images && mkdir images $ mount --bind $another/images images (3) You add everything to start the project; $ git init && git add . Up to this point it would work (you are at the top of the working tree). And this is the point we _could_ notice and warn that you will have trouble in step (4). (4) Go down to a subdirectory and start futzing; $ cd images && gimp naughty.jpg && git add -u Documentation/git.txt | 12 ++++++++---- setup.c | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index bf1b45e..aa62083 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -531,10 +531,14 @@ git so take care if using Cogito etc. (Useful for excluding slow-loading network directories.) 'GIT_ONE_FILESYSTEM':: - If set to a true value ("true" or a non-zero integer), stop at - filesystem boundaries when looking for a repository directory. - Like 'GIT_CEILING_DIRECTORIES', it will not affect an explicit - respository directory set via 'GIT_DIR' or on the command line. + When run in a directory that does not have ".git" repository + directory, git tries to find such a directory in the parent + directories to find the top of the working tree, but by default it + does not cross filesystem boundaries. This environment variable + can be set to false value ("false" or zero) to tell git not to + stop at filesystem boundaries. Like 'GIT_CEILING_DIRECTORIES', + this will not affect an explicit respository directory set via + 'GIT_DIR' or on the command line. git Commits ~~~~~~~~~~~ diff --git a/setup.c b/setup.c index 8b911b1..d290633 100644 --- a/setup.c +++ b/setup.c @@ -323,7 +323,7 @@ const char *setup_git_directory_gently(int *nongit_ok) const char *gitdirenv; const char *gitfile_dir; int len, offset, ceil_offset, root_len; - int current_device = 0, one_filesystem = 0; + int current_device = 0, one_filesystem = 1; struct stat buf; /* @@ -444,7 +444,7 @@ const char *setup_git_directory_gently(int *nongit_ok) } cwd[offset] = '\0'; die("Not a git repository (or any parent up to mount parent %s)\n" - "Stopping at filesystem boundary since GIT_ONE_FILESYSTEM is set.", cwd); + "Stopping at filesystem boundary since GIT_ONE_FILESYSTEM is true.", cwd); } } if (chdir("..")) { -- 1.7.0.4.552.gc303c1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM 2010-04-04 18:00 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano @ 2010-04-04 22:30 ` Junio C Hamano 2010-04-04 22:37 ` Matthieu Moy 2010-04-04 22:39 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2010-04-04 22:30 UTC (permalink / raw) To: Lars R. Damerow; +Cc: Linus Torvalds, Jeff King, Erik Faye-Lund, git If a missing ONE_FILESYSTEM defaults to true, the only users who set this variable set it to false to tell git not to limit the discovery to one filesystem; there are too many negations in one sentence to make a simple panda brain dizzy. Use the variable GIT_DISCOVERY_ACROSS_FILESYSTEM that changes the behaviour from the default "limit to one filesystem" to "cross the boundary as I ask you to"; makes the semantics much more straight forward. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git.txt | 10 +++++----- setup.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index aa62083..eb78a2b 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -530,15 +530,15 @@ git so take care if using Cogito etc. a GIT_DIR set on the command line or in the environment. (Useful for excluding slow-loading network directories.) -'GIT_ONE_FILESYSTEM':: +'GIT_DISCOVERY_ACROSS_FILESYSTEM':: When run in a directory that does not have ".git" repository directory, git tries to find such a directory in the parent directories to find the top of the working tree, but by default it does not cross filesystem boundaries. This environment variable - can be set to false value ("false" or zero) to tell git not to - stop at filesystem boundaries. Like 'GIT_CEILING_DIRECTORIES', - this will not affect an explicit respository directory set via - 'GIT_DIR' or on the command line. + can be set to true to tell git not to stop at filesystem + boundaries. Like 'GIT_CEILING_DIRECTORIES', this will not affect + an explicit respository directory set via 'GIT_DIR' or on the + command line. git Commits ~~~~~~~~~~~ diff --git a/setup.c b/setup.c index d290633..5a26b5b 100644 --- a/setup.c +++ b/setup.c @@ -392,7 +392,7 @@ const char *setup_git_directory_gently(int *nongit_ok) * etc. */ offset = len = strlen(cwd); - one_filesystem = git_env_bool("GIT_ONE_FILESYSTEM", 0); + one_filesystem = !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0); if (one_filesystem) { if (stat(".", &buf)) die_errno("failed to stat '.'"); -- 1.7.0.4.552.gc303c1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM 2010-04-04 22:30 ` [PATCH] Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM Junio C Hamano @ 2010-04-04 22:37 ` Matthieu Moy 0 siblings, 0 replies; 20+ messages in thread From: Matthieu Moy @ 2010-04-04 22:37 UTC (permalink / raw) To: Junio C Hamano Cc: Lars R. Damerow, Linus Torvalds, Jeff King, Erik Faye-Lund, git Junio C Hamano <gitster@pobox.com> writes: > If a missing ONE_FILESYSTEM defaults to true, the only users who set this > variable set it to false to tell git not to limit the discovery to one > filesystem; there are too many negations in one sentence to make a simple > panda brain dizzy. Right. + an explicit respository directory set via 'GIT_DIR' or on the While you're there, you can spell respository as "repository" ;-). > diff --git a/setup.c b/setup.c > index d290633..5a26b5b 100644 > --- a/setup.c > +++ b/setup.c > @@ -392,7 +392,7 @@ const char *setup_git_directory_gently(int *nongit_ok) > * etc. > */ > offset = len = strlen(cwd); > - one_filesystem = git_env_bool("GIT_ONE_FILESYSTEM", 0); > + one_filesystem = !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0); > if (one_filesystem) { > if (stat(".", &buf)) > die_errno("failed to stat '.'"); I guess you still have one instance of GIT_ONE_FILESYSTEM a little below in the error message: "Stopping at filesystem boundary since GIT_ONE_FILESYSTEM is true." which should become "Stopping at filesystem boundary since GIT_DISCOVERY_ACROSS_FILESYSTEM\n" "is not set." or so. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries 2010-04-04 18:00 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano 2010-04-04 22:30 ` [PATCH] Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM Junio C Hamano @ 2010-04-04 22:39 ` Junio C Hamano 1 sibling, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2010-04-04 22:39 UTC (permalink / raw) To: Lars R. Damerow; +Cc: Linus Torvalds, Jeff King, Erik Faye-Lund, git Junio C Hamano <gitster@pobox.com> writes: > I wonder if "git add" and friends should also notice it and warn. If > you have more than one values of ce->ce_dev in the index, it means that > the working tree spans more than one filesystem and from a subdirectory > with an entry that has a ce->ce_dev different from the value for a path > at the top of the work tree, you will not be able to discover the top > of the tree without GIT_ONE_FILESYSTEM set to true. A likely scenario > for this to happen would be: > > (1) You have a tarball of some sort; you extract it $there; > > $ mkdir $there && cd $there > $ tar xf /var/tmp/tarball.tar > > (2) You notice the filesystem lacks enough free space, and move some > part (say "images/") to a separate filesystem, and bind-mount; > > $ mv images $another/. && rm -fr images && mkdir images > $ mount --bind $another/images images > > (3) You add everything to start the project; > > $ git init && git add . > > Up to this point it would work (you are at the top of the working > tree). And this is the point we _could_ notice and warn that you > will have trouble in step (4). > > (4) Go down to a subdirectory and start futzing; > > $ cd images && gimp naughty.jpg && git add -u And this adds such a check. read-cache.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/read-cache.c b/read-cache.c index f1f789b..486bb2a 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1550,6 +1550,7 @@ int write_index(struct index_state *istate, int newfd) struct cache_entry **cache = istate->cache; int entries = istate->cache_nr; struct stat st; + int more_than_one_dev; for (i = removed = extended = 0; i < entries; i++) { if (cache[i]->ce_flags & CE_REMOVE) @@ -1572,6 +1573,7 @@ int write_index(struct index_state *istate, int newfd) if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0) return -1; + more_than_one_dev = 0; for (i = 0; i < entries; i++) { struct cache_entry *ce = cache[i]; if (ce->ce_flags & CE_REMOVE) @@ -1580,8 +1582,15 @@ int write_index(struct index_state *istate, int newfd) ce_smudge_racily_clean_entry(ce); if (ce_write_entry(&c, newfd, ce) < 0) return -1; + if (i && ce->ce_dev != cache[0]->ce_dev) + more_than_one_dev = 1; } + if (more_than_one_dev && + !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0)) + warning("working tree spans across filesystems but " + "GIT_DISCOVERY_ACROSS_FILESYSTEM is not set."); + /* Write extension data here */ if (istate->cache_tree) { struct strbuf sb = STRBUF_INIT; ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2010-04-04 22:40 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-17 19:55 [PATCH v3 0/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow 2010-03-17 19:55 ` [PATCH 1/3] config.c: remove static keyword from git_env_bool() Lars R. Damerow 2010-03-17 19:55 ` [PATCH 2/3] truncate cwd string before printing error message Lars R. Damerow 2010-03-17 19:55 ` [PATCH 3/3] Add support for GIT_ONE_FILESYSTEM Lars R. Damerow 2010-03-28 9:22 ` Jeff King 2010-03-28 12:05 ` Jeff King 2010-03-28 16:44 ` Junio C Hamano 2010-03-28 17:32 ` Lars Damerow 2010-03-30 15:58 ` Jeff King 2010-03-30 22:43 ` Linus Torvalds 2010-03-30 22:59 ` Thomas Rast 2010-03-30 23:04 ` Jeff King 2010-03-30 23:02 ` Jeff King 2010-03-30 23:10 ` Junio C Hamano 2010-03-30 23:12 ` Linus Torvalds 2010-03-31 0:54 ` Erik Faye-Lund 2010-04-04 18:00 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano 2010-04-04 22:30 ` [PATCH] Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM Junio C Hamano 2010-04-04 22:37 ` Matthieu Moy 2010-04-04 22:39 ` [PATCH] GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).