threads / patch / 23031

patchAdd support for GIT_ONE_FILESYSTEM

Subject: [PATCH] Add support for GIT_ONE_FILESYSTEM

## tl;dr

8 messages between Mar 15, 2010 and Mar 16, 2010. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Lars Damerow· Mar 15, 2010, 21:40 UTC · lore

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               |   11 +++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)
Show changes to 2 files +14 −0

Documentation/git.txt, setup.c

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 5716d90..63dc52e 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;
+	struct stat buf;
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -390,6 +392,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 *   etc.
 	 */
 	offset = len = strlen(cwd);
+	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 +427,12 @@ 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 (getenv("GIT_ONE_FILESYSTEM") != NULL) {
+			if (stat("..", &buf))
+				die_errno("failed to stat '..'");
+			if (buf.st_dev != current_device)
+				die("refusing to cross filesystem boundary '%s/..'", cwd);
+		}
 		if (chdir(".."))
 			die_errno("Cannot change to '%s/..'", cwd);
 	}

-- 
1.6.5.2
Sverre Rabbelier· Mar 15, 2010, 23:16 UTC · re: Lars Damerow · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

Heya,
On Mon, Mar 15, 2010 at 22:40, Lars Damerow <lars@pixar.com> wrote:
> +                       if (buf.st_dev != current_device)
> +                               die("refusing to cross filesystem boundary '%s/..'", cwd);

Great, that's going to tell the unfortunate user who runs into this a whole lot of nothing. Shouldn't you at least tell them what git was trying to do, how they can fix it if this is not the desired behavior, etc. etc.?

-- 
Cheers,

Sverre Rabbelier
Lars Damerow· Mar 15, 2010, 23:36 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

Show 11 quoted lines
>From Sverre Rabbelier <srabbelier@gmail.com>, Tue, Mar 16, 2010 at 12:16:19AM +0100:
> Heya,
> 
> On Mon, Mar 15, 2010 at 22:40, Lars Damerow <lars@pixar.com> wrote:
> > if (buf.st_dev != current_device)
> >     die("refusing to cross filesystem boundary '%s/..'", cwd);
> 
> Great, that's going to tell the unfortunate user who runs into this a
> whole lot of nothing. Shouldn't you at least tell them what git was
> trying to do, how they can fix it if this is not the desired behavior,
> etc. etc.?
Hi Sverre,
You're right, that's a badly-worded error. How about this?
    Not a git repository (or any parent up to /path/to/mountpoint)
    Stopping at filesystem boundary since GIT_ONE_FILESYSTEM is set.
-lars

-- lars r. damerow :: button pusher :: pixar animation studios

Science Factoid:  The common cockroach shares 99.5 percent of its DNA code
                  with e-mail spammers and telemarketers.
Jeff King· Mar 16, 2010, 02:33 UTC · re: Lars Damerow · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

On Mon, Mar 15, 2010 at 02:40:03PM -0700, Lars Damerow wrote:
> +		if (getenv("GIT_ONE_FILESYSTEM") != NULL) {

Should this really trigger for GIT_ONE_FILESYSTEM=0? We already have git_env_bool, which will handle 0/1, true/false, etc. Probably you should use it here.

I am not a big fan of the environment variable name, either, but I don't have another good suggestion. It is closely related to GIT_CEILING_DIRECTORIES (in fact, you could probably solve the same problem with GIT_CEILING_DIRECTORIES, but I think your solution is much nicer in that it lets the user get away with being less verbose).

Show 5 quoted lines
> +			if (stat("..", &buf))
> +				die_errno("failed to stat '..'");
> +			if (buf.st_dev != current_device)
> +				die("refusing to cross filesystem boundary '%s/..'", cwd);
> +		}

I agree with Sverre that this message isn't descriptive enough, but I like the suggestion you posted elsewhere in the thread.

-Peff
Lars Damerow· Mar 16, 2010, 03:28 UTC · re: Jeff King · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

Show 8 quoted lines
>From Jeff King <peff@peff.net>, Mon, Mar 15, 2010 at 10:33:06PM -0400:
> On Mon, Mar 15, 2010 at 02:40:03PM -0700, Lars Damerow wrote:
> 
> > +		if (getenv("GIT_ONE_FILESYSTEM") != NULL) {
> 
> Should this really trigger for GIT_ONE_FILESYSTEM=0? We already have
> git_env_bool, which will handle 0/1, true/false, etc. Probably you
> should use it here.

Thanks for the pointer--I wasn't aware of git_env_bool. I'll use it instead.

Show 5 quoted lines
> I am not a big fan of the environment variable name, either, but I don't
> have another good suggestion. It is closely related to
> GIT_CEILING_DIRECTORIES (in fact, you could probably solve the same
> problem with GIT_CEILING_DIRECTORIES, but I think your solution is much
> nicer in that it lets the user get away with being less verbose).

I was thinking about cp's -x option and its description in the man page when I chose GIT_ONE_FILESYSTEM. Before I wrote this patch I configured my machines to have their most popular automount points in GIT_CEILING_DIRECTORIES, but need a more complete solution and don't want to have a cron job running to keep the list up to date.

Do the overall idea and implementation have enough merit that the patch could get accepted?

thanks, -lars

-- lars r. damerow :: button pusher :: pixar animation studios

The big print giveth and the small print taketh away.
Jeff King· Mar 16, 2010, 03:37 UTC · re: Lars Damerow · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

On Mon, Mar 15, 2010 at 08:28:58PM -0700, Lars Damerow wrote:
> I was thinking about cp's -x option and its description in the man page
> when I chose GIT_ONE_FILESYSTEM. Before I wrote this patch I configured

Ah, I always just used "-x" and didn't know about "--one-file-system". So in that way, it makes sense.

> Do the overall idea and implementation have enough merit that the patch
> could get accepted?
Yeah, I think it is an improvement overall.
-Peff
Junio C Hamano· Mar 16, 2010, 04:19 UTC · re: Jeff King · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

Jeff King <peff@peff.net> writes:
Show 13 quoted lines
> On Mon, Mar 15, 2010 at 02:40:03PM -0700, Lars Damerow wrote:
>
>> +		if (getenv("GIT_ONE_FILESYSTEM") != NULL) {
>
> Should this really trigger for GIT_ONE_FILESYSTEM=0? We already have
> git_env_bool, which will handle 0/1, true/false, etc. Probably you
> should use it here.
>
> I am not a big fan of the environment variable name, either, but I don't
> have another good suggestion. It is closely related to
> GIT_CEILING_DIRECTORIES (in fact, you could probably solve the same
> problem with GIT_CEILING_DIRECTORIES, but I think your solution is much
> nicer in that it lets the user get away with being less verbose).

Yeah, that, or toplevel global option --one-file-system given to "git", but I think that is probably too cumbersome.

How inefficient is it to repeatedly call getenv() in a loop, by the way? I think the patch would become easier to read if env_bool() is called once at the beginning to set a local variable, and not even do the initial stat() for "." if this feature is not asked for.

Lars Damerow· Mar 16, 2010, 05:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add support for GIT_ONE_FILESYSTEM

Show 6 quoted lines
>From Junio C Hamano <gitster@pobox.com>, Mon, Mar 15, 2010 at 09:19:17PM -0700:
> 
> How inefficient is it to repeatedly call getenv() in a loop, by the way?
> I think the patch would become easier to read if env_bool() is called once
> at the beginning to set a local variable, and not even do the initial stat()
> for "." if this feature is not asked for.

Agreed, I didn't do this too efficiently. Thanks for the feedback! I'll make a new patch and resubmit.

cheers, -lars

-- lars r. damerow :: button pusher :: pixar animation studios

When in doubt, use brute force.

← back to recent threads