* Re: defined behaviour for multiple urls for a remote
From: Sitaram Chamarty @ 2011-10-17 3:41 UTC (permalink / raw)
To: Kirill Likhodedov; +Cc: git
In-Reply-To: <loom.20111014T113208-660@post.gmane.org>
On Fri, Oct 14, 2011 at 3:06 PM, Kirill Likhodedov
<Kirill.Likhodedov@jetbrains.com> wrote:
> Sitaram Chamarty <sitaramc <at> gmail.com> writes:
>
>> What's the defined behaviour if I do this:
>>
>> [remote "both"]
>> url = https://code.google.com/p/gitolite/
>> url = git <at> github.com:sitaramc/gitolite.git
>>
>> I know what I'm seeing (a fetch only goes to the first URL, and does a
>> HEAD->FETCH_HEAD because I didn't provide a refspec line, while a push
>> seems to push all to both), but I was curious what the official
>> position is, because I couldn't find it in the docs.
>
> Please see the message from Linus about that: http://marc.info/?l=git&m=116231242118202&w=2
cool; thanks!
> You also may check how Git understands your remotes by running
> git remote -v
Aah that's very clear!
> It will show, where it is going to fetch from and push to.
>
> I agree though, that documentation should be updated.
well in theory yes, but now that you mention it, 'git remote -v' is sufficient.
^ permalink raw reply
* Re: regression in git-gui since 2c5c66b... Merge branch 'jp/get-ref-dir-unsorted
From: Michael Haggerty @ 2011-10-17 3:40 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Junio C Hamano, git
In-Reply-To: <4E9B8719.1090203@gmail.com>
On 10/17/2011 03:38 AM, Mark Levedahl wrote:
> On 10/16/2011 08:35 PM, Junio C Hamano wrote:
>> Mark Levedahl<mlevedahl@gmail.com> writes:
>>> I have a project organized as a number of nested git modules (not
>>> using git-submodule), and frequently use git-new-workdir to create the
>>> nested modules.
>>>
>>> Since the above merge-commit, git-gui is confused by this arrangement
>>> and reports every file in every nested module as being an untracked
>>> file in the top-level (super) project.
> [...]
>
> The following shows the problem for me:
>
> #!/bin/bash
> mkdir super sub
> cd sub
> git init
> touch a
> git add a
> git commit -m 'file' a
> git pack-refs --all
> cd ../super
> git init
> git new-workdir ../sub sub
> git-gui
>
>
> git-gui shows "sub/a" in the list of Unstaged Changes. Note that the
> "git pack-refs" call is needed to get the failure.
Please bear with me because I don't use git-gui so I don't really know
what to expect.
When I check out 2c5c66b and run the above script (actually, the script
listed below) what I see in git-gui is:
* In the "Unstaged Changes" window, "sub" is listed (not "sub/a").
* When I click on "sub", then in the "Untracked, not staged" window,
"Git Repository (subproject)" appears.
I see the exact same thing when I run the same test script on the
version before merge 2c5c66b.
What do you see?
What do you expect to see?
What versions of git, exactly, are you testing (what version do you
consider "good"; presumably it is version 2c5c66b that you consider "bad")?
Are you certain that you are using the same git version for all commands
("git", "git-gui", and "git-new-workdir")? Please especially note that
git-new-workdir is not part of a default git install, and therefore it
would be easy to accidentally use a different version of this script
than of the other commands.
Michael
#!/bin/bash
SRC=$(cd $(dirname $0); pwd)
GIT=$SRC/git
GIT_NEW_WORKDIR=$SRC/contrib/workdir/git-new-workdir
GITGUI=$SRC/git-gui/git-gui
rm -rf super sub
mkdir super sub
cd sub
$GIT init
touch a
$GIT add a
$GIT commit -m 'file' a
$GIT pack-refs --all
cd ../super
$GIT init
$GIT_NEW_WORKDIR ../sub sub
$GITGUI &
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2011, #05; Fri, 14)
From: Jeff King @ 2011-10-17 3:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcrorh49.fsf@alter.siamese.dyndns.org>
On Sun, Oct 16, 2011 at 10:17:10AM -0700, Junio C Hamano wrote:
> >> * jk/daemon-msgs (2011-10-14) 1 commit
> >> - daemon: give friendlier error messages to clients
> >>
> >> Will merge to 'next'.
> >
> > I'm happy to tweak the "access denied" message if other people want. I
> > kind of hoped it wouldn't matter, and that most sites would use
> > --informative-errors.
>
> I've already updated it with the "not exported" bit from Sitaram.
What you queued in d5570f4 looks sane, but the merge into next is
curious. It's an evil merge that turns on informative errors by default:
$ git show 415cf53
commit 415cf53e710b2ff44d485e253bb35a1ca5dff636
Merge: fbc2ee6 d5570f4
Author: Junio C Hamano <gitster@pobox.com>
Date: Sat Oct 15 21:25:41 2011 -0700
Merge branch 'jk/daemon-msgs' into next
* jk/daemon-msgs:
daemon: give friendlier error messages to clients
Conflicts:
daemon.c
diff --cc daemon.c
index 91c4d9b,72fb53a..95b7df5
--- a/daemon.c
+++ b/daemon.c
@@@ -20,6 -20,7 +20,7 @@@
static int log_syslog;
static int verbose;
static int reuseaddr;
-static int informative_errors;
++static int informative_errors = 1;
static const char daemon_usage[] =
"git daemon [--verbose] [--syslog] [--export-all]\n"
The conflicts come from merging with Duy's version, which was already in
next. But the introduced line comes from my second patch, which was
rightly dropped, and should not be there. Was this intentional, or did
you do something clever with "git am" and my original series during
resolution and accidentally pull in that change?
-Peff
^ permalink raw reply
* [PATCH v4 7/7] clear_ref_cache(): inline function
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
clear_ref_cache() was only called from one place, so inline it
there.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index a888cea..d8a4fa3 100644
--- a/refs.c
+++ b/refs.c
@@ -172,12 +172,6 @@ static void clear_loose_ref_cache(struct ref_cache *refs)
refs->did_loose = 0;
}
-static void clear_ref_cache(struct ref_cache *refs)
-{
- clear_packed_ref_cache(refs);
- clear_loose_ref_cache(refs);
-}
-
static struct ref_cache *create_ref_cache(const char *submodule)
{
int len;
@@ -215,7 +209,9 @@ static struct ref_cache *get_ref_cache(const char *submodule)
void invalidate_ref_cache(const char *submodule)
{
- clear_ref_cache(get_ref_cache(submodule));
+ struct ref_cache *refs = get_ref_cache(submodule);
+ clear_packed_ref_cache(refs);
+ clear_loose_ref_cache(refs);
}
static void read_packed_refs(FILE *f, struct ref_array *array)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v4 2/7] invalidate_ref_cache(): take the submodule as parameter
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Instead of invalidating the ref cache on an all-or-nothing basis,
invalidate the cache for a specific submodule.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 02b1ef0..68b73aa 100644
--- a/refs.c
+++ b/refs.c
@@ -202,13 +202,9 @@ static struct ref_cache *get_ref_cache(const char *submodule)
return refs;
}
-static void invalidate_ref_cache(void)
+static void invalidate_ref_cache(const char *submodule)
{
- struct ref_cache *refs = ref_cache;
- while (refs) {
- clear_ref_cache(refs);
- refs = refs->next;
- }
+ clear_ref_cache(get_ref_cache(submodule));
}
static void read_packed_refs(FILE *f, struct ref_array *array)
@@ -1228,7 +1224,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
ret |= repack_without_ref(refname);
unlink_or_warn(git_path("logs/%s", lock->ref_name));
- invalidate_ref_cache();
+ invalidate_ref_cache(NULL);
unlock_ref(lock);
return ret;
}
@@ -1527,7 +1523,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_ref_cache();
+ invalidate_ref_cache(NULL);
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v4 6/7] write_ref_sha1(): only invalidate the loose ref cache
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Since write_ref_sha1() can only write loose refs and cannot write
symbolic refs, there is no need for it to invalidate the packed ref
cache.
Suggested by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 2a21dc3..a888cea 100644
--- a/refs.c
+++ b/refs.c
@@ -1534,7 +1534,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_ref_cache(NULL);
+ clear_loose_ref_cache(get_ref_cache(NULL));
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v4 4/7] clear_ref_cache(): rename parameter
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
...for consistency with the rest of this module.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index ddd799e..623d673 100644
--- a/refs.c
+++ b/refs.c
@@ -158,13 +158,13 @@ static void free_ref_array(struct ref_array *array)
array->refs = NULL;
}
-static void clear_ref_cache(struct ref_cache *ca)
+static void clear_ref_cache(struct ref_cache *refs)
{
- if (ca->did_loose)
- free_ref_array(&ca->loose);
- if (ca->did_packed)
- free_ref_array(&ca->packed);
- ca->did_loose = ca->did_packed = 0;
+ if (refs->did_loose)
+ free_ref_array(&refs->loose);
+ if (refs->did_packed)
+ free_ref_array(&refs->packed);
+ refs->did_loose = refs->did_packed = 0;
}
static struct ref_cache *create_ref_cache(const char *submodule)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v4 5/7] clear_ref_cache(): extract two new functions
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Extract two new functions from clear_cached_refs():
clear_loose_ref_cache() and clear_packed_ref_cache().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 623d673..2a21dc3 100644
--- a/refs.c
+++ b/refs.c
@@ -158,13 +158,24 @@ static void free_ref_array(struct ref_array *array)
array->refs = NULL;
}
-static void clear_ref_cache(struct ref_cache *refs)
+static void clear_packed_ref_cache(struct ref_cache *refs)
{
- if (refs->did_loose)
- free_ref_array(&refs->loose);
if (refs->did_packed)
free_ref_array(&refs->packed);
- refs->did_loose = refs->did_packed = 0;
+ refs->did_packed = 0;
+}
+
+static void clear_loose_ref_cache(struct ref_cache *refs)
+{
+ if (refs->did_loose)
+ free_ref_array(&refs->loose);
+ refs->did_loose = 0;
+}
+
+static void clear_ref_cache(struct ref_cache *refs)
+{
+ clear_packed_ref_cache(refs);
+ clear_loose_ref_cache(refs);
}
static struct ref_cache *create_ref_cache(const char *submodule)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v4 0/7] Provide API to invalidate refs cache
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
These patches are re-rolled onto the current master and incorporate
Junio's suggestion to rename "struct cached_refs" to "ref_cache".
This patch series provides an API for external code to invalidate the
ref cache that is used internally to refs.c. It also allows code
*within* refs.c to invalidate only the packed or only the loose refs
for a module/submodule.
IMPORTANT:
I won't myself have time to figure out who, outside of refs.c, has to
*call* invalidate_ref_cache(). The candidates that I know off the top
of my head are git-clone, git-submodule [1], and git-pack-refs. It
would be great if experts in those areas would insert calls to
invalidate_ref_cache() where needed.
Even better would be if the meddlesome code were changed to use the
refs API. I'd be happy to help expanding the refs API if needed to
accommodate your needs.
This is why the API for invalidating only packed or loose refs is
private. After code outside refs.c is changed to use the refs API, it
will get the optimal behavior for free (and at that time
invalidate_ref_cache() can be removed again).
[1] http://marc.info/?l=git&m=131827641227965&w=2
In this mailing list thread, Heiko Voigt stated that git-submodule
does not modify any references, so it should not have to use the
API.
Michael Haggerty (7):
invalidate_ref_cache(): rename function from invalidate_cached_refs()
invalidate_ref_cache(): take the submodule as parameter
invalidate_ref_cache(): expose this function in the refs API
clear_ref_cache(): rename parameter
clear_ref_cache(): extract two new functions
write_ref_sha1(): only invalidate the loose ref cache
clear_ref_cache(): inline function
refs.c | 59 +++++++++++++++++++++++++++++++----------------------------
refs.h | 8 ++++++++
2 files changed, 39 insertions(+), 28 deletions(-)
--
1.7.7.rc2
^ permalink raw reply
* [PATCH v4 3/7] invalidate_ref_cache(): expose this function in the refs API
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Make invalidate_ref_cache() an official part of the refs API. It is
currently a fact of life that code outside of refs.c mucks about with
references. This change gives such code a way of informing the refs
module that it should no longer trust its cache.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 +-
refs.h | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 68b73aa..ddd799e 100644
--- a/refs.c
+++ b/refs.c
@@ -202,7 +202,7 @@ static struct ref_cache *get_ref_cache(const char *submodule)
return refs;
}
-static void invalidate_ref_cache(const char *submodule)
+void invalidate_ref_cache(const char *submodule)
{
clear_ref_cache(get_ref_cache(submodule));
}
diff --git a/refs.h b/refs.h
index 0229c57..f439c54 100644
--- a/refs.h
+++ b/refs.h
@@ -80,6 +80,14 @@ extern void unlock_ref(struct ref_lock *lock);
/** Writes sha1 into the ref specified by the lock. **/
extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
+/*
+ * Invalidate the reference cache for the specified submodule. Use
+ * submodule=NULL to invalidate the cache for the main module. This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_ref_cache(const char *submodule);
+
/** Setup reflog before using. **/
int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v4 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs()
From: mhagger @ 2011-10-17 2:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318819091-7066-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
It is the cache that is being invalidated, not the references, and the
new name makes this unambiguous. Rename other items analogously:
* struct cached_refs -> struct ref_cache
* cached_refs (the variable) -> ref_cache
* clear_cached_refs() -> clear_ref_cache()
* create_cached_refs() -> create_ref_cache()
* get_cached_refs() -> get_ref_cache()
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 40 ++++++++++++++++++++--------------------
1 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/refs.c b/refs.c
index 9911c97..02b1ef0 100644
--- a/refs.c
+++ b/refs.c
@@ -134,15 +134,15 @@ static struct ref_entry *search_ref_array(struct ref_array *array, const char *n
* Future: need to be in "struct repository"
* when doing a full libification.
*/
-static struct cached_refs {
- struct cached_refs *next;
+static struct ref_cache {
+ struct ref_cache *next;
char did_loose;
char did_packed;
struct ref_array loose;
struct ref_array packed;
/* The submodule name, or "" for the main repo. */
char name[FLEX_ARRAY];
-} *cached_refs;
+} *ref_cache;
static struct ref_entry *current_ref;
@@ -158,7 +158,7 @@ static void free_ref_array(struct ref_array *array)
array->refs = NULL;
}
-static void clear_cached_refs(struct cached_refs *ca)
+static void clear_ref_cache(struct ref_cache *ca)
{
if (ca->did_loose)
free_ref_array(&ca->loose);
@@ -167,27 +167,27 @@ static void clear_cached_refs(struct cached_refs *ca)
ca->did_loose = ca->did_packed = 0;
}
-static struct cached_refs *create_cached_refs(const char *submodule)
+static struct ref_cache *create_ref_cache(const char *submodule)
{
int len;
- struct cached_refs *refs;
+ struct ref_cache *refs;
if (!submodule)
submodule = "";
len = strlen(submodule) + 1;
- refs = xcalloc(1, sizeof(struct cached_refs) + len);
+ refs = xcalloc(1, sizeof(struct ref_cache) + len);
memcpy(refs->name, submodule, len);
return refs;
}
/*
- * Return a pointer to a cached_refs for the specified submodule. For
+ * Return a pointer to a ref_cache for the specified submodule. For
* the main repository, use submodule==NULL. The returned structure
* will be allocated and initialized but not necessarily populated; it
* should not be freed.
*/
-static struct cached_refs *get_cached_refs(const char *submodule)
+static struct ref_cache *get_ref_cache(const char *submodule)
{
- struct cached_refs *refs = cached_refs;
+ struct ref_cache *refs = ref_cache;
if (!submodule)
submodule = "";
while (refs) {
@@ -196,17 +196,17 @@ static struct cached_refs *get_cached_refs(const char *submodule)
refs = refs->next;
}
- refs = create_cached_refs(submodule);
- refs->next = cached_refs;
- cached_refs = refs;
+ refs = create_ref_cache(submodule);
+ refs->next = ref_cache;
+ ref_cache = refs;
return refs;
}
-static void invalidate_cached_refs(void)
+static void invalidate_ref_cache(void)
{
- struct cached_refs *refs = cached_refs;
+ struct ref_cache *refs = ref_cache;
while (refs) {
- clear_cached_refs(refs);
+ clear_ref_cache(refs);
refs = refs->next;
}
}
@@ -257,7 +257,7 @@ void clear_extra_refs(void)
static struct ref_array *get_packed_refs(const char *submodule)
{
- struct cached_refs *refs = get_cached_refs(submodule);
+ struct ref_cache *refs = get_ref_cache(submodule);
if (!refs->did_packed) {
const char *packed_refs_file;
@@ -379,7 +379,7 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
static struct ref_array *get_loose_refs(const char *submodule)
{
- struct cached_refs *refs = get_cached_refs(submodule);
+ struct ref_cache *refs = get_ref_cache(submodule);
if (!refs->did_loose) {
get_ref_dir(submodule, "refs", &refs->loose);
@@ -1228,7 +1228,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
ret |= repack_without_ref(refname);
unlink_or_warn(git_path("logs/%s", lock->ref_name));
- invalidate_cached_refs();
+ invalidate_ref_cache();
unlock_ref(lock);
return ret;
}
@@ -1527,7 +1527,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_cached_refs();
+ invalidate_ref_cache();
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* Re: [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Kato Kazuyoshi @ 2011-10-17 2:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbotgper8.fsf@alter.siamese.dyndns.org>
On Mon, Oct 17, 2011 at 10:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I think a better organization would be
>
> [1/2] change code so that $diff_class does not have leading SP
> optionally catch a case where $diff_class stays empty as an error?
>
I think we don't have to treat empty $diff_class as an error because
$diff_class will be an empty when $line is around modification
(ex. foo or quux).
foo
- bar
+ baz
quuz
And class attributes are CDATA. "diff[SP]" and "diff" have same
meanings.
http://www.w3.org/TR/html401/struct/global.html#h-7.5.2
http://www.w3.org/TR/html401/types.html#type-cdata
--
Kato Kazuyoshi
^ permalink raw reply
* Re: [PATCH 2/2] daemon: report permission denied error to clients
From: Jeff King @ 2011-10-17 2:09 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git, Junio C Hamano
In-Reply-To: <1318803076-4229-2-git-send-email-drizzd@aon.at>
On Mon, Oct 17, 2011 at 12:11:16AM +0200, Clemens Buchacher wrote:
> If passed an inaccessible url, git daemon returns the
> following error:
>
> $ git clone git://host/repo
> fatal: remote error: no such repository: /repo
>
> In case of a permission denied error, return the following
> instead:
>
> fatal: remote error: permission denied: /repo
>
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>
> ---
I like the intent. This actually does leak a little more information
than the existing --informative-errors, as before you couldn't tell the
difference between "not found" and "not exported". But I think the
spirit of --informative-errors is to let that information leak, and this
is a good change.
> -static char *path_ok(char *directory)
> +static int path_ok(char *directory, const char **return_path)
> {
> static char rpath[PATH_MAX];
> static char interp_path[PATH_MAX];
> @@ -120,13 +120,13 @@ static char *path_ok(char *directory)
>
> if (daemon_avoid_alias(dir)) {
> logerror("'%s': aliased", dir);
> - return NULL;
> + return -1;
> }
>
> if (*dir == '~') {
> if (!user_path) {
> logerror("'%s': User-path not allowed", dir);
> - return NULL;
> + return EACCES;
The new calling conventions for this function seem a little weird. I
would expect either "return negative, and set errno" for usual library
code, or possibly "return negative error value". But "return -1, or a
positive error code" seems unusual to me.
One of:
errno = EACCESS;
return -1;
or
return -EACCESS;
would be more idiomatic, I think.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] daemon: add tests
From: Jeff King @ 2011-10-17 2:01 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git, Junio C Hamano
In-Reply-To: <1318803076-4229-1-git-send-email-drizzd@aon.at>
On Mon, Oct 17, 2011 at 12:11:15AM +0200, Clemens Buchacher wrote:
> The semantics of the git daemon tests are similar to the http
> transport tests. In fact, they are only a slightly modified copy
> of t5550, plus the newly added remote error tests.
>
> All daemon tests will be skipped unless the environment variable
> GIT_TEST_DAEMON is set.
Thanks, it's nice to have some tests. Overall, some of the tests feel a
little silly, because the results should be exactly the same as fetching
or pushing a local repository (so the "set-head" thing, for example,
really has little to do with git-daemon). At the same time, maybe it's a
good thing to re-confirm that the results really are the same. :)
> diff --git a/t/lib-daemon.sh b/t/lib-daemon.sh
> new file mode 100644
> index 0000000..30a89ea
> --- /dev/null
> +++ b/t/lib-daemon.sh
> @@ -0,0 +1,52 @@
> +#!/bin/sh
> +
> +if test -z "$GIT_TEST_DAEMON"
> +then
> + skip_all="Daemon testing disabled (define GIT_TEST_DAEMON to enable)"
> + test_done
> +fi
> +
> +LIB_DAEMON_PORT=${LIB_DAEMON_PORT-'8121'}
I assume you picked this arbitrarily to be LIB_HTTPD_PORT+10. It's fine
to have a default, but note that each of the httpd tests actually
defaults the port to their test number, so they can be run in parallel.
So that would be:
> --- /dev/null
> +++ b/t/t5570-git-daemon.sh
> @@ -0,0 +1,148 @@
> +#!/bin/sh
> +
> +test_description='test fetching over git protocol'
> +. ./test-lib.sh
> +
> +. "$TEST_DIRECTORY"/lib-daemon.sh
> +start_daemon
LIB_DAEMON_PORT=${LIB_DAEMON_PORT-'5570'}
here.
^ permalink raw reply
* Re: [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Junio C Hamano @ 2011-10-17 1:51 UTC (permalink / raw)
To: Kato Kazuyoshi; +Cc: git
In-Reply-To: <CAFo4x0+_Uy=yjbO61qj8krS0-iovzC5WnBE8-1n5OzxgGeg6JA@mail.gmail.com>
Kato Kazuyoshi <kato.kazuyoshi@gmail.com> writes:
> Probably like:
>
> - format_diff_line returns a class with an line.
> - remove trailing space from the class.
> - add side-by-side feature and CSS.
> - add form.
>
> Thank you for your correction!
My wording came out a bit too strong; I didn't mean to "correct" anything.
I think a better organization would be
[1/2] change code so that $diff_class does not have leading SP
optionally catch a case where $diff_class stays empty as an error?
[2/2] add side-by-side feature, which would involve:
- making format_diff_line() to return $diff_class separately;
- necessary addition of CSS;
- addition of form to trigger the feature.
I do not think splitting the second patch into pieces smaller than that
makes sense.
Thanks.
^ permalink raw reply
* Re: regression in git-gui since 2c5c66b... Merge branch 'jp/get-ref-dir-unsorted
From: Mark Levedahl @ 2011-10-17 1:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfwispi8u.fsf@alter.siamese.dyndns.org>
On 10/16/2011 08:35 PM, Junio C Hamano wrote:
> Mark Levedahl<mlevedahl@gmail.com> writes:
>
>> I have a project organized as a number of nested git modules (not
>> using git-submodule), and frequently use git-new-workdir to create the
>> nested modules.
>>
>> Since the above merge-commit, git-gui is confused by this arrangement
>> and reports every file in every nested module as being an untracked
>> file in the top-level (super) project.
> Could you come up with a simple reproduction recipe that prepares a
> superproject that has no file of its own, a submodule with a single file
> tracked, and attaches another workdir? If running git-gui in the resulting
> directory makes it misbehave, we could then isolate what git command that
> is invoked by git-gui has changed its behaviour.
>
> Thanks.
>
The following shows the problem for me:
#!/bin/bash
mkdir super sub
cd sub
git init
touch a
git add a
git commit -m 'file' a
git pack-refs --all
cd ../super
git init
git new-workdir ../sub sub
git-gui
git-gui shows "sub/a" in the list of Unstaged Changes. Note that the
"git pack-refs" call is needed to get the failure.
Mark
^ permalink raw reply
* Re: [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Kato Kazuyoshi @ 2011-10-17 1:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobxgpixo.fsf@alter.siamese.dyndns.org>
On Mon, Oct 17, 2011 at 9:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> What are these changes for, except perhaps to make the patch larger than
> necesssary to make it harder to review?
>
> It would leave an unnecessary SP like 'class="diff "' when all the arms of
> if/elsif cascade fall off and $diff_class is left empty. It isn't a major
> issue (I suspect that such a case might even be an error), and I even
> think the code after the above patch would be easier to read and more
> sensible, but shouldn't that kind of "style fix" be in a separate clean-up
> patch that does not introduce any new feature?
>
Sorry. I will separate my patch into two or three commits. Probably like:
- format_diff_line returns a class with an line.
- remove trailing space from the class.
- add side-by-side feature and CSS.
- add form.
Thank you for your correction!
--
Kato Kazuyoshi
^ permalink raw reply
* Re: regression in git-gui since 2c5c66b... Merge branch 'jp/get-ref-dir-unsorted
From: Junio C Hamano @ 2011-10-17 0:35 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <4E9B1E32.7030101@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
> I have a project organized as a number of nested git modules (not
> using git-submodule), and frequently use git-new-workdir to create the
> nested modules.
>
> Since the above merge-commit, git-gui is confused by this arrangement
> and reports every file in every nested module as being an untracked
> file in the top-level (super) project.
Could you come up with a simple reproduction recipe that prepares a
superproject that has no file of its own, a submodule with a single file
tracked, and attaches another workdir? If running git-gui in the resulting
directory makes it misbehave, we could then isolate what git command that
is invoked by git-gui has changed its behaviour.
Thanks.
^ permalink raw reply
* Re: [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Junio C Hamano @ 2011-10-17 0:20 UTC (permalink / raw)
To: Kato Kazuyoshi; +Cc: git
In-Reply-To: <CAFo4x0Kb651CyxoP8wxR36aDe5=Md2UV3qjh+HPo4ad6NB=Emg@mail.gmail.com>
Kato Kazuyoshi <kato.kazuyoshi@gmail.com> writes:
> @@ -2235,25 +2238,25 @@ sub format_diff_line {
> # combined diff
> my $prefix = substr($line, 0, scalar @{$from->{'href'}});
> if ($line =~ m/^\@{3}/) {
> - $diff_class = " chunk_header";
> + $diff_class = "chunk_header";
> } elsif ($line =~ m/^\\/) {
> - $diff_class = " incomplete";
> + $diff_class = "incomplete";
> } elsif ($prefix =~ tr/+/+/) {
> - $diff_class = " add";
> + $diff_class = "add";
> } elsif ($prefix =~ tr/-/-/) {
> - $diff_class = " rem";
> + $diff_class = "rem";
> }
> } else {
> # assume ordinary diff
> my $char = substr($line, 0, 1);
> if ($char eq '+') {
> - $diff_class = " add";
> + $diff_class = "add";
> } elsif ($char eq '-') {
> - $diff_class = " rem";
> + $diff_class = "rem";
> } elsif ($char eq '@') {
> - $diff_class = " chunk_header";
> + $diff_class = "chunk_header";
> } elsif ($char eq "\\") {
> - $diff_class = " incomplete";
> + $diff_class = "incomplete";
> }
> }
> $line = untabify($line);
> @@ -2274,7 +2277,7 @@ sub format_diff_line {
> }
> $line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
> "<span class=\"section\">" . esc_html($section, -nbsp=>1) .
> "</span>";
> - return "<div class=\"diff$diff_class\">$line</div>\n";
> + return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
What are these changes for, except perhaps to make the patch larger than
necesssary to make it harder to review?
It would leave an unnecessary SP like 'class="diff "' when all the arms of
if/elsif cascade fall off and $diff_class is left empty. It isn't a major
issue (I suspect that such a case might even be an error), and I even
think the code after the above patch would be easier to read and more
sensible, but shouldn't that kind of "style fix" be in a separate clean-up
patch that does not introduce any new feature?
> @@ -2307,9 +2310,9 @@ sub format_diff_line {
> }
> $line .= " $prefix</span>" .
> "<span class=\"section\">" . esc_html($section, -nbsp=>1)
> . "</span>";
> - return "<div class=\"diff$diff_class\">$line</div>\n";
> + return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
> }
> - return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1)
> . "</div>\n";
> + return $diff_class, "<div class=\"diff $diff_class\">" .
> esc_html($line, -nbsp=>1) . "</div>\n";
> }
And everything else, including this hunk to change what is returned from
the subroutine belongs to a separate patch that implements the side-by-side
feature.
^ permalink raw reply
* [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Kato Kazuyoshi @ 2011-10-16 23:57 UTC (permalink / raw)
To: git
---
Hello,
I want to add some features that provided by Trac to Gitweb.
First, I've implemented "side-by-side" style diff.
gitweb/gitweb.perl | 97 ++++++++++++++++++++++++++++++++++++++--------
gitweb/static/gitweb.css | 16 ++++++++
2 files changed, 96 insertions(+), 17 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 85d64b2..9d7609f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -757,6 +757,7 @@ our @cgi_param_mapping = (
extra_options => "opt",
search_use_regexp => "sr",
ctag => "by_tag",
+ diff_style => "ds",
# this must be last entry (for manipulation from JavaScript)
javascript => "js"
);
@@ -1072,6 +1073,8 @@ sub evaluate_and_validate_params {
}
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
}
+
+ $input_params{diff_style} ||= 'inline';
}
# path to the current git repository
@@ -2235,25 +2238,25 @@ sub format_diff_line {
# combined diff
my $prefix = substr($line, 0, scalar @{$from->{'href'}});
if ($line =~ m/^\@{3}/) {
- $diff_class = " chunk_header";
+ $diff_class = "chunk_header";
} elsif ($line =~ m/^\\/) {
- $diff_class = " incomplete";
+ $diff_class = "incomplete";
} elsif ($prefix =~ tr/+/+/) {
- $diff_class = " add";
+ $diff_class = "add";
} elsif ($prefix =~ tr/-/-/) {
- $diff_class = " rem";
+ $diff_class = "rem";
}
} else {
# assume ordinary diff
my $char = substr($line, 0, 1);
if ($char eq '+') {
- $diff_class = " add";
+ $diff_class = "add";
} elsif ($char eq '-') {
- $diff_class = " rem";
+ $diff_class = "rem";
} elsif ($char eq '@') {
- $diff_class = " chunk_header";
+ $diff_class = "chunk_header";
} elsif ($char eq "\\") {
- $diff_class = " incomplete";
+ $diff_class = "incomplete";
}
}
$line = untabify($line);
@@ -2274,7 +2277,7 @@ sub format_diff_line {
}
$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
"<span class=\"section\">" . esc_html($section, -nbsp=>1) .
"</span>";
- return "<div class=\"diff$diff_class\">$line</div>\n";
+ return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
} elsif ($from && $to && $line =~ m/^\@{3}/) {
my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
@@ -2307,9 +2310,9 @@ sub format_diff_line {
}
$line .= " $prefix</span>" .
"<span class=\"section\">" . esc_html($section, -nbsp=>1)
. "</span>";
- return "<div class=\"diff$diff_class\">$line</div>\n";
+ return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
}
- return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1)
. "</div>\n";
+ return $diff_class, "<div class=\"diff $diff_class\">" .
esc_html($line, -nbsp=>1) . "</div>\n";
}
# Generates undef or something like "_snapshot_" or "snapshot (_tbz2_ _zip_)",
@@ -4826,8 +4829,32 @@ sub git_difftree_body {
print "</table>\n";
}
+sub format_diff_chunk {
+ my @chunk = @_;
+
+ my $first_class = $chunk[0]->[0];
+ my @partial = map { $_->[1] } grep { $_->[0] eq $first_class } @chunk;
+
+ if (scalar @partial < scalar @chunk) {
+ return join '', ("<div class='chunk'><div class='old'>",
+ @partial,
+ "</div>",
+ "<div class='new'>",
+ (map {
+ $_->[1];
+ } @chunk[scalar @partial..scalar @chunk-1]),
+ "</div></div>");
+ } else {
+ return join '', ("<div class='chunk'><div class='",
+ ($first_class eq 'add' ? 'new' : 'old'),
+ "'>",
+ @partial,
+ "</div></div>");
+ }
+}
+
sub git_patchset_body {
- my ($fd, $difftree, $hash, @hash_parents) = @_;
+ my ($fd, $is_inline, $difftree, $hash, @hash_parents) = @_;
my ($hash_parent) = $hash_parents[0];
my $is_combined = (@hash_parents > 1);
@@ -4938,12 +4965,31 @@ sub git_patchset_body {
# the patch itself
LINE:
+ my @chunk;
while ($patch_line = <$fd>) {
chomp $patch_line;
next PATCH if ($patch_line =~ m/^diff /);
- print format_diff_line($patch_line, \%from, \%to);
+ my ($class, $line) = format_diff_line($patch_line, \%from, \%to);
+ if ($is_inline) {
+ print $line;
+ } elsif ($class eq 'add' || $class eq 'rem') {
+ push @chunk, [ $class, $line ];
+ } else {
+ if (@chunk) {
+ print format_diff_chunk(@chunk);
+ @chunk = ();
+ } elsif ($class eq 'chunk_header') {
+ print $line;
+ } else {
+ print q{<div class='chunk'><div class='old'>},
+ $line,
+ q{</div><div class='new'>},
+ $line,
+ q{</div></div>};
+ }
+ }
}
} continue {
@@ -7022,7 +7068,7 @@ sub git_blobdiff {
"raw");
git_header_html(undef, $expires);
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
- git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+ git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base,
$formats_nav . diff_nav($input_params{diff_style}));
git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
} else {
print "<div class=\"page_nav\"><br/>$formats_nav<br/></div>\n";
@@ -7051,7 +7097,7 @@ sub git_blobdiff {
if ($format eq 'html') {
print "<div class=\"page_body\">\n";
- git_patchset_body($fd, [ \%diffinfo ], $hash_base, $hash_parent_base);
+ git_patchset_body($fd, $input_params{diff_style} eq 'inline', [
\%diffinfo ], $hash_base, $hash_parent_base);
close $fd;
print "</div>\n"; # class="page_body"
@@ -7076,6 +7122,22 @@ sub git_blobdiff_plain {
git_blobdiff('plain');
}
+sub diff_nav {
+ my ($style) = @_;
+
+ my %pairs = (inline => 'inline', 'sidebyside' => 'side by side');
+ join '', ($cgi->start_form({ method => 'get' }),
+ $cgi->hidden('p'),
+ $cgi->hidden('a'),
+ $cgi->hidden('h'),
+ $cgi->hidden('hp'),
+ $cgi->hidden('hb'),
+ $cgi->hidden('hpb'),
+ $cgi->popup_menu('ds', [keys %pairs], $style, \%pairs),
+ $cgi->submit('change'),
+ $cgi->end_form);
+}
+
sub git_commitdiff {
my %params = @_;
my $format = $params{-format} || 'html';
@@ -7228,7 +7290,7 @@ sub git_commitdiff {
my $ref = format_ref_marker($refs, $co{'id'});
git_header_html(undef, $expires);
- git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
+ git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash,
$formats_nav . diff_nav($input_params{diff_style}));
git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
print "<div class=\"title_text\">\n" .
"<table class=\"object_header\">\n";
@@ -7282,7 +7344,8 @@ sub git_commitdiff {
$use_parents ? @{$co{'parents'}} : $hash_parent);
print "<br/>\n";
- git_patchset_body($fd, \@difftree, $hash,
+ git_patchset_body($fd, $input_params{diff_style} eq 'inline',
+ \@difftree, $hash,
$use_parents ? @{$co{'parents'}} : $hash_parent);
close $fd;
print "</div>\n"; # class="page_body"
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 7d88509..a6872f9 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -618,6 +618,22 @@ div.remote {
cursor: pointer;
}
+/* side-by-side diff */
+div.chunk {
+ overflow: hidden;
+}
+
+div.chunk div.old {
+ float: left;
+ width: 50%;
+ overflow: hidden;
+}
+
+div.chunk div.new {
+ margin-left: 50%;
+ width: 50%;
+}
+
/* Style definition generated by highlight 2.4.5, http://www.andre-simon.de/ */
--
1.7.7.213.g8b0e1.dirty
^ permalink raw reply related
* Re: [PATCH] gitweb: add extensions to highlight feature map
From: Philip Oakley @ 2011-10-16 23:18 UTC (permalink / raw)
To: Lénaïc Huard; +Cc: git
In-Reply-To: <201110161859.49817.lenaic@lhuard.fr.eu.org>
From: "Lénaïc Huard" <lenaic@lhuard.fr.eu.org>
Sent: Sunday, October 16, 2011 5:59 PM
> added: hpp, m
'm' files can be matlab files, and matlab can benefit from git's branch
model. Matlab expectes filenames to be identical to function names(!), so
branching function variants makes for easier development. Without git you
can't easily refactor.
Philip
>
> Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr.eu.org>
> ---
> gitweb/gitweb.perl | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 85d64b2..75e4854 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -262,11 +262,12 @@ our %highlight_ext = (
> # alternate extensions, see /etc/highlight/filetypes.conf
> 'h' => 'c',
> map { $_ => 'sh' } qw(bash zsh ksh),
> - map { $_ => 'cpp' } qw(cxx c++ cc),
> + map { $_ => 'cpp' } qw(cxx c++ cc hpp),
> map { $_ => 'php' } qw(php3 php4 php5 phps),
> map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
> map { $_ => 'make'} qw(mak mk),
> map { $_ => 'xml' } qw(xhtml html htm),
> + 'm' => 'objc',
> );
>
> # You define site-wide feature defaults here; override them with
> --
> 1.7.7
> --
> 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
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2012.0.1831 / Virus Database: 2090/4554 - Release Date: 10/15/11
>
^ permalink raw reply
* Re: [PATCH 1/6] git-p4 tests: refactor and cleanup
From: Pete Wyckoff @ 2011-10-16 22:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Luke Diamand, Chris Li, Stefano Lattarini
In-Reply-To: <20111016144319.GD22144@arf.padd.com>
pw@padd.com wrote on Sun, 16 Oct 2011 10:43 -0400:
> Introduce a library for functions that are common to
> multiple git-p4 test files.
Update, replacing bare "python" invocation with "$PYTHON_PATH".
-------------------------8<-----------------------------
>From 3a26dbf40d9aa9f8432d994d5697ad10fce8ed91 Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd.com>
Date: Mon, 22 Aug 2011 22:20:33 -0400
Subject: [PATCH 1/6] git-p4 tests: refactor and cleanup
Introduce a library for functions that are common to
multiple git-p4 test files.
Be a bit more clever about starting and stopping p4d.
Specify a unique port number for each test, so that
tests can run in parallel. Start p4d not in daemon mode,
and save the pid, to be able to kill it cleanly later.
Never kill p4d at startup; always shutdown cleanly.
Handle directory changes better. Always chdir inside
a subshell, and remove any post-test directory changes.
Clean up whitespace, and use test_cmp and test_must_fail
more consistently.
Separate the tests related to detecting p4 branches
into their own file, and add a few more.
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/lib-git-p4.sh | 74 +++++++
t/t9800-git-p4.sh | 546 ++++++++++++++++++++--------------------------
t/t9801-git-p4-branch.sh | 233 ++++++++++++++++++++
3 files changed, 544 insertions(+), 309 deletions(-)
create mode 100644 t/lib-git-p4.sh
create mode 100755 t/t9801-git-p4-branch.sh
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
new file mode 100644
index 0000000..a870f9a
--- /dev/null
+++ b/t/lib-git-p4.sh
@@ -0,0 +1,74 @@
+#
+# Library code for git-p4 tests
+#
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON; then
+ skip_all='skipping git-p4 tests; python not available'
+ test_done
+fi
+( p4 -h && p4d -h ) >/dev/null 2>&1 || {
+ skip_all='skipping git-p4 tests; no p4 or p4d'
+ test_done
+}
+
+GITP4="$GIT_BUILD_DIR/contrib/fast-import/git-p4"
+
+# Try to pick a unique port: guess a large number, then hope
+# no more than one of each test is running.
+#
+# This does not handle the case where somebody else is running the
+# same tests and has chosen the same ports.
+testid=${this_test#t}
+git_p4_test_start=9800
+P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
+
+export P4PORT=localhost:$P4DPORT
+export P4CLIENT=client
+
+db="$TRASH_DIRECTORY/db"
+cli="$TRASH_DIRECTORY/cli"
+git="$TRASH_DIRECTORY/git"
+pidfile="$TRASH_DIRECTORY/p4d.pid"
+
+start_p4d() {
+ mkdir -p "$db" "$cli" "$git" &&
+ (
+ p4d -q -r "$db" -p $P4DPORT &
+ echo $! >"$pidfile"
+ ) &&
+ for i in 1 2 3 4 5 ; do
+ p4 info >/dev/null 2>&1 && break || true &&
+ echo waiting for p4d to start &&
+ sleep 1
+ done &&
+ # complain if it never started
+ p4 info >/dev/null &&
+ (
+ cd "$cli" &&
+ p4 client -i <<-EOF
+ Client: client
+ Description: client
+ Root: $cli
+ View: //depot/... //client/...
+ EOF
+ )
+}
+
+kill_p4d() {
+ pid=$(cat "$pidfile")
+ # it had better exist for the first kill
+ kill $pid &&
+ for i in 1 2 3 4 5 ; do
+ kill $pid >/dev/null 2>&1 || break
+ sleep 1
+ done &&
+ # complain if it would not die
+ test_must_fail kill $pid >/dev/null 2>&1 &&
+ rm -rf "$db" "$cli" "$pidfile"
+}
+
+cleanup_git() {
+ rm -rf "$git"
+}
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 01ba041..272de3f 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -2,76 +2,51 @@
test_description='git-p4 tests'
-. ./test-lib.sh
+. ./lib-git-p4.sh
-( p4 -h && p4d -h ) >/dev/null 2>&1 || {
- skip_all='skipping git-p4 tests; no p4 or p4d'
- test_done
-}
-
-GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4
-P4DPORT=10669
-
-export P4PORT=localhost:$P4DPORT
-
-db="$TRASH_DIRECTORY/db"
-cli="$TRASH_DIRECTORY/cli"
-git="$TRASH_DIRECTORY/git"
-
-test_debug 'echo p4d -q -d -r "$db" -p $P4DPORT'
-test_expect_success setup '
- mkdir -p "$db" &&
- p4d -q -d -r "$db" -p $P4DPORT &&
- mkdir -p "$cli" &&
- mkdir -p "$git" &&
- export P4PORT=localhost:$P4DPORT
+test_expect_success 'start p4d' '
+ start_p4d
'
test_expect_success 'add p4 files' '
- cd "$cli" &&
- p4 client -i <<-EOF &&
- Client: client
- Description: client
- Root: $cli
- View: //depot/... //client/...
- EOF
- export P4CLIENT=client &&
- echo file1 >file1 &&
- p4 add file1 &&
- p4 submit -d "file1" &&
- echo file2 >file2 &&
- p4 add file2 &&
- p4 submit -d "file2" &&
- cd "$TRASH_DIRECTORY"
+ (
+ cd "$cli" &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "file1" &&
+ echo file2 >file2 &&
+ p4 add file2 &&
+ p4 submit -d "file2"
+ )
'
-cleanup_git() {
- cd "$TRASH_DIRECTORY" &&
- rm -rf "$git" &&
- mkdir "$git"
-}
-
test_expect_success 'basic git-p4 clone' '
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git log --oneline >lines &&
- test_line_count = 1 lines
+ (
+ cd "$git" &&
+ git log --oneline >lines &&
+ test_line_count = 1 lines
+ )
'
test_expect_success 'git-p4 clone @all' '
"$GITP4" clone --dest="$git" //depot@all &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git log --oneline >lines &&
- test_line_count = 2 lines
+ (
+ cd "$git" &&
+ git log --oneline >lines &&
+ test_line_count = 2 lines
+ )
'
test_expect_success 'git-p4 sync uninitialized repo' '
test_create_repo "$git" &&
test_when_finished cleanup_git &&
- cd "$git" &&
- test_must_fail "$GITP4" sync
+ (
+ cd "$git" &&
+ test_must_fail "$GITP4" sync
+ )
'
#
@@ -81,17 +56,19 @@ test_expect_success 'git-p4 sync uninitialized repo' '
test_expect_success 'git-p4 sync new branch' '
test_create_repo "$git" &&
test_when_finished cleanup_git &&
- cd "$git" &&
- test_commit head &&
- "$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
- git log --oneline p4/depot >lines &&
- test_line_count = 2 lines
+ (
+ cd "$git" &&
+ test_commit head &&
+ "$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
+ git log --oneline p4/depot >lines &&
+ test_line_count = 2 lines
+ )
'
test_expect_success 'exit when p4 fails to produce marshaled output' '
badp4dir="$TRASH_DIRECTORY/badp4dir" &&
- mkdir -p "$badp4dir" &&
- test_when_finished "rm -rf $badp4dir" &&
+ mkdir "$badp4dir" &&
+ test_when_finished "rm \"$badp4dir/p4\" && rmdir \"$badp4dir\"" &&
cat >"$badp4dir"/p4 <<-EOF &&
#!$SHELL_PATH
exit 1
@@ -103,61 +80,61 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
'
test_expect_success 'add p4 files with wildcards in the names' '
- cd "$cli" &&
- echo file-wild-hash >file-wild#hash &&
- echo file-wild-star >file-wild\*star &&
- echo file-wild-at >file-wild@at &&
- echo file-wild-percent >file-wild%percent &&
- p4 add -f file-wild* &&
- p4 submit -d "file wildcards"
+ (
+ cd "$cli" &&
+ echo file-wild-hash >file-wild#hash &&
+ echo file-wild-star >file-wild\*star &&
+ echo file-wild-at >file-wild@at &&
+ echo file-wild-percent >file-wild%percent &&
+ p4 add -f file-wild* &&
+ p4 submit -d "file wildcards"
+ )
'
test_expect_success 'wildcard files git-p4 clone' '
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- test -f file-wild#hash &&
- test -f file-wild\*star &&
- test -f file-wild@at &&
- test -f file-wild%percent
+ (
+ cd "$git" &&
+ test -f file-wild#hash &&
+ test -f file-wild\*star &&
+ test -f file-wild@at &&
+ test -f file-wild%percent
+ )
'
test_expect_success 'clone bare' '
"$GITP4" clone --dest="$git" --bare //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- test ! -d .git &&
- bare=`git config --get core.bare` &&
- test "$bare" = true
+ (
+ cd "$git" &&
+ test ! -d .git &&
+ bare=`git config --get core.bare` &&
+ test "$bare" = true
+ )
'
p4_add_user() {
- name=$1
- fullname=$2
- p4 user -f -i <<EOF &&
-User: $name
-Email: $name@localhost
-FullName: $fullname
-EOF
- p4 passwd -P secret $name
+ name=$1 fullname=$2 &&
+ p4 user -f -i <<-EOF &&
+ User: $name
+ Email: $name@localhost
+ FullName: $fullname
+ EOF
+ p4 passwd -P secret $name
}
p4_grant_admin() {
- name=$1
- p4 protect -o |\
- awk "{print}END{print \" admin user $name * //depot/...\"}" |\
- p4 protect -i
+ name=$1 &&
+ {
+ p4 protect -o &&
+ echo " admin user $name * //depot/..."
+ } | p4 protect -i
}
p4_check_commit_author() {
- file=$1
- user=$2
- if p4 changes -m 1 //depot/$file | grep $user > /dev/null ; then
- return 0
- else
- echo "file $file not modified by user $user" 1>&2
- return 1
- fi
+ file=$1 user=$2 &&
+ p4 changes -m 1 //depot/$file | grep -q $user
}
make_change_by_user() {
@@ -174,15 +151,17 @@ test_expect_success 'preserve users' '
p4_grant_admin alice &&
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- echo "username: a change by alice" >> file1 &&
- echo "username: a change by bob" >> file2 &&
- git commit --author "Alice <alice@localhost>" -m "a change by alice" file1 &&
- git commit --author "Bob <bob@localhost>" -m "a change by bob" file2 &&
- git config git-p4.skipSubmitEditCheck true &&
- P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
- p4_check_commit_author file1 alice &&
- p4_check_commit_author file2 bob
+ (
+ cd "$git" &&
+ echo "username: a change by alice" >>file1 &&
+ echo "username: a change by bob" >>file2 &&
+ git commit --author "Alice <alice@localhost>" -m "a change by alice" file1 &&
+ git commit --author "Bob <bob@localhost>" -m "a change by bob" file2 &&
+ git config git-p4.skipSubmitEditCheck true &&
+ P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
+ p4_check_commit_author file1 alice &&
+ p4_check_commit_author file2 bob
+ )
'
# Test username support, submitting as bob, who lacks admin rights. Should
@@ -190,32 +169,37 @@ test_expect_success 'preserve users' '
test_expect_success 'refuse to preserve users without perms' '
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git config git-p4.skipSubmitEditCheck true &&
- echo "username-noperms: a change by alice" >> file1 &&
- git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
- ! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&
- ! git diff --exit-code HEAD..p4/master > /dev/null
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
+ echo "username-noperms: a change by alice" >>file1 &&
+ git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
+ P4EDITOR=touch P4USER=bob P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
+ test_must_fail git diff --exit-code HEAD..p4/master
+ )
'
# What happens with unknown author? Without allowMissingP4Users it should fail.
test_expect_success 'preserve user where author is unknown to p4' '
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git config git-p4.skipSubmitEditCheck true &&
- echo "username-bob: a change by bob" >> file1 &&
- git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
- echo "username-unknown: a change by charlie" >> file1 &&
- git commit --author "Charlie <charlie@localhost>" -m "preserve: a change by charlie" file1 &&
- ! P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
- ! git diff --exit-code HEAD..p4/master > /dev/null &&
- echo "$0: repeat with allowMissingP4Users enabled" &&
- git config git-p4.allowMissingP4Users true &&
- git config git-p4.preserveUser true &&
- P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
- git diff --exit-code HEAD..p4/master > /dev/null &&
- p4_check_commit_author file1 alice
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
+ echo "username-bob: a change by bob" >>file1 &&
+ git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
+ echo "username-unknown: a change by charlie" >>file1 &&
+ git commit --author "Charlie <charlie@localhost>" -m "preserve: a change by charlie" file1 &&
+ P4EDITOR=touch P4USER=alice P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
+ test_must_fail git diff --exit-code HEAD..p4/master &&
+
+ echo "$0: repeat with allowMissingP4Users enabled" &&
+ git config git-p4.allowMissingP4Users true &&
+ git config git-p4.preserveUser true &&
+ P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
+ git diff --exit-code HEAD..p4/master &&
+ p4_check_commit_author file1 alice
+ )
'
# If we're *not* using --preserve-user, git-p4 should warn if we're submitting
@@ -225,33 +209,35 @@ test_expect_success 'preserve user where author is unknown to p4' '
test_expect_success 'not preserving user with mixed authorship' '
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git config git-p4.skipSubmitEditCheck true &&
- p4_add_user derek Derek &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
+ p4_add_user derek Derek &&
- make_change_by_user usernamefile3 Derek derek@localhost &&
- P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
- grep "git author derek@localhost does not match" actual &&
+ make_change_by_user usernamefile3 Derek derek@localhost &&
+ P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+ grep "git author derek@localhost does not match" &&
- make_change_by_user usernamefile3 Charlie charlie@localhost &&
- P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
- grep "git author charlie@localhost does not match" actual &&
+ make_change_by_user usernamefile3 Charlie charlie@localhost &&
+ P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+ grep "git author charlie@localhost does not match" &&
- make_change_by_user usernamefile3 alice alice@localhost &&
- P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
- ! grep "git author.*does not match" actual &&
+ make_change_by_user usernamefile3 alice alice@localhost &&
+ P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" |\
+ test_must_fail grep "git author.*does not match" &&
- git config git-p4.skipUserNameCheck true &&
- make_change_by_user usernamefile3 Charlie charlie@localhost &&
- P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
- ! grep "git author.*does not match" actual &&
+ git config git-p4.skipUserNameCheck true &&
+ make_change_by_user usernamefile3 Charlie charlie@localhost &&
+ P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+ test_must_fail grep "git author.*does not match" &&
- p4_check_commit_author usernamefile3 alice
+ p4_check_commit_author usernamefile3 alice
+ )
'
marshal_dump() {
what=$1
- python -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
+ "$PYTHON_PATH" -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
}
# Sleep a bit so that the top-most p4 change did not happen "now". Then
@@ -263,10 +249,12 @@ test_expect_success 'initial import time from top change time' '
sleep 3 &&
"$GITP4" clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
- cd "$git" &&
- gittime=$(git show -s --raw --pretty=format:%at HEAD) &&
- echo $p4time $gittime &&
- test $p4time = $gittime
+ (
+ cd "$git" &&
+ gittime=$(git show -s --raw --pretty=format:%at HEAD) &&
+ echo $p4time $gittime &&
+ test $p4time = $gittime
+ )
'
# Rename a file and confirm that rename is not detected in P4.
@@ -279,47 +267,49 @@ test_expect_success 'initial import time from top change time' '
test_expect_success 'detect renames' '
"$GITP4" clone --dest="$git" //depot@all &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git config git-p4.skipSubmitEditCheck true &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
- git mv file1 file4 &&
- git commit -a -m "Rename file1 to file4" &&
- git diff-tree -r -M HEAD &&
- "$GITP4" submit &&
- p4 filelog //depot/file4 &&
- ! p4 filelog //depot/file4 | grep -q "branch from" &&
+ git mv file1 file4 &&
+ git commit -a -m "Rename file1 to file4" &&
+ git diff-tree -r -M HEAD &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file4 &&
+ p4 filelog //depot/file4 | test_must_fail grep -q "branch from" &&
- git mv file4 file5 &&
- git commit -a -m "Rename file4 to file5" &&
- git diff-tree -r -M HEAD &&
- git config git-p4.detectRenames true &&
- "$GITP4" submit &&
- p4 filelog //depot/file5 &&
- p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
+ git mv file4 file5 &&
+ git commit -a -m "Rename file4 to file5" &&
+ git diff-tree -r -M HEAD &&
+ git config git-p4.detectRenames true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file5 &&
+ p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
- git mv file5 file6 &&
- echo update >>file6 &&
- git add file6 &&
- git commit -a -m "Rename file5 to file6 with changes" &&
- git diff-tree -r -M HEAD &&
- level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
- test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
- git config git-p4.detectRenames $((level + 2)) &&
- "$GITP4" submit &&
- p4 filelog //depot/file6 &&
- ! p4 filelog //depot/file6 | grep -q "branch from" &&
+ git mv file5 file6 &&
+ echo update >>file6 &&
+ git add file6 &&
+ git commit -a -m "Rename file5 to file6 with changes" &&
+ git diff-tree -r -M HEAD &&
+ level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
+ test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
+ git config git-p4.detectRenames $(($level + 2)) &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file6 &&
+ p4 filelog //depot/file6 | test_must_fail grep -q "branch from" &&
- git mv file6 file7 &&
- echo update >>file7 &&
- git add file7 &&
- git commit -a -m "Rename file6 to file7 with changes" &&
- git diff-tree -r -M HEAD &&
- level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
- test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
- git config git-p4.detectRenames $((level - 2)) &&
- "$GITP4" submit &&
- p4 filelog //depot/file7 &&
- p4 filelog //depot/file7 | grep -q "branch from //depot/file6"
+ git mv file6 file7 &&
+ echo update >>file7 &&
+ git add file7 &&
+ git commit -a -m "Rename file6 to file7 with changes" &&
+ git diff-tree -r -M HEAD &&
+ level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
+ test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
+ git config git-p4.detectRenames $(($level - 2)) &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file7 &&
+ p4 filelog //depot/file7 | grep -q "branch from //depot/file6"
+ )
'
# Copy a file and confirm that copy is not detected in P4.
@@ -336,141 +326,79 @@ test_expect_success 'detect renames' '
test_expect_success 'detect copies' '
"$GITP4" clone --dest="$git" //depot@all &&
test_when_finished cleanup_git &&
- cd "$git" &&
- git config git-p4.skipSubmitEditCheck true &&
-
- cp file2 file8 &&
- git add file8 &&
- git commit -a -m "Copy file2 to file8" &&
- git diff-tree -r -C HEAD &&
- "$GITP4" submit &&
- p4 filelog //depot/file8 &&
- ! p4 filelog //depot/file8 | grep -q "branch from" &&
-
- cp file2 file9 &&
- git add file9 &&
- git commit -a -m "Copy file2 to file9" &&
- git diff-tree -r -C HEAD &&
- git config git-p4.detectCopies true &&
- "$GITP4" submit &&
- p4 filelog //depot/file9 &&
- ! p4 filelog //depot/file9 | grep -q "branch from" &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
- echo "file2" >>file2 &&
- cp file2 file10 &&
- git add file2 file10 &&
- git commit -a -m "Modify and copy file2 to file10" &&
- git diff-tree -r -C HEAD &&
- "$GITP4" submit &&
- p4 filelog //depot/file10 &&
- p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+ cp file2 file8 &&
+ git add file8 &&
+ git commit -a -m "Copy file2 to file8" &&
+ git diff-tree -r -C HEAD &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file8 &&
+ p4 filelog //depot/file8 | test_must_fail grep -q "branch from" &&
- cp file2 file11 &&
- git add file11 &&
- git commit -a -m "Copy file2 to file11" &&
- git diff-tree -r -C --find-copies-harder HEAD &&
- src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- test "$src" = file10 &&
- git config git-p4.detectCopiesHarder true &&
- "$GITP4" submit &&
- p4 filelog //depot/file11 &&
- p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+ cp file2 file9 &&
+ git add file9 &&
+ git commit -a -m "Copy file2 to file9" &&
+ git diff-tree -r -C HEAD &&
+ git config git-p4.detectCopies true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file9 &&
+ p4 filelog //depot/file9 | test_must_fail grep -q "branch from" &&
- cp file2 file12 &&
- echo "some text" >>file12 &&
- git add file12 &&
- git commit -a -m "Copy file2 to file12 with changes" &&
- git diff-tree -r -C --find-copies-harder HEAD &&
- level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
- test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
- src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- test "$src" = file10 &&
- git config git-p4.detectCopies $((level + 2)) &&
- "$GITP4" submit &&
- p4 filelog //depot/file12 &&
- ! p4 filelog //depot/file12 | grep -q "branch from" &&
+ echo "file2" >>file2 &&
+ cp file2 file10 &&
+ git add file2 file10 &&
+ git commit -a -m "Modify and copy file2 to file10" &&
+ git diff-tree -r -C HEAD &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file10 &&
+ p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
- cp file2 file13 &&
- echo "different text" >>file13 &&
- git add file13 &&
- git commit -a -m "Copy file2 to file13 with changes" &&
- git diff-tree -r -C --find-copies-harder HEAD &&
- level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
- test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
- src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- test "$src" = file10 &&
- git config git-p4.detectCopies $((level - 2)) &&
- "$GITP4" submit &&
- p4 filelog //depot/file13 &&
- p4 filelog //depot/file13 | grep -q "branch from //depot/file"
-'
+ cp file2 file11 &&
+ git add file11 &&
+ git commit -a -m "Copy file2 to file11" &&
+ git diff-tree -r -C --find-copies-harder HEAD &&
+ src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
+ test "$src" = file10 &&
+ git config git-p4.detectCopiesHarder true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file11 &&
+ p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
-# Create a simple branch structure in P4 depot to check if it is correctly
-# cloned.
-test_expect_success 'add simple p4 branches' '
- cd "$cli" &&
- mkdir branch1 &&
- cd branch1 &&
- echo file1 >file1 &&
- echo file2 >file2 &&
- p4 add file1 file2 &&
- p4 submit -d "branch1" &&
- p4 integrate //depot/branch1/... //depot/branch2/... &&
- p4 submit -d "branch2" &&
- echo file3 >file3 &&
- p4 add file3 &&
- p4 submit -d "add file3 in branch1" &&
- p4 open file2 &&
- echo update >>file2 &&
- p4 submit -d "update file2 in branch1" &&
- p4 integrate //depot/branch1/... //depot/branch3/... &&
- p4 submit -d "branch3" &&
- cd "$TRASH_DIRECTORY"
-'
+ cp file2 file12 &&
+ echo "some text" >>file12 &&
+ git add file12 &&
+ git commit -a -m "Copy file2 to file12 with changes" &&
+ git diff-tree -r -C --find-copies-harder HEAD &&
+ level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
+ test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
+ src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
+ test "$src" = file10 &&
+ git config git-p4.detectCopies $(($level + 2)) &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file12 &&
+ p4 filelog //depot/file12 | test_must_fail grep -q "branch from" &&
-# Configure branches through git-config and clone them.
-# All files are tested to make sure branches were cloned correctly.
-# Finally, make an update to branch1 on P4 side to check if it is imported
-# correctly by git-p4.
-test_expect_success 'git-p4 clone simple branches' '
- test_when_finished cleanup_git &&
- test_create_repo "$git" &&
- cd "$git" &&
- git config git-p4.branchList branch1:branch2 &&
- git config --add git-p4.branchList branch1:branch3 &&
- "$GITP4" clone --dest=. --detect-branches //depot@all &&
- git log --all --graph --decorate --stat &&
- git reset --hard p4/depot/branch1 &&
- test -f file1 &&
- test -f file2 &&
- test -f file3 &&
- grep -q update file2 &&
- git reset --hard p4/depot/branch2 &&
- test -f file1 &&
- test -f file2 &&
- test ! -f file3 &&
- ! grep -q update file2 &&
- git reset --hard p4/depot/branch3 &&
- test -f file1 &&
- test -f file2 &&
- test -f file3 &&
- grep -q update file2 &&
- cd "$cli" &&
- cd branch1 &&
- p4 edit file2 &&
- echo file2_ >>file2 &&
- p4 submit -d "update file2 in branch1" &&
- cd "$git" &&
- git reset --hard p4/depot/branch1 &&
- "$GITP4" rebase &&
- grep -q file2_ file2
+ cp file2 file13 &&
+ echo "different text" >>file13 &&
+ git add file13 &&
+ git commit -a -m "Copy file2 to file13 with changes" &&
+ git diff-tree -r -C --find-copies-harder HEAD &&
+ level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
+ test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
+ src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
+ test "$src" = file10 &&
+ git config git-p4.detectCopies $(($level - 2)) &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file13 &&
+ p4 filelog //depot/file13 | grep -q "branch from //depot/file"
+ )
'
-test_expect_success 'shutdown' '
- pid=`pgrep -f p4d` &&
- test -n "$pid" &&
- test_debug "ps wl `echo $pid`" &&
- kill $pid
+test_expect_success 'kill p4d' '
+ kill_p4d
'
test_done
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
new file mode 100755
index 0000000..a25f18d
--- /dev/null
+++ b/t/t9801-git-p4-branch.sh
@@ -0,0 +1,233 @@
+#!/bin/sh
+
+test_description='git-p4 p4 branching tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+#
+# 1: //depot/main/f1
+# 2: //depot/main/f2
+# 3: integrate //depot/main/... -> //depot/branch1/...
+# 4: //depot/main/f4
+# 5: //depot/branch1/f5
+# .: named branch branch2
+# 6: integrate -b branch2
+# 7: //depot/branch2/f7
+# 8: //depot/main/f8
+#
+test_expect_success 'basic p4 branches' '
+ (
+ cd "$cli" &&
+ mkdir -p main &&
+
+ echo f1 >main/f1 &&
+ p4 add main/f1 &&
+ p4 submit -d "main/f1" &&
+
+ echo f2 >main/f2 &&
+ p4 add main/f2 &&
+ p4 submit -d "main/f2" &&
+
+ p4 integrate //depot/main/... //depot/branch1/... &&
+ p4 submit -d "integrate main to branch1" &&
+
+ echo f4 >main/f4 &&
+ p4 add main/f4 &&
+ p4 submit -d "main/f4" &&
+
+ echo f5 >branch1/f5 &&
+ p4 add branch1/f5 &&
+ p4 submit -d "branch1/f5" &&
+
+ p4 branch -i <<-EOF &&
+ Branch: branch2
+ View: //depot/main/... //depot/branch2/...
+ EOF
+
+ p4 integrate -b branch2 &&
+ p4 submit -d "integrate main to branch2" &&
+
+ echo f7 >branch2/f7 &&
+ p4 add branch2/f7 &&
+ p4 submit -d "branch2/f7" &&
+
+ echo f8 >main/f8 &&
+ p4 add main/f8 &&
+ p4 submit -d "main/f8"
+ )
+'
+
+test_expect_success 'import main, no branch detection' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot/main@all &&
+ (
+ cd "$git" &&
+ git log --oneline --graph --decorate --all &&
+ git rev-list master >wc &&
+ test_line_count = 4 wc
+ )
+'
+
+test_expect_success 'import branch1, no branch detection' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot/branch1@all &&
+ (
+ cd "$git" &&
+ git log --oneline --graph --decorate --all &&
+ git rev-list master >wc &&
+ test_line_count = 2 wc
+ )
+'
+
+test_expect_success 'import branch2, no branch detection' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot/branch2@all &&
+ (
+ cd "$git" &&
+ git log --oneline --graph --decorate --all &&
+ git rev-list master >wc &&
+ test_line_count = 2 wc
+ )
+'
+
+test_expect_success 'import depot, no branch detection' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+ git log --oneline --graph --decorate --all &&
+ git rev-list master >wc &&
+ test_line_count = 8 wc
+ )
+'
+
+test_expect_success 'import depot, branch detection' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" --detect-branches //depot@all &&
+ (
+ cd "$git" &&
+
+ git log --oneline --graph --decorate --all &&
+
+ # 4 main commits
+ git rev-list master >wc &&
+ test_line_count = 4 wc &&
+
+ # 3 main, 1 integrate, 1 on branch2
+ git rev-list p4/depot/branch2 >wc &&
+ test_line_count = 5 wc &&
+
+ # no branch1, since no p4 branch created for it
+ test_must_fail git show-ref p4/depot/branch1
+ )
+'
+
+test_expect_success 'import depot, branch detection, branchList branch definition' '
+ test_when_finished cleanup_git &&
+ test_create_repo "$git" &&
+ (
+ cd "$git" &&
+ git config git-p4.branchList main:branch1 &&
+ "$GITP4" clone --dest=. --detect-branches //depot@all &&
+
+ git log --oneline --graph --decorate --all &&
+
+ # 4 main commits
+ git rev-list master >wc &&
+ test_line_count = 4 wc &&
+
+ # 3 main, 1 integrate, 1 on branch2
+ git rev-list p4/depot/branch2 >wc &&
+ test_line_count = 5 wc &&
+
+ # 2 main, 1 integrate, 1 on branch1
+ git rev-list p4/depot/branch1 >wc &&
+ test_line_count = 4 wc
+ )
+'
+
+test_expect_success 'restart p4d' '
+ kill_p4d &&
+ start_p4d
+'
+
+#
+# 1: //depot/branch1/file1
+# //depot/branch1/file2
+# 2: integrate //depot/branch1/... -> //depot/branch2/...
+# 3: //depot/branch1/file3
+# 4: //depot/branch1/file2 (edit)
+# 5: integrate //depot/branch1/... -> //depot/branch3/...
+#
+## Create a simple branch structure in P4 depot.
+test_expect_success 'add simple p4 branches' '
+ (
+ cd "$cli" &&
+ mkdir branch1 &&
+ cd branch1 &&
+ echo file1 >file1 &&
+ echo file2 >file2 &&
+ p4 add file1 file2 &&
+ p4 submit -d "branch1" &&
+ p4 integrate //depot/branch1/... //depot/branch2/... &&
+ p4 submit -d "branch2" &&
+ echo file3 >file3 &&
+ p4 add file3 &&
+ p4 submit -d "add file3 in branch1" &&
+ p4 open file2 &&
+ echo update >>file2 &&
+ p4 submit -d "update file2 in branch1" &&
+ p4 integrate //depot/branch1/... //depot/branch3/... &&
+ p4 submit -d "branch3"
+ )
+'
+
+# Configure branches through git-config and clone them.
+# All files are tested to make sure branches were cloned correctly.
+# Finally, make an update to branch1 on P4 side to check if it is imported
+# correctly by git-p4.
+test_expect_success 'git-p4 clone simple branches' '
+ test_when_finished cleanup_git &&
+ test_create_repo "$git" &&
+ (
+ cd "$git" &&
+ git config git-p4.branchList branch1:branch2 &&
+ git config --add git-p4.branchList branch1:branch3 &&
+ "$GITP4" clone --dest=. --detect-branches //depot@all &&
+ git log --all --graph --decorate --stat &&
+ git reset --hard p4/depot/branch1 &&
+ test -f file1 &&
+ test -f file2 &&
+ test -f file3 &&
+ grep -q update file2 &&
+ git reset --hard p4/depot/branch2 &&
+ test -f file1 &&
+ test -f file2 &&
+ test ! -f file3 &&
+ test_must_fail grep -q update file2 &&
+ git reset --hard p4/depot/branch3 &&
+ test -f file1 &&
+ test -f file2 &&
+ test -f file3 &&
+ grep -q update file2 &&
+ cd "$cli" &&
+ cd branch1 &&
+ p4 edit file2 &&
+ echo file2_ >>file2 &&
+ p4 submit -d "update file2 in branch3" &&
+ cd "$git" &&
+ git reset --hard p4/depot/branch1 &&
+ "$GITP4" rebase &&
+ grep -q file2_ file2
+ )
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 2/6] git-p4: handle utf16 filetype properly
From: Pete Wyckoff @ 2011-10-16 22:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Luke Diamand, Chris Li, Stefano Lattarini
In-Reply-To: <7vaa90reqm.fsf@alter.siamese.dyndns.org>
gitster@pobox.com wrote on Sun, 16 Oct 2011 11:08 -0700:
> Pete Wyckoff <pw@padd.com> writes:
> > + python "$TRASH_DIRECTORY/k_smush.py" <"$cli/k-text-k" >cli-k-text-k-smush &&
>
> Shouldn't this pay attention to PYTHON_PATH in any way?
Yes. I did not notice that the configuration variable existed.
I'll respond to 1/6 to fix that one too. Below is the new 2/6,
please. Includes the extra \n that Stefano noticed too.
-- Pete
--------------------8<--------------------------
From f177a75182576251ae4c895861d1e0f1f92794fc Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd.com>
Date: Sat, 17 Sep 2011 19:16:14 -0400
Subject: [PATCH 2/6] git-p4: handle utf16 filetype properly
One of the filetypes that p4 supports is utf16. Its behavior is
odd in this case. The data delivered through "p4 -G print" is
not encoded in utf16, although "p4 print -o" will produce the
proper utf16-encoded file.
When dealing with this filetype, discard the data from -G, and
instead read the contents directly.
An alternate approach would be to try to encode the data in
python. That worked for true utf16 files, but for other files
marked as utf16, p4 delivers mangled text in no recognizable encoding.
Add a test case to check utf16 handling, and +k and +ko handling.
Reported-by: Chris Li <git@chrisli.org>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
contrib/fast-import/git-p4 | 11 +++++
t/t9802-git-p4-filetype.sh | 108 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 119 insertions(+), 0 deletions(-)
create mode 100755 t/t9802-git-p4-filetype.sh
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2f7b270..e69caf3 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1238,6 +1238,15 @@ class P4Sync(Command, P4UserMap):
data = ''.join(contents)
contents = [data[:-1]]
+ if file['type'].startswith("utf16"):
+ # p4 delivers different text in the python output to -G
+ # than it does when using "print -o", or normal p4 client
+ # operations. utf16 is converted to ascii or utf8, perhaps.
+ # But ascii text saved as -t utf16 is completely mangled.
+ # Invoke print -o to get the real contents.
+ text = p4_read_pipe('print -q -o - "%s"' % file['depotFile'])
+ contents = [ text ]
+
if self.isWindows and file["type"].endswith("text"):
mangled = []
for data in contents:
@@ -1245,6 +1254,8 @@ class P4Sync(Command, P4UserMap):
mangled.append(data)
contents = mangled
+ # Note that we do not try to de-mangle keywords on utf16 files,
+ # even though in theory somebody may want that.
if file['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
contents = map(lambda text: re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text), contents)
elif file['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
new file mode 100755
index 0000000..3b358ef
--- /dev/null
+++ b/t/t9802-git-p4-filetype.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+test_description='git-p4 p4 filetype tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'utf-16 file create' '
+ (
+ cd "$cli" &&
+
+ # p4 saves this verbatim
+ printf "three\nline\ntext\n" >f-ascii &&
+ p4 add -t text f-ascii &&
+
+ # p4 adds \377\376 header
+ cp f-ascii f-ascii-as-utf16 &&
+ p4 add -t utf16 f-ascii-as-utf16 &&
+
+ # p4 saves this exactly as iconv produced it
+ printf "three\nline\ntext\n" | iconv -f ascii -t utf-16 >f-utf16 &&
+ p4 add -t utf16 f-utf16 &&
+
+ # this also is unchanged
+ cp f-utf16 f-utf16-as-text &&
+ p4 add -t text f-utf16-as-text &&
+
+ p4 submit -d "f files" &&
+
+ # force update of client files
+ p4 sync -f
+ )
+'
+
+test_expect_success 'utf-16 file test' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+
+ test_cmp "$cli/f-ascii" f-ascii &&
+ test_cmp "$cli/f-ascii-as-utf16" f-ascii-as-utf16 &&
+ test_cmp "$cli/f-utf16" f-utf16 &&
+ test_cmp "$cli/f-utf16-as-text" f-utf16-as-text
+ )
+'
+
+test_expect_success 'keyword file create' '
+ (
+ cd "$cli" &&
+
+ printf "id\n\$Id\$\n\$Author\$\ntext\n" >k-text-k &&
+ p4 add -t text+k k-text-k &&
+
+ cp k-text-k k-text-ko &&
+ p4 add -t text+ko k-text-ko &&
+
+ cat k-text-k | iconv -f ascii -t utf-16 >k-utf16-k &&
+ p4 add -t utf16+k k-utf16-k &&
+
+ cp k-utf16-k k-utf16-ko &&
+ p4 add -t utf16+ko k-utf16-ko &&
+
+ p4 submit -d "k files" &&
+ p4 sync -f
+ )
+'
+
+build_smush() {
+ cat >k_smush.py <<-\EOF &&
+ import re, sys
+ sys.stdout.write(re.sub(r'(?i)\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', sys.stdin.read()))
+ EOF
+ cat >ko_smush.py <<-\EOF
+ import re, sys
+ sys.stdout.write(re.sub(r'(?i)\$(Id|Header):[^$]*\$', r'$\1$', sys.stdin.read()))
+ EOF
+}
+
+test_expect_success 'keyword file test' '
+ build_smush &&
+ test_when_finished rm -f k_smush.py ko_smush.py &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+
+ # text, ensure unexpanded
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/k_smush.py" <"$cli/k-text-k" >cli-k-text-k-smush &&
+ test_cmp cli-k-text-k-smush k-text-k &&
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/ko_smush.py" <"$cli/k-text-ko" >cli-k-text-ko-smush &&
+ test_cmp cli-k-text-ko-smush k-text-ko &&
+
+ # utf16, even though p4 expands keywords, git-p4 does not
+ # try to undo that
+ test_cmp "$cli/k-utf16-k" k-utf16-k &&
+ test_cmp "$cli/k-utf16-ko" k-utf16-ko
+ )
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 2/4] git-gui: add smart case search mode in searchbar
From: Andrew Ardill @ 2011-10-16 22:32 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Pat Thoyts, git
In-Reply-To: <9350c86dc58e6345b237de5af186718d97fdd19b.1318579823.git.bert.wesarg@googlemail.com>
On 14 October 2011 19:14, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Setting config gui.search.smartcase to true, the search mode in the
> searchbar (from the blame view) is by default case-insensitive. But
> entering an upper case letter into the search field activates the case-
> sensitive search mode.
>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
> lib/search.tcl | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/lib/search.tcl b/lib/search.tcl
> index ef3486f..461c66d 100644
> --- a/lib/search.tcl
> +++ b/lib/search.tcl
> @@ -7,7 +7,8 @@ field w
> field ctext
>
> field searchstring {}
> -field casesensitive 1
> +field casesensitive
> +field default_casesensitive
> field searchdirn -forwards
>
> field smarktop
> @@ -18,6 +19,12 @@ constructor new {i_w i_text args} {
> set w $i_w
> set ctext $i_text
>
> + if {[is_config_true gui.search.smartcase]} {
> + set default_casesensitive 0
> + } else {
> + set default_casesensitive 1
> + }
> +
> ${NS}::frame $w
> ${NS}::label $w.l -text [mc Find:]
> entry $w.ent -textvariable ${__this}::searchstring -background lightgreen
> @@ -45,6 +52,7 @@ constructor new {i_w i_text args} {
> method show {} {
> if {![visible $this]} {
> grid $w
> + set casesensitive $default_casesensitive
> }
> focus -force $w.ent
> }
> @@ -125,6 +133,9 @@ method _incrsearch {} {
> if {[catch {$ctext index anchor}]} {
> $ctext mark set anchor [_get_new_anchor $this]
> }
> + if {[regexp {[[:upper:]]} $searchstring]} {
> + set casesensitive 1
> + }
> if {$searchstring ne {}} {
> set here [_do_search $this anchor mlen]
> if {$here ne {}} {
> --
> 1.7.6.789.gb4599
>
I don't really know tcl so I'm not certain, but it looks like you
never reset the case sensitive flag once it has been set by entering
an upper case letter. If I accidentally enter an upper case letter and
have set smartcase true, I would expect that deleting that character
would turn case sensitivity off again.
Regards,
Andrew Ardill
^ permalink raw reply
* [PATCH 2/2] daemon: report permission denied error to clients
From: Clemens Buchacher @ 2011-10-16 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1318803076-4229-1-git-send-email-drizzd@aon.at>
If passed an inaccessible url, git daemon returns the
following error:
$ git clone git://host/repo
fatal: remote error: no such repository: /repo
In case of a permission denied error, return the following
instead:
fatal: remote error: permission denied: /repo
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
daemon.c | 32 +++++++++++++++++++++-----------
path.c | 31 +++++++++++++++++++++----------
t/t5570-git-daemon.sh | 2 +-
3 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/daemon.c b/daemon.c
index 72fb53a..1442b5b 100644
--- a/daemon.c
+++ b/daemon.c
@@ -109,7 +109,7 @@ static void NORETURN daemon_die(const char *err, va_list params)
exit(1);
}
-static char *path_ok(char *directory)
+static int path_ok(char *directory, const char **return_path)
{
static char rpath[PATH_MAX];
static char interp_path[PATH_MAX];
@@ -120,13 +120,13 @@ static char *path_ok(char *directory)
if (daemon_avoid_alias(dir)) {
logerror("'%s': aliased", dir);
- return NULL;
+ return -1;
}
if (*dir == '~') {
if (!user_path) {
logerror("'%s': User-path not allowed", dir);
- return NULL;
+ return EACCES;
}
if (*user_path) {
/* Got either "~alice" or "~alice/foo";
@@ -158,7 +158,7 @@ static char *path_ok(char *directory)
if (*dir != '/') {
/* Allow only absolute */
logerror("'%s': Non-absolute path denied (interpolated-path active)", dir);
- return NULL;
+ return EACCES;
}
strbuf_expand(&expanded_path, interpolated_path,
@@ -173,7 +173,7 @@ static char *path_ok(char *directory)
if (*dir != '/') {
/* Allow only absolute */
logerror("'%s': Non-absolute path denied (base-path active)", dir);
- return NULL;
+ return EACCES;
}
snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
dir = rpath;
@@ -190,10 +190,14 @@ static char *path_ok(char *directory)
}
if (!path) {
+ int ret = -1;
+ if (errno == EACCES)
+ ret = EACCES;
logerror("'%s' does not appear to be a git repository", dir);
- return NULL;
+ return ret;
}
+ *return_path = path;
if ( ok_paths && *ok_paths ) {
char **pp;
int pathlen = strlen(path);
@@ -211,17 +215,17 @@ static char *path_ok(char *directory)
!memcmp(*pp, path, len) &&
(path[len] == '\0' ||
(!strict_paths && path[len] == '/')))
- return path;
+ return 0;
}
}
else {
/* be backwards compatible */
if (!strict_paths)
- return path;
+ return 0;
}
logerror("'%s': not in whitelist", path);
- return NULL; /* Fallthrough. Deny by default */
+ return EACCES; /* Fallthrough. Deny by default */
}
typedef int (*daemon_service_fn)(void);
@@ -258,6 +262,7 @@ static int daemon_error(const char *dir, const char *msg)
static int run_service(char *dir, struct daemon_service *service)
{
+ int err;
const char *path;
int enabled = service->enabled;
@@ -269,8 +274,13 @@ static int run_service(char *dir, struct daemon_service *service)
return daemon_error(dir, "service not enabled");
}
- if (!(path = path_ok(dir)))
- return daemon_error(dir, "no such repository");
+ err = path_ok(dir, &path);
+ if (err) {
+ if (err == EACCES)
+ return daemon_error(dir, "permission denied");
+ else
+ return daemon_error(dir, "no such repository");
+ }
/*
* Security on the cheap.
diff --git a/path.c b/path.c
index 6f3f5d5..227d8d7 100644
--- a/path.c
+++ b/path.c
@@ -288,6 +288,7 @@ char *enter_repo(char *path, int strict)
static char used_path[PATH_MAX];
static char validated_path[PATH_MAX];
+ errno = 0;
if (!path)
return NULL;
@@ -301,12 +302,15 @@ char *enter_repo(char *path, int strict)
path[len-1] = 0;
len--;
}
- if (PATH_MAX <= len)
+ if (PATH_MAX <= len) {
+ errno = ENAMETOOLONG;
return NULL;
+ }
if (path[0] == '~') {
char *newpath = expand_user_path(path);
if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
free(newpath);
+ errno = 0;
return NULL;
}
/*
@@ -319,9 +323,10 @@ char *enter_repo(char *path, int strict)
strcpy(validated_path, path);
path = used_path;
}
- else if (PATH_MAX - 10 < len)
+ else if (PATH_MAX - 10 < len) {
+ errno = ENAMETOOLONG;
return NULL;
- else {
+ } else {
path = strcpy(used_path, path);
strcpy(validated_path, path);
}
@@ -331,23 +336,29 @@ char *enter_repo(char *path, int strict)
if (!access(path, F_OK)) {
strcat(validated_path, suffix[i]);
break;
+ } else if (errno == EACCES) {
+ return NULL;
}
}
- if (!suffix[i] || chdir(path))
+ if (!suffix[i])
+ return NULL;
+ if (chdir(path))
return NULL;
path = validated_path;
}
else if (chdir(path))
return NULL;
- if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
- validate_headref("HEAD") == 0) {
- set_git_dir(".");
- check_repository_format();
- return path;
+ if (access("objects", X_OK) || access("refs", X_OK))
+ return NULL;
+ if (validate_headref("HEAD")) {
+ errno = 0;
+ return NULL;
}
- return NULL;
+ set_git_dir(".");
+ check_repository_format();
+ return path;
}
int set_shared_perm(const char *path, int mode)
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index aa5771a..e6482eb 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -141,7 +141,7 @@ start_daemon --informative-errors
test_expect_success 'clone non-existent' "test_remote_error clone nowhere.git 'no such repository'"
test_expect_success 'push disabled' "test_remote_error push repo.git 'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git 'no such repository'"
+test_expect_success 'read access denied' "test_remote_error -x fetch repo.git 'permission denied'"
test_expect_success 'not exported' "test_remote_error -n fetch repo.git 'repository not exported'"
stop_daemon
--
1.7.7
^ permalink raw reply related
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