* Re: [JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache
From: Robin Rosenberg @ 2009-09-03 23:14 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Shawn O. Pearce
In-Reply-To: <1251847010-9992-3-git-send-email-spearce@spearce.org>
onsdag 02 september 2009 01:16:50 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> If the cache has no entries, we want to return -1 rather than throw
> ArrayIndexOutOfBoundsException. This binary search loop was stolen
> from some other code which contained a test before the loop to see if
> the collection was empty or not, but we failed to include that here.
>
> Flipping the loop around to a standard while loop ensures we test
> the condition properly first.
>
> Signed-off-by: Shawn O. Pearce <sop@google.com>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Is this a new policy?
-- robin
^ permalink raw reply
* Re: [JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache
From: Shawn O. Pearce @ 2009-09-03 23:19 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200909040114.12980.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Signed-off-by: Shawn O. Pearce <sop@google.com>
> > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
>
> Is this a new policy?
Call it a new habit. I send from my @spearce.org because that's my
"public identity", but this was on work time, so I added an extra
SOB line to indicate that yes, my employer is also OK with this.
Not that anyone probably doubted that in the first place though...
I work for a company that is pretty friendly towards open source.
--
Shawn.
^ permalink raw reply
* Re: [PATCHv2] git-config: Parse config files leniently
From: Junio C Hamano @ 2009-09-03 23:42 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4A9F733D.5050205@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>> Why was I CC'ed, if the patch wasn't even self tested?
>
> Because
> - not CC'ing you would have meant culling you from the existing CC,
> - we've discussed v1 of this patch before,
> - I asked in this patch (v2) whether to go for an alternative.
Oh, so you did not mean this was for inclusion but as another round
of RFC. I misread your intent. Sorry about that.
The eralier analysis of the cause of the breakage indicated that the
implementation in this patch was flawed. What it essentially did was to
re-define all die() to warn() in the codepaths around configuration
variable handling [*1*].
However, it does not mean that the idea of "ignoring syntax errors while
keeping other errors still noticed for all commands, not limited to config
nor not limited to 'config -e'" is necessarily flawed.
For example, the test I noticed the breakage with was stuffing an invalid
value to branch.autosetuprebase and wanted to see "git branch" fail.
Obviously we do want to fail a "git branch newbranch origin/master" when
the value given to branch.autosetuprebase is misspelled, to avoid creating
bogus settings the user did not intend to. We do care about semantic
errors (i.e. this variable can take only one of these values, but the
value given in the file is bogus) in such a case. But if you are running
"git branch" only to view, but not to create, there is no reason for us to
care about the branch.autosetuprebase variable [*2*].
This observation suggests that it may make sense to make the error
handling _even looser_ than what you intended to do in your patch (which I
assume was "we ignore syntax errors and try to recover, pretending that we
saw a comment until the end of line, but we still keep the validation of
values assigned to variables that the existing commands rely on").
Ideally, the rule would be "we care about the values of variables we are
going to use, but allow misspelled values in variables that are not used
by us---we have no business complaining about them." Unfortunately that
is much harder to arrange with the current code structure, but under that
rule, "config -e" would care only about "core.editor" and nothing else, so
as long as that variable can be sanely read, it should be able to start.
[Footnotes]
*1* The additional test that came with the patch only checked for the
positive case (i.e. "does the system with this patch treats errors looser
than before?"), but failed to check the negative case (i.e. "does the
change too much and stop catching errors that should be caught?"), which
unfortunately is a common mistake that easily lets bugs go unnoticed.
*2* Worse yet, the parsing of branch.autosetuprebase is part of the
default_config and commands that do not have anything to do with new
branch creation will fail with the current setup.
^ permalink raw reply
* Re: [JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache
From: Nicolas Pitre @ 2009-09-04 0:06 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Robin Rosenberg, git
In-Reply-To: <20090903231910.GN1033@spearce.org>
On Thu, 3 Sep 2009, Shawn O. Pearce wrote:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > > Signed-off-by: Shawn O. Pearce <sop@google.com>
> > > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> >
> > Is this a new policy?
>
> Call it a new habit. I send from my @spearce.org because that's my
> "public identity", but this was on work time, so I added an extra
> SOB line to indicate that yes, my employer is also OK with this.
You might as well simply use your Google SOB alone, even if your From is
@spearce.org. That's what I do with my Linux patches done on $day_job
time.
Nicolas
^ permalink raw reply
* [PATCH] pack-objects: free preferred base memory after usage
From: Nicolas Pitre @ 2009-09-04 1:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When adding objects for preferred delta base, the content from tree
objects leading to given paths is kept in a cache. This has the
potential to grow significantly, especially with large directories as
the whole tree object content is loaded in memory, even if in practice
the number of those objects is limited to the 256 cache entries plus the
$window root tree objects. Still, that can't hurt freeing that up after
object enumeration is done, and before more memory is needed for delta
search.
Signed-off-by: Nicolas Pitre <nico@cam.org>
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 1ff2ce6..ae7ce0f 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1008,6 +1008,33 @@ static void add_preferred_base(unsigned char *sha1)
it->pcache.tree_size = size;
}
+static void cleanup_preferred_base(void)
+{
+ struct pbase_tree *it;
+ unsigned i;
+
+ it = pbase_tree;
+ pbase_tree = NULL;
+ while (it) {
+ struct pbase_tree *this = it;
+ it = this->next;
+ free(this->pcache.tree_data);
+ free(this);
+ }
+
+ for (i = 0; i < ARRAY_SIZE(pbase_tree_cache); i++) {
+ if (!pbase_tree_cache[i])
+ continue;
+ free(pbase_tree_cache[i]->tree_data);
+ free(pbase_tree_cache[i]);
+ pbase_tree_cache[i] = NULL;
+ }
+
+ free(done_pbase_paths);
+ done_pbase_paths = NULL;
+ done_pbase_paths_num = done_pbase_paths_alloc = 0;
+}
+
static void check_object(struct object_entry *entry)
{
if (entry->in_pack) {
@@ -2312,6 +2339,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
rp_av[rp_ac] = NULL;
get_object_list(rp_ac, rp_av);
}
+ cleanup_preferred_base();
if (include_tag && nr_result)
for_each_ref(add_ref_tag, NULL);
stop_progress(&progress_state);
^ permalink raw reply related
* [PATCH 0/8] VCS helpers
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This is the next version of the db/vcs-helper series in pu.
The first patch is new, a rework of the remote-curl build to produce
"remote-curl" and call it as a special case for the sorts of URLs that
we accept as indicating something that it now handled by this helper.
The series is rebased onto current next, with some conflicts resolved.
Two patches have been dropped: a memory leak fix for code that was
removed entirely by the first patch, and the "mark" helper capability,
which is not needed (I believe) due to the "option" fast-import command.
Daniel Barkalow (8):
Make the "traditionally-supported" URLs a special case
Use a clearer style to issue commands to remote helpers
Use a function to determine whether a remote is valid
Allow fetch to modify refs
Allow programs to not depend on remotes having urls
Add a config option for remotes to specify a foreign vcs
Add support for "import" helper command
Allow helpers to report in "list" command that the ref is unchanged
Documentation/config.txt | 4 +
Documentation/git-remote-helpers.txt | 14 ++++-
Makefile | 17 +-----
builtin-clone.c | 6 +-
builtin-fetch.c | 7 ++-
builtin-ls-remote.c | 4 +-
builtin-push.c | 67 +++++++++++++--------
remote.c | 15 ++++-
remote.h | 2 +
transport-helper.c | 107 +++++++++++++++++++++++++++++-----
transport.c | 23 +++++--
transport.h | 43 +++++++++++++-
12 files changed, 234 insertions(+), 75 deletions(-)
^ permalink raw reply
* [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Instead of trying to make http://, https://, and ftp:// URLs
indicative of some sort of pattern of transport helper usage, make
them a special case which runs the "curl" helper, and leave the
mechanism by which arbitrary helpers will be chosen entirely to future
work.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Makefile | 17 ++---------------
transport-helper.c | 7 ++-----
transport.c | 2 +-
transport.h | 2 +-
4 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/Makefile b/Makefile
index 263e191..3ac12ec 100644
--- a/Makefile
+++ b/Makefile
@@ -972,8 +972,7 @@ else
else
CURL_LIBCURL = -lcurl
endif
- CURL_SYNONYMS = git-remote-https$X git-remote-ftp$X
- PROGRAMS += git-remote-http$X $(CURL_SYNONYMS) git-http-fetch$X
+ PROGRAMS += git-remote-curl$X git-http-fetch$X
curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
ifeq "$(curl_check)" "070908"
ifndef NO_EXPAT
@@ -1483,16 +1482,10 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
-git-remote-http$X: remote-curl.o http.o http-walker.o $(GITLIBS)
+git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
-$(CURL_SYNONYMS): git-remote-http$X
- $(QUIET_LNCP)$(RM) $@ && \
- ln $< $@ 2>/dev/null || \
- ln -s $< $@ 2>/dev/null || \
- cp $< $@
-
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
builtin-revert.o wt-status.o: wt-status.h
@@ -1674,12 +1667,6 @@ endif
ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
cp "$$execdir/git$X" "$$execdir/$$p" || exit; \
done; } && \
- { for p in $(CURL_SYNONYMS); do \
- $(RM) "$$execdir/$$p" && \
- ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
- ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
- cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; \
- done; } && \
./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
install-doc:
diff --git a/transport-helper.c b/transport-helper.c
index 43fdc0a..4684877 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -152,13 +152,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
return ret;
}
-int transport_helper_init(struct transport *transport)
+int transport_helper_init(struct transport *transport, const char *name)
{
struct helper_data *data = xcalloc(sizeof(*data), 1);
- char *eom = strchr(transport->url, ':');
- if (!eom)
- return -1;
- data->name = xstrndup(transport->url, eom - transport->url);
+ data->name = name;
transport->data = data;
transport->get_refs_list = get_refs_list;
diff --git a/transport.c b/transport.c
index f2bd998..4cb8077 100644
--- a/transport.c
+++ b/transport.c
@@ -823,7 +823,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
} else if (!prefixcmp(url, "http://")
|| !prefixcmp(url, "https://")
|| !prefixcmp(url, "ftp://")) {
- transport_helper_init(ret);
+ transport_helper_init(ret, "curl");
#ifdef NO_CURL
error("git was compiled without libcurl support.");
#else
diff --git a/transport.h b/transport.h
index bfd542f..c14da6f 100644
--- a/transport.h
+++ b/transport.h
@@ -80,6 +80,6 @@ int transport_disconnect(struct transport *transport);
char *transport_anonymize_url(const char *url);
/* Transport methods defined outside transport.c */
-int transport_helper_init(struct transport *transport);
+int transport_helper_init(struct transport *transport, const char *name);
#endif
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 2/8] Use a clearer style to issue commands to remote helpers
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This style is overkill for some commands, but it's worthwhile to use
the same style to issue all commands, and it's useful to avoid
open-coding string lengths.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
transport-helper.c | 21 ++++++++++++++-------
1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 4684877..b1ea7e6 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)
die("Unable to run helper: git %s", helper->argv[0]);
data->helper = helper;
- write_in_full(data->helper->in, "capabilities\n", 13);
+ strbuf_addstr(&buf, "capabilities\n");
+ write_in_full(helper->in, buf.buf, buf.len);
+ strbuf_reset(&buf);
+
file = fdopen(helper->out, "r");
while (1) {
if (strbuf_getline(&buf, file, '\n') == EOF)
@@ -78,11 +81,12 @@ static int fetch_with_fetch(struct transport *transport,
const struct ref *posn = to_fetch[i];
if (posn->status & REF_STATUS_UPTODATE)
continue;
- write_in_full(helper->in, "fetch ", 6);
- write_in_full(helper->in, sha1_to_hex(posn->old_sha1), 40);
- write_in_full(helper->in, " ", 1);
- write_in_full(helper->in, posn->name, strlen(posn->name));
- write_in_full(helper->in, "\n", 1);
+
+ strbuf_addf(&buf, "fetch %s %s\n",
+ sha1_to_hex(posn->old_sha1), posn->name);
+ write_in_full(helper->in, buf.buf, buf.len);
+ strbuf_reset(&buf);
+
if (strbuf_getline(&buf, file, '\n') == EOF)
exit(128); /* child died, message supplied already */
}
@@ -119,7 +123,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
FILE *file;
helper = get_helper(transport);
- write_in_full(helper->in, "list\n", 5);
+
+ strbuf_addstr(&buf, "list\n");
+ write_in_full(helper->in, buf.buf, buf.len);
+ strbuf_reset(&buf);
file = fdopen(helper->out, "r");
while (1) {
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 3/8] Use a function to determine whether a remote is valid
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Currently, it only checks url, but it will allow other things in the future.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
remote.c | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/remote.c b/remote.c
index 4b5b905..fec63fa 100644
--- a/remote.c
+++ b/remote.c
@@ -48,6 +48,11 @@ static int rewrite_nr;
#define BUF_SIZE (2048)
static char buffer[BUF_SIZE];
+static int valid_remote(const struct remote *remote)
+{
+ return !!remote->url;
+}
+
static const char *alias_url(const char *url)
{
int i, j;
@@ -667,14 +672,14 @@ struct remote *remote_get(const char *name)
ret = make_remote(name, 0);
if (valid_remote_nick(name)) {
- if (!ret->url)
+ if (!valid_remote(ret))
read_remotes_file(ret);
- if (!ret->url)
+ if (!valid_remote(ret))
read_branches_file(ret);
}
- if (name_given && !ret->url)
+ if (name_given && !valid_remote(ret))
add_url_alias(ret, name);
- if (!ret->url)
+ if (!valid_remote(ret))
return NULL;
ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr, ret->fetch_refspec);
ret->push = parse_push_refspec(ret->push_refspec_nr, ret->push_refspec);
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 4/8] Allow fetch to modify refs
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This allows the transport to use the null sha1 for a ref reported to
be present in the remote repository to indicate that a ref exists but
its actual value is presently unknown and will be set if the objects
are fetched.
Also adds documentation to the API to specify exactly what the methods
should do and how they should interpret arguments.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
builtin-clone.c | 6 ++++--
transport-helper.c | 4 ++--
transport.c | 13 +++++++------
transport.h | 41 +++++++++++++++++++++++++++++++++++++++--
4 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index ad04808..deef435 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -520,8 +520,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_upload_pack);
refs = transport_get_remote_refs(transport);
- if (refs)
- transport_fetch_refs(transport, refs);
+ if (refs) {
+ struct ref *ref_cpy = copy_ref_list(refs);
+ transport_fetch_refs(transport, ref_cpy);
+ }
}
if (refs) {
diff --git a/transport-helper.c b/transport-helper.c
index b1ea7e6..e2b5270 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -70,7 +70,7 @@ static int disconnect_helper(struct transport *transport)
}
static int fetch_with_fetch(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct child_process *helper = get_helper(transport);
FILE *file = fdopen(helper->out, "r");
@@ -94,7 +94,7 @@ static int fetch_with_fetch(struct transport *transport,
}
static int fetch(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct helper_data *data = transport->data;
int i, count;
diff --git a/transport.c b/transport.c
index 4cb8077..93430fa 100644
--- a/transport.c
+++ b/transport.c
@@ -204,7 +204,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)
}
static int fetch_objs_via_rsync(struct transport *transport,
- int nr_objs, const struct ref **to_fetch)
+ int nr_objs, struct ref **to_fetch)
{
struct strbuf buf = STRBUF_INIT;
struct child_process rsync;
@@ -408,7 +408,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
}
static int fetch_refs_from_bundle(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct bundle_transport_data *data = transport->data;
return unbundle(&data->header, data->fd);
@@ -486,7 +486,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
}
static int fetch_refs_via_pack(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct git_transport_data *data = transport->data;
char **heads = xmalloc(nr_heads * sizeof(*heads));
@@ -922,16 +922,17 @@ const struct ref *transport_get_remote_refs(struct transport *transport)
return transport->remote_refs;
}
-int transport_fetch_refs(struct transport *transport, const struct ref *refs)
+int transport_fetch_refs(struct transport *transport, struct ref *refs)
{
int rc;
int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
- const struct ref **heads = NULL;
- const struct ref *rm;
+ struct ref **heads = NULL;
+ struct ref *rm;
for (rm = refs; rm; rm = rm->next) {
nr_refs++;
if (rm->peer_ref &&
+ !is_null_sha1(rm->old_sha1) &&
!hashcmp(rm->peer_ref->old_sha1, rm->old_sha1))
continue;
ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
diff --git a/transport.h b/transport.h
index c14da6f..503db11 100644
--- a/transport.h
+++ b/transport.h
@@ -18,11 +18,48 @@ struct transport {
int (*set_option)(struct transport *connection, const char *name,
const char *value);
+ /**
+ * Returns a list of the remote side's refs. In order to allow
+ * the transport to try to share connections, for_push is a
+ * hint as to whether the ultimate operation is a push or a fetch.
+ *
+ * If the transport is able to determine the remote hash for
+ * the ref without a huge amount of effort, it should store it
+ * in the ref's old_sha1 field; otherwise it should be all 0.
+ **/
struct ref *(*get_refs_list)(struct transport *transport, int for_push);
- int (*fetch)(struct transport *transport, int refs_nr, const struct ref **refs);
+
+ /**
+ * Fetch the objects for the given refs. Note that this gets
+ * an array, and should ignore the list structure.
+ *
+ * If the transport did not get hashes for refs in
+ * get_refs_list(), it should set the old_sha1 fields in the
+ * provided refs now.
+ **/
+ int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
+
+ /**
+ * Push the objects and refs. Send the necessary objects, and
+ * then, for any refs where peer_ref is set and
+ * peer_ref->new_sha1 is different from old_sha1, tell the
+ * remote side to update each ref in the list from old_sha1 to
+ * peer_ref->new_sha1.
+ *
+ * Where possible, set the status for each ref appropriately.
+ *
+ * The transport must modify new_sha1 in the ref to the new
+ * value if the remote accepted the change. Note that this
+ * could be a different value from peer_ref->new_sha1 if the
+ * process involved generating new commits.
+ **/
int (*push_refs)(struct transport *transport, struct ref *refs, int flags);
int (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);
+ /** get_refs_list(), fetch(), and push_refs() can keep
+ * resources (such as a connection) reserved for futher
+ * use. disconnect() releases these resources.
+ **/
int (*disconnect)(struct transport *connection);
char *pack_lockfile;
signed verbose : 2;
@@ -74,7 +111,7 @@ int transport_push(struct transport *connection,
const struct ref *transport_get_remote_refs(struct transport *transport);
-int transport_fetch_refs(struct transport *transport, const struct ref *refs);
+int transport_fetch_refs(struct transport *transport, struct ref *refs);
void transport_unlock_pack(struct transport *transport);
int transport_disconnect(struct transport *transport);
char *transport_anonymize_url(const char *url);
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 5/8] Allow programs to not depend on remotes having urls
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
For fetch and ls-remote, which use the first url of a remote, have
transport_get() determine this by passing a remote and passing NULL
for the url. For push, which uses every url of a remote, use each url
in turn if there are any, and use NULL if there are none.
This will allow the transport code to do something different if the
location is not specified with a url.
Also, have the message for a fetch say "foreign" if there is no url.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
builtin-fetch.c | 7 ++++-
builtin-ls-remote.c | 4 +-
builtin-push.c | 67 ++++++++++++++++++++++++++++++++-------------------
transport.c | 3 ++
4 files changed, 52 insertions(+), 29 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 817dd6b..63a4ff0 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -309,7 +309,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
if (!fp)
return error("cannot open %s: %s\n", filename, strerror(errno));
- url = transport_anonymize_url(raw_url);
+ if (raw_url)
+ url = transport_anonymize_url(raw_url);
+ else
+ url = xstrdup("foreign");
for (rm = ref_map; rm; rm = rm->next) {
struct ref *ref = NULL;
@@ -663,7 +666,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
if (!remote)
die("Where do you want to fetch from today?");
- transport = transport_get(remote, remote->url[0]);
+ transport = transport_get(remote, NULL);
if (verbosity >= 2)
transport->verbose = 1;
if (verbosity < 0)
diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..4c6fc58 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -87,9 +87,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
}
}
remote = nongit ? NULL : remote_get(dest);
- if (remote && !remote->url_nr)
+ if (!nongit && !remote)
die("remote %s has no configured URL", dest);
- transport = transport_get(remote, remote ? remote->url[0] : dest);
+ transport = transport_get(remote, remote ? NULL : dest);
if (uploadpack != NULL)
transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
diff --git a/builtin-push.c b/builtin-push.c
index 67f6d96..8e9b917 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -88,6 +88,34 @@ static void setup_default_push_refspecs(void)
}
}
+static int push_with_options(struct transport *transport, int flags)
+{
+ int err;
+ int nonfastforward;
+ if (receivepack)
+ transport_set_option(transport,
+ TRANS_OPT_RECEIVEPACK, receivepack);
+ if (thin)
+ transport_set_option(transport, TRANS_OPT_THIN, "yes");
+
+ if (flags & TRANSPORT_PUSH_VERBOSE)
+ fprintf(stderr, "Pushing to %s\n", transport->url);
+ err = transport_push(transport, refspec_nr, refspec, flags,
+ &nonfastforward);
+ err |= transport_disconnect(transport);
+
+ if (!err)
+ return 0;
+
+ if (nonfastforward) {
+ printf("To prevent you from losing history, non-fast-forward updates were rejected.\n"
+ "Merge the remote changes before pushing again.\n"
+ "See 'non-fast forward' section of 'git push --help' for details.\n");
+ }
+
+ return 1;
+}
+
static int do_push(const char *repo, int flags)
{
int i, errs;
@@ -136,33 +164,22 @@ static int do_push(const char *repo, int flags)
url = remote->url;
url_nr = remote->url_nr;
}
- for (i = 0; i < url_nr; i++) {
+ if (url_nr) {
+ for (i = 0; i < url_nr; i++) {
+ struct transport *transport =
+ transport_get(remote, url[i]);
+ if (push_with_options(transport, flags)) {
+ error("failed to push some refs to '%s'", url[i]);
+ errs++;
+ }
+ }
+ } else {
struct transport *transport =
- transport_get(remote, url[i]);
- int err;
- int nonfastforward;
- if (receivepack)
- transport_set_option(transport,
- TRANS_OPT_RECEIVEPACK, receivepack);
- if (thin)
- transport_set_option(transport, TRANS_OPT_THIN, "yes");
-
- if (flags & TRANSPORT_PUSH_VERBOSE)
- fprintf(stderr, "Pushing to %s\n", url[i]);
- err = transport_push(transport, refspec_nr, refspec, flags,
- &nonfastforward);
- err |= transport_disconnect(transport);
-
- if (!err)
- continue;
-
- error("failed to push some refs to '%s'", url[i]);
- if (nonfastforward) {
- printf("To prevent you from losing history, non-fast-forward updates were rejected.\n"
- "Merge the remote changes before pushing again.\n"
- "See 'non-fast forward' section of 'git push --help' for details.\n");
+ transport_get(remote, NULL);
+ if (push_with_options(transport, flags)) {
+ error("failed to push some refs to foreign system");
+ errs++;
}
- errs++;
}
return !!errs;
}
diff --git a/transport.c b/transport.c
index 93430fa..684fd6c 100644
--- a/transport.c
+++ b/transport.c
@@ -813,6 +813,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
struct transport *ret = xcalloc(1, sizeof(*ret));
ret->remote = remote;
+
+ if (!url && remote && remote->url)
+ url = remote->url[0];
ret->url = url;
if (!prefixcmp(url, "rsync:")) {
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 6/8] Add a config option for remotes to specify a foreign vcs
From: Daniel Barkalow @ 2009-09-04 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If this is set, the url is not required, and the transport always uses
a helper named "git-remote-<value>".
It is a separate configuration option in order to allow a sensible
configuration for foreign systems which either have no meaningful urls
for repositories or which require urls that do not specify the system
used by the repository at that location. However, this only affects
how the name of the helper is determined, not anything about the
interaction with the helper, and the contruction is such that, if the
foreign scm does happen to use a co-named url method, a url with that
method may be used directly.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Documentation/config.txt | 4 ++++
remote.c | 4 +++-
remote.h | 2 ++
transport.c | 5 +++++
4 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5256c7f..436ee91 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1380,6 +1380,10 @@ remote.<name>.tagopt::
Setting this value to \--no-tags disables automatic tag following when
fetching from remote <name>
+remote.<name>.vcs::
+ Setting this to a value <vcs> will cause git to interact with
+ the remote with the git-remote-<vcs> helper.
+
remotes.<group>::
The list of remotes which are fetched by "git remote update
<group>". See linkgit:git-remote[1].
diff --git a/remote.c b/remote.c
index fec63fa..e9dbb3b 100644
--- a/remote.c
+++ b/remote.c
@@ -50,7 +50,7 @@ static char buffer[BUF_SIZE];
static int valid_remote(const struct remote *remote)
{
- return !!remote->url;
+ return !remote->url != !remote->foreign_vcs;
}
static const char *alias_url(const char *url)
@@ -427,6 +427,8 @@ static int handle_config(const char *key, const char *value, void *cb)
} else if (!strcmp(subkey, ".proxy")) {
return git_config_string((const char **)&remote->http_proxy,
key, value);
+ } else if (!strcmp(subkey, ".vcs")) {
+ return git_config_string(&remote->foreign_vcs, key, value);
}
return 0;
}
diff --git a/remote.h b/remote.h
index 5db8420..ac0ce2f 100644
--- a/remote.h
+++ b/remote.h
@@ -11,6 +11,8 @@ struct remote {
const char *name;
int origin;
+ const char *foreign_vcs;
+
const char **url;
int url_nr;
int url_alloc;
diff --git a/transport.c b/transport.c
index 684fd6c..38bebe3 100644
--- a/transport.c
+++ b/transport.c
@@ -818,6 +818,11 @@ struct transport *transport_get(struct remote *remote, const char *url)
url = remote->url[0];
ret->url = url;
+ if (remote && remote->foreign_vcs) {
+ transport_helper_init(ret, remote->foreign_vcs);
+ return ret;
+ }
+
if (!prefixcmp(url, "rsync:")) {
ret->get_refs_list = get_refs_via_rsync;
ret->fetch = fetch_objs_via_rsync;
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 7/8] Add support for "import" helper command
From: Daniel Barkalow @ 2009-09-04 2:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This command, supported if the "import" capability is advertized,
allows a helper to support fetching by outputting a git-fast-import
stream.
If both "fetch" and "import" are advertized, git itself will use
"fetch" (although other users may use "import" in this case).
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Documentation/git-remote-helpers.txt | 10 ++++++
transport-helper.c | 53 ++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 173ee23..e9aa67e 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -43,6 +43,13 @@ Commands are given by the caller on the helper's standard input, one per line.
+
Supported if the helper has the "fetch" capability.
+'import' <name>::
+ Produces a fast-import stream which imports the current value
+ of the named ref. It may additionally import other refs as
+ needed to construct the history efficiently.
++
+Supported if the helper has the "import" capability.
+
If a fatal error occurs, the program writes the error message to
stderr and exits. The caller should expect that a suitable error
message has been printed if the child closes the connection without
@@ -57,6 +64,9 @@ CAPABILITIES
'fetch'::
This helper supports the 'fetch' command.
+'import'::
+ This helper supports the 'import' command.
+
REF LIST ATTRIBUTES
-------------------
diff --git a/transport-helper.c b/transport-helper.c
index e2b5270..3825441 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -11,6 +11,7 @@ struct helper_data
const char *name;
struct child_process *helper;
unsigned fetch : 1;
+ unsigned import : 1;
};
static struct child_process *get_helper(struct transport *transport)
@@ -50,6 +51,8 @@ static struct child_process *get_helper(struct transport *transport)
break;
if (!strcmp(buf.buf, "fetch"))
data->fetch = 1;
+ if (!strcmp(buf.buf, "import"))
+ data->import = 1;
}
return data->helper;
}
@@ -93,6 +96,53 @@ static int fetch_with_fetch(struct transport *transport,
return 0;
}
+static int get_importer(struct transport *transport, struct child_process *fastimport)
+{
+ struct helper_data *data = transport->data;
+ struct child_process *helper = get_helper(transport);
+ memset(fastimport, 0, sizeof(*fastimport));
+ fastimport->in = helper->out;
+ fastimport->argv = xcalloc(5, sizeof(*fastimport->argv));
+ fastimport->argv[0] = "fast-import";
+ fastimport->argv[1] = "--quiet";
+
+ fastimport->git_cmd = 1;
+ return start_command(fastimport);
+}
+
+static int fetch_with_import(struct transport *transport,
+ int nr_heads, struct ref **to_fetch)
+{
+ struct child_process fastimport;
+ struct child_process *helper = get_helper(transport);
+ int i;
+ struct ref *posn;
+ struct strbuf buf = STRBUF_INIT;
+
+ if (get_importer(transport, &fastimport))
+ die("Couldn't run fast-import");
+
+ for (i = 0; i < nr_heads; i++) {
+ posn = to_fetch[i];
+ if (posn->status & REF_STATUS_UPTODATE)
+ continue;
+
+ strbuf_addf(&buf, "import %s\n", posn->name);
+ write_in_full(helper->in, buf.buf, buf.len);
+ strbuf_reset(&buf);
+ }
+ disconnect_helper(transport);
+ finish_command(&fastimport);
+
+ for (i = 0; i < nr_heads; i++) {
+ posn = to_fetch[i];
+ if (posn->status & REF_STATUS_UPTODATE)
+ continue;
+ read_ref(posn->name, posn->old_sha1);
+ }
+ return 0;
+}
+
static int fetch(struct transport *transport,
int nr_heads, struct ref **to_fetch)
{
@@ -110,6 +160,9 @@ static int fetch(struct transport *transport,
if (data->fetch)
return fetch_with_fetch(transport, nr_heads, to_fetch);
+ if (data->import)
+ return fetch_with_import(transport, nr_heads, to_fetch);
+
return -1;
}
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* [PATCH 8/8] Allow helpers to report in "list" command that the ref is unchanged
From: Daniel Barkalow @ 2009-09-04 2:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Helpers may use a line like "? name unchanged" to specify that there
is nothing new at that name, without any git-specific code to
determine the correct response.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Documentation/git-remote-helpers.txt | 4 +++-
transport-helper.c | 22 ++++++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index e9aa67e..2c5130f 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -70,7 +70,9 @@ CAPABILITIES
REF LIST ATTRIBUTES
-------------------
-None are defined yet, but the caller must accept any which are supplied.
+'unchanged'::
+ This ref is unchanged since the last import or fetch, although
+ the helper cannot necessarily determine what value that produced.
Documentation
-------------
diff --git a/transport-helper.c b/transport-helper.c
index 3825441..1a05f0b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -166,6 +166,22 @@ static int fetch(struct transport *transport,
return -1;
}
+static int has_attribute(const char *attrs, const char *attr) {
+ int len;
+ if (!attrs)
+ return 0;
+
+ len = strlen(attr);
+ for (;;) {
+ const char *space = strchrnul(attrs, ' ');
+ if (len == space - attrs && !strncmp(attrs, attr, len))
+ return 1;
+ if (!*space)
+ return 0;
+ attrs = space + 1;
+ }
+}
+
static struct ref *get_refs_list(struct transport *transport, int for_push)
{
struct child_process *helper;
@@ -202,6 +218,12 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
(*tail)->symref = xstrdup(buf.buf + 1);
else if (buf.buf[0] != '?')
get_sha1_hex(buf.buf, (*tail)->old_sha1);
+ if (eon) {
+ if (has_attribute(eon + 1, "unchanged")) {
+ (*tail)->status |= REF_STATUS_UPTODATE;
+ read_ref((*tail)->name, (*tail)->old_sha1);
+ }
+ }
tail = &((*tail)->next);
}
strbuf_release(&buf);
--
1.6.4.2.419.gc86f8
^ permalink raw reply related
* Re: [PATCH] clone: disconnect transport after fetching
From: Jeff King @ 2009-09-04 2:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Daniel Barkalow, Sverre Rabbelier, Björn Steinbrink,
Matthieu Moy, Sitaram Chamarty
In-Reply-To: <20090902063647.GA29559@coredump.intra.peff.net>
On Wed, Sep 02, 2009 at 02:36:47AM -0400, Jeff King wrote:
> This patch just explicitly calls disconnect after we are
> done with the remote end, which sends a flush packet to
> upload-pack and cleanly disconnects, avoiding the error
> message.
I see you applied this with some extra tests. I should have mentioned in
the original cover letter that I considered tests but intentionally did
not include them.
The problem is that clone forks upload-pack, and then hangs up on it by
exiting, and then upload-pack spews the unwanted message. But control
has returned to the shell after clone exits, meaning that the message
from upload-pack may or may not have gotten there by the time we grep
stderr.
So I don't think your test will ever incorrectly show a failure, but I
believe that it would pass randomly even without the related fix to the
code.
-Peff
^ permalink raw reply
* Re: [PATCH v6 5/6] fast-import: add option command
From: Ian Clatworthy @ 2009-09-04 3:42 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Matt McClure, Miklos Vajna, Julian Phillips, vcs-fast-import-devs
In-Reply-To: <fabb9a1e0909022155r254c41c6s9ed962313c241e9@mail.gmail.com>
Sverre Rabbelier wrote:
> Heya,
>
> On Thu, Sep 3, 2009 at 04:41, Junio C Hamano<gitster@pobox.com> wrote:
>> I think at least the function should be made conditional to die() if it
>> was called from parse_argv() but simply ignore unknown if it was called
>> from the input stream.
>
> Makes sense, what do the fast-import devs think?
Sounds ok to me.
Ian C.
^ permalink raw reply
* Re: [JGIT] Request for help
From: Gabe McArthur @ 2009-09-04 5:00 UTC (permalink / raw)
To: git
In-Reply-To: <20090903155219.GI1033@spearce.org>
Shawn O. Pearce <spearce <at> spearce.org> writes:
>
> Please post patches; formatted with -M. I do want to do this, I just
> don't have the patience and Maven-fu to write the new poms myself.
>
Hey,
I'm a build engineer with a considerable amount of "Maven-fu" :). I've actually
generated a patch that does everything you want (and a bit more). I'm not that
familiar with git's command line yet, so it's a bit tricky to get the patch
thing right. However, here's a rough overview of what I did:
ROOT
====
README
/bin
bash.env -- A script that you can source from Bash that
will add the 'jgit' executable and the other
scripts in this 'bin' directory to your PATH
build.sh -- A general build script, that hides some
Maven complexities for initiates.
tag.sh -- Ok, this is the only thing that will have to
be re-written. It's too tied in with git commands for
me to fully extract what it's supposed to do.
/docs
LICENSE
SUBMITTING_PATCHES
TODO
pom.xml -- A considerable amount of build logic has been
centralized here. It references 3 sub-module
projects, listed below.
/sources
/jgit-lib
pom.xml
/src/main/java....
/src/test
/java....
/resources
/exttst -- Don't know exactly where this goes, as it
doesn't seem to be doing much/being run
currently.
/jgit-pgm
pom.xml -- Does the work to do a 'jar-with-dependencies'
so that org.spearce.jgit.pgm.build can be removed.
/src/main/java....
/jgit-exec
pom.xml -- Actually generates the 'jgit' executable and
installs it in ROOT/target/bin, so that it will
be on your path after sourcing 'bin/bash.env'
/src/main/scripts/jgit
I'll try to submit a full patch later, using your conventions.
My appreciation to Shawn for pointing out this thread....
-Gabe
^ permalink raw reply
* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Sverre Rabbelier @ 2009-09-04 5:29 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909032213180.28290@iabervon.org>
Heya,
On Fri, Sep 4, 2009 at 04:13, Daniel Barkalow<barkalow@iabervon.org> wrote:
> Instead of trying to make http://, https://, and ftp:// URLs
> indicative of some sort of pattern of transport helper usage, make
> them a special case which runs the "curl" helper, and leave the
> mechanism by which arbitrary helpers will be chosen entirely to future
> work.
I'm sorry, I missed a few emails I think :(. Would you mind explaining
why we chose to special-case the curl helpers instead of the symlink
scheme?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Clemens Buchacher @ 2009-09-04 7:02 UTC (permalink / raw)
To: Jeff King; +Cc: SZEDER Gábor, git
In-Reply-To: <20090902081917.GA5447@coredump.intra.peff.net>
On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:
> [1] I would prefer "git add -u ." to add only the current directory, and
> "git add -u" to touch everything.
FWIW, I feel the same way. And there is no easy way to do that now. (cd `git
rev-parse --show-cdup`; git add -u) ?
> But then, I am one of the people who
> turn off status.relativepaths, so I think I may be in the minority in
> always wanting to think of the project as a whole.
That mindset is one of git's greatest strengths IMO.
Clemens
^ permalink raw reply
* Re: [PATCH 0/8] VCS helpers
From: Junio C Hamano @ 2009-09-04 7:04 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0909032213120.28290@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> This is the next version of the db/vcs-helper series in pu.
Thanks.
> The first patch is new, a rework of the remote-curl build to produce
> "remote-curl" and call it as a special case for the sorts of URLs that
> we accept as indicating something that it now handled by this helper.
>
> The series is rebased onto current next, with some conflicts resolved.
Because the theme of the topic does not have anything to do with fixing
the deepening of shallow clones nor giving an extended error message from
non-fast-forward git-push, I queued the series after reverse-rebasing onto
old db/vcs-helper~8, in order to keep the topic branch pure, instead of
merging unrelated topics from maint, master or next into it. The result
merged in 'pu' obviously has to match what you expected by applying the
patches on top of 'next', and I am reasonably sure it does.
> Two patches have been dropped: a memory leak fix for code that was
> removed entirely by the first patch, and the "mark" helper capability,
> which is not needed (I believe) due to the "option" fast-import command.
Johan's cvs-helper series were depending on the previous iteration of this
series, but I thought it is being rerolled, so I'd drop it from pu for now.
^ permalink raw reply
* Re: [JGIT] Request for help
From: Mark Struberg @ 2009-09-04 7:12 UTC (permalink / raw)
To: Jonas Fonseca, Shawn O. Pearce
Cc: Johannes Schindelin, Nasser Grainawi, Git Mailing List
Hi!
Since I work on the sonatype repo and also being a maven guy, I'd be happy to help!
There are a few patches from the work we've done to come the next weeks anyway, starting with IgnoreRules and stuff. I think we still have to improve the code quality of SimpleRepository but I'd be happy to hear your opinion on this too, so I maybe send a RFC.
If you like to go with maven for JGIT, we have 2 options:
1.) Use the current directory structure and use the configuration you can see in the sonatype poms Jason did. E.g all paths have to be set in pom.sml
2.) Do a complete rework and move over to the standard maven layout [1] . This may include moving org.spearce.jgit.test/ to org.spearce.jgit/src/test/java resp org.spearce.jgit/src/test/resources.
In the meantime Eclipse is really fine with handling separate target folders for production code and test classes (target/classes vs target/test-classes) so this is not a showstopper any more.
LieGrue,
strub
[1] http://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html
--- On Thu, 9/3/09, Shawn O. Pearce <spearce@spearce.org> wrote:
> From: Shawn O. Pearce <spearce@spearce.org>
> Subject: Re: [JGIT] Request for help
> To: "Jonas Fonseca" <jonas.fonseca@gmail.com>
> Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>, "Nasser Grainawi" <nasser@codeaurora.org>, "Git Mailing List" <git@vger.kernel.org>
> Date: Thursday, September 3, 2009, 5:52 PM
> Jonas Fonseca <jonas.fonseca@gmail.com>
> wrote:
> > On Thu, Sep 3, 2009 at 10:42, Shawn O. Pearce<spearce@spearce.org>
> wrote:
> > > Actually, now that we have forked out of the
> egit.git repository,
> > > I want to refactor the layout of the JGit project
> to be more maven
> > > like, and have a proper top-level pom to build
> things.
> >
> > What kind of module structure do you have in mind? Do
> you want to move
> > some of the modules/subdirectories?
> > Some refactoring of the maven setup for JGit back was
> done back in
> > April in sonatype's (a maven company) JGit clone. It
> is not
> > signed-off, but can serve as a reference.
>
> Yea, I was hoping they would contribute this back as
> patches,
> but thus far they haven't.
>
> > The Maven layout in the sonatype clone simply uses the
> Eclipse project layout.
> >
> > pom.xml: JGit :: Parent
> > |- org.spearce.jgit/pom.xml: JGit :: Core
> > |- org.spearce.jgit.pgm/pom.xml: JGit ::
> Programs
> > `- org.spearce.jgit.test/pom.xml: JGit :: Test
> >
> > However, having tests in a separate module can be both
> good/bad. For
> > example, they will not automatically get run when you
> only build the
> > Core module.
>
> Yea, I know. This is one area where Maven is just
> whack, by putting
> the tests in the same project the Maven plugin for Eclipse
> puts
> them into the same classpath, which means you can see test
> code
> from project code. Wrong. They should be
> different projects so
> the test classpath is isolated.
>
> However. This is a bug in the Eclipse plugin I think,
> not
> necessarily with Maven's approach of trying to keep tests
> alongside
> the code they test. Thus we probably want:
>
> pom.xml: JGit :: Parent
> |- jgit-lib/pom.xml: JGit
> |
> src/main/java <-- from
> org.spearce.jgit/src
> |
> src/test/java <-- from
> org.spearce.jgit.test/src
> |
> `- jgit-pgm/pom.xml: JGit pgm
> src/main/java
> <-- from org.spearce.jgit.pgm/src
>
> IIRC there is Maven support to create proper MANIFEST.MF
> files for
> OSGI bundles, which is what we need for the Eclipse plugin
> support.
> That should be able to replace the META-INF/MANIFEST.MF in
> the top
> of each of the current directories.
>
> > Anyway, I would like to help.
>
> Please post patches; formatted with -M. I do want to
> do this, I just
> don't have the patience and Maven-fu to write the new poms
> myself.
>
> --
> Shawn.
> --
> 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
* Re: [PATCHv2] git-config: Parse config files leniently
From: Michael J Gruber @ 2009-09-04 7:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqfz4mu3.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 04.09.2009 01:42:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>>> Why was I CC'ed, if the patch wasn't even self tested?
>>
>> Because
>> - not CC'ing you would have meant culling you from the existing CC,
>> - we've discussed v1 of this patch before,
>> - I asked in this patch (v2) whether to go for an alternative.
>
> Oh, so you did not mean this was for inclusion but as another round
> of RFC. I misread your intent. Sorry about that.
OK. I'll try to distinguish better between RFC and RFI in the future.
And, yes, usually I run *all* tests before sending patches. I had even
grepped all tests for "config" in order not to miss any, but failed to
note how the patch affects all other commands as well.
>
> The eralier analysis of the cause of the breakage indicated that the
> implementation in this patch was flawed. What it essentially did was to
> re-define all die() to warn() in the codepaths around configuration
> variable handling [*1*].
>
> However, it does not mean that the idea of "ignoring syntax errors while
> keeping other errors still noticed for all commands, not limited to config
> nor not limited to 'config -e'" is necessarily flawed.
>
> For example, the test I noticed the breakage with was stuffing an invalid
> value to branch.autosetuprebase and wanted to see "git branch" fail.
>
> Obviously we do want to fail a "git branch newbranch origin/master" when
> the value given to branch.autosetuprebase is misspelled, to avoid creating
> bogus settings the user did not intend to. We do care about semantic
> errors (i.e. this variable can take only one of these values, but the
> value given in the file is bogus) in such a case. But if you are running
> "git branch" only to view, but not to create, there is no reason for us to
> care about the branch.autosetuprebase variable [*2*].
>
> This observation suggests that it may make sense to make the error
> handling _even looser_ than what you intended to do in your patch (which I
> assume was "we ignore syntax errors and try to recover, pretending that we
> saw a comment until the end of line, but we still keep the validation of
> values assigned to variables that the existing commands rely on").
>
> Ideally, the rule would be "we care about the values of variables we are
> going to use, but allow misspelled values in variables that are not used
> by us---we have no business complaining about them." Unfortunately that
> is much harder to arrange with the current code structure, but under that
> rule, "config -e" would care only about "core.editor" and nothing else, so
> as long as that variable can be sanely read, it should be able to start.
>
>
> [Footnotes]
>
> *1* The additional test that came with the patch only checked for the
> positive case (i.e. "does the system with this patch treats errors looser
> than before?"), but failed to check the negative case (i.e. "does the
> change too much and stop catching errors that should be caught?"), which
> unfortunately is a common mistake that easily lets bugs go unnoticed.
>
> *2* Worse yet, the parsing of branch.autosetuprebase is part of the
> default_config and commands that do not have anything to do with new
> branch creation will fail with the current setup.
Thanks for the thorough explanations. Especially *2* makes me think that
quite some restructuring would be necessary in order to "do it right".
That would be above my head (and time constraints).
Given that, I think the practical options are
a) make "git config" parse leniently (both -e and others)
b) leave as is and document how to recover from bad config
c) launch the editor on a tmp copy, check and refuse or loop on fail...
I still think of "git config -e" as a shortcut only (meaning it doesn't
warrant large specific code efforts), and it's problematic because the
user base is split in two sets:
- Those who know their way around .git/config and editors.
- Those who should stick with the get and set modes of "git config".
"git config -e" helps users from the 2nd group shoot themselves in their
feet badly enough that they can recover only with insight from the first
group...
Michael
^ permalink raw reply
* Re: [JGIT] Request for help
From: Mark Struberg @ 2009-09-04 7:33 UTC (permalink / raw)
To: git, Gabe McArthur
In-Reply-To: <loom.20090904T064602-511@post.gmane.org>
Seems this speeds up lately ;)
Gabe, please allow me a few questions:
.) why do we need the /sources directory layer? I think /jgit and /jgit-pgm would be enough.
.) imho the docs should stay in / at least the LICENSE file
.) we don't need a tag.sh any more if we work with maven. Maven now has the maven-scm-provider-gitexe activated by default (since early 2008), so
mvn release:prepare
mvn release:perform
should work if we set the proper <scm> section. Any feedback or bugreporting on the maven-git integration is highly welcome btw ;)
LieGrue,
strub
--- On Fri, 9/4/09, Gabe McArthur <gabriel.mcarthur@gmail.com> wrote:
> From: Gabe McArthur <gabriel.mcarthur@gmail.com>
> Subject: Re: [JGIT] Request for help
> To: git@vger.kernel.org
> Date: Friday, September 4, 2009, 7:00 AM
>
> Shawn O. Pearce <spearce <at> spearce.org>
> writes:
>
> >
> > Please post patches; formatted with -M. I do
> want to do this, I just
> > don't have the patience and Maven-fu to write the new
> poms myself.
> >
>
>
> Hey,
> I'm a build engineer with a considerable amount of
> "Maven-fu" :). I've actually
> generated a patch that does everything you want (and a bit
> more). I'm not that
> familiar with git's command line yet, so it's a bit tricky
> to get the patch
> thing right. However, here's a rough overview of what
> I did:
>
> ROOT
> ====
> README
> /bin
> bash.env -- A script that you can
> source from Bash that
>
> will add the 'jgit' executable and the other
>
> scripts in this 'bin' directory to your PATH
> build.sh -- A general build script,
> that hides some
>
> Maven complexities for initiates.
> tag.sh -- Ok, this is the
> only thing that will have to
> be
> re-written. It's too tied in with git commands for
> me
> to fully extract what it's supposed to do.
> /docs
> LICENSE
> SUBMITTING_PATCHES
> TODO
> pom.xml -- A considerable amount of
> build logic has been
>
> centralized here. It references 3 sub-module
>
> projects, listed below.
> /sources
> /jgit-lib
> pom.xml
> /src/main/java....
> /src/test
> /java....
> /resources
> /exttst -- Don't know
> exactly where this goes, as it
>
> doesn't seem to be doing much/being run
>
> currently.
> /jgit-pgm
> pom.xml -- Does the
> work to do a 'jar-with-dependencies'
>
> so that org.spearce.jgit.pgm.build can be
> removed.
> /src/main/java....
> /jgit-exec
> pom.xml -- Actually
> generates the 'jgit' executable and
>
> installs it in ROOT/target/bin, so that it
> will
>
> be on your path after sourcing
> 'bin/bash.env'
> /src/main/scripts/jgit
>
> I'll try to submit a full patch later, using your
> conventions.
>
> My appreciation to Shawn for pointing out this thread....
> -Gabe
>
>
> --
> 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
* Re: Deleted folder keeps showing up?
From: Michael J Gruber @ 2009-09-04 7:41 UTC (permalink / raw)
To: Benjamin Buch; +Cc: git
In-Reply-To: <34230C98-81B8-4DC8-846F-8B6FA2A022DA@gmx.de>
Benjamin Buch venit, vidit, dixit 03.09.2009 18:59:
> I made a branch and deleted a folder there with git rm -rf foldername.
> So now i have to branches, A with the folder and B without the folder.
>
> I'm on B, the folder is not there.
> Then I check out A, the folder shows up like it should.
> When I check out B again, the folder is still there.
>
> A git rm -rf folder gives me:
>
> fatal: pathspec 'folder/' did not match any files
>
> , so git is not tracking the folder.
>
> I can rm -rf the filder without git and start the whole game from the
> beginning,
> but there has to be a better way?
>
> Strange enough this happens just to two folders I removed,
> with others there is no problem.
What does "git status" say when you've checked out B? Could some
contents of folder/ possibly be being ignored (.git/info/excludes etc.)?
Michael
^ permalink raw reply
* Re: Deleted folder keeps showing up?
From: Benjamin Buch @ 2009-09-04 8:18 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4AA0C4A2.5040405@drmicha.warpmail.net>
Hi Michael,
thank you for your answer.
Strangely, I can't reproduce the error today.
As I did quite a lot branching and merging yesterday (learning git),
I can't remember the steps that led to the error.
And it doesn't have to be an error though -
perhaps it was just my clouded perception due to the lots of branches.
But I will keep an eye on this and see if it happens again.
Sorry for bothering,
- benjamin
Am 04.09.2009 um 09:41 schrieb Michael J Gruber:
> Benjamin Buch venit, vidit, dixit 03.09.2009 18:59:
>> I made a branch and deleted a folder there with git rm -rf
>> foldername.
>> So now i have to branches, A with the folder and B without the
>> folder.
>>
>> I'm on B, the folder is not there.
>> Then I check out A, the folder shows up like it should.
>> When I check out B again, the folder is still there.
>>
>> A git rm -rf folder gives me:
>>
>> fatal: pathspec 'folder/' did not match any files
>>
>> , so git is not tracking the folder.
>>
>> I can rm -rf the filder without git and start the whole game from the
>> beginning,
>> but there has to be a better way?
>>
>> Strange enough this happens just to two folders I removed,
>> with others there is no problem.
>
> What does "git status" say when you've checked out B? Could some
> contents of folder/ possibly be being ignored (.git/info/excludes
> etc.)?
>
> Michael
> --
> 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
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