* [PATCH 2/3] read-cache: reduce malloc/free during writing index
From: Nguyễn Thái Ngọc Duy @ 2012-02-05 8:30 UTC (permalink / raw)
To: git; +Cc: Joshua Redstone, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328430605-4566-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
read-cache.c | 26 ++++++++++++++++++--------
1 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 2dbf923..7b9a989 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1521,12 +1521,19 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
}
}
-static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)
+static int ce_prepare_ondisk_entry(struct cache_entry *ce,
+ void **ondisk_p, int *ondisk_size)
{
int size = ondisk_ce_size(ce);
- struct ondisk_cache_entry *ondisk = xcalloc(1, size);
+ struct ondisk_cache_entry *ondisk;
char *name;
- int result;
+
+ if (size <= *ondisk_size)
+ ondisk = *ondisk_p;
+ else {
+ ondisk = *ondisk_p = xrealloc(*ondisk_p, size);
+ *ondisk_size = size;
+ }
ondisk->ctime.sec = htonl(ce->ce_ctime.sec);
ondisk->mtime.sec = htonl(ce->ce_mtime.sec);
@@ -1549,10 +1556,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)
else
name = ondisk->name;
memcpy(name, ce->name, ce_namelen(ce));
-
- result = ce_write(c, fd, ondisk, size);
- free(ondisk);
- return result;
+ return size;
}
static int has_racy_timestamp(struct index_state *istate)
@@ -1588,6 +1592,8 @@ int write_index(struct index_state *istate, int newfd)
struct cache_entry **cache = istate->cache;
int entries = istate->cache_nr;
struct stat st;
+ void *ce_ondisk = NULL;
+ int ce_ondisk_size = 0;
for (i = removed = extended = 0; i < entries; i++) {
if (cache[i]->ce_flags & CE_REMOVE)
@@ -1612,13 +1618,17 @@ int write_index(struct index_state *istate, int newfd)
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
+ int size;
+
if (ce->ce_flags & CE_REMOVE)
continue;
if (!ce_uptodate(ce) && is_racy_timestamp(istate, ce))
ce_smudge_racily_clean_entry(ce);
- if (ce_write_entry(&c, newfd, ce) < 0)
+ size = ce_prepare_ondisk_entry(ce, &ce_ondisk, &ce_ondisk_size);
+ if (ce_write(&c, newfd, ce_ondisk, size) < 0)
return -1;
}
+ free(ce_ondisk);
/* Write extension data here */
if (istate->cache_tree) {
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 3/3] Support compressing index when GIT_ZCACHE=1
From: Nguyễn Thái Ngọc Duy @ 2012-02-05 8:30 UTC (permalink / raw)
To: git; +Cc: Joshua Redstone, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328430605-4566-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 1 +
read-cache.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 112 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 10afd71..112bc52 100644
--- a/cache.h
+++ b/cache.h
@@ -99,6 +99,7 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long);
*/
#define CACHE_SIGNATURE 0x44495243 /* "DIRC" */
+#define ZCACHE_SIGNATURE 0x4452435A /* "DRCZ" */
struct cache_header {
unsigned int hdr_signature;
unsigned int hdr_version;
diff --git a/read-cache.c b/read-cache.c
index 7b9a989..45c1712 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1182,12 +1182,17 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall
return refresh_cache_ent(&the_index, ce, really, NULL, NULL);
}
-static int verify_hdr(struct cache_header *hdr, unsigned long size)
+static int verify_hdr(struct cache_header *hdr, unsigned long size,
+ int *deflated)
{
git_SHA_CTX c;
unsigned char sha1[20];
- if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
+ if (hdr->hdr_signature == htonl(CACHE_SIGNATURE))
+ *deflated = 0;
+ else if (hdr->hdr_signature == htonl(ZCACHE_SIGNATURE))
+ *deflated = 1;
+ else
return error("bad signature");
if (hdr->hdr_version != htonl(2) && hdr->hdr_version != htonl(3))
return error("bad index version");
@@ -1273,6 +1278,43 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk)
return ce;
}
+static int inflate_cache_entries(struct index_state *istate,
+ const unsigned char *mmap, size_t mmap_size,
+ unsigned long src_offset)
+{
+ unsigned char buf[sizeof(struct ondisk_cache_entry) + PATH_MAX];
+ struct ondisk_cache_entry *disk_ce;
+ struct cache_entry *ce;
+ struct git_zstream stream;
+ int i, status;
+
+ memset(&stream, 0, sizeof(stream));
+ stream.next_in = (unsigned char*)mmap + src_offset;
+ stream.avail_in = mmap_size - src_offset;
+ stream.next_out = buf;
+ stream.avail_out = sizeof(buf);
+ git_inflate_init(&stream);
+
+ for (i = 0; i < istate->cache_nr; i++) {
+ int remaining;
+ do {
+ status = git_inflate(&stream, Z_FINISH);
+ } while (status == Z_OK);
+
+ disk_ce = (struct ondisk_cache_entry *)buf;
+ ce = create_from_disk(disk_ce);
+ set_index_entry(istate, i, ce);
+
+ remaining = stream.next_out - (buf + ondisk_ce_size(ce));
+ memmove(buf, buf + ondisk_ce_size(ce), remaining);
+ stream.next_out = buf + remaining;
+ stream.avail_out = sizeof(buf) - remaining;
+ }
+ assert(status == Z_STREAM_END);
+ git_inflate_end(&stream);
+ return stream.next_in - mmap;
+}
+
static int read_cache_entries(struct index_state *istate,
const char *mmap, unsigned long src_offset)
{
@@ -1300,6 +1342,7 @@ int read_index_from(struct index_state *istate, const char *path)
struct cache_header *hdr;
void *mmap;
size_t mmap_size;
+ int deflated;
errno = EBUSY;
if (istate->initialized)
@@ -1329,7 +1372,7 @@ int read_index_from(struct index_state *istate, const char *path)
die_errno("unable to map index file");
hdr = mmap;
- if (verify_hdr(hdr, mmap_size) < 0)
+ if (verify_hdr(hdr, mmap_size, &deflated) < 0)
goto unmap;
istate->cache_nr = ntohl(hdr->hdr_entries);
@@ -1337,7 +1380,11 @@ int read_index_from(struct index_state *istate, const char *path)
istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *));
istate->initialized = 1;
- src_offset = read_cache_entries(istate, mmap, sizeof(*hdr));
+ if (deflated)
+ src_offset = inflate_cache_entries(istate, mmap, mmap_size,
+ sizeof(*hdr));
+ else
+ src_offset = read_cache_entries(istate, mmap, sizeof(*hdr));
istate->timestamp.sec = st.st_mtime;
istate->timestamp.nsec = ST_MTIME_NSEC(st);
@@ -1594,6 +1641,10 @@ int write_index(struct index_state *istate, int newfd)
struct stat st;
void *ce_ondisk = NULL;
int ce_ondisk_size = 0;
+ struct git_zstream stream;
+ int deflate, status;
+ unsigned char *dbuf_out;
+ unsigned char *dbuf_in;
for (i = removed = extended = 0; i < entries; i++) {
if (cache[i]->ce_flags & CE_REMOVE)
@@ -1607,7 +1658,8 @@ int write_index(struct index_state *istate, int newfd)
}
}
- hdr.hdr_signature = htonl(CACHE_SIGNATURE);
+ deflate = getenv("GIT_ZCACHE") != NULL;
+ hdr.hdr_signature = htonl(deflate ? ZCACHE_SIGNATURE : CACHE_SIGNATURE);
/* for extended format, increase version so older git won't try to read it */
hdr.hdr_version = htonl(extended ? 3 : 2);
hdr.hdr_entries = htonl(entries - removed);
@@ -1616,6 +1668,17 @@ int write_index(struct index_state *istate, int newfd)
if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)
return -1;
+ if (deflate) {
+ dbuf_out = xmalloc(WRITE_BUFFER_SIZE);
+ dbuf_in = xmalloc(WRITE_BUFFER_SIZE);
+ memset(&stream, 0, sizeof(stream));
+ stream.next_out = dbuf_out;
+ stream.avail_out = WRITE_BUFFER_SIZE;
+ stream.next_in = dbuf_in;
+ stream.avail_in = 0;
+ git_deflate_init(&stream, zlib_compression_level);
+ }
+
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
int size;
@@ -1625,11 +1688,52 @@ int write_index(struct index_state *istate, int newfd)
if (!ce_uptodate(ce) && is_racy_timestamp(istate, ce))
ce_smudge_racily_clean_entry(ce);
size = ce_prepare_ondisk_entry(ce, &ce_ondisk, &ce_ondisk_size);
- if (ce_write(&c, newfd, ce_ondisk, size) < 0)
- return -1;
+ if (!deflate) {
+ if (ce_write(&c, newfd, ce_ondisk, size) < 0)
+ return -1;
+ continue;
+ }
+
+ if (stream.avail_in)
+ memmove(dbuf_in, stream.next_in, stream.avail_in);
+ memcpy(dbuf_in + stream.avail_in, ce_ondisk, size);
+ stream.next_in = dbuf_in;
+ stream.avail_in += size;
+ do {
+ status = git_deflate(&stream, 0);
+ if (stream.next_out > dbuf_out) {
+ size = stream.next_out - dbuf_out;
+ if (ce_write(&c, newfd, dbuf_out, size) < 0)
+ return -1;
+ stream.next_out = dbuf_out;
+ stream.avail_out = WRITE_BUFFER_SIZE;
+ }
+ } while (status == Z_OK);
}
free(ce_ondisk);
+ if (deflate) {
+ do {
+ status = git_deflate(&stream, Z_FINISH);
+ if (stream.next_out > dbuf_out) {
+ int size = stream.next_out - dbuf_out;
+ if (ce_write(&c, newfd, dbuf_out, size) < 0)
+ return -1;
+ stream.next_out = dbuf_out;
+ stream.avail_out = WRITE_BUFFER_SIZE;
+ }
+ } while (status == Z_OK);
+
+ git_deflate_end(&stream);
+ if (stream.next_out > dbuf_out) {
+ int size = stream.next_out - dbuf_out;
+ if (ce_write(&c, newfd, dbuf_out, size) < 0)
+ return -1;
+ }
+ free(dbuf_in);
+ free(dbuf_out);
+ }
+
/* Write extension data here */
if (istate->cache_tree) {
struct strbuf sb = STRBUF_INIT;
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 1/3] read-cache: factor out cache entries reading code
From: Nguyễn Thái Ngọc Duy @ 2012-02-05 8:30 UTC (permalink / raw)
To: git; +Cc: Joshua Redstone, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328430605-4566-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
read-cache.c | 32 ++++++++++++++++++++------------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index a51bba1..2dbf923 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1273,10 +1273,28 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk)
return ce;
}
+static int read_cache_entries(struct index_state *istate,
+ const char *mmap, unsigned long src_offset)
+{
+ const char *buf = mmap + src_offset;
+ int i;
+
+ for (i = 0; i < istate->cache_nr; i++) {
+ struct ondisk_cache_entry *disk_ce;
+ struct cache_entry *ce;
+
+ disk_ce = (struct ondisk_cache_entry *)buf;
+ ce = create_from_disk(disk_ce);
+ set_index_entry(istate, i, ce);
+ buf += ondisk_ce_size(ce);
+ }
+ return buf - mmap;
+}
+
/* remember to discard_cache() before reading a different cache! */
int read_index_from(struct index_state *istate, const char *path)
{
- int fd, i;
+ int fd;
struct stat st;
unsigned long src_offset;
struct cache_header *hdr;
@@ -1319,17 +1337,7 @@ int read_index_from(struct index_state *istate, const char *path)
istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *));
istate->initialized = 1;
- src_offset = sizeof(*hdr);
- for (i = 0; i < istate->cache_nr; i++) {
- struct ondisk_cache_entry *disk_ce;
- struct cache_entry *ce;
-
- disk_ce = (struct ondisk_cache_entry *)((char *)mmap + src_offset);
- ce = create_from_disk(disk_ce);
- set_index_entry(istate, i, ce);
-
- src_offset += ondisk_ce_size(ce);
- }
+ src_offset = read_cache_entries(istate, mmap, sizeof(*hdr));
istate->timestamp.sec = st.st_mtime;
istate->timestamp.nsec = ST_MTIME_NSEC(st);
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Installing git-svn on Linux without root
From: Jakub Narebski @ 2012-02-05 10:11 UTC (permalink / raw)
To: Andrew Keller; +Cc: git
In-Reply-To: <AD682311-372A-4AED-B575-E77EB862ABD8@kellerfarm.com>
Andrew Keller wrote:
> On Feb 4, 2012, at 6:32 AM, Jakub Narebski wrote:
> > Andrew Keller <andrew@kellerfarm.com> writes:
> > > So, the module does exist, but not in a location included by @INC.
> >
> > From the above error message it looks like
> >
> > /homedirs/kelleran/local/lib/perl5/site_perl/5.8.8
> >
> > is in @INC, but
> > /homedirs/kelleran/local/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi/
> >
> > is not. Strange.
> >
> > Do you use local::lib?
>
> No - Where should I use it?
local::lib is a way to install Perl modules / packages e.g. in your
home directory. Please read and follow the instructions on local::lib
manpage ("The bootstrapping technique" section):
http://search.cpan.org/~apeiron/local-lib-1.008004/lib/local/lib.pm
Then you can install Alien::SVN (or any other Perl package) using
'cpan' client (or intstall 'cpanm' / 'App::cpanminus').
HTH
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] Explicitly set X to avoid potential build breakage
From: Michael @ 2012-02-05 10:41 UTC (permalink / raw)
To: git
$X is appended to binary names for Windows builds (ie. git.exe).
Pollution from the environment can inadvertently trigger this behaviour,
resulting in 'git' turning into 'gitwhatever' without warning.
Signed-off-by: Michael <kensington@astralcloak.net>
---
Makefile | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index c457c34..380d96f 100644
--- a/Makefile
+++ b/Makefile
@@ -388,6 +388,9 @@ SCRIPT_SH =
SCRIPT_LIB =
TEST_PROGRAMS_NEED_X =
+# Binary suffix used for Windows builds
+X =
+
# Having this variable in your environment would break pipelines because
# you cause "cd" to echo its destination to stdout. It can also take
# scripts to unexpected places. If you like CDPATH, define it for your
--
1.7.8.4
^ permalink raw reply related
* Re: Git performance results on a large repository
From: David Barr @ 2012-02-05 11:24 UTC (permalink / raw)
To: david; +Cc: Joshua Redstone, git@vger.kernel.org
In-Reply-To: <alpine.DEB.2.02.1202042026280.6541@asgard.lang.hm>
On Sun, Feb 5, 2012 at 3:30 PM, <david@lang.hm> wrote:
> On Fri, 3 Feb 2012, Joshua Redstone wrote:
>
>> The test repo has 4 million commits, linear history and about 1.3 million
>> files. The size of the .git directory is about 15GB, and has been
>> repacked with 'git repack -a -d -f --max-pack-size=10g --depth=100
>> --window=250'. This repack took about 2 days on a beefy machine (I.e.,
>> lots of ram and flash). The size of the index file is 191 MB.
>
>
> This may be a silly thought, but what if instead of one pack file of your
> entire history (4 million commits) you create multiple packs (say every half
> million commits) and mark all but the most recent pack as .keep (so that
> they won't be modified by a repack)
>
> that way things that only need to worry about recent history (blame, etc)
> will probably never have to go past the most recent pack file or two
>
> I may be wrong, but I think that when git is looking for 'similar files' for
> delta compression, it limits it's search to the current pack, so this will
> also keep you from searching the entire project history.
I don't know if there is an easy way to determine with the with the
current tools
in git but one useful statistic for tuning packing performance is the
size of the
largest component in the delta-chain graph. The significance of this number is
that the product of window-size and maximum depth need not be larger than it.
I've found that with some older repositories I could have a depth as low as 3
and still get good performance from a moderate window size.
--
David Barr
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Ben Walton @ 2012-02-05 14:33 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <201202042045.54114.jnareb@gmail.com>
Excerpts from Jakub Narebski's message of Sat Feb 04 14:45:53 -0500 2012:
Hi Jakub,
These items are as much about UI as anything else, I think. UI that
better helps users to know the state of their commits and branches can
only be a good thing. People that have used git for a while and are
comfortable with it may not see the need/point of these, but I think
they could both really help new users.
> In Mercurial 2.1 there are three available phases: 'public' for
> published commits, 'draft' for local un-published commits and
> 'secret' for local un-published commits which are not meant to be
> published.
How do you envision such a feature in git?
A 'draft' commit (or chain of commits) could be determined from the
push matching definitions and then marked with simple decorations in
log output...This would extend the ability of status to note that your
are X commits ahead of foo. This would see any commit on a branch
that would be pushed automatically decorated with a 'draft' status.
> While default "push matching" behavior makes it possible to have
> "secret" commits, being able to explicitly mark commits as not for
> publishing might be a good idea also for Git.
Do you see using configuration or convention to achieve this?
For example, any branch named private/foo could, by convention, be
un-pushable without a force option? Alternately, a config item
similar to the push matching stuff to allow the users to designate
un-pushable branches could work too.
Please don't take the above implementation possibilities as anything
more than a starting point for discussion as they may be deeply
flawed. I'm just tossing a few things out there as I think this is a
good discussion to have.
Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302
^ permalink raw reply
* Fwd: Breakage in master?
From: Erik Faye-Lund @ 2012-02-05 14:46 UTC (permalink / raw)
To: bug-gnu-gettext; +Cc: msysGit, Git Mailing List
In-Reply-To: <CABPQNSbj8QKqkdY49Y7tpAOQd53t+z6Gc5U-CS0-TZWyNz1WfQ@mail.gmail.com>
Git has recently switched to using gettext for translations, and I
have observed a breakage on Windows due to the way gettext handles
vsnprintf.
On MinGW, vsnprintf and _vsnprintf are two different implementations;
vsnprintf is from MinGW-runtime, and provides a reasonably sane
implementation. _vnsprintf on the other hand is from MSVCRT.dll, and
has some issues with it's return value. Before using gettext, the
MinGW-built version of Git called the version from mingw-runtime, and
everything worked fine. When built with MSVC, a shim was used to fixup
the bogus return value. This shim was injected through a define,
similar to what gettext does.
The shim in gettext lead to issues for Git, both on MinGW and on MSVC.
For MinGW, the problem is that libintl_vsnprintf calls _vsnprintf
rather than vsnprintf, giving us the same, broken return value that we
tried to prevent. This means that our code intended to call the
MinGW-runtime version, but gettext ended up calling the MSVCRT.dll
version. I don't find this very reasonable; a call to vsnprintf ends
up as a call to _vsnprintf.
On MSVC the problem is a bit easier to spot; libgnuintl.h.in contains
the following:
---8<---
#if !(defined vsnprintf && defined _GL_STDIO_H) /* don't override gnulib */
#undef vsnprintf
#define vsnprintf libintl_vsnprintf
extern int vsnprintf (char *, size_t, const char *, va_list);
#endif
---8<---
Uhm, what? Unless we're using Gnulib, our definition of vsnprintf
should simply be ignored?
I'm not saying figuring out what to do here is exactly trivial; but I
think undefining any definitions of vsnprintf that aren't exactly
"_vsnprintf" is dangerous. The forwarded mail below contains a
quick-fix I did locally that seems to side-step the problem for me, by
not using _vsnprintf on MinGW. But perhaps there's something better we
can do?
---------- Forwarded message ----------
From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Sat, Feb 4, 2012 at 10:55 PM
Subject: Re: Breakage in master?
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>, msysGit
<msysgit@googlegroups.com>, Ævar Arnfjörð <avarab@gmail.com>
On Fri, Feb 3, 2012 at 1:28 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:
>>
>>> But here's the REALLY puzzling part: If I add a simple, unused
>>> function to diff-lib.c, like this:
>>> [...]
>>> "git status" starts to error out with that same vsnprintf complaint!
>>>
>>> ---8<---
>>> $ git status
>>> # On branch master
>>> # Changes not staged for commit:
>>> # (use "git add <file>..." to update what will be committed)
>>> fatal: BUG: your vsnprintf is broken (returned -1)
>>> ---8<---
>>
>> OK, that's definitely odd.
>>
>> At the moment of the die() in strbuf_vaddf, what does errno say?
>
> If I apply this patch:
> ---8<---
> diff --git a/strbuf.c b/strbuf.c
> index ff0b96b..52dfdd6 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char
> *fmt, va_list ap)
> len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
> va_end(cp);
> if (len < 0)
> - die("BUG: your vsnprintf is broken (returned %d)", len);
> + die_errno("BUG: your vsnprintf is broken (returned %d)", len);
> if (len > strbuf_avail(sb)) {
> strbuf_grow(sb, len);
> len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> ---8<---
>
> Then I get "fatal: BUG: your vsnprintf is broken (returned -1): Result
> too large". This goes both for both failure cases I described. I
> assume this means errno=ERANGE.
>
>> vsnprintf should generally never be returning -1 (it should return the
>> number of characters that would have been written). Since you're on
>> Windows, I assume you're using the replacement version in
>> compat/snprintf.c.
>
> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
> the MinGW target. I'm assuming that means MinGW-runtime has a sane
> vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,
> the problem occurs. And it's still "Result too large".
>
> So I decided to do a bit of stepping, and it seems libintl takes over
> vsnprintf, directing us to libintl_vsnprintf instead. I guess this is
> so it can ensure we support reordering the parameters with $1 etc...
> And aparently this vsnprintf implementation calls the system vnsprintf
> if the format string does not contain '$', and it's using _vsnprintf
> rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,
> and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.
>
> So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.
>
Indeed, I just got around to testing this, and doing this on top of
gettext seems to fix the problem for me. For the MSVC, a more
elaborate fix is needed, as it doesn't have a sane vsnprintf.
---
diff --git a/gettext-runtime/intl/printf.c b/gettext-runtime/intl/printf.c
index b7cdc5d..f55023e 100644
--- a/gettext-runtime/intl/printf.c
+++ b/gettext-runtime/intl/printf.c
@@ -192,7 +192,7 @@ libintl_sprintf (char *resultbuf, const char *format, ...)
#if HAVE_SNPRINTF
-# if HAVE_DECL__SNPRINTF
+# if HAVE_DECL__SNPRINTF && !defined(__MINGW32__)
/* Windows. */
# define system_vsnprintf _vsnprintf
# else
^ permalink raw reply related
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-05 15:05 UTC (permalink / raw)
To: Ben Walton; +Cc: git
In-Reply-To: <1328452328-sup-6643@pinkfloyd.chass.utoronto.ca>
On Sun, 5 Feb 2012, Ben Walton wrote:
> Excerpts from Jakub Narebski's message of Sat Feb 04 14:45:53 -0500 2012:
>
> Hi Jakub,
>
> These items are as much about UI as anything else, I think. UI that
> better helps users to know the state of their commits and branches can
> only be a good thing. People that have used git for a while and are
> comfortable with it may not see the need/point of these, but I think
> they could both really help new users.
As I said, 1500+ git users would like to have such feature, according
to latest Git User's Survey.
> > In Mercurial 2.1 there are three available phases: 'public' for
> > published commits, 'draft' for local un-published commits and
> > 'secret' for local un-published commits which are not meant to be
> > published.
>
> How do you envision such a feature in git?
>
> A 'draft' commit (or chain of commits) could be determined from the
> push matching definitions and then marked with simple decorations in
> log output...This would extend the ability of status to note that your
> are X commits ahead of foo. This would see any commit on a branch
> that would be pushed automatically decorated with a 'draft' status.
I think that in its basic form (treating all remotes equally) commits
in 'public' phase would be those reachable from remote-tracking branches.
Otherwise commits would be in 'draft' phase, unless explicitly marked
as 'secret' (it we implement 'secret' phase, that is).
The safety new I think of would (similarly to Mercurial phases) prevent
or warn about amending published commit, and rebasing commits which were
already published (in 'public' phase). That would require modifications
to git-commit and git-amend, I think...
Maybe even Git could refuse or warn on the local side about non
fast-forward update of public branch, to help users of third-party tools.
> > While default "push matching" behavior makes it possible to have
> > "secret" commits, being able to explicitly mark commits as not for
> > publishing might be a good idea also for Git.
>
> Do you see using configuration or convention to achieve this?
>
> For example, any branch named private/foo could, by convention, be
> un-pushable without a force option? Alternately, a config item
> similar to the push matching stuff to allow the users to designate
> un-pushable branches could work too.
I'm not sure, but the config item might be a good solution. Git would
skip publishing 'secret' commits (commits from 'secret' branch) if it
would otherwise publish it due to glob refspec, and refuse (or warn)
publishing 'secret' branches explicitly.
Currently if you use default "push matching", then those branches that
you didn't push explicitly wouldn't be pushed. But that does not prevent
pushing them by accident, and does not give UI to check if branch is
private or not (e.g. to use in git-aware shell prompt).
--
Jakub Narebski
Poland
^ permalink raw reply
* Fw: [RFD] Rewriting safety - warn before/when rewriting published history
From: Philip Oakley @ 2012-02-05 15:10 UTC (permalink / raw)
To: Git List
Oops, forget 'reply all' to the list.
Philip
----- Original Message -----
From: "Philip Oakley" <philipoakley@iee.org>
To: "Jakub Narebski" <jnareb@gmail.com>
Sent: Sunday, February 05, 2012 2:31 PM
Subject: Re: [RFD] Rewriting safety - warn before/when rewriting published
history
> From: "Jakub Narebski" <jnareb@gmail.com>
> Sent: Saturday, February 04, 2012 7:45 PM
>> Git includes protection against rewriting published history on the
>> receive side with fast-forward check by default (which can be
>> overridden) and various receive.deny* configuration variables,
>> including receive.denyNonFastForwards.
>>
>> Nevertheless git users requested (among others in Git User's Survey)
>> more help on creation side, namely preventing rewriting parts of
>> history which was already made public (or at least warning that one is
>> about to rewrite published history). The "warn before/when rewriting
>> published history" answer in "17. Which of the following features would
>> you like to see implemented in git?" multiple-choice question in latest
>> Git User's Survey 2011[1] got 24% (1525) responses.
>>
>> [1]: https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
>>
>> So people would like for git to warn them about rewriting history before
>> they attempt a push and it turns out to not fast-forward.
>>
>
> Another area that is implicitly related is that of (lack of) publication
> of sub-module updates. A mechanisms that, in the super project, knows the
> status of the (local) submodules, such as where they would be sourced
> from, i.e. what was last pushed & where, could help in such instances.
>
>>
>> What prompted this email is the fact that Mercurial includes support for
>> tracking which revisions (changesets) are safe to modify in its 2.1
>> latest version:
>>
>> http://lwn.net/Articles/478795/
>> http://mercurial.selenic.com/wiki/WhatsNew
>>
>> It does that by tracking so called "phase" of a changeset (revision).
>>
>> http://mercurial.selenic.com/wiki/Phases
>> http://mercurial.selenic.com/wiki/PhasesDevel
>>
>> http://www.logilab.org/blogentry/88203
>> http://www.logilab.org/blogentry/88219
>> http://www.logilab.org/blogentry/88259
>>
>>
>> While we don't have to play catch-up with Mercurial features, I think
>> something similar to what Mercurial has to warn about rewriting
>> published history (amend, rebase, perhaps even filter-branch) would
>> be nice to have. Perhaps even follow UI used by Mercurial, and/or
>> translating its implementation into git terms.
>>
>> In Mercurial 2.1 there are three available phases: 'public' for
>> published commits, 'draft' for local un-published commits and
>> 'secret' for local un-published commits which are not meant to
>> be published.
>>
>> The phase of a changeset is always equal to or higher than the phase
>> of it's descendants, according to the following order:
>>
>> public < draft < secret
>>
>> Commits start life as 'draft', and move to 'public' on push.
>
> Recording where they wer pushed to would be useful for synchronising
> sub-modules and their super projects. That is, giving remote users a clue
> as to where they might find mising sub-modules.
>
>>
>> Mercurial documentation talks about phase of a commit, which might
>> be a good UI, ut also about commits in 'public' phase being "immutable".
>> As commits in Git are immutable, and rewriting history is in fact
>> re-doing commits, this description should probably be changed.
>>
>> While default "push matching" behavior makes it possible to have
>> "secret" commits, being able to explicitly mark commits as not for
>> publishing might be a good idea also for Git.
>>
>
> Being able to mark temporary, out of sequence or other hacks as Secret
> could be useful, as would recording where Public commits had been sent.
>
>>
>> What do you think about this?
>> --
>> Jakub Narebski
>> Poland
>> --
>
> Philip Oakley
>
^ permalink raw reply
* Re: Git performance results on a large repository
From: Nguyen Thai Ngoc Duy @ 2012-02-05 15:17 UTC (permalink / raw)
To: Tomas Carnecky; +Cc: Joshua Redstone, git@vger.kernel.org
In-Reply-To: <4F2E99C2.7090609@dbservice.com>
On Sun, Feb 5, 2012 at 10:01 PM, Tomas Carnecky <tom@dbservice.com> wrote:
> On 2/4/12 7:53 AM, Nguyen Thai Ngoc Duy wrote:
>>
>> On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone<joshua.redstone@fb.com>
>> wrote:
>>>
>>> I timed a few common operations with both a warm OS file cache and a cold
>>> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then
>>> did
>>> the operation in question a few times (first timing is the cold timing,
>>> the next few are the warm timings). The following results are on a
>>> server
>>> with average hard drive (I.e., not flash) and> 10GB of ram.
>>>
>>> 'git status' : 39 minutes cold, and 24 seconds warm.
>>>
>>> 'git blame': 44 minutes cold, 11 minutes warm.
>>>
>>> 'git add' (appending a few chars to the end of a file and adding it): 7
>>> seconds cold and 5 seconds warm.
>>>
>>> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
>>> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
>>> of git to remove the three or four places where 'git commit' stats every
>>> file in the repo, and this dropped the times to 30 minutes cold and 8
>>> seconds warm.
>>
>> Have you tried "git update-index --assume-unchaged"? That should
>> reduce mass lstat() and hopefully improve the above numbers. The
>> interface is not exactly easy-to-use, but if it has significant gain,
>> then we can try to improve UI.
>>
>> On the index size issue, ideally we should make minimum writes to
>> index instead of rewriting 191 MB index. An improvement we could do
>> now is to compress it, reduce disk footprint, thus disk I/O. If you
>> compress the index with gzip, how big is it?
>
> If you're not afraid to add filesystem-specific code to git, you could
> leverage the btrfs find-new command (or use the ioctl directly) to quickly
> find changed files since a certain point in time. Other CoW filesystems may
> have similar mechanisms. You could for example store the last generation id
> in an index extension, that's what those extensions are for, right?
Sure they could be stored as index extensions. I'm more concerned of
the index size. I guess fs-specific code, if properly implemented
(e.g. clean, handling repos crossing fs boundaries, moving repos...),
may get Junio's approval. There were also talks of implementing NTFS's
journal (or something) on msysgit for similar goal.
--
Duy
^ permalink raw reply
* Re: Git performance results on a large repository
From: Tomas Carnecky @ 2012-02-05 15:01 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Joshua Redstone, git@vger.kernel.org
In-Reply-To: <CACsJy8DkLCK0ZUKNz_PJazsxjsRbWVVZwjAU5n2EAjJfCYtpoQ@mail.gmail.com>
On 2/4/12 7:53 AM, Nguyen Thai Ngoc Duy wrote:
> On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone<joshua.redstone@fb.com> wrote:
>> I timed a few common operations with both a warm OS file cache and a cold
>> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
>> the operation in question a few times (first timing is the cold timing,
>> the next few are the warm timings). The following results are on a server
>> with average hard drive (I.e., not flash) and> 10GB of ram.
>>
>> 'git status' : 39 minutes cold, and 24 seconds warm.
>>
>> 'git blame': 44 minutes cold, 11 minutes warm.
>>
>> 'git add' (appending a few chars to the end of a file and adding it): 7
>> seconds cold and 5 seconds warm.
>>
>> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
>> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
>> of git to remove the three or four places where 'git commit' stats every
>> file in the repo, and this dropped the times to 30 minutes cold and 8
>> seconds warm.
> Have you tried "git update-index --assume-unchaged"? That should
> reduce mass lstat() and hopefully improve the above numbers. The
> interface is not exactly easy-to-use, but if it has significant gain,
> then we can try to improve UI.
>
> On the index size issue, ideally we should make minimum writes to
> index instead of rewriting 191 MB index. An improvement we could do
> now is to compress it, reduce disk footprint, thus disk I/O. If you
> compress the index with gzip, how big is it?
If you're not afraid to add filesystem-specific code to git, you could
leverage the btrfs find-new command (or use the ioctl directly) to
quickly find changed files since a certain point in time. Other CoW
filesystems may have similar mechanisms. You could for example store the
last generation id in an index extension, that's what those extensions
are for, right?
tom
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-05 16:15 UTC (permalink / raw)
To: Philip Oakley; +Cc: git
In-Reply-To: <CAFA910035B74E56A52A96097E76AC39@PhilipOakley>
Please don't remove git mailing list from Cc... Oh, I see that you
forgot to send to list, but resend your email there.
On Sun, 5 Feb 2012, Philip Oakley wrote:
> From: "Jakub Narebski" <jnareb@gmail.com>
> Sent: Saturday, February 04, 2012 7:45 PM
> > Git includes protection against rewriting published history on the
> > receive side with fast-forward check by default (which can be
> > overridden) and various receive.deny* configuration variables,
> > including receive.denyNonFastForwards.
> >
> > Nevertheless git users requested (among others in Git User's Survey)
> > more help on creation side, namely preventing rewriting parts of
> > history which was already made public (or at least warning that one is
> > about to rewrite published history). The "warn before/when rewriting
> > published history" answer in "17. Which of the following features would
> > you like to see implemented in git?" multiple-choice question in latest
> > Git User's Survey 2011[1] got 24% (1525) responses.
> >
> > [1]: https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
> >
> > So people would like for git to warn them about rewriting history before
> > they attempt a push and it turns out to not fast-forward.
>
> Another area that is implicitly related is that of (lack of) publication of
> sub-module updates. A mechanisms that, in the super project, knows the
> status of the (local) submodules, such as where they would be sourced from,
> i.e. what was last pushed & where, could help in such instances.
"Better support for submodules" had almost the same number of requests
in the latest Git User's Survey 2011 (25% which means 1582 responses).
Remembering when to do recursive push and where would be a very nice thing.
[...]
> Recording where they were pushed to would be useful for synchronising
> sub-modules and their super projects. That is, giving remote users a clue as
> to where they might find mising sub-modules.
Is it a matter of correctly writing configuration with current git?
I don't use submodules myself, so I cannot say.
> > Mercurial documentation talks about phase of a commit, which might
> > be a good UI, ut also about commits in 'public' phase being "immutable".
> > As commits in Git are immutable, and rewriting history is in fact
> > re-doing commits, this description should probably be changed.
> >
> > While default "push matching" behavior makes it possible to have
> > "secret" commits, being able to explicitly mark commits as not for
> > publishing might be a good idea also for Git.
> >
>
> Being able to mark temporary, out of sequence or other hacks as Secret could
> be useful, as would recording where Public commits had been sent.
Marking as 'secret' must I think be explicit, but I think 'public' phase
should be inferred from remote-tracking branches. The idea of phases is
to allow UI to ask about status of commits: can we amend / rebase it or
not, can we push it or not.
--
Jakub Narebski
Poland
^ permalink raw reply
* [ANNOUNCE] libgit2 v0.16.0
From: Vicent Marti @ 2012-02-05 16:28 UTC (permalink / raw)
To: libgit2, git, git-dev
Hello everyone,
another minor libgit2 release is here, albeit slightly delayed. This
one ships from Brussels, damn it's cold.
The release has been tagged at:
https://github.com/libgit2/libgit2/tree/v0.16.0
A dist package can be found at:
https://github.com/downloads/libgit2/libgit2/libgit2-0.16.0.tar.gz
Updated documentation can be found at:
http://libgit2.github.com/libgit2/
The full change log follows after the message.
Cheers,
Vicent
===================================
libgit2 v0.16.0 "Dutch Fries"
This lovely and much delayed release of libgit2 ships from the cold city
of Brussels, which is currently hosting FOSDEM 2012.
There's been plenty of changes since the latest stable release, here's a
full summary:
- Git Attributes support (see git2/attr.h)
There is now support to efficiently parse and retrieve information
from `.gitattribute` files in a repository. Note that this
information is not yet used e.g. when checking out files.
- .gitignore support
Likewise, all the operations that are affected by `.gitignore` files
now take into account the global, user and local ignores when
skipping the relevant files.
- Cleanup of the object ownership semantics
The ownership semantics for all repository subparts (index, odb,
config files, etc) has been redesigned. All these objects are now
reference counted, and can be hot-swapped in the middle of
execution, allowing for instance to add a working directory and an
index to a repository that was previously opened as bare, or to
change the source of the ODB objects after initialization.
Consequently, the repository API has been simplified to remove all
the `_openX` calls that allowed setting these subparts *before*
initialization.
- git_index_read_tree()
Git trees can now be read into the index.
- More reflog functionality
The reference log has been optimized, and new API calls to rename
and delete the logs for a reference have been added.
- Rewrite of the References code with explicit ownership semantics
The references code has been mostly rewritten to take into account
the cases where another Git application was modifying a repository's
references while the Library was running.
References are now explicitly loaded and free'd by the user, and
they may be reloaded in the middle of execution if the user suspects
that their values may have changed on disk. Despite the new
ownership semantics, the references API stays the same.
- Simplified the Remotes API
Some of the more complex Remote calls have been refactored into
higher level ones, to facilitate the usual `fetch` workflow of a
repository.
- Greatly improved thread-safety
The library no longer has race conditions when loading objects from
the same ODB and different threads at the same time. There's now
full TLS support, even for error codes. When the library is built
with `THREADSAFE=1`, the threading support must be globally
initialized before it can be used (see `git_threads_init()`)
- Tree walking API
A new API can recursively traverse trees and subtrees issuing callbacks for
every single entry.
- Tree diff API
There is basic support for diff'ing an index against two trees.
- Improved windows support
The Library is now codepage aware under Windows32: new API calls
allow the user to set the default codepage for the OS in order to
avoid strange Unicode errors.
^ permalink raw reply
* [RFC/PATCH] git-new-workdir: add option --rsync
From: Michael Schubert @ 2012-02-05 16:27 UTC (permalink / raw)
To: git
Currently, git-new-workdir doesn't allow you to rather duplicate your
old workdir instead of creating a new one based on a branch. This can be
annoying when you are used to carry untracked helper scripts in your
workdir (e.g. test.sh).
Add a new option -s | --rsync to allow users to "sync" the new workdir.
Signed-off-by: Michael Schubert <mschub@elegosoft.com>
---
Maybe we even want to stop after rsync'ing without doing the checkout -f.
Opinions?
contrib/workdir/git-new-workdir | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 75e8b25..bac75f9 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -10,9 +10,30 @@ die () {
exit 128
}
-if test $# -lt 2 || test $# -gt 3
+if test $# -lt 2
then
- usage "$0 <repository> <new_workdir> [<branch>]"
+ usage "$0 [-s | --rsync] <repository> <new_workdir> [<branch>]"
+fi
+
+rsync=
+
+while test $# != 0
+do
+ case "$1" in
+ -s|--rsync)
+ rsync=t ;;
+ *)
+ break ;;
+ esac
+ shift
+done
+
+if test -n "$rsync"
+then
+ if ! $(hash rsync 2>/dev/null)
+ then
+ die "cannot find rsync"
+ fi
fi
orig_git=$1
@@ -77,6 +98,12 @@ done
cd "$new_workdir"
# copy the HEAD from the original repository as a default branch
cp "$git_dir/HEAD" .git/HEAD
+
+if test -n "$rsync"
+then
+ rsync --archive --exclude '.*' "../$orig_git/" .
+fi
+
# checkout the branch (either the same as HEAD from the original repository, or
# the one that was asked for)
git checkout -f $branch
--
1.7.9.230.gbd302
^ permalink raw reply related
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Johan Herland @ 2012-02-05 17:29 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Philip Oakley, git
In-Reply-To: <201202051715.38896.jnareb@gmail.com>
2012/2/5 Jakub Narebski <jnareb@gmail.com>:
>> Being able to mark temporary, out of sequence or other hacks as Secret could
>> be useful, as would recording where Public commits had been sent.
>
> Marking as 'secret' must I think be explicit, but I think 'public' phase
> should be inferred from remote-tracking branches. The idea of phases is
> to allow UI to ask about status of commits: can we amend / rebase it or
> not, can we push it or not.
I agree that the 'public' state should (by default) be automatically
inferred from remote-tracking branches. As it stands, we can do this
with current git, by writing a pre-rebase hook that checks if any of
the commits to-be-rebased are reachable from any remote-tracking
branch.
Unfortunately, the pre-rebase hook only affects 'git rebase', and in
order to do the same check on 'git commit --amend' you'd have to write
a similar pre-commit hook (don't know how easy it is to find the
amended commit from within the hook). Maybe we should add a
pre-rewrite hook that trigger in the same situations as the
post-rewrite hook.
This should take care of the simplest 'public' use case in a
push-based workflow. If you publish commits by other means
(send-email, bundles, pulling directly from your repo), you need some
other way to mark the 'public' commits. One solution would be using
'git notes' to annotate the 'public' commits on a 'refs/notes/public'
notes ref. Your pre-rebase/pre-rewrite hook should then check if any
of the commits to-be-rewritten are reachable from any commit annotated
as 'public'.
Also, if you want to record where 'public' commits have been sent
(other than what can be inferred from the remote-tracking branches),
you could write this into the refs/notes/public annotation.
As for 'secret' commits, you could annotate these on a
refs/notes/secret notes ref, and then teach 'git push' (or whatever
other method for publishing commits you use) to refuse to publish
commits annotated on this notes ref. Possibly we would want to add a
"pre-push" or "pre-publish" hook.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] send-email: add extra safetly in address sanitazion
From: Thomas Rast @ 2012-02-05 19:39 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Brandon Casey, Uwe Kleine-König, Brian Gernhardt,
Robin H. Johnson, Ævar Arnfjörð Bjarmason
In-Reply-To: <1328373162-25188-1-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Currently bad addresses like 'Foo Bar <foo@bar.com>>' will just be sent
> verbatim -- that's not good; we should either error out, or sanitize
> them.
>
> The following patch adds extra sanitazion so the following
> transformations are performed:
>
> 'Foo Bar <foo@bar.com>' -> 'Foo Bar <foo@bar.com>'
> '"Foo Bar" <foo@bar.com>' -> '"Foo Bar" <foo@bar.com>'
> 'foo@bar.com' -> 'foo@bar.com'
> '<foo@bar.com>' -> 'foo@bar.com'
> 'Foo Bar' -> 'Foo Bar'
Am I the only one who stared at this for ten seconds, only to then
realize that there is no sanitizing whatsoever going on here?
> 'Foo Bar <foo@bar.com>>' -> 'Foo Bar <foo@bar.com>'
> '"Foo Bar" <foo@bar.com>>' -> '"Foo Bar" <foo@bar.com>'
> '<foo@bar.com>>' -> 'foo@bar.com'
All of these are the same underlying issue. Does your patch fix any
other malformed addresses, or just this particular type?
> Basically, we try to check that the address is in the form of
> "Name <email>", and if not, assume it's "email". According to commit
> 155197e[1], the "prhase" should not be empty, so if it is, remove the
^^^^^^
"phrase"
> git-send-email.perl | 14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
Tests?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 1/3] blame: fix email output with mailmap
From: Thomas Rast @ 2012-02-05 19:57 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1328385024-6955-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>, "Brian Gianforcaro" <b.gianfo@gmail.com>, "Marius Storm-Olsen" <marius@trolltech.com>, "Junio C Hamano" <junkio@cox.net>
I hope you noticed that Cc lists like the above really prove my point
that you cannot automate common sense. Your cccmd script apparently
uses the line range in
> diff --git a/builtin/blame.c b/builtin/blame.c
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1403,10 +1403,13 @@
to determine who to Cc. But in doing so, it dug up an old address for
Junio. It also added Brian whose only fault in all of this was fixing
style of the 'if' you are patching.
And on top of that it had absolutely no way of knowing that Cc'ing Peff
would have been a good idea, seeing as you two were involved in an
earlier discussion about this precise bug.
(Granted, omitting *Peff* doesn't make that much of a difference, since
for all I know he reads every email that crosses this list. But my
point still stands.)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-05 20:16 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120204182611.GA31091@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
>
>> In any case, the one to blame for the header corruption is git:
>> [...]
>> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>>
>> Notice the mail is wrong.
> ...
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 5a67c20..9b886fa 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
> /* Add a trailing '>' to email, since map_user returns plain emails
> Note: It already has '<', since we replace from mail+1 */
> mailpos = memchr(mail, '\0', mail_len);
> - if (mailpos && mailpos-mail < mail_len - 1) {
> + if (mailpos && mailpos-mail < mail_len - 1 &&
> + mailpos > mail && *(mailpos-1) != '>') {
> *mailpos = '>';
> *(mailpos+1) = '\0';
> }
>
> but it feels like the fix should go into map_user.
Thanks.
The map_user() API takes an email address that is terminated by either NUL
or '>' to allow the caller to learn the corresponding up-to-date email
address that is NUL terminated, while indicating with its return value
that if the caller even needs to replace what it already has. But the
function does not properly terminate email when it only touched the name
part. And I think that is the real bug.
So I agree that the real fix should go to map_user() so that when it says
"I've updated something, so pick up the updated result from the i/o
arguments you gave me, i.e. email and name", it makes sure what it claims
to be an e-mail address does not have the extra '>' in it.
Working around the current behaviour by forcing all callers that pass '>'
terminated e-mail address to have the code like the above quoted patch
does not feel right.
^ permalink raw reply
* Re: Specifying revisions in the future
From: Matthieu Moy @ 2012-02-05 20:18 UTC (permalink / raw)
To: jpaugh; +Cc: git
In-Reply-To: <jgjkk0$qrg$1@dough.gmane.org>
jpaugh@gmx.us writes:
> Hello.
>
> Is it possible to specify revisions in the future?
You mean, the opposite of <commit>^ or <commit>~n?
AFAIK, there isn't, and there's a good reason for that: <commit>^ is
well-defined, it's the first parent of <commit>, and it won't change
unless one rewrites this commit.
"the successor of <commit>", OTOH, is not well defined, since there can
be several successors, and one can't order them reliably (you can't
really know the set of successors, because they can exist in different
repositories).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Junio C Hamano @ 2012-02-05 20:38 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Jonathan Nieder, git, Marius Storm-Olsen
In-Reply-To: <CAMP44s2djXurzMSXLOAkx84Sm8P2YV67M1yS2AuidkfGbTdmEQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
>>> Look at the title:
>>> add 'git blame -e' tests
>>>
>>> s/blame/blame -e/
>>
>> And? After copy/pasting this particular test with that substitution,
>> what do we get a test for?
>
> For 'git blame -e'.
>
>> What class of problem is it supposed to catch?
>
> Problems related to 'git blame -e'?
You very well know that we know you better than that, so it is no use to
pretend to be dumb. It does not do anything other than wasting bandwidth
and irritating readers.
You know that you are not addressing "git blame with the -e option shows
wrong line numbers on its output". The symptom you are checking with is
"e-mail address output from 'blame -e' used to add an extra '>' at the end
when only name is mapped, and I fixed it with the previous patch."
Why is it so hard to either
(1) give the more descriptive answer upfront when somebody who did not
read the first patch of the 3-patch series pointed it out the comment
is not descriptive enough the first time; or
(2) give the more descriptive answer and then say "we could do that, but
when somebody views this change in "git log" as a part of 3-patch
series, it should be clear enough---so let's aim for brevity instead
of adding that two-line description" to defend the line in the patch?
> Or just apply it. Don't let the perfect be the enemy of the good.
Perfect is the enemy of the good, but it is not an excuse to be sloppy.
I tend to think that a single line "# blame -e" is sufficient if this were
a part of just a single patch that has the fix and the test to guard the
fix against future breakage (i.e. "not sloppy"). I would even say that not
even "# blame" is necessary in such a case.
But if this is a standalone patch to add this test, it does not describe
what it wants to test very well.
In any case, I have doubts that the fix should go to blame and not to
map_user(), so I'll see what happens in the further discussions.
Thanks.
^ permalink raw reply
* Re: [PATCH] Change include order in two compat/ files to avoid compiler warning
From: Junio C Hamano @ 2012-02-05 20:41 UTC (permalink / raw)
To: Ben Walton; +Cc: git
In-Reply-To: <1328404107-15757-1-git-send-email-bwalton@artsci.utoronto.ca>
Ben Walton <bwalton@artsci.utoronto.ca> writes:
> The inet_ntop and inet_pton compatibility wrapper source files
> included system headers before git-compat-utils.h.
Thanks, that is definitely a breakage.
> diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
> index 60b5a1d..f1bf81c 100644
> --- a/compat/inet_ntop.c
> +++ b/compat/inet_ntop.c
> @@ -15,11 +15,9 @@
> * SOFTWARE.
> */
>
> +#include "../git-compat-util.h"
> #include <errno.h>
> #include <sys/types.h>
> -
> -#include "../git-compat-util.h"
> -
> #include <stdio.h>
> #include <string.h>
I actually have to wonder if any of these four inclusion of the system headers
are warranted. Wouldn't they be included as part of git-compat-util.h anyway?
^ permalink raw reply
* Re: [PATCH v4 4/4] completion: simplify __gitcomp*
From: Junio C Hamano @ 2012-02-05 20:45 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Felipe Contreras, git, Jonathan Nieder, Thomas Rast,
Shawn O. Pearce
In-Reply-To: <20120204135404.GF16099@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
> And it does make a difference, it breaks the completion of a single
> word in multiple steps, e.g. git log --pretty=<TAB> master..<TAB>. In
> such cases we pass "${cur##--pretty=}" and "${cur_#*..}" as third
> argument to __gitcomp() and __gitcomp_nl(), which can be empty strings
> when the user hits TAB right after the '=' and '..'.
After saying "this rewrite is wrong", I was actually wondering if I should
have said "this rewrite is not faithful to the original". Based on your
analysis, the difference does break the callers, so the rewrite is indeed
wrong.
Thanks for following up.
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-05 20:46 UTC (permalink / raw)
To: Johan Herland; +Cc: Philip Oakley, git
In-Reply-To: <CALKQrgcPW5VnVtGYDo6i00bvmWr6PvnWfEdWW+9ttG4hVQm58A@mail.gmail.com>
On Sun, 5 Feb 2012, Johan Herland wrote:
> 2012/2/5 Jakub Narebski <jnareb@gmail.com>:
> > > Being able to mark temporary, out of sequence or other hacks as Secret could
> > > be useful, as would recording where Public commits had been sent.
> >
> > Marking as 'secret' must I think be explicit, but I think 'public' phase
> > should be inferred from remote-tracking branches. The idea of phases is
> > to allow UI to ask about status of commits: can we amend / rebase it or
> > not, can we push it or not.
>
> I agree that the 'public' state should (by default) be automatically
> inferred from remote-tracking branches. As it stands, we can do this
> with current git, by writing a pre-rebase hook that checks if any of
> the commits to-be-rebased are reachable from any remote-tracking
> branch.
It is nice that we can achieve a large part of this feature with existing
infrastructure. It would be nice if we ship such pre-rebase hook with
git, so people can just enable it if they want to use this functionality,
like the default pre-commit hook that checks for whitespace errors.
> Unfortunately, the pre-rebase hook only affects 'git rebase', and in
> order to do the same check on 'git commit --amend' you'd have to write
> a similar pre-commit hook (don't know how easy it is to find the
> amended commit from within the hook). Maybe we should add a
> pre-rewrite hook that trigger in the same situations as the
> post-rewrite hook.
pre-rewrite hook would be a really nice to have, especially that it would
(I hope) cover third party tools like various GUIs for git; and also
git-filter-branch.
Note however that the safety net, i.e. refusing or warning against attempted
rewrite of published history is only part of issue. Another important part
is querying and showing "phase" of a commit. What I'd like to see is
ability to show among others in "git log" and "git show" output if commit
was already published or not (and if it is marked 'secret').
> This should take care of the simplest 'public' use case in a
> push-based workflow. If you publish commits by other means
> (send-email, bundles, pulling directly from your repo), you need some
> other way to mark the 'public' commits. One solution would be using
> 'git notes' to annotate the 'public' commits on a 'refs/notes/public'
> notes ref. Your pre-rebase/pre-rewrite hook should then check if any
> of the commits to-be-rewritten are reachable from any commit annotated
> as 'public'.
Another solution would be to create "fake" remote-tracking branches
by git-bundle and git-send-email.
> Also, if you want to record where 'public' commits have been sent
> (other than what can be inferred from the remote-tracking branches),
> you could write this into the refs/notes/public annotation.
I wonder if this too can be done by hook...
> As for 'secret' commits, you could annotate these on a
> refs/notes/secret notes ref, and then teach 'git push' (or whatever
> other method for publishing commits you use) to refuse to publish
> commits annotated on this notes ref. Possibly we would want to add a
> "pre-push" or "pre-publish" hook.
Well, addition of pre-push / pre-publish was resisted on the grounds
that all it does is something that can be as easy done by hand before
push. Perhaps this new use case would help bring it forward, don't
you think?
Thanks for all the comments.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v3 1/4] completion: be nicer with zsh
From: Junio C Hamano @ 2012-02-05 20:49 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <CAMP44s2QdJ4+qgg4fF5-DOWHx3Btd0pTivTT9s_E=qqxg16YLQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
>> Do not blame pobox.com; they have nothing to do with the corruption of
>> your headers.
>
> No, but they have everything to do with *silently* dropping it. Why
> couldn't they _at least_ return an error saying that the headers are
> wrong? Note that other servers didn't even complain, they processed
> the mail happily.
Again, please do not blame pobox.com; they fall into your "other servers"
category. You are probably talking about vger.kernel.org that is extra
picky and wants to avoid wasting their outgoing bandwidth because they get
so much spams.
> % git blame -e -L 947,+7 contrib/completion/git-completion.bash v1.7.9
> ...
> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
I am glad to see that something useful came out from your digging, and a
fix is being worked on it, while I was away from my machines. Thanks for
getting the ball rolling.
^ 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