* Re: [RFC PATCH 2/2] MSVC: Fix an "incompatible pointer types" compiler warning
From: Johannes Sixt @ 2009-12-04 7:44 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, Marius Storm-Olsen, GIT Mailing-list
In-Reply-To: <4B1806FB.2050401@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> In order to avoid the compiler warning, we use the appropriate
> structure type names (and function names) from the msvc headers.
> This allows us to compile with -D_USE_32BIT_TIME_T if necessary.
"if necessary"? Who defines when -D_USE_32BIT_TIME_T is necessary?
> Also, I added the "&& defined(_stati64)" in the hope that it would work with
> older msvc/sdk versions.
I think that this is an unnecessary complication and I did wonder why you
added this extra check. Anybody doing some serious development with MS's
tools is using VS2005 at least. But isn't the .vcproj file made for VS2008
anyway?
> The reason for the RFC is:
>
> - maybe we don't need the flexibility of compiling with/without the 32-bit
> time_t definition (which *works* BTW) and can revert to the original patch?
Indeed I'm wondering why we should cater for 64 bit time_t. It is an
unnessary complication as long as MinGW gcc supports only 32 bit time_t
and the old MSVCRT.DLL.
> - I *think* this will work with older msvc, but I can't test it!
This should not be a concern, IMHO.
> - I've tried to be careful not to break the MinGW build, but again I can't
> test it. (I will be shocked if I have ;-)
It compiles without warnings and doesn't break t/t[01]* ;)
-- Hannes
^ permalink raw reply
* Re: [PATCH resend 0/3] transport: catch non-fast-forwards
From: Daniel Barkalow @ 2009-12-04 7:05 UTC (permalink / raw)
To: Tay Ray Chuan
Cc: git, Shawn O. Pearce, Daniel Barkalow, Sverre Rabbelier,
Clemens Buchacher, Jeff King, Junio C Hamano
In-Reply-To: <20091204144822.a61355d2.rctay89@gmail.com>
On Fri, 4 Dec 2009, Tay Ray Chuan wrote:
> This patch series applies on top of 'next', and deals with alerting
> the user to non-fast-forward pushes when using helpers (smart).
>
> Previously, git silently exited. This situation involves the curl
> helper and the smart protocol. The fast-forward push is only detected
> when curl executes the rpc client (git-send-pack). Now, we detect it
> before telling the helper to push.
>
> The first patch refactors ref status logic out of builtin-send-pack.c.
>
> The second patch lets git know of non-fast-forwards before actually
> telling the helper to push anything.
>
> The third patch changes the return code when ref status indicate an
> error (determined by push_had_errors()), so that the caller of
> transport_push() is alerted.
Seems to me like transport_push() ought to set this sort of status
information before calling the push method, rather than requiring all of
the implementations to remember to call the generic function, since
there's not really anything else sane they can do, right?
(There's eventually going to be at least a third push implementation:
export the data to a foreign system a la "git svn dcommit")
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [StGit PATCH v2 3/6] stg mail: make __send_message do more
From: Karl Wiberg @ 2009-12-04 7:00 UTC (permalink / raw)
To: Alex Chiang; +Cc: catalin.marinas, git
In-Reply-To: <20091203193018.GF23258@ldl.fc.hp.com>
On Thu, Dec 3, 2009 at 8:30 PM, Alex Chiang <achiang@hp.com> wrote:
> * Karl Wiberg <kha@treskal.com>:
>
> > On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:
> >
> > > + for (p, n) in zip(patches, range(1, total_nr + 1)):
> > > + msg_id = __send_message('patch', tmpl, options, p, n, total_nr, ref_id)
> >
> > Can be written as
> >
> > for (n, p) in enumerate(patches):
> >
> > if you use n + 1 instead of n in the loop body.
>
> That is a little cleaner, but I decided to keep it as zip(). Why?
> Because using n + 1 in the loop body will push that line past 80
> columns. ;)
>
> It's also the original code (albeit with a simple variable rename).
>
> I know this isn't the kernel, and that there are plenty of other
> lines that are 80+ characters, but if you can keep it short, why
> not?
Oh, I fully favor keeping lines within the 80 columns allotted to us
by the ancestors---I just didn't realize it was going to be a problem
here.
In general, though, programmer time is worth optimizing for, and
thinking through exactly what zip(patches, range(1, total_nr + 1))
means (and getting it right!) is a small but not insignificant cost
every time someone reads the code.
--
Karl Wiberg, kha@treskal.com
subrabbit.wordpress.com
www.treskal.com/kalle
^ permalink raw reply
* [PATCH resend 3/3] transport.c::transport_push(): make ref status affect return value
From: Tay Ray Chuan @ 2009-12-04 6:58 UTC (permalink / raw)
To: git
Cc: Shawn O. Pearce, Daniel Barkalow, Sverre Rabbelier,
Clemens Buchacher, Jeff King, Junio C Hamano
In-Reply-To: <20091204145437.1a9910db.rctay89@gmail.com>
When determining whether to print the ref status table,
push_had_errors() is called. Piggy-back this and modify the return
value accordingly.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
transport.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/transport.c b/transport.c
index 3eea836..32c122b 100644
--- a/transport.c
+++ b/transport.c
@@ -889,10 +889,13 @@ int transport_push(struct transport *transport,
ret = transport->push_refs(transport, remote_refs, flags);
- if (!quiet || push_had_errors(remote_refs))
- print_push_status(transport->url, remote_refs,
- verbose | porcelain, porcelain,
- nonfastforward);
+ if (!quiet)
+ if (push_had_errors(remote_refs)) {
+ ret = -1;
+ print_push_status(transport->url, remote_refs,
+ verbose | porcelain, porcelain,
+ nonfastforward);
+ }
if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
struct ref *ref;
--
1.6.6.rc1.286.gbc15a
^ permalink raw reply related
* [PATCH resend 2/3] transport-helper.c::push_refs(): know more about refs before pushing
From: Tay Ray Chuan @ 2009-12-04 6:57 UTC (permalink / raw)
To: git
Cc: Shawn O. Pearce, Daniel Barkalow, Sverre Rabbelier,
Clemens Buchacher, Jeff King, Junio C Hamano
In-Reply-To: <20091204145437.1a9910db.rctay89@gmail.com>
Know about rejected non-fast-forward refs, in addition to up-to-date
ones, by calling set_ref_status_for_push() in remote.[ch], before
passing push commands to the helper.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
transport-helper.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 11f3d7e..6ed7b55 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -334,16 +334,12 @@ static int push_refs(struct transport *transport,
else if (!mirror)
continue;
- ref->deletion = is_null_sha1(ref->new_sha1);
- if (!ref->deletion &&
- !hashcmp(ref->old_sha1, ref->new_sha1)) {
- ref->status = REF_STATUS_UPTODATE;
- continue;
- }
-
if (force_all)
ref->force = 1;
+ if (set_ref_status_for_push(ref, force_all))
+ continue;
+
strbuf_addstr(&buf, "push ");
if (!ref->deletion) {
if (ref->force)
--
1.6.6.rc1.286.gbc15a
^ permalink raw reply related
* [PATCH resend 1/3] refactor ref status logic for pushing
From: Tay Ray Chuan @ 2009-12-04 6:56 UTC (permalink / raw)
To: git
Cc: Shawn O. Pearce, Daniel Barkalow, Sverre Rabbelier,
Clemens Buchacher, Jeff King, Junio C Hamano
In-Reply-To: <20091204145437.1a9910db.rctay89@gmail.com>
Move the logic to a new function in remote.[ch],
set_ref_status_for_push().
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
builtin-send-pack.c | 39 +++------------------------------------
remote.c | 42 ++++++++++++++++++++++++++++++++++++++++++
remote.h | 1 +
3 files changed, 46 insertions(+), 36 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8fffdbf..8c98624 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -412,44 +412,11 @@ int send_pack(struct send_pack_args *args,
else if (!args->send_mirror)
continue;
- ref->deletion = is_null_sha1(ref->new_sha1);
- if (ref->deletion && !allow_deleting_refs) {
- ref->status = REF_STATUS_REJECT_NODELETE;
- continue;
- }
- if (!ref->deletion &&
- !hashcmp(ref->old_sha1, ref->new_sha1)) {
- ref->status = REF_STATUS_UPTODATE;
+ if (set_ref_status_for_push(ref, args->force_update))
continue;
- }
- /* This part determines what can overwrite what.
- * The rules are:
- *
- * (0) you can always use --force or +A:B notation to
- * selectively force individual ref pairs.
- *
- * (1) if the old thing does not exist, it is OK.
- *
- * (2) if you do not have the old thing, you are not allowed
- * to overwrite it; you would not know what you are losing
- * otherwise.
- *
- * (3) if both new and old are commit-ish, and new is a
- * descendant of old, it is OK.
- *
- * (4) regardless of all of the above, removing :B is
- * always allowed.
- */
-
- ref->nonfastforward =
- !ref->deletion &&
- !is_null_sha1(ref->old_sha1) &&
- (!has_sha1_file(ref->old_sha1)
- || !ref_newer(ref->new_sha1, ref->old_sha1));
-
- if (ref->nonfastforward && !ref->force && !args->force_update) {
- ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ if (ref->deletion && !allow_deleting_refs) {
+ ref->status = REF_STATUS_REJECT_NODELETE;
continue;
}
diff --git a/remote.c b/remote.c
index e3afecd..188869a 100644
--- a/remote.c
+++ b/remote.c
@@ -1247,6 +1247,48 @@ int match_refs(struct ref *src, struct ref **dst,
return 0;
}
+int set_ref_status_for_push(struct ref *ref, int force_update)
+{
+ ref->deletion = is_null_sha1(ref->new_sha1);
+ if (!ref->deletion &&
+ !hashcmp(ref->old_sha1, ref->new_sha1)) {
+ ref->status = REF_STATUS_UPTODATE;
+ return 1;
+ }
+
+ /* This part determines what can overwrite what.
+ * The rules are:
+ *
+ * (0) you can always use --force or +A:B notation to
+ * selectively force individual ref pairs.
+ *
+ * (1) if the old thing does not exist, it is OK.
+ *
+ * (2) if you do not have the old thing, you are not allowed
+ * to overwrite it; you would not know what you are losing
+ * otherwise.
+ *
+ * (3) if both new and old are commit-ish, and new is a
+ * descendant of old, it is OK.
+ *
+ * (4) regardless of all of the above, removing :B is
+ * always allowed.
+ */
+
+ ref->nonfastforward =
+ !ref->deletion &&
+ !is_null_sha1(ref->old_sha1) &&
+ (!has_sha1_file(ref->old_sha1)
+ || !ref_newer(ref->new_sha1, ref->old_sha1));
+
+ if (ref->nonfastforward && !ref->force && !force_update) {
+ ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ return 1;
+ }
+
+ return 0;
+}
+
struct branch *branch_get(const char *name)
{
struct branch *ret;
diff --git a/remote.h b/remote.h
index 8b7ecf9..0f45553 100644
--- a/remote.h
+++ b/remote.h
@@ -98,6 +98,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
int match_refs(struct ref *src, struct ref **dst,
int nr_refspec, const char **refspec, int all);
+int set_ref_status_for_push(struct ref *ref, int force_update);
/*
* Given a list of the remote refs and the specification of things to
--
1.6.6.rc1.286.gbc15a
^ permalink raw reply related
* [PATCH resend 0/3] transport: catch non-fast-forwards
From: Tay Ray Chuan @ 2009-12-04 6:54 UTC (permalink / raw)
To: git
Cc: Shawn O. Pearce, Daniel Barkalow, Sverre Rabbelier,
Clemens Buchacher, Jeff King, Junio C Hamano
This patch series applies on top of 'next', and deals with alerting
the user to non-fast-forward pushes when using helpers (smart).
Previously, git silently exited. This situation involves the curl
helper and the smart protocol. The fast-forward push is only detected
when curl executes the rpc client (git-send-pack). Now, we detect it
before telling the helper to push.
The first patch refactors ref status logic out of builtin-send-pack.c.
The second patch lets git know of non-fast-forwards before actually
telling the helper to push anything.
The third patch changes the return code when ref status indicate an
error (determined by push_had_errors()), so that the caller of
transport_push() is alerted.
PS. There are at least 2 bug fixes related to this series. If you
usually do so from repo.or.cz, please fetch from git.kernel.org; the
former is 2 days old.
builtin-send-pack.c | 39 +++------------------------------------
remote.c | 42 ++++++++++++++++++++++++++++++++++++++++++
remote.h | 1 +
transport-helper.c | 10 +++-------
transport.c | 11 +++++++----
5 files changed, 56 insertions(+), 47 deletions(-)
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.6.5.4
From: Matthieu Moy @ 2009-12-04 6:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Eugene Sajine, Todd Zullinger, Michael J Gruber, Andreas Schwab,
git
In-Reply-To: <7vocmfxnxh.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Eugene Sajine <euguess@gmail.com> writes:
>
>> We have RH with xmlto 0.0.18. I was getting ready to update our
>> installation to 1.6.5.4, but as i understand the documentation will
>> not be fully available untill this issue is resolved. Could you,
>> please, advise if this is going to be in next 1.6.5.5?
>
> I've applied the patch in the thread you are responding to already on
> 'maint' so it will appear in both 1.6.5.5 and 1.6.6. In the meantime, if
> you want to run 1.6.5.4 or preferably 1.6.6-rc1, you can locally revert
> 8dd35c7 (Unconditionally set man.base.url.for.relative.links,
> 2009-12-03).
Also, one can download the man pages from git.git :
# in a clone of git://git.kernel.org/pub/scm/git/git.git
git archive --format=tar origin/man | tar -x -C /usr/share/man/ -vf -
(or leave with 1.6.5.5 and the docs of 1.6.5.4 for some time)
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.6.5.4
From: Junio C Hamano @ 2009-12-04 6:18 UTC (permalink / raw)
To: Eugene Sajine
Cc: Junio C Hamano, Todd Zullinger, Michael J Gruber, Andreas Schwab,
git
In-Reply-To: <76c5b8580912031949k7f4193f9q94f9a2040b877571@mail.gmail.com>
Eugene Sajine <euguess@gmail.com> writes:
> We have RH with xmlto 0.0.18. I was getting ready to update our
> installation to 1.6.5.4, but as i understand the documentation will
> not be fully available untill this issue is resolved. Could you,
> please, advise if this is going to be in next 1.6.5.5?
I've applied the patch in the thread you are responding to already on
'maint' so it will appear in both 1.6.5.5 and 1.6.6. In the meantime, if
you want to run 1.6.5.4 or preferably 1.6.6-rc1, you can locally revert
8dd35c7 (Unconditionally set man.base.url.for.relative.links, 2009-12-03).
^ permalink raw reply
* [PATCH 1/3] refactor ref status logic for pushing
From: Tay Ray Chuan @ 2009-12-04 4:54 UTC (permalink / raw)
To: git; +Cc: Clemens Buchacher, Junio C Hamano
In-Reply-To: <20091204125042.c64f347d.rctay89@gmail.com>
Move the logic to a new function in remote.[ch],
set_ref_status_for_push().
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
builtin-send-pack.c | 39 +++------------------------------------
remote.c | 42 ++++++++++++++++++++++++++++++++++++++++++
remote.h | 1 +
3 files changed, 46 insertions(+), 36 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8fffdbf..8c98624 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -412,44 +412,11 @@ int send_pack(struct send_pack_args *args,
else if (!args->send_mirror)
continue;
- ref->deletion = is_null_sha1(ref->new_sha1);
- if (ref->deletion && !allow_deleting_refs) {
- ref->status = REF_STATUS_REJECT_NODELETE;
- continue;
- }
- if (!ref->deletion &&
- !hashcmp(ref->old_sha1, ref->new_sha1)) {
- ref->status = REF_STATUS_UPTODATE;
+ if (set_ref_status_for_push(ref, args->force_update))
continue;
- }
- /* This part determines what can overwrite what.
- * The rules are:
- *
- * (0) you can always use --force or +A:B notation to
- * selectively force individual ref pairs.
- *
- * (1) if the old thing does not exist, it is OK.
- *
- * (2) if you do not have the old thing, you are not allowed
- * to overwrite it; you would not know what you are losing
- * otherwise.
- *
- * (3) if both new and old are commit-ish, and new is a
- * descendant of old, it is OK.
- *
- * (4) regardless of all of the above, removing :B is
- * always allowed.
- */
-
- ref->nonfastforward =
- !ref->deletion &&
- !is_null_sha1(ref->old_sha1) &&
- (!has_sha1_file(ref->old_sha1)
- || !ref_newer(ref->new_sha1, ref->old_sha1));
-
- if (ref->nonfastforward && !ref->force && !args->force_update) {
- ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ if (ref->deletion && !allow_deleting_refs) {
+ ref->status = REF_STATUS_REJECT_NODELETE;
continue;
}
diff --git a/remote.c b/remote.c
index e3afecd..188869a 100644
--- a/remote.c
+++ b/remote.c
@@ -1247,6 +1247,48 @@ int match_refs(struct ref *src, struct ref **dst,
return 0;
}
+int set_ref_status_for_push(struct ref *ref, int force_update)
+{
+ ref->deletion = is_null_sha1(ref->new_sha1);
+ if (!ref->deletion &&
+ !hashcmp(ref->old_sha1, ref->new_sha1)) {
+ ref->status = REF_STATUS_UPTODATE;
+ return 1;
+ }
+
+ /* This part determines what can overwrite what.
+ * The rules are:
+ *
+ * (0) you can always use --force or +A:B notation to
+ * selectively force individual ref pairs.
+ *
+ * (1) if the old thing does not exist, it is OK.
+ *
+ * (2) if you do not have the old thing, you are not allowed
+ * to overwrite it; you would not know what you are losing
+ * otherwise.
+ *
+ * (3) if both new and old are commit-ish, and new is a
+ * descendant of old, it is OK.
+ *
+ * (4) regardless of all of the above, removing :B is
+ * always allowed.
+ */
+
+ ref->nonfastforward =
+ !ref->deletion &&
+ !is_null_sha1(ref->old_sha1) &&
+ (!has_sha1_file(ref->old_sha1)
+ || !ref_newer(ref->new_sha1, ref->old_sha1));
+
+ if (ref->nonfastforward && !ref->force && !force_update) {
+ ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ return 1;
+ }
+
+ return 0;
+}
+
struct branch *branch_get(const char *name)
{
struct branch *ret;
diff --git a/remote.h b/remote.h
index 8b7ecf9..0f45553 100644
--- a/remote.h
+++ b/remote.h
@@ -98,6 +98,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
int match_refs(struct ref *src, struct ref **dst,
int nr_refspec, const char **refspec, int all);
+int set_ref_status_for_push(struct ref *ref, int force_update);
/*
* Given a list of the remote refs and the specification of things to
--
1.6.6.rc1.286.gbc15a
^ permalink raw reply related
* Re: [ANNOUNCE] Git 1.6.5.4
From: Eugene Sajine @ 2009-12-04 3:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Todd Zullinger, Michael J Gruber, Andreas Schwab, git
In-Reply-To: <7vbpif4rn2.fsf@alter.siamese.dyndns.org>
On Thu, Dec 3, 2009 at 5:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Todd Zullinger <tmz@pobox.com> writes:
>
>> I tested with this in Documentation/manpage-base.xsl on a CentOS 5 box
>> and it builds fine, leaving no cruft in the man pages regarding the
>> man.base.url...
>>
>> <!-- set a base URL for relative links -->
>> <xsl:param name="man.base.url.for.relative.links"
>> >/path/to/git/docs</xsl:param>
>>
>> Of course, the relative links looked just like they did in older
>> docbook releases:
>>
>> 1. Everyday Git
>> everyday.html
>>
>> Is it worth the effort to have @@MAN_BASE_URL@@ in
>> Documentation/manpage-base.xsl or similar and replace it at build
>> time?
>
> I think it depends on the likelihood that a distro has xmlto so old that
> it does not understand --stringparam yet it uses stylesheet so new that
> setting the parameter makes a positive difference (either it gives the
> full URL or at least squelches the "You should define the parameter"
> noise) in the output.
>
> I am guessing that the answer would be that is a very unlikely combination
> and not worth worrying about it, but I've been wrong before in this exact
> area ;-)
Hello,
We have RH with xmlto 0.0.18. I was getting ready to update our
installation to 1.6.5.4, but as i understand the documentation will
not be fully available untill this issue is resolved. Could you,
please, advise if this is going to be in next 1.6.5.5?
Thanks,
Eugene
^ permalink raw reply
* Re: How do you best store structured data in git repositories?
From: Avery Pennarun @ 2009-12-04 1:45 UTC (permalink / raw)
To: David Aguilar; +Cc: sebastianspublicaddress, git
In-Reply-To: <20091204001359.GA6709@gmail.com>
On Thu, Dec 3, 2009 at 7:14 PM, David Aguilar <davvid@gmail.com> wrote:
> JSON's not too bad for data structures and is known to
> be friendly to XML expats.
>
> http://json.org/
yaml is also really good for storing structured data, and its
line-by-line format lends itself to easy merging (if you don't feel
like writing a custom merge algorithm).
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH] reset: add --quiet option
From: Felipe Contreras @ 2009-12-04 1:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7v7ht3zgaz.fsf@alter.siamese.dyndns.org>
On Fri, Dec 4, 2009 at 3:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> Because the less trivial the patches, the less luck I have of getting
>> them applied :)
>
> Well, the name of the game is not "let me have more commits under my name
> in a well known project". It is "let's work together to make the system
> better without stepping on each other's toes and without introducing
> unintended side effects".
Except that different people have different opinions about what's
"better", when it's OK to step on somebody else's toes, and what's an
important side-effect.
> I actually do not think it is the complexity that matters. It largely
> depends on what other patches are in flight that may have interactions,
> and if the change is suitable for the phase of the cycle.
And whether or not you consider the change desirable at all.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] reset: add --quiet option
From: Junio C Hamano @ 2009-12-04 1:19 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Stephen Boyd, git
In-Reply-To: <94a0d4530912030133n7e2fbf2asfea6e3896980dc7c@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Mon, Nov 30, 2009 at 11:45 PM, Stephen Boyd <bebarino@gmail.com> wrote:
>> If you're already touching the line why not just do it once? I agree a
>> follow-up patch to cover the other commands would be good.
>
> Because the less trivial the patches, the less luck I have of getting
> them applied :)
Well, the name of the game is not "let me have more commits under my name
in a well known project". It is "let's work together to make the system
better without stepping on each other's toes and without introducing
unintended side effects".
I actually do not think it is the complexity that matters. It largely
depends on what other patches are in flight that may have interactions,
and if the change is suitable for the phase of the cycle.
> Anyway, I sent a patch to use OPT__QUIET directly in two places.
Yeah, I saw it and queued it to 'pu'. Thanks.
We _might_ want to think about doing something about the lossage of "long
messages" by this conversion, and we may end up updating OPT__QUIET() to
allow its users supply messages that are more suitable than the default
one, but I do not want to see such a change to parse-options before 1.6.6
happens on the 'master' branch, as I do not have infinite mental
bandwidth.
^ permalink raw reply
* Re: [PATCH] builtin-push: don't access freed transport->url
From: Junio C Hamano @ 2009-12-04 0:31 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Tay Ray Chuan, Git Mailing List, Sverre Rabbelier, Junio C Hamano
In-Reply-To: <alpine.LNX.2.00.0912031837570.14365@iabervon.org>
Thanks, both; queued at the top of sr/vcs-helpers topic and merged to
'next'.
^ permalink raw reply
* Requesting for pull-requests for 1.6.6
From: Junio C Hamano @ 2009-12-04 0:27 UTC (permalink / raw)
To: spearce, Eric Wong, Paul Mackerras; +Cc: git
Sorry, I should have sent this out before tagging -rc1, but if you have
fixes and well tested new features that should be in 1.6.6 in your
respective areas, please give me a pull order soonish.
Thanks.
^ permalink raw reply
* Re: How do you best store structured data in git repositories?
From: David Aguilar @ 2009-12-04 0:14 UTC (permalink / raw)
To: Avery Pennarun; +Cc: sebastianspublicaddress, git
In-Reply-To: <32541b130912021317y705d1d4cj28e230a3e727df2e@mail.gmail.com>
On Wed, Dec 02, 2009 at 04:17:10PM -0500, Avery Pennarun wrote:
> On Wed, Dec 2, 2009 at 4:08 PM, Sebastian Setzer
> <sebastianspublicaddress@googlemail.com> wrote:
> > Do you store everything in a single file and configure git to use
> > special diff- and merge-tools?
> > Do you use XML for this purpose?
>
> XML is terrible for most data storage purposes. Data exchange, maybe,
> but IMHO the best thing you can do when you get XML data is to put it
> in some other format ASAP.
I agree 100%.
JSON's not too bad for data structures and is known to
be friendly to XML expats.
http://json.org/
> That said, however, you should still try to make your files as stable
> as possible, because:
>
> - If your program outputs the data in random order, it's just being
> sloppy anyway
>
> - 'git diff' doesn't work usefully otherwise (for examining the data
> and debugging)
If you were using Python + simplejson then using something
like the sort_keys=True flag would ensure that your data
is stable as the dictionaries keys will always appear in a
deterministic order.
Since I mentioned JSON and git in the same email then I might as
well also mention an old UGFWIINI candidate:
http://www.ordecon.com/2009/04/22/is-git-more-than-just-a-version-control-system/
Lastly, BERT might not be a good choice for storing inside
of a git repository, but it is a nice format for representing
data structures:
http://github.com/blog/531-introducing-bert-and-bert-rpc
We've been using git for tracking changes to a large set of
JSON files at $dayjob and it's worked out pretty well.
I'd suggest that you try to break your data up into multiple
files if possible. As someone else mentioned, it's often
easier to diff and merge stuff if you structure things in a
merge-friendly way.
One feature that we've implemented is file referencing
where data can "#include" another data file. That is
the kind of thing that can make things easier on you if
you foresee having a lot of common data that can be
shared amongst the various different files.
--
David
^ permalink raw reply
* Re: [PATCH] builtin-push: don't access freed transport->url
From: Daniel Barkalow @ 2009-12-03 23:38 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List, Sverre Rabbelier, Junio C Hamano
In-Reply-To: <20091204073144.f98115f9.rctay89@gmail.com>
On Fri, 4 Dec 2009, Tay Ray Chuan wrote:
> Move the failed push message to before transport_disconnect() so that
> it doesn't access transport->url after transport has been free()'d (in
> transport_disconnect()).
>
> Additionally, make the failed push message more accurate by moving it
> before transport_disconnect(), so that it doesn't report errors due
> to a failed disconnect.
>
> Cc: "Daniel Barkalow" <barkalow@iabervon.org>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Acked-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> builtin-push.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-push.c b/builtin-push.c
> index a21e46c..dcfb53f 100644
> --- a/builtin-push.c
> +++ b/builtin-push.c
> @@ -101,13 +101,14 @@ static int push_with_options(struct transport *transport, int flags)
> fprintf(stderr, "Pushing to %s\n", transport->url);
> err = transport_push(transport, refspec_nr, refspec, flags,
> &nonfastforward);
> + if (err != 0)
> + error("failed to push some refs to '%s'", transport->url);
> +
> err |= transport_disconnect(transport);
>
> if (!err)
> return 0;
>
> - error("failed to push some refs to '%s'", transport->url);
> -
> if (nonfastforward && advice_push_nonfastforward) {
> printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
> "Merge the remote changes before pushing again. See the 'non-fast-forward'\n"
> --
> 1.6.6.rc1.249.g048b3
>
^ permalink raw reply
* [PATCH] builtin-push: don't access freed transport->url
From: Tay Ray Chuan @ 2009-12-03 23:31 UTC (permalink / raw)
To: Git Mailing List; +Cc: Daniel Barkalow, Sverre Rabbelier, Junio C Hamano
Move the failed push message to before transport_disconnect() so that
it doesn't access transport->url after transport has been free()'d (in
transport_disconnect()).
Additionally, make the failed push message more accurate by moving it
before transport_disconnect(), so that it doesn't report errors due
to a failed disconnect.
Cc: "Daniel Barkalow" <barkalow@iabervon.org>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
builtin-push.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin-push.c b/builtin-push.c
index a21e46c..dcfb53f 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -101,13 +101,14 @@ static int push_with_options(struct transport *transport, int flags)
fprintf(stderr, "Pushing to %s\n", transport->url);
err = transport_push(transport, refspec_nr, refspec, flags,
&nonfastforward);
+ if (err != 0)
+ error("failed to push some refs to '%s'", transport->url);
+
err |= transport_disconnect(transport);
if (!err)
return 0;
- error("failed to push some refs to '%s'", transport->url);
-
if (nonfastforward && advice_push_nonfastforward) {
printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
"Merge the remote changes before pushing again. See the 'non-fast-forward'\n"
--
1.6.6.rc1.249.g048b3
^ permalink raw reply related
* Re: [ANNOUNCE] Git 1.6.5.4
From: Junio C Hamano @ 2009-12-03 22:30 UTC (permalink / raw)
To: Todd Zullinger; +Cc: Michael J Gruber, Andreas Schwab, git
In-Reply-To: <20091203220020.GS23717@inocybe.localdomain>
Todd Zullinger <tmz@pobox.com> writes:
> I tested with this in Documentation/manpage-base.xsl on a CentOS 5 box
> and it builds fine, leaving no cruft in the man pages regarding the
> man.base.url...
>
> <!-- set a base URL for relative links -->
> <xsl:param name="man.base.url.for.relative.links"
> >/path/to/git/docs</xsl:param>
>
> Of course, the relative links looked just like they did in older
> docbook releases:
>
> 1. Everyday Git
> everyday.html
>
> Is it worth the effort to have @@MAN_BASE_URL@@ in
> Documentation/manpage-base.xsl or similar and replace it at build
> time?
I think it depends on the likelihood that a distro has xmlto so old that
it does not understand --stringparam yet it uses stylesheet so new that
setting the parameter makes a positive difference (either it gives the
full URL or at least squelches the "You should define the parameter"
noise) in the output.
I am guessing that the answer would be that is a very unlikely combination
and not worth worrying about it, but I've been wrong before in this exact
area ;-)
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.6.5.4
From: Todd Zullinger @ 2009-12-03 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Andreas Schwab, git
In-Reply-To: <7vfx7r4we7.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]
Junio C Hamano wrote:
> This is what I plan to use.
>
> -- >8 ---
> From: Junio C Hamano <gitster@pobox.com>
> Date: Thu, 3 Dec 2009 11:12:32 -0800
> Subject: [PATCH] Documentation: xmlto 0.0.18 does not know --stringparam
>
> Newer DocBook stylesheets want man.base.url.for.relative.links
> parameter set when formatting manpages with external references
> to turn them into full URLs, and leave a helpful "you should
> set this parameter" message in the output. Earlier we added
> the MAN_BASE_URL make variable to specify the value for it.
>
> When MAN_BASE_URL is not given, it ought to be safe to set the
> parameter to empty; it would result in an empty leading path for
> older stylesheets that ignore the parameter, and newer ones
> would produce the same "relative URL" without the message.
>
> Unfortunately, older xmlto (at least version 0.0.18 released in
> early 2004 that comes with RHEL/CentOS 5) does not understand
> the --stringparam command line option, so we cannot add the
> parameter definition unconditionally to the command line. Work
> it around by passing the parameter only when set.
Is it worth sidestepping the xmlto part entirely? If we set this
directly in a .xsl file, it will work on older systems without any
effort. Then we can default MAN_BASE_URL to something and let distro
packagers override it.
I tested with this in Documentation/manpage-base.xsl on a CentOS 5 box
and it builds fine, leaving no cruft in the man pages regarding the
man.base.url...
<!-- set a base URL for relative links -->
<xsl:param name="man.base.url.for.relative.links"
>/path/to/git/docs</xsl:param>
Of course, the relative links looked just like they did in older
docbook releases:
1. Everyday Git
everyday.html
Is it worth the effort to have @@MAN_BASE_URL@@ in
Documentation/manpage-base.xsl or similar and replace it at build
time?
--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Just because everything is different doesn't mean anything has
changed.
-- Irene Peter
[-- Attachment #2: Type: application/pgp-signature, Size: 542 bytes --]
^ permalink raw reply
* svn svn returning 'fatal: Not a valid object name' on sourceforge svn repo
From: Stephen Bannasch @ 2009-12-03 21:00 UTC (permalink / raw)
To: git
I use git svn often and normally it works fine.
I getting a fatal error trying to clone the asciimathm svn repo at sourceforge:
$ git svn clone --trunk=trunk --branches=branches http://asciimathml.svn.sourceforge.net/svnroot/asciimathml asciimathml-svn-git
fatal: Not a valid object name
ls-tree -z ./: command returned error: 128
Anybody seen this kind of problem before.
A svn co works fine.
I'm using git version 1.6.5.1 on mac os 10.5.8.
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.6.5.4
From: Junio C Hamano @ 2009-12-03 20:48 UTC (permalink / raw)
To: Todd Zullinger; +Cc: Michael J Gruber, Andreas Schwab, git
In-Reply-To: <20091203202738.GP23717@inocybe.localdomain>
Todd Zullinger <tmz@pobox.com> writes:
>> Either we require 0.0.20 or we revert the tip one on this topic. I
>> think the latter is a safe thing to do.
>
> That sounds good to me. I'd like to get the EPEL builds for
> RHEL/CentOS updated sometime soon, as they're currently still on
> 1.5.5.6 and that lacks too many of the great improvements in newer git
> releases. Not having to patch for building the docs is one less thing
> to worry about.
This is what I plan to use.
-- >8 ---
From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 3 Dec 2009 11:12:32 -0800
Subject: [PATCH] Documentation: xmlto 0.0.18 does not know --stringparam
Newer DocBook stylesheets want man.base.url.for.relative.links
parameter set when formatting manpages with external references
to turn them into full URLs, and leave a helpful "you should
set this parameter" message in the output. Earlier we added
the MAN_BASE_URL make variable to specify the value for it.
When MAN_BASE_URL is not given, it ought to be safe to set the
parameter to empty; it would result in an empty leading path for
older stylesheets that ignore the parameter, and newer ones
would produce the same "relative URL" without the message.
Unfortunately, older xmlto (at least version 0.0.18 released in
early 2004 that comes with RHEL/CentOS 5) does not understand
the --stringparam command line option, so we cannot add the
parameter definition unconditionally to the command line. Work
it around by passing the parameter only when set.
If you do not have a suitable URL prefix, you can pass a quoted empty
string to it, like so:
$ make MAN_BASE_URL='""'
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/Makefile | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index d4c05ca..1c9dfce 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -108,7 +108,14 @@ endif
# use MAN_BASE_URL=http://www.kernel.org/pub/software/scm/git/docs/
# but distros may want to set it to /usr/share/doc/git-core/docs/ or
# something like that.
+#
+# As older stylesheets simply ignore this parameter, it ought to be
+# safe to set it to empty string when the base URL is not specified,
+# but unfortunately we cannot do so unconditionally because at least
+# xmlto 0.0.18 is reported to lack --stringparam option.
+ifdef MAN_BASE_URL
XMLTO_EXTRA += --stringparam man.base.url.for.relative.links=$(MAN_BASE_URL)
+endif
# If your target system uses GNU groff, it may try to render
# apostrophes as a "pretty" apostrophe using unicode. This breaks
--
1.6.6.rc1.5.ge21a85
^ permalink raw reply related
* [StGit PATCH v3 1/6] stg mail: Refactor __send_message and friends
From: Alex Chiang @ 2009-12-03 20:46 UTC (permalink / raw)
To: Karl Wiberg; +Cc: catalin.marinas, git
In-Reply-To: <b8197bcb0912012253l399bb542sab141021e7ff6353@mail.gmail.com>
Instead of passing all the various smtp* args to __send_message
individually, let's just pass the options list instead.
The main motivation is for future patches. The end goal is to
thin out stg mail's implementation and make it a minimal wrapper
around git send-email. By passing the options list to __send_message
we prepare to pass options directly to git send-email.
As a bonus, this change results in a cleaner internal API.
Finally, it also pushes the smtp logic where it belongs, viz. into
__send_message_smtp, instead of cluttering up the main body of
mail.func().
Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---
Catalin,
This is the only patch in the series that changed, so no sense in
sending out all the others.
stgit/commands/mail.py | 43 +++++++++++++++++++------------------------
1 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index abd42e4..777ee36 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -190,10 +190,20 @@ def __send_message_sendmail(sendmail, msg):
cmd = sendmail.split()
Run(*cmd).raw_input(msg).discard_output()
-def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
- smtpuser, smtppassword, use_tls):
+def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
"""Send the message using the given SMTP server
"""
+ smtppassword = options.smtp_password or config.get('stgit.smtppassword')
+ smtpuser = options.smtp_user or config.get('stgit.smtpuser')
+ smtpusetls = options.smtp_tls or config.get('stgit.smtptls') == 'yes'
+
+ if (smtppassword and not smtpuser):
+ raise CmdException('SMTP password supplied, username needed')
+ if (smtpusetls and not smtpuser):
+ raise CmdException('SMTP over TLS requested, username needed')
+ if (smtpuser and not smtppassword):
+ smtppassword = getpass.getpass("Please enter SMTP password: ")
+
try:
s = smtplib.SMTP(smtpserver)
except Exception, err:
@@ -203,7 +213,7 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
try:
if smtpuser and smtppassword:
s.ehlo()
- if use_tls:
+ if smtpusetls:
if not hasattr(socket, 'ssl'):
raise CmdException, "cannot use TLS - no SSL support in Python"
s.starttls()
@@ -218,17 +228,17 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
s.quit()
-def __send_message(smtpserver, from_addr, to_addr_list, msg,
- smtpuser, smtppassword, use_tls):
+def __send_message(from_addr, to_addr_list, msg, options):
"""Message sending dispatcher.
"""
+ smtpserver = options.smtp_server or config.get('stgit.smtpserver')
+
if smtpserver.startswith('/'):
# Use the sendmail tool
__send_message_sendmail(smtpserver, msg)
else:
# Use the SMTP server (we have host and port information)
- __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
- smtpuser, smtppassword, use_tls)
+ __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options)
def __build_address_headers(msg, options, extra_cc = []):
"""Build the address headers and check existing headers in the
@@ -543,8 +553,6 @@ def func(parser, options, args):
"""Send the patches by e-mail using the patchmail.tmpl file as
a template
"""
- smtpserver = options.smtp_server or config.get('stgit.smtpserver')
-
applied = crt_series.get_applied()
if options.all:
@@ -564,17 +572,6 @@ def func(parser, options, args):
raise CmdException, 'Cannot send empty patch "%s"' % p
out.done()
- smtppassword = options.smtp_password or config.get('stgit.smtppassword')
- smtpuser = options.smtp_user or config.get('stgit.smtpuser')
- smtpusetls = options.smtp_tls or config.get('stgit.smtptls') == 'yes'
-
- if (smtppassword and not smtpuser):
- raise CmdException, 'SMTP password supplied, username needed'
- if (smtpusetls and not smtpuser):
- raise CmdException, 'SMTP over TLS requested, username needed'
- if (smtpuser and not smtppassword):
- smtppassword = getpass.getpass("Please enter SMTP password: ")
-
total_nr = len(patches)
if total_nr == 0:
raise CmdException, 'No patches to send'
@@ -616,8 +613,7 @@ def func(parser, options, args):
out.stdout_raw(msg_string + '\n')
else:
out.start('Sending the cover message')
- __send_message(smtpserver, from_addr, to_addr_list, msg_string,
- smtpuser, smtppassword, smtpusetls)
+ __send_message(from_addr, to_addr_list, msg_string, options)
time.sleep(sleep)
out.done()
@@ -648,8 +644,7 @@ def func(parser, options, args):
out.stdout_raw(msg_string + '\n')
else:
out.start('Sending patch "%s"' % p)
- __send_message(smtpserver, from_addr, to_addr_list, msg_string,
- smtpuser, smtppassword, smtpusetls)
+ __send_message(from_addr, to_addr_list, msg_string, options)
# give recipients a chance of receiving related patches in the
# correct order.
if patch_nr < total_nr:
^ permalink raw reply related
* Re: [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: Junio C Hamano @ 2009-12-03 20:37 UTC (permalink / raw)
To: y; +Cc: git, Matthieu Moy
In-Reply-To: <1259784061-25143-1-git-send-email-y>
y@imag.fr writes:
> diff --git a/cache.h b/cache.h
> index 0e69384..5c8cb5f 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)
> #define DEFAULT_ABBREV 7
>
> extern int get_sha1(const char *str, unsigned char *sha1);
> -extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
> +static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
> +{
> + return get_sha1_with_mode_1(str, sha1, mode, 0, NULL);
> +}
> +extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix);
Do I understand correctly that "fatal" here is the same as "!gently"
elsewhere in the API?
> diff --git a/sha1_name.c b/sha1_name.c
> index 44bb62d..030e2ac 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -804,7 +804,77 @@ int get_sha1(const char *name, unsigned char *sha1)
> return get_sha1_with_mode(name, sha1, &unused);
> }
>
> -int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
> +static void diagnose_invalid_sha1_path(const char *prefix,
> + const char *filename,
> + const char *tree_sha1,
> + const char *object_name)
> +{
> + struct stat st;
> + unsigned char sha1[20];
> + unsigned mode;
> +
> + if (!prefix)
> + prefix = "";
> +
> + if (!lstat(filename, &st))
> + die("Path '%s' exists on disk, but not in '%s'.",
> + filename, object_name);
> + if (errno == ENOENT || errno == ENOTDIR) {
> + char *fullname = malloc(strlen(filename)
> + + strlen(prefix) + 1);
> + strcpy(fullname, prefix);
> + strcat(fullname, filename);
What if malloc fails here (and elsewhere in your patch)?
> + if (!get_tree_entry(tree_sha1, fullname,
> + sha1, &mode)) {
> + die("Path '%s' exists, but not '%s'.\n"
> + "Did you mean '%s:%s'?",
> + fullname,
> + filename,
> + object_name,
> + fullname);
> + }
> + die("Path '%s' does not exist in '%s'",
> + filename, object_name);
> + }
> +}
> +
> +static void diagnose_invalid_index_path(int stage,
> + const char *prefix,
> + const char *filename)
> +{
> + struct stat st;
> +
> + if (!prefix)
> + prefix = "";
> +
> + if (!lstat(filename, &st))
> + die("Path '%s' exists on disk, but not in the index.", filename);
> + if (errno == ENOENT || errno == ENOTDIR) {
> + struct cache_entry *ce;
> + int pos;
> + int namelen = strlen(filename) + strlen(prefix);
> + char *fullname = malloc(namelen + 1);
> + strcpy(fullname, prefix);
> + strcat(fullname, filename);
> + pos = cache_name_pos(fullname, namelen);
> + if (pos < 0)
> + pos = -pos - 1;
> + ce = active_cache[pos];
> + if (ce_namelen(ce) == namelen &&
> + !memcmp(ce->name, fullname, namelen))
> + die("Path '%s' is in the index, but not '%s'.\n"
> + "Did you mean ':%d:%s'?",
> + fullname, filename,
> + stage, fullname);
What happens if the user asked for ":2:Makefile" while in directory "t/",
and there is ":1:t/Makefile" but not ":2:t/Makefile" in the index?
What should happen if the user asked for ":2:t/Makefile" in such a case?
> @@ -850,6 +920,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
> }
> pos++;
> }
> + if (fatal)
> + diagnose_invalid_index_path(stage, prefix, cp);
> return -1;
> }
> for (cp = name, bracket_depth = 0; *cp; cp++) {
> @@ -862,9 +934,24 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
> }
> if (*cp == ':') {
> unsigned char tree_sha1[20];
> - if (!get_sha1_1(name, cp-name, tree_sha1))
> - return get_tree_entry(tree_sha1, cp+1, sha1,
> - mode);
> + char *object_name;
> + if (fatal) {
> + object_name = malloc(cp-name+1);
Where is this freed?
Instead of doing a leaky allocation, it may make sense to pass the tree
object name as <const char *, size_t> pair, and print it with "%.*s" in
the error reporting codepath. After all, object_name is used only for
that purpose in diagnose_invalid_sha1_path(), no?
> + strncpy(object_name, name, cp-name);
> + object_name[cp-name] = '\0';
> + }
> + if (!get_sha1_1(name, cp-name, tree_sha1)) {
> + const char *filename = cp+1;
> + ret = get_tree_entry(tree_sha1, filename, sha1, mode);
> + if (fatal)
> + diagnose_invalid_sha1_path(prefix, filename,
> + tree_sha1, object_name);
> +
> + return ret;
> + } else {
> + if (fatal)
> + die("Invalid object name '%s'.", object_name);
> + }
> }
> return ret;
> }
> diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
> new file mode 100755
> index 0000000..8112d56
> --- /dev/null
> +++ b/t/t1506-rev-parse-diagnosis.sh
> @@ -0,0 +1,67 @@
> +#!/bin/sh
> +
> +test_description='test git rev-parse diagnosis for invalid argument'
> +
> +exec </dev/null
> +
> +. ./test-lib.sh
> +
> +HASH_file=
> +
> +test_expect_success 'set up basic repo' '
> + echo one > file.txt &&
> + mkdir subdir &&
> + echo two > subdir/file.txt &&
> + echo three > subdir/file2.txt &&
> + git add . &&
> + git commit -m init &&
> + echo four > index-only.txt &&
> + git add index-only.txt &&
> + echo five > disk-only.txt
> +'
> +
> +test_expect_success 'correct file objects' '
> + HASH_file=$(git rev-parse HEAD:file.txt) &&
> + git rev-parse HEAD:subdir/file.txt &&
> + git rev-parse :index-only.txt &&
> + cd subdir &&
> + git rev-parse HEAD:file.txt &&
> + git rev-parse HEAD:subdir/file2.txt &&
> + test $HASH_file = $(git rev-parse HEAD:file.txt) &&
> + test $HASH_file = $(git rev-parse :file.txt) &&
> + test $HASH_file = $(git rev-parse :0:file.txt) &&
> + cd ..
> +'
Please make it a habit of not doing "cd" without forcing a subprocess
using (). If 'rev-parse HEAD:file.txt' fails after "cd subdir", the next
test will start running from that directory.
> +test_expect_success 'incorrect revision id' '
> + test_must_fail git rev-parse foobar:file.txt 2>&1 |
> + grep "Invalid object name '"'"'foobar'"'"'." &&
It always is better to write this in separate steps, because exit status
of the upstream of pipe is discarded by the shell.
If you expect an error exit and want to make sure a particular error
message is given, do this:
test_must_fail git rev-parse foobar:file.txt 2>error &&
grep "Invalid ..." error
If you expect an error exit and want to make sure an incorrect error
message is not produced, do this:
test_must_fail git rev-parse foobar:file.txt 2>error &&
! grep "Invalid ..." error
^ 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