Git development
 help / color / mirror / Atom feed
* Re: [LIBGIT2 PATCH] git_odb_open ckeck for valid path to database
From: Ramsay Jones @ 2009-12-03 18:54 UTC (permalink / raw)
  To: Esben Mose Hansen; +Cc: ae, git
In-Reply-To: <200911301037.53512.kde@mosehansen.dk>

Esben Mose Hansen wrote:
> On Tuesday 10 November 2009 22:07:04 Esben Mose Hansen wrote:
>> On Monday 09 November 2009 20:16:15 Ramsay Jones wrote:
>>
>> I have made 2 new patchsets: One
> 
> Did these get lost in transit? Or am I going about it in the wrong way? :)

Oh, sorry, I didn't notice that this was sent to me! *ahem*
(I thought/expected them to be addressed to Andreas)

Maybe you could resend the first patch, inline rather than as an attachment,
addressed to Andreas (ae@op5.com) so that he can comment on them (or commit
them).

ATB,
Ramsay Jones

^ permalink raw reply

* [RFC PATCH 2/2] MSVC: Fix an "incompatible pointer types" compiler warning
From: Ramsay Jones @ 2009-12-03 18:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list


In particular, the following warning is issued while compiling
compat/msvc.c:

    ...mingw.c(223) : warning C4133: 'function' : incompatible \
types - from '_stati64 *' to '_stat64 *'

which relates to a call of _fstati64() in the mingw_fstat()
function definition.

This is caused by various layers of macro magic and attempts to
avoid macro redefinition compiler warnings. For example, the call
to _fstati64() mentioned above is actually a call to _fstat64(),
since macro _USE_32BIT_TIME_T is not defined, and expects a pointer
to a struct _stat64 rather than the struct _stati64 which is passed
to mingw_fstat().

The definition of struct _stati64 given in compat/msvc.h had the
same "shape" as the definition of struct _stat64, so the call to
_fstat64() does not actually cause any runtime errors, but the
structure types are indeed incompatible. Also, the "shape" of
struct _stati64 changes, depending on the _USE_32BIT_TIME_T
macro, since the time_t type is defined as either __time64_t or
__time32_t.

When _USE_32BIT_TIME_T is defined, the call to _fstati64() is
actually a call to _fstat32i64() and expects a struct _stat32i64
pointer parameter. (struct _stati64 would again have the same
"shape" as struct _stat32i64).

The _USE_32BIT_TIME_T macro, along with all of the additional
structure type definitions, function definitions, and overloading
macro magic was introduced in msvc 2005.

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.
Note that the original mingw code should work with an msvc/sdk
prior to 2005. We attempt to detect this by checking for _stati64
being defined as a macro and, if not defined, conditionally
compiling the original code.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

The first version of this patch was much simpler; the diffstat showed
a net decrease of 15 lines of code! The extra fat comes from additions
to the compat/mingw.h file. The original change looked like this (along
with the identical change to compat/msvc.h):

    @@ -175,13 +175,21 @@ int mingw_getpagesize(void);
      * mingw_fstat() instead of fstat() on Windows.
      */
     #define off_t off64_t
    -#define stat _stati64
     #define lseek _lseeki64
    +#if defined(_MSC_VER)
    +#define stat _stat64
    +#else
    +#define stat _stati64
    +#endif
     int mingw_lstat(const char *file_name, struct stat *buf);
     int mingw_fstat(int fd, struct stat *buf);
     #define fstat mingw_fstat
     #define lstat mingw_lstat
    +#if defined(_MSC_VER)
    +#define _stat64(x,y) mingw_lstat(x,y)
    +#else
     #define _stati64(x,y) mingw_lstat(x,y)
    +#endif
 
     int mingw_utime(const char *file_name, const struct utimbuf *times);
     #define utime mingw_utime

This works with my version of msvc/sdk, provided we have no need to compile
with -D_USE_32BIT_TIME_T. (I was a little concerned when I noticed that the
time_t type was 64-bits; I checked a few of the obvious places to see if this
causes any breakage, but didn't find any).

Also, I added the "&& defined(_stati64)" in the hope that it would work with
older msvc/sdk versions.

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?
    - I *think* this will work with older msvc, but I can't test it!
    - 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 ;-)

ATB,
Ramsay Jones

 compat/mingw.h |   27 ++++++++++++++++++++++++++-
 compat/msvc.h  |   25 +------------------------
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 5b5258b..98d233b 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -175,14 +175,39 @@ int mingw_getpagesize(void);
  * mingw_fstat() instead of fstat() on Windows.
  */
 #define off_t off64_t
-#define stat _stati64
 #define lseek _lseeki64
+
+#if defined(_MSC_VER) && defined(_stati64)
+
+# if defined(_USE_32BIT_TIME_T)
+#  define stat _stat32i64
+# else
+#  define stat _stat64
+# endif
+
+  int mingw_lstat(const char *file_name, struct stat *buf);
+  int mingw_fstat(int fd, struct stat *buf);
+
+# define fstat mingw_fstat
+# define lstat mingw_lstat
+
+# if defined(_USE_32BIT_TIME_T)
+#  define _stat32i64(x,y) mingw_lstat(x,y)
+# else
+#  define _stat64(x,y) mingw_lstat(x,y)
+# endif
+
+#else  /* !defined(_MSC_VER) || !defined(_stati64) */
+
+#define stat _stati64
 int mingw_lstat(const char *file_name, struct stat *buf);
 int mingw_fstat(int fd, struct stat *buf);
 #define fstat mingw_fstat
 #define lstat mingw_lstat
 #define _stati64(x,y) mingw_lstat(x,y)
 
+#endif
+
 int mingw_utime(const char *file_name, const struct utimbuf *times);
 #define utime mingw_utime
 
diff --git a/compat/msvc.h b/compat/msvc.h
index 9c753a5..c099fe0 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -21,30 +21,7 @@ static __inline int strcasecmp (const char *s1, const char *s2)
 }
 
 #undef ERROR
-#undef stat
-#undef _stati64
+
 #include "compat/mingw.h"
-#undef stat
-#define stat _stati64
-#define _stat64(x,y) mingw_lstat(x,y)
 
-/*
-   Even though _stati64 is normally just defined at _stat64
-   on Windows, we specify it here as a proper struct to avoid
-   compiler warnings about macro redefinition due to magic in
-   mingw.h. Struct taken from ReactOS (GNU GPL license).
-*/
-struct _stati64 {
-	_dev_t  st_dev;
-	_ino_t  st_ino;
-	unsigned short st_mode;
-	short   st_nlink;
-	short   st_uid;
-	short   st_gid;
-	_dev_t  st_rdev;
-	__int64 st_size;
-	time_t  st_atime;
-	time_t  st_mtime;
-	time_t  st_ctime;
-};
 #endif
-- 
1.6.5

^ permalink raw reply related

* Running commands in wrong environment
From: Marinescu Paul dan @ 2009-12-03 19:19 UTC (permalink / raw)
  To: git@vger.kernel.org


git's start_command (run_command.c) executes a command (e.g. hook) but does not verify that it has properly set up the environment. It seems that in the unlikely case where putenv (run_command.c:117) fails, the command may have undesirable effects e.g. GIT_INDEX_FILE should have been set (interactive pre-commit hooks) but the default index will be used instead. It would be safer not to run the command but just exit in that case.

^ permalink raw reply

* Re: [StGit PATCH v2 0/6] add support for git send-email
From: Alex Chiang @ 2009-12-03 19:27 UTC (permalink / raw)
  To: Karl Wiberg; +Cc: catalin.marinas, git
In-Reply-To: <b8197bcb0912012246n3b83866cjb93654effc000242@mail.gmail.com>

* Karl Wiberg <kha@treskal.com>:
> On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:
> > The upshot is that in stg mail, --git and --mbox don't interact
> > well, and the resulting mbox file will lack the recipients. This
> > might be fixed in the future if we teach git send-email how to
> > generate mbox files, but then we introduce a versioning problem.
> 
> One wild idea: git send-email's --smtp-server flag will accept the
> (full) path of a sendmail program; writing such a program, just
> capable enough to receive the outgoing emails and dumping them to a
> file, should be easy. Another option would be a program that speaks
> just enough SMTP to accept the mails. (Incidentally, these two would
> be useful in testing stg mail even without the --git option.)
 
Hm, I think this is getting to be a bit of overkill. I could see
adding --mbox support to git send-email as being a better use of
time (IMO).

> I fully understand if you'd rather get on with scratching your actual
> itch, though ...
 
:)

> > So let's just accept this wart for now, and say, if you want an mbox
> > file generated, don't use --git. That seems reasonable to me.
> 
> Sure.

Thanks,
/ac

^ permalink raw reply

* Re: [StGit PATCH v2 1/6] stg mail: Refactor __send_message and friends
From: Alex Chiang @ 2009-12-03 19:27 UTC (permalink / raw)
  To: Karl Wiberg; +Cc: catalin.marinas, git
In-Reply-To: <b8197bcb0912012253l399bb542sab141021e7ff6353@mail.gmail.com>

* Karl Wiberg <kha@treskal.com>:
> On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:
> 
> > +    if (smtppassword and not smtpuser):
> > +        raise Exception('SMTP password supplied, username needed')
> > +    if (smtpusetls and not smtpuser):
> > +        raise Exception('SMTP over TLS requested, username needed')
> > +    if (smtpuser and not smtppassword):
> > +        smtppassword = getpass.getpass("Please enter SMTP password: ")
> 
> Sorry if I confused you with my earlier explanation; I only meant that
> you should use the _form_ "raise Exception('message')", not that you
> should change the exception type from CmdException to Exception. If
> you try to trigger these errors, I think you'll find that in the case
> of CmdException, StGit will print just the message and exit with an
> error; whereas for straight Exception, it'll print the full backtrace
> as well under the assumption that it's a program bug.

Ah, ok. Will update.

/ac

^ permalink raw reply

* Re: [StGit PATCH v2 3/6] stg mail: make __send_message do more
From: Alex Chiang @ 2009-12-03 19:30 UTC (permalink / raw)
  To: Karl Wiberg; +Cc: catalin.marinas, git
In-Reply-To: <b8197bcb0912012303i3bd1061fhdb391de096996a27@mail.gmail.com>

* Karl Wiberg <kha@treskal.com>:
> Just pointing out a couple of Python tricks you might've wanted to
> use. No need to update the patch, though.
> 
> On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:
> 
> > +        (patch_nr, total_nr) = (args[1], args[2])
> 
> Can be written as
> 
>   (patch_nr, total_nr) = args[1:3]

Thanks, I did it this way.

> or, if args[2] is the last element of the list (which it isn't in this
> case?),
> 
>   (patch_nr, total_nr) = args[1:]

No, ref_id is the last arg, so that won't work.

> > +    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?

Thanks,
/ac

^ permalink raw reply

* Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports
From: Shawn O. Pearce @ 2009-12-03 19:42 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: git
In-Reply-To: <20091202201008.GB11301@Knoppix>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> On Wed, Dec 02, 2009 at 09:04:57AM -0800, Shawn O. Pearce wrote:
> > Modify transport-helper.c to allow pushing TRANS_OPT_UPLOADPACK and
> > TRANS_OPT_RECEIVEPACK down into the helper via the option capability.
> 
> NAK. Modified _process_connect_or_invoke (now _process_connect) to pass
> new option that appiles to connecting all subprotocols (if needed).
...
> And from helper POV, all subprotocols should appear identical from
> layer 6 POV so it doesn't make sense to diffrentiate between path
> for upload-pack and receive-pack (or upload-archive!).

That may be true, but the remote.origin.uploadpack and
remote.origin.receivepack configuration options exist
and are passed through as these option uploadpack and
option receivepack callbacks.

If you want to pass through a single option with the remote program
name, you need to do that immediately before the connect invoke
occurs, and instead buffer the two different configuration options
in the struct transport.  I think that would make the series messier
than it is.
 
-- 
Shawn.

^ permalink raw reply

* installation problems with version 1.6.5.4
From: Oliver Kullmann @ 2009-12-03 20:09 UTC (permalink / raw)
  To: git

Hi,

I get when installing 1.6.5.4

    GEN technical/api-index.txt
    ASCIIDOC technical/api-index.html
    ASCIIDOC git-add.xml
    XMLTO git-add.1
xmlto: unrecognised option `--stringparam'
usage: xmlto [OPTION]... FORMAT XML
OPTIONs are:
  -v              verbose output (-vv for very verbose)
  -x stylesheet   use the specified stylesheet instead of choosing one
  -m fragment     use the XSL fragment to customize the stylesheet
  -o directory    put output in the specified directory instead of
                  the current working directory
  -p postprocopts pass option to postprocessor
  --extensions    turn on stylesheet extensions for this tool chain
  --searchpath    colon-separated list of fallback directories
  --skip-validation
                  do not attempt to validate the input before processing


where

> xmlto --version
xmlto version 0.0.18

Couldn't find something on versions of xmlto required?

Oliver

^ permalink raw reply

* [PATCH] cvsserver: make the output of 'update' more compatible with cvs.
From: Sergei Organov @ 2009-12-03 20:12 UTC (permalink / raw)
  To: git; +Cc: gitster

Native cvs update outputs the string "cvs update: Updating <DIR>" for
every directory it processes (to stderr). This is used, e.g., by emacs
pcl-cvs to split files by directory. This commit implements this
feature in cvsserver.
---
 git-cvsserver.perl |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 6dc45f5..5994951 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -981,10 +981,22 @@ sub req_update
 
     #$log->debug("update state : " . Dumper($state));
 
+    my $last_dirname = "///";
+    
     # foreach file specified on the command line ...
     foreach my $filename ( @{$state->{args}} )
     {
         $filename = filecleanup($filename);
+        my $cur_dirname = dirname($filename);
+        if ( $cur_dirname ne $last_dirname )
+        {
+            $last_dirname = $cur_dirname;
+            if ( $cur_dirname eq "" )
+            {
+                $cur_dirname = ".";
+            }
+            print "E cvs update: Updating $cur_dirname\n";
+        }
 
         $log->debug("Processing file $filename");
 
-- 
1.6.6.rc0.67.g68b144.dirty

^ permalink raw reply related

* Re: installation problems with version 1.6.5.4
From: Todd Zullinger @ 2009-12-03 20:22 UTC (permalink / raw)
  To: Oliver Kullmann; +Cc: git
In-Reply-To: <20091203200921.GA29478@cs-wsok.swansea.ac.uk>

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

Oliver Kullmann wrote:
> I get when installing 1.6.5.4
>
>     GEN technical/api-index.txt
>     ASCIIDOC technical/api-index.html
>     ASCIIDOC git-add.xml
>     XMLTO git-add.1
> xmlto: unrecognised option `--stringparam'

See http://thread.gmane.org/gmane.linux.kernel/921919/focus=134449 and
follow-ups.

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fleas can be taught nearly anything that a Congressman can.
    -- Mark Twain


[-- Attachment #2: Type: application/pgp-signature, Size: 542 bytes --]

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.6.5.4
From: Todd Zullinger @ 2009-12-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, Andreas Schwab, git
In-Reply-To: <7viqco54xh.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

Junio C Hamano wrote:
> I did the second one after seeing that both my Debian box and the
> k.org machine that manpages tarballs are made (FC11 IIRC) had that
> option, and my impression has been that it usually is safe to say
> "even Debian has it, and there wouldn't be many things older than
> that distro", but that is apparently not true.

Heh, that surely used to be a very good rule of thumb.  Perhaps these
days a rule of whichever is older Debian stable or current RHEL/CentOS
would suffice?

> 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.

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Rowe's Rule: The odds are five to six that the light at the end of the
tunnel is the headlight of an oncoming train.
    -- Paul Dickson


[-- Attachment #2: Type: application/pgp-signature, Size: 542 bytes --]

^ permalink raw reply

* 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

* [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: [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

* 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: 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

* 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

* [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: [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

* 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

* 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: [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

* 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] 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: 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox