* Re: [PATCH 2/3] git p4 test: should honor symlink in p4 client root
From: Johannes Sixt @ 2013-03-08 6:42 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git, Miklós Fazekas, John Keeping
In-Reply-To: <1362698357-7334-3-git-send-email-pw@padd.com>
Am 3/8/2013 0:19, schrieb Pete Wyckoff:
> +# When the p4 client Root is a symlink, make sure chdir() does not use
> +# getcwd() to convert it to a physical path.
> +test_expect_failure 'p4 client root symlink should stay symbolic' '
> + physical="$TRASH_DIRECTORY/physical" &&
> + symbolic="$TRASH_DIRECTORY/symbolic" &&
> + test_when_finished "rm -rf \"$physical\"" &&
> + test_when_finished "rm \"$symbolic\"" &&
> + mkdir -p "$physical" &&
> + ln -s "$physical" "$symbolic" &&
This test needs a SYMLINKS prerequisite to future-proof it, in case the
Windows port gains p4 support some time.
> + test_when_finished cleanup_git &&
> + (
> + P4CLIENT=client-sym &&
> + p4 client -i <<-EOF &&
> + Client: $P4CLIENT
> + Description: $P4CLIENT
> + Root: $symbolic
> + LineEnd: unix
> + View: //depot/... //$P4CLIENT/...
> + EOF
> + git p4 clone --dest="$git" //depot &&
> + cd "$git" &&
> + test_commit file2 &&
> + git config git-p4.skipSubmitEdit true &&
> + git p4 submit
> + )
> +'
-- Hannes
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Torsten Bögershausen @ 2013-03-08 7:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Torsten Bögershausen, Duy Nguyen, Ramkumar Ramachandra,
Robert Zeh, Git List, finnag
In-Reply-To: <7vr4jqkb9g.fsf@alter.siamese.dyndns.org>
On 08.03.13 01:04, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index d6dd3df..6a5ba11 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1158,6 +1158,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>> unsigned char sha1[20];
>> static struct option builtin_status_options[] = {
>> OPT__VERBOSE(&verbose, N_("be verbose")),
>> + OPT_BOOLEAN('c', "changed-only", &s.check_changed_only,
>> + N_("Ignore untracked files. Check if files known to git are modified")),
>
> Doesn't this make one wonder why a separate bit and implementation
> is necessary to say "I am not interested in untracked files" when
> "-uno" option is already there?
Thanks Junio,
this is good news.
I need to admit that I wasn't aware about "git status -uno".
Thinking about it, how many git users are aware of the speed penalty
when running git status to find out which (tracked) files they had changed?
Or to put it the other way, when a developer wants a quick overview
about the files she changed, then git status -uno may be a good and fast friend.
Does it make sence to stress put that someway in the documentation?
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f1ef9a..360d813 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -51,13 +51,18 @@ default is 'normal', i.e. show untracked files and directori
+
The possible options are:
+
- - 'no' - Show no untracked files
+ - 'no' - Show no untracked files (this is fastest)
- 'normal' - Shows untracked files and directories
- 'all' - Also shows individual files in untracked directories.
+
The default can be changed using the status.showUntrackedFiles
configuration variable documented in linkgit:git-config[1].
++
+Note: Searching for untracked files or directories may take some time.
+A fast way to get a status of files tracked by git is to use
+'git status -uno'
+
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply related
* [PATCH] setup: suppress implicit "." work-tree for bare repos
From: Jeff King @ 2013-03-08 7:15 UTC (permalink / raw)
To: Mark Lodato; +Cc: Junio C Hamano, git list
In-Reply-To: <20130308054824.GA24429@sigill.intra.peff.net>
If an explicit GIT_DIR is given without a working tree, we
implicitly assume that the current working directory should
be used as the working tree. E.g.,:
GIT_DIR=/some/repo.git git status
would compare against the cwd.
Unfortunately, we fool this rule for sub-invocations of git
by setting GIT_DIR internally ourselves. For example:
git init foo
cd foo/.git
git status ;# fails, as we expect
git config alias.st status
git status ;# does not fail, but should
What happens is that we run setup_git_directory when doing
alias lookup (since we need to see the config), set GIT_DIR
as a result, and then leave GIT_WORK_TREE blank (because we
do not have one). Then when we actually run the status
command, we do setup_git_directory again, which sees our
explicit GIT_DIR and uses the cwd as an implicit worktree.
It's tempting to argue that we should be suppressing that
second invocation of setup_git_directory, as it could use
the values we already found in memory. However, the problem
still exists for sub-processes (e.g., if "git status" were
an external command).
You can see another example with the "--bare" option, which
sets GIT_DIR explicitly. For example:
git init foo
cd foo/.git
git status ;# fails
git --bare status ;# does NOT fail
We need some way of telling sub-processes "even though
GIT_DIR is set, do not use cwd as an implicit working tree".
We could do it by putting a special token into
GIT_WORK_TREE, but the obvious choice (an empty string) has
some portability problems, and could potentially be
triggered accidentally by a user.
Instead, we add a new boolean variable, GIT_IMPLICIT_WORK_TREE,
which suppresses the use of cwd as a working tree when
GIT_DIR is set. We trigger the new variable when we know we
are in a bare setting.
The variable is left intentionally undocumented, as this is
an internal detail (for now, anyway). If somebody comes up
with a good alternate use for it, and once we are confident
we have shaken any bugs out of it, we can consider promoting
it further.
Signed-off-by: Jeff King <peff@peff.net>
---
So I think this just ends up being a cleaner and smaller change than
trying to support $GIT_BARE. I think $GIT_BARE could allow more
flexibility, but it's flexibility nobody is particularly asking for, and
there are lots of nasty corner cases around it. I'm pretty sure this is
doing the right thing.
Having written this, I'm still tempted to signal the same thing by
putting /dev/null into GIT_WORK_TREE (Junio's suggestion from the old
thread). This one works OK because we only check GIT_WORK_TREE_IMPLICIT
_after_ exhausting all of the other working tree options, so it is
always subordinate to a later setting of GIT_WORK_TREE. But it seems a
little cleaner for somebody setting GIT_WORK_TREE To clear this
"implicit" flag automatically.
At the same time, I would wonder how other git implementations would
react to GIT_WORK_TREE=/dev/null. Would they try to chdir() there and
barf, when they could happily exist without a working tree? Doing it
this way seems a bit safer from regressions (those other implementations
do not get the _benefit_ of this patch unless they support
GIT_WORK_TREE_IMPLICIT, of course, but at least we are not breaking
them).
cache.h | 1 +
git.c | 1 +
setup.c | 8 ++++++++
t/t1510-repo-setup.sh | 19 +++++++++++++++++++
4 files changed, 29 insertions(+)
diff --git a/cache.h b/cache.h
index e493563..070169a 100644
--- a/cache.h
+++ b/cache.h
@@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_DIR_ENVIRONMENT "GIT_DIR"
#define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
#define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
+#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
#define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
#define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
#define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
diff --git a/git.c b/git.c
index b10c18b..24b7984 100644
--- a/git.c
+++ b/git.c
@@ -125,6 +125,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
static char git_dir[PATH_MAX+1];
is_bare_repository_cfg = 1;
setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
+ setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "-c")) {
diff --git a/setup.c b/setup.c
index 1dee47e..6c87660 100644
--- a/setup.c
+++ b/setup.c
@@ -523,6 +523,12 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
set_git_work_tree(core_worktree);
}
}
+ else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
+ /* #16d */
+ set_git_dir(gitdirenv);
+ free(gitfile);
+ return NULL;
+ }
else /* #2, #10 */
set_git_work_tree(".");
@@ -601,6 +607,8 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi
if (check_repository_format_gently(".", nongit_ok))
return NULL;
+ setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
+
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
const char *gitdir;
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index 80aedfc..cf2ee78 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -517,6 +517,25 @@ test_expect_success '#16c: bare .git has no worktree' '
"$here/16c/.git" "(null)" "$here/16c/sub" "(null)"
'
+test_expect_success '#16d: bareness preserved across alias' '
+ setup_repo 16d unset "" unset &&
+ (
+ cd 16d/.git &&
+ test_must_fail git status &&
+ git config alias.st status &&
+ test_must_fail git st
+ )
+'
+
+test_expect_success '#16e: bareness preserved by --bare' '
+ setup_repo 16e unset "" unset &&
+ (
+ cd 16e/.git &&
+ test_must_fail git status &&
+ test_must_fail git --bare status
+ )
+'
+
test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is accepted (bare case)' '
# Just like #16.
setup_repo 17a unset "" true &&
--
1.8.2.rc2.4.g3e774bb
^ permalink raw reply related
* Re: [PATCH] setup: suppress implicit "." work-tree for bare repos
From: Johannes Sixt @ 2013-03-08 7:44 UTC (permalink / raw)
To: Jeff King; +Cc: Mark Lodato, Junio C Hamano, git list
In-Reply-To: <20130308071554.GB24429@sigill.intra.peff.net>
Am 3/8/2013 8:15, schrieb Jeff King:
> --- a/cache.h
> +++ b/cache.h
> @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode)
> #define GIT_DIR_ENVIRONMENT "GIT_DIR"
> #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
> #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
> +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
> #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
> #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
> #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
This new variable needs to be added to environment.c:local_repo_env, right?
-- Hannes
^ permalink raw reply
* Re: [PATCH] setup: suppress implicit "." work-tree for bare repos
From: Junio C Hamano @ 2013-03-08 7:54 UTC (permalink / raw)
To: Jeff King; +Cc: Mark Lodato, git list
In-Reply-To: <20130308071554.GB24429@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> diff --git a/cache.h b/cache.h
> index e493563..070169a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode)
> #define GIT_DIR_ENVIRONMENT "GIT_DIR"
> #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
> #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
> +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
> #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
> #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
> #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
Not adding any user documentation is fine (you explained why in the
log message), but I would really prefer to have some in-code comment
to clarify its meaning. Is it "Please do use implicit work tree"
boolean? Is it "This is the path to the work tree we have already
figured out" string? Is it something else? What is it used for,
who sets it, what other codepath that will be invented in the future
need to be careful to set it (or unset it) and how does one who
writes that new codepath decides that he needs to do so (or
shouldn't)?
I would know *today* that it is a bool to affect us, after having
discovered that we are in bare and we have set GIT_DIR (so if the
end user already had GIT_DIR, we shouldn't set it ourselves), and
also our child processes, but I am not confident that I will
remember this thread 6 months down the road.
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Junio C Hamano @ 2013-03-08 8:15 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Duy Nguyen, Ramkumar Ramachandra, Robert Zeh, Git List, finnag
In-Reply-To: <51398CD5.1070603@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
>> Doesn't this make one wonder why a separate bit and implementation
>> is necessary to say "I am not interested in untracked files" when
>> "-uno" option is already there?
> ...
> I need to admit that I wasn't aware about "git status -uno".
Not so fast. I did not ask you "Why do you need a new one to solve
the same problem -uno already solves?"
> Thinking about it, how many git users are aware of the speed penalty
> when running git status to find out which (tracked) files they had changed?
>
> Or to put it the other way, when a developer wants a quick overview
> about the files she changed, then git status -uno may be a good and fast friend.
>
> Does it make sence to stress put that someway in the documentation?
>
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 9f1ef9a..360d813 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -51,13 +51,18 @@ default is 'normal', i.e. show untracked files and directori
> +
> The possible options are:
> +
> - - 'no' - Show no untracked files
> + - 'no' - Show no untracked files (this is fastest)
There is a trade-off around the use of -uno between safety and
performance. The default is not to use -uno so that you will not
forget to add a file you newly created (i.e safety). You would pay
for the safety with the cost to find such untracked files (i.e.
performance).
I suspect that the documentation was written with the assumption
that at least for the people who are reading this part of the
documentation, the trade-off is obvious. In order to find more
information, you naturally need to spend more cycles.
If the trade-off is not so obvious, however, I do not object at all
to describing it. But if we are to do so, I do object to mentioning
only one side of the trade-off. People who choose "fastest" needs
to be made very aware that they are disabling "safety".
That brings us back to the "Why a separate implementation when -uno
is there?" question.
Your patch adds new code; it does not just enables the same logic as
what the existing -uno does with a new flag. Does the new compute
different things? Does it find more stuff by spending extra cycles?
Does it find less stuff by being extra faster?
These questions are important.
If the new option strikes the trade-off between safety and
performance at a point different from the point where the existing
-uno option does, it _might_ still be worth adding as a separate
option. I didn't get that impression when I saw the patch, but I
admit that I did not follow the code carefully myself.
That is the reason why I was wondering why a separate bit and
implementation had to be added by the patch.
^ permalink raw reply
* Re: [PATCH] setup: suppress implicit "." work-tree for bare repos
From: Jeff King @ 2013-03-08 8:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Mark Lodato, Junio C Hamano, git list
In-Reply-To: <513996D4.6060009@viscovery.net>
On Fri, Mar 08, 2013 at 08:44:20AM +0100, Johannes Sixt wrote:
> Am 3/8/2013 8:15, schrieb Jeff King:
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode)
> > #define GIT_DIR_ENVIRONMENT "GIT_DIR"
> > #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
> > #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
> > +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
> > #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
> > #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
> > #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
>
> This new variable needs to be added to environment.c:local_repo_env, right?
It does; I had no idea local_repo_env existed. We should add a comment
to cache.h to that effect, too.
-Peff
^ permalink raw reply
* Re: [PATCH] setup: suppress implicit "." work-tree for bare repos
From: Jeff King @ 2013-03-08 8:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Lodato, git list
In-Reply-To: <7vboaujphx.fsf@alter.siamese.dyndns.org>
On Thu, Mar 07, 2013 at 11:54:18PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > diff --git a/cache.h b/cache.h
> > index e493563..070169a 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode)
> > #define GIT_DIR_ENVIRONMENT "GIT_DIR"
> > #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
> > #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
> > +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
> > #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
> > #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
> > #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
>
> Not adding any user documentation is fine (you explained why in the
> log message), but I would really prefer to have some in-code comment
> to clarify its meaning. Is it "Please do use implicit work tree"
> boolean? Is it "This is the path to the work tree we have already
> figured out" string? Is it something else? What is it used for,
> who sets it, what other codepath that will be invented in the future
> need to be careful to set it (or unset it) and how does one who
> writes that new codepath decides that he needs to do so (or
> shouldn't)?
My intent was that the commit message would be enough to explain it, but
it is a pain for a later reader to have to blame the line back to that
commit to read it. I'll re-roll with a comment.
-Peff
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Torsten Bögershausen @ 2013-03-08 9:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Torsten Bögershausen, Duy Nguyen, Ramkumar Ramachandra,
Robert Zeh, Git List, finnag
In-Reply-To: <7v7glijoiy.fsf@alter.siamese.dyndns.org>
On 08.03.13 09:15, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>>> Doesn't this make one wonder why a separate bit and implementation
>>> is necessary to say "I am not interested in untracked files" when
>>> "-uno" option is already there?
>> ...
>> I need to admit that I wasn't aware about "git status -uno".
>
> Not so fast. I did not ask you "Why do you need a new one to solve
> the same problem -uno already solves?"
>
>> Thinking about it, how many git users are aware of the speed penalty
>> when running git status to find out which (tracked) files they had changed?
>>
>> Or to put it the other way, when a developer wants a quick overview
>> about the files she changed, then git status -uno may be a good and fast friend.
>>
>> Does it make sence to stress put that someway in the documentation?
>>
>> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
>> index 9f1ef9a..360d813 100644
>> --- a/Documentation/git-status.txt
>> +++ b/Documentation/git-status.txt
>> @@ -51,13 +51,18 @@ default is 'normal', i.e. show untracked files and directori
>> +
>> The possible options are:
>> +
>> - - 'no' - Show no untracked files
>> + - 'no' - Show no untracked files (this is fastest)
>
> There is a trade-off around the use of -uno between safety and
> performance. The default is not to use -uno so that you will not
> forget to add a file you newly created (i.e safety). You would pay
> for the safety with the cost to find such untracked files (i.e.
> performance).
>
> I suspect that the documentation was written with the assumption
> that at least for the people who are reading this part of the
> documentation, the trade-off is obvious. In order to find more
> information, you naturally need to spend more cycles.
>
> If the trade-off is not so obvious, however, I do not object at all
> to describing it. But if we are to do so, I do object to mentioning
> only one side of the trade-off. People who choose "fastest" needs
> to be made very aware that they are disabling "safety".
>
> That brings us back to the "Why a separate implementation when -uno
> is there?" question.
[...]
The short version:
The -uno option does exactly what the -c option intended to do ;-)
(The code path to disable the "expensive" call to read_directory_recursive()
in dir.c is slightly different).
Making benchmarks (again, sorry for the noise) shows that -uno and -c are equally fast,
making 5 git status on a linux tree, take the best of 5:
git status
real 0m0.697s
git status -uno
real 0m0.291s
(with the patch) git status -c
real 0m0.289s
These are not really scientific numbers, but all in all we have motivation enough to drop
the "git status -c" patch completely.
My feeling is still that the suggested documentation "this is fastest" is not a good choice either.
Let me try to come up with a better suggestion.
/Torsten
^ permalink raw reply
* [PATCHv2] setup and GIT_IMPLICIT_WORK_TREE
From: Jeff King @ 2013-03-08 9:28 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Junio C Hamano, Mark Lodato
In-Reply-To: <20130308084225.GA10963@sigill.intra.peff.net>
Here's a re-roll of the GIT_IMPLICIT_WORK_TREE patch which should
address the comments in the last round. I've added an explanatory
comment near the variable definition, and added it to local_repo_env.
While doing that, I noticed some cleanup opportunities around
local_repo_env, which resulted in the first two patches.
[1/3]: cache.h: drop LOCAL_REPO_ENV_SIZE
[2/3]: environment: add GIT_PREFIX to local_repo_env
[3/3]: setup: suppress implicit "." work-tree for bare repos
-Peff
^ permalink raw reply
* [PATCH v2 1/3] cache.h: drop LOCAL_REPO_ENV_SIZE
From: Jeff King @ 2013-03-08 9:29 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Junio C Hamano, Mark Lodato
In-Reply-To: <20130308092824.GA9127@sigill.intra.peff.net>
We keep a static array of variables that should be cleared
when invoking a sub-process on another repo. We statically
size the array with the LOCAL_REPO_ENV_SIZE macro so that
any readers do not have to count it themselves.
As it turns out, no readers actually use the macro, and it
creates a maintenance headache, as modifications to the
array need to happen in two places (one to add the new
element, and another to bump the size).
Since it's NULL-terminated, we can just drop the size macro
entirely. While we're at it, we'll clean up some comments
around it, and add a new mention of it at the top of the
list of environment variable macros. Even though
local_repo_env is right below that list, it's easy to miss,
and additions to that list should consider local_repo_env.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 12 ++++++------
environment.c | 6 ++----
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/cache.h b/cache.h
index e493563..b90044a 100644
--- a/cache.h
+++ b/cache.h
@@ -341,6 +341,7 @@ static inline enum object_type object_type(unsigned int mode)
OBJ_BLOB;
}
+/* Double-check local_repo_env below if you add to this list. */
#define GIT_DIR_ENVIRONMENT "GIT_DIR"
#define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
#define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
@@ -365,13 +366,12 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_LITERAL_PATHSPECS_ENVIRONMENT "GIT_LITERAL_PATHSPECS"
/*
- * Repository-local GIT_* environment variables
- * The array is NULL-terminated to simplify its usage in contexts such
- * environment creation or simple walk of the list.
- * The number of non-NULL entries is available as a macro.
+ * Repository-local GIT_* environment variables; these will be cleared
+ * when git spawns a sub-process that runs inside another repository.
+ * The array is NULL-terminated, which makes it easy to pass in the "env"
+ * parameter of a run-command invocation, or to do a simple walk.
*/
-#define LOCAL_REPO_ENV_SIZE 9
-extern const char *const local_repo_env[LOCAL_REPO_ENV_SIZE + 1];
+extern const char * const local_repo_env[];
extern int is_bare_repository_cfg;
extern int is_bare_repository(void);
diff --git a/environment.c b/environment.c
index 89d6c70..dc73927 100644
--- a/environment.c
+++ b/environment.c
@@ -83,11 +83,9 @@ static char *git_object_dir, *git_index_file, *git_graft_file;
static char *git_object_dir, *git_index_file, *git_graft_file;
/*
- * Repository-local GIT_* environment variables
- * Remember to update local_repo_env_size in cache.h when
- * the size of the list changes
+ * Repository-local GIT_* environment variables; see cache.h for details.
*/
-const char * const local_repo_env[LOCAL_REPO_ENV_SIZE + 1] = {
+const char * const local_repo_env[] = {
ALTERNATE_DB_ENVIRONMENT,
CONFIG_ENVIRONMENT,
CONFIG_DATA_ENVIRONMENT,
--
1.8.2.rc2.4.g3e774bb
^ permalink raw reply related
* [PATCH v2 2/3] environment: add GIT_PREFIX to local_repo_env
From: Jeff King @ 2013-03-08 9:30 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Junio C Hamano, Mark Lodato
In-Reply-To: <20130308092824.GA9127@sigill.intra.peff.net>
The GIT_PREFIX variable is set based on our location within
the working tree. It should therefore be cleared whenever
GIT_WORK_TREE is cleared.
In practice, this doesn't cause any bugs, because none of
the sub-programs we invoke with local_repo_env cleared
actually care about GIT_PREFIX. But this is the right thing
to do, and future proofs us again that assumption changing.
While we're at it, let's define a GIT_PREFIX_ENVIRONMENT
macro; this avoids repetition of the string literal, which
can help catch any spelling mistakes in the code.
Signed-off-by: Jeff King <peff@peff.net>
---
I noticed this one because it was near code I was touching in an earlier
iteration of patch 3. I gave a quick skim and did not notice any other
variables which would want to receive the same treatment.
cache.h | 1 +
environment.c | 1 +
setup.c | 4 ++--
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index b90044a..23e6e62 100644
--- a/cache.h
+++ b/cache.h
@@ -345,6 +345,7 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_DIR_ENVIRONMENT "GIT_DIR"
#define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
#define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
+#define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX"
#define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
#define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
#define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
diff --git a/environment.c b/environment.c
index dc73927..2bd1c37 100644
--- a/environment.c
+++ b/environment.c
@@ -95,6 +95,7 @@ const char * const local_repo_env[] = {
GRAFT_ENVIRONMENT,
INDEX_ENVIRONMENT,
NO_REPLACE_OBJECTS_ENVIRONMENT,
+ GIT_PREFIX_ENVIRONMENT,
NULL
};
diff --git a/setup.c b/setup.c
index 1dee47e..1996295 100644
--- a/setup.c
+++ b/setup.c
@@ -794,9 +794,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
prefix = setup_git_directory_gently_1(nongit_ok);
if (prefix)
- setenv("GIT_PREFIX", prefix, 1);
+ setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
else
- setenv("GIT_PREFIX", "", 1);
+ setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
if (startup_info) {
startup_info->have_repository = !nongit_ok || !*nongit_ok;
--
1.8.2.rc2.4.g3e774bb
^ permalink raw reply related
* [PATCH v2 3/3] setup: suppress implicit "." work-tree for bare repos
From: Jeff King @ 2013-03-08 9:32 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Junio C Hamano, Mark Lodato
In-Reply-To: <20130308092824.GA9127@sigill.intra.peff.net>
If an explicit GIT_DIR is given without a working tree, we
implicitly assume that the current working directory should
be used as the working tree. E.g.,:
GIT_DIR=/some/repo.git git status
would compare against the cwd.
Unfortunately, we fool this rule for sub-invocations of git
by setting GIT_DIR internally ourselves. For example:
git init foo
cd foo/.git
git status ;# fails, as we expect
git config alias.st status
git status ;# does not fail, but should
What happens is that we run setup_git_directory when doing
alias lookup (since we need to see the config), set GIT_DIR
as a result, and then leave GIT_WORK_TREE blank (because we
do not have one). Then when we actually run the status
command, we do setup_git_directory again, which sees our
explicit GIT_DIR and uses the cwd as an implicit worktree.
It's tempting to argue that we should be suppressing that
second invocation of setup_git_directory, as it could use
the values we already found in memory. However, the problem
still exists for sub-processes (e.g., if "git status" were
an external command).
You can see another example with the "--bare" option, which
sets GIT_DIR explicitly. For example:
git init foo
cd foo/.git
git status ;# fails
git --bare status ;# does NOT fail
We need some way of telling sub-processes "even though
GIT_DIR is set, do not use cwd as an implicit working tree".
We could do it by putting a special token into
GIT_WORK_TREE, but the obvious choice (an empty string) has
some portability problems.
Instead, we add a new boolean variable, GIT_IMPLICIT_WORK_TREE,
which suppresses the use of cwd as a working tree when
GIT_DIR is set. We trigger the new variable when we know we
are in a bare setting.
The variable is left intentionally undocumented, as this is
an internal detail (for now, anyway). If somebody comes up
with a good alternate use for it, and once we are confident
we have shaken any bugs out of it, we can consider promoting
it further.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 12 ++++++++++++
environment.c | 1 +
git.c | 1 +
setup.c | 8 ++++++++
t/t1510-repo-setup.sh | 19 +++++++++++++++++++
5 files changed, 41 insertions(+)
diff --git a/cache.h b/cache.h
index 23e6e62..635f2e9 100644
--- a/cache.h
+++ b/cache.h
@@ -367,6 +367,18 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_LITERAL_PATHSPECS_ENVIRONMENT "GIT_LITERAL_PATHSPECS"
/*
+ * This environment variable is expected to contain a boolean indicating
+ * whether we should or should not treat:
+ *
+ * GIT_DIR=foo.git git ...
+ *
+ * as if GIT_WORK_TREE=. was given. It's not expected that users will make use
+ * of this, but we use it internally to communicate to sub-processes that we
+ * are in a bare repo. If not set, defaults to true.
+ */
+#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
+
+/*
* Repository-local GIT_* environment variables; these will be cleared
* when git spawns a sub-process that runs inside another repository.
* The array is NULL-terminated, which makes it easy to pass in the "env"
diff --git a/environment.c b/environment.c
index 2bd1c37..e2e75c1 100644
--- a/environment.c
+++ b/environment.c
@@ -92,6 +92,7 @@ const char * const local_repo_env[] = {
DB_ENVIRONMENT,
GIT_DIR_ENVIRONMENT,
GIT_WORK_TREE_ENVIRONMENT,
+ GIT_IMPLICIT_WORK_TREE_ENVIRONMENT,
GRAFT_ENVIRONMENT,
INDEX_ENVIRONMENT,
NO_REPLACE_OBJECTS_ENVIRONMENT,
diff --git a/git.c b/git.c
index b10c18b..24b7984 100644
--- a/git.c
+++ b/git.c
@@ -125,6 +125,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
static char git_dir[PATH_MAX+1];
is_bare_repository_cfg = 1;
setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
+ setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "-c")) {
diff --git a/setup.c b/setup.c
index 1996295..9107f54 100644
--- a/setup.c
+++ b/setup.c
@@ -523,6 +523,12 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
set_git_work_tree(core_worktree);
}
}
+ else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
+ /* #16d */
+ set_git_dir(gitdirenv);
+ free(gitfile);
+ return NULL;
+ }
else /* #2, #10 */
set_git_work_tree(".");
@@ -601,6 +607,8 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi
if (check_repository_format_gently(".", nongit_ok))
return NULL;
+ setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
+
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
const char *gitdir;
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index 80aedfc..cf2ee78 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -517,6 +517,25 @@ test_expect_success '#16c: bare .git has no worktree' '
"$here/16c/.git" "(null)" "$here/16c/sub" "(null)"
'
+test_expect_success '#16d: bareness preserved across alias' '
+ setup_repo 16d unset "" unset &&
+ (
+ cd 16d/.git &&
+ test_must_fail git status &&
+ git config alias.st status &&
+ test_must_fail git st
+ )
+'
+
+test_expect_success '#16e: bareness preserved by --bare' '
+ setup_repo 16e unset "" unset &&
+ (
+ cd 16e/.git &&
+ test_must_fail git status &&
+ test_must_fail git --bare status
+ )
+'
+
test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is accepted (bare case)' '
# Just like #16.
setup_repo 17a unset "" true &&
--
1.8.2.rc2.4.g3e774bb
^ permalink raw reply related
* Re: [PATCH] git svn: ignore partial svn:mergeinfo
From: Eric Wong @ 2013-03-08 9:53 UTC (permalink / raw)
To: Jan Pešta
Cc: git, 'Junio C Hamano', 'Matthieu Moy',
'Sam Vilain'
In-Reply-To: <000e01ce1b26$dbb65570$93230050$@certicon.cz>
Jan Pešta <jan.pesta@certicon.cz> wrote:
> Currently this is cosmetic change - the merges are ignored, becuase the methods
> (lookup_svn_merge, find_rev_before, find_rev_after) are failing on comparing text with number.
>
> See http://www.open.collab.net/community/subversion/articles/merge-info.html
> Extract:
> The range r30430:30435 that was added to 1.5.x in this merge has a '*' suffix for 1.5.x\www.
> This '*' is the marker for a non-inheritable mergeinfo range.
> The '*' means that only the path on which the mergeinfo is explicitly set has had this range merged into it.
>
> Signed-off-by: Jan Pesta <jan.pesta@certicon.cz>
> ---
> perl/Git/SVN.pm | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
> index 0ebc68a..74d49bb 100644
> --- a/perl/Git/SVN.pm
> +++ b/perl/Git/SVN.pm
> @@ -1493,6 +1493,11 @@ sub lookup_svn_merge {
> my @merged_commit_ranges;
> # find the tip
> for my $range ( @ranges ) {
> + if ($range =~ /[*]$/) {
> + warn "W:Ignoring partial merge in svn:mergeinfo "
Thanks,
I've pushed this with a minor formatting change (added space after
"W:") will push another change for formatting existing warnings more
consistently.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
^ permalink raw reply
* [PATCH] git svn: consistent spacing after "W:" in warnings
From: Eric Wong @ 2013-03-08 10:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, 'Matthieu Moy', 'Sam Vilain', Jan Pešta
In-Reply-To: <20130308095330.GA20205@dcvr.yhbt.net>
Eric Wong <normalperson@yhbt.net> wrote:
> will push another change for formatting existing warnings more
> consistently.
Just pushed. My master is sitting at git://git.bogomips.org/git-svn.git
commit eae6cf5aa8ae2d8a90a99bbe4aeb01c29e01fd02
Eric Wong (1):
git svn: consistent spacing after "W:" in warnings
Jan Pešta (1):
git svn: ignore partial svn:mergeinfo
I don't have further updates planned, maybe others do.
----------------------------8<------------------------------
From eae6cf5aa8ae2d8a90a99bbe4aeb01c29e01fd02 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Fri, 8 Mar 2013 09:46:41 +0000
Subject: [PATCH] git svn: consistent spacing after "W:" in warnings
All other instances of "W:"-prefixed warning messages have a space after
the "W:" to help with readability.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
perl/Git/SVN.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 46aeb85..5273ee8 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1504,7 +1504,7 @@ sub lookup_svn_merge {
my $top_commit = $gs->find_rev_before( $top, 1, $bottom );
unless ($top_commit and $bottom_commit) {
- warn "W:unknown path/rev in svn:mergeinfo "
+ warn "W: unknown path/rev in svn:mergeinfo "
."dirprop: $source:$range\n";
next;
}
--
Eric Wong
^ permalink raw reply related
* Re: [ANNOUNCE] Git v1.8.2-rc3
From: Matthieu Moy @ 2013-03-08 10:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7glils5k.fsf@alter.siamese.dyndns.org>
A few suggestions on the release notes (you may safely ignore)
Junio C Hamano <gitster@pobox.com> writes:
> In the next major release Git 2.0 (not *this* one), we will change the
> behavior of the "git push" command.
It's not entirely clear from the formatting whether this sentenece
introduces the whole "Backward compatibility notes" section or just the
paragraph above. I'd merge this paragraph with the one below
(push.default).
It may make sense to split "Backward compatibility notes" in two
sections "Backward compatibility notes (this release)" and "Backward
compatibility notes (future Git 2.0 release)".
> At Git 2.0 (not *this* one), we plan to change these commands without
> pathspec to operate on the entire tree. Forming a habit to type "."
> when you mean to limit the command to the current working directory
> will protect you against the planned future change, and that is the
> whole point of the new message (there will be no configuration
> variable to squelch this warning---it goes against the "habit forming"
> objective).
I think it will make sense to have a way to shut down the warning
starting from Git 2.0 (people who upgrade and don't look back aren't
interested in warnings about compatibility with previous versions).
So I'd say just "there is no configuration" instead of "there will be no
configuration".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* svn-fe + git-fast-import bails out on certain SVN commits
From: Björn Töpel @ 2013-03-08 10:21 UTC (permalink / raw)
To: git
I'm importing a huge repository using svn-fe and git-fast-import.
$ cat repo.svn_dump | svn-fe 'svn+ssh://bjornto@example.com/pub/repo/'
3<backchannel | git fast-import --cat-blob-fd=3 3>backchannel
git-fast-import crashes with:
fatal: Empty path component found in input
fast-import: dumping crash report to .git/fast_import_crash_10844
Crashing command:
* ls :202791
Where the crashing SVN commit is "svn cp $SVN_ROOT $SVN_ROOT/subdirectory".
A Google search led me to:
http://git.661346.n2.nabble.com/BUG-fast-import-ls-command-on-commit-root-returns-missing-was-Bug-in-svn-fe-copying-the-root-directo-td7353801.html
As far as I understand, this issue is fixed in fast-import, or am I
missing something? Is there a way workaround this problem?
Björn
$ git --verion
git version 1.8.2.rc3.dirty
$ uname -a # Ubuntu 12.04
Linux bjorntodesktop 3.2.0-38-generic #61-Ubuntu SMP Tue Feb 19
12:18:21 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
^ permalink raw reply
* Re: git-scm.com/book/ru -- incorrect "next" link containing a question mark
From: Jeff King @ 2013-03-08 10:40 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git-users, Aleksey Rozhkov, git
In-Reply-To: <20130307124736.3d738c5aab39345fa9ca2930@domain007.com>
On Thu, Mar 07, 2013 at 12:47:36PM +0400, Konstantin Khomoutov wrote:
> On Thu, 7 Mar 2013 00:01:31 -0800 (PST)
> Aleksey Rozhkov <ekkertan@gmail.com> wrote:
>
> > The page http://git-scm.com/book/ru/
> > Введение-Первоначальная-настройка-Git contains incorrect link "next"
> > Now this link to the page
> > http://git-scm.com/book/ru/Введение-Как-получить-помощь? , but this
> > page does not exist
>
> I would say "?" is just interpreted as a request part of the URL.
Yes, the problem is that the page's URL actually has the "?" in it,
but it needs to be URL-encoded in links.
> > So, correct link is
> > http://git-scm.com/book/ru/Введение-Как-получить-помощь%3F
>
> Good point, thanks. I Cc'ed the main Git list and made the message
> title a bit more clear.
Exactly. I fixed a few instances of this a while ago, but not the
prev/next pointers. This PR should fix all sites:
https://github.com/github/gitscm-next/pull/259
> To the Pro Git book maintainer: is it possible to somehow fix the HTML
> generation script to URL-encode any special characters if they're to
> appear in an URL?
I don't think Scott reads the list too closely these days. In general,
the best place to report git-scm.com problems is as an issue in the repo
linked above.
-Peff
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Duy Nguyen @ 2013-03-08 10:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: Torsten Bögershausen, Ramkumar Ramachandra, Robert Zeh,
Git List, finnag
In-Reply-To: <7v7glijoiy.fsf@alter.siamese.dyndns.org>
On Fri, Mar 8, 2013 at 3:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> The possible options are:
>> +
>> - - 'no' - Show no untracked files
>> + - 'no' - Show no untracked files (this is fastest)
>
> There is a trade-off around the use of -uno between safety and
> performance. The default is not to use -uno so that you will not
> forget to add a file you newly created (i.e safety). You would pay
> for the safety with the cost to find such untracked files (i.e.
> performance).
>
> I suspect that the documentation was written with the assumption
> that at least for the people who are reading this part of the
> documentation, the trade-off is obvious. In order to find more
> information, you naturally need to spend more cycles.
>
> If the trade-off is not so obvious, however, I do not object at all
> to describing it. But if we are to do so, I do object to mentioning
> only one side of the trade-off. People who choose "fastest" needs
> to be made very aware that they are disabling "safety".
On the topic of trading off, I was thinking about new -uauto as
default that is like -uall if it takes less than a certan amount of
time (e.g. 0.5 seconds), if it exceeds that limit, the operation is
aborted (i.e. it turns to -uno). The safety net is still there, "git
status" advices to use -u to show full information.
Or a less intrusive approach: measure the time and advice the user to
(read doc and) use -uno.
But it's probably worth waiting for the first cut of inotify support
from Ram. It's better with inotify anyway.
--
Duy
^ permalink raw reply
* Re: [PATCH] git-push.txt: mention about remote.*.push when no refspec is given
From: Duy Nguyen @ 2013-03-08 10:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8v60qu2i.fsf@alter.siamese.dyndns.org>
On Thu, Mar 7, 2013 at 1:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I agree that saying what it is, what it does or what it is for
> upfront (i.e. "Specifies what are pushed") before how it is spelled
> is an improvement. I however think describing "If not specified"
> here was a mistake, and you are making it worse by burying the
> description of what happens when <refspec>... are missing in the
> middle of the description for <refspec>...
>
> I would rather see this done in the direction the attached "how
> about doing it this way" patch illustrates. The way how "where" and
> "what" are determined when the command line does not specify is the
> proparty of the entire command, not of an individual parameter.
I agree your patch looks better than mine. Put it on 'pu', perhaps?
--
Duy
^ permalink raw reply
* Re: [PATCH v2 3/4] status: show more info than "currently not on any branch"
From: Duy Nguyen @ 2013-03-08 11:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matthieu Moy, Jonathan Niedier
In-Reply-To: <7vzjygpcd5.fsf@alter.siamese.dyndns.org>
On Thu, Mar 7, 2013 at 2:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> +static void wt_status_get_detached_from(struct wt_status_state *state)
>> +{
>> + struct grab_1st_switch_cbdata cb;
>> + struct commit *commit;
>> + unsigned char sha1[20];
>> + char *ref = NULL;
>> +
>> + strbuf_init(&cb.buf, 0);
>> + if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
>> + for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
>> + if (!cb.buf.len)
>> + return;
>
> Is this correct? What if the recent entries (i.e. the tail 4k) of
> the HEAD reflog did not have *any* checkout? Your callback never
> returns non-zero, so as long as the HEAD reflog is sufficiently
> long, for_each_recent_reflog_ent() will return 0 to signal success,
> and you do not dig deeper by retrying the full reflog for HEAD,
> missing the checkout that exists before the final 4k, no?
>
> It should be more like this, I would think:
>
> for_each_recent_reflog_ent();
> if (!found)
> for_each_reflog_ent();
> if (!found)
> return;
Yes. This "recent" optimization is tricky.
> Using cb.buf.len as the "found" marker may be correct, but I found
> it a bit subtle to my taste, without explanation. Adding an
> explicit bit to "struct grab_1st_switch_cbdata" would be cleaner and
> more resistant to future changes, I think.
OK
>
>> +
>> + if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
>> + (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
>> + !hashcmp(cb.nsha1, commit->object.sha1)) {
>
> That feels unnecessarily expensive. Why not hashcmp before checking
> the type of the object to reject the case where the ref moved since
> the last checkout early?
>
> For that matter, does this even need to check the type of the object
> that currently sits at the ref? Isn't it sufficient to reject this
> case by seeing if sha1 is the same as cb.nsha1?
nsha1 is always a commit sha-1, sha-1 could be a tag sha-1 that refers
to the same commit. hashcmp before lookup is a good idea. Although I
don't think it's expensive in the big picture. When HEAD is not
detached, we show "<n> commits ahead of @{u}" which is way more
expensive than this. As long as "git status" on detached HEAD does not
use up all the time that "git status" on branches normally does, I
think we're fine.
--
Duy
^ permalink raw reply
* Re: rebase: strange failures to apply patc 3-way
From: Max Horn @ 2013-03-08 11:34 UTC (permalink / raw)
Cc: git
In-Reply-To: <7A483B92-D671-46CA-9EFD-83C6F4C97B5E@quendi.de>
A follow up:
I was able to reproduce this behavior on my Mac with multiple git versions down to 1.6.0.6. On the other hand, I copied the whole work tree to a virtual machine running Ubuntu, and there the issue did not occur.
All in all, I suspect that Mac OS X and/or the filesystem (HFS+ with journaling, not case sensitive (the default)) might be at fault. Still, this is quite puzzling and annoying, and so I still wonder if anybody has any insights on this.
And: is this a known issue, I wonder?
Cheers,
Max
On 07.03.2013, at 11:16, Max Horn wrote:
> Recently I have observed very strange failures in "git rebase" that cause it to fail to work automatically in situations where it should trivially be able to do so.
>
> In case it matter, here's my setup: git 1.8.2.rc2.4.g7799588 (i.e. git.git master branch) on Mac OS X. The repos clone is on a HFS+ partition, not on a network volume. No gitattributes are being used. Regarding the origin of the repos (I hope it doesn't matter, but just in case): The repository in question used to be a CVS repository; it was decided to switch to Mercurial (not my decision ;-) and I performed the conversion; I first converted it to git using cvs2git (from the cvs2svn suite), then performed some history cleanup, and finally used "hg convert" to convert it to git. Recently, I have been accessing the repository from git via the gitifyhg tool <https://github.com/buchuki/gitifyhg>.
>
> Anyway, several strange things just happened (and I had similar experiences in the past days and weeks, but that was using an older git, and I thought I was just doing something wrong).
>
> Specifically, I wanted to rebase a branch with some experimental commits. The strange things started with this:
>
> $ git rebase MY-NEW-BASE
> First, rewinding head to replay your work on top of it...
> Applying: SOME COMMIT
> Applying: SOME OTHER COMMIT
> ...
> Applying: COMMIT A
> Applying: COMMIT B
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> error: Your local changes to the following files would be overwritten by merge:
> some/source.file
> Please, commit your changes or stash them before you can merge.
> Aborting
> Failed to merge in the changes.
> Patch failed at 0014 COMMIT B
> The copy of the patch that failed is found in:
> /path/to/my/repo/.git/rebase-apply/patch
>
> When you have resolved this problem, run "git rebase --continue".
> If you prefer to skip this patch, run "git rebase --skip" instead.
> To check out the original branch and stop rebasing, run "git rebase --abort".
>
>
> Now, what is strange about this is that the failed patch actually applies cleanly:
>
> $ patch -p1 < /path/to/my/repo/.git/rebase-apply/patch
> patching file some/source.file
> $
>
> And there is no subtle merge issue here, either: That patch is the only one to have touched the surrounding code since 1999! There is no source of conflict there!
>
> Anyway. The tale gets stranger, as I was trying to do this again (no changes were made to the repos in between, this is a straight continuation from above):
>
> $ git rebase --abort
> $ git rebase MY-NEW-BASE
> First, rewinding head to replay your work on top of it...
> Applying: SOME COMMIT
> Applying: SOME OTHER COMMIT
> ...
> Applying: COMMIT A
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> error: Your local changes to the following files would be overwritten by merge:
> some/othersource.file
> some/yetanother.file
> Please, commit your changes or stash them before you can merge.
> Aborting
> Failed to merge in the changes.
> Patch failed at 0013 COMMIT A
> The copy of the patch that failed is found in:
> /path/to/my/repo/.git/rebase-apply/patch
>
> When you have resolved this problem, run "git rebase --continue".
> If you prefer to skip this patch, run "git rebase --skip" instead.
> To check out the original branch and stop rebasing, run "git rebase --abort".
>
>
>
> So suddenly it fails to apply the commit A, the one before the previously failing commit. Huh? But again, the failing patch applies cleanly (and after all, rebase was able to apply it in my previous attempt). And again, the patch actually applies cleanly. So one more try:
>
>
> $ git rebase --abort
> $ git rebase MY-NEW-BASE
> First, rewinding head to replay your work on top of it...
> Applying: SOME COMMIT
> Applying: SOME OTHER COMMIT
> ...
> Applying: COMMIT A
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> error: Your local changes to the following files would be overwritten by merge:
> some/othersource.file
> Please, commit your changes or stash them before you can merge.
> Aborting
> Failed to merge in the changes.
> Patch failed at 0013 COMMIT A
> The copy of the patch that failed is found in:
> /path/to/my/repo/.git/rebase-apply/patch
>
> When you have resolved this problem, run "git rebase --continue".
> If you prefer to skip this patch, run "git rebase --skip" instead.
> To check out the original branch and stop rebasing, run "git rebase --abort".
>
>
> Again it fails in commit A -- but this time, it only thinks one file is problematic. HUH? Again, the patch actually applies cleanly:
>
> $ patch -p1 < /path/to/my/repo/.git/rebase-apply/patch
> patching file some/othersource.file
> patching file some/yetanother.file
>
>
> At this point, things stabilized, and when I now abort and reattempt the merge, it always fails in the same way. This time trying to apply a change to a code comment that was last changed in 1997 (though for one hunk, a few lines before the changed lines, there is a line that was changed in 2008... but I assure you, that line is there in the ancestors of both the branch I want to rebase, and also in the MY-NEW-BASE branch I rebase onto).
>
>
> Something seems to be really fishy here and I wonder if anybody has an idea what's going wrong here. Is this a bug in git? Is my repos broken in some way? Note that "git fsck" reported nothing except some dangling objects. Any other ideas?
>
>
> Cheers,
> Max
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Memory corruption when rebasing with git version 1.8.1.5 on arch
From: Bernhard Posselt @ 2013-03-08 12:19 UTC (permalink / raw)
To: git
Hi im running arch linux and core/glibc 2.17-3
When i try to rebase with:
git clonehttps://github.com/owncloud/core.git
cd core/
git pull --rebasehttps://github.com/PatrickHeller/core.git master
I'm getting:
$ git pull --rebasehttps://github.com/PatrickHeller/core.git master
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 3 (delta 2), reused 3 (delta 2)
Unpacking objects: 100% (3/3), done.
Fromhttps://github.com/PatrickHeller/core
* branch master -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: distinguish between touch and write
Applying: remove debug output
*** Error in `git': malloc(): memory corruption: 0x0000000000be14e0 ***
Using valgrind gives me:
$ valgrind /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git master
==5995== Memcheck, a memory error detector
==5995== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5995== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5995== Command: /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git master
==5995==
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 3 (delta 2), reused 3 (delta 2)
Unpacking objects: 100% (3/3), done.
Fromhttps://github.com/PatrickHeller/core
* branch master -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: distinguish between touch and write
Applying: remove debug output
*** Error in `git': malloc(): memory corruption: 0x00000000027f14e0 ***
^C==5995==
==5995== HEAP SUMMARY:
==5995== in use at exit: 1,076 bytes in 11 blocks
==5995== total heap usage: 53 allocs, 42 frees, 11,038 bytes allocated
==5995==
==5995== LEAK SUMMARY:
==5995== definitely lost: 0 bytes in 0 blocks
==5995== indirectly lost: 0 bytes in 0 blocks
==5995== possibly lost: 0 bytes in 0 blocks
^ permalink raw reply
* question re tags
From: John Stean @ 2013-03-08 15:16 UTC (permalink / raw)
To: git
Ive been tagging some commits using tortoise git , for example with "v1.0", "v1.1" etc. In tortoise git log the tag sits alongside the commit as I expect.
But when I do a git describe it outputs the first tag along with the latest commit. What am I doing wrong?
^ permalink raw reply
* Re: rebase: strange failures to apply patc 3-way
From: Andrew Wong @ 2013-03-08 15:32 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <494292C5-EBD9-487B-8846-9D9DD23ACB83@quendi.de>
On 3/8/13, Max Horn <max@quendi.de> wrote:
> All in all, I suspect that Mac OS X and/or the filesystem (HFS+ with
> journaling, not case sensitive (the default)) might be at fault. Still, this
> is quite puzzling and annoying, and so I still wonder if anybody has any
> insights on this.
When "rebase" errors out at COMMIT A, try manually running "git apply"
on the patch file (rebase-apply/patch) a couple times, and see if the
error occurs randomly. You'd have to do a "reset --hard" to revert the
changes done by "git apply" every time before you run it again. The
error from "git apply" might shed more light on the issue.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox