Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor
All,
I applied Kyle's latest patch
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index 622535e2..96888228 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -391,6 +391,7 @@ sub longest_common_path {
> sub gs_fetch_loop_common {
> my ($self, $base, $head, $gsv, $globs) = @_;
> return if ($base > $head);
> + $::_repository->_open_cat_blob_if_needed;
> my $gpool = SVN::Pool->new_default;
> my $ra_url = $self->url;
> my $reload_ra = sub {
alone and this fixes the bug for me. Thanks a lot, Kyle!
Cheers,
Nico
On Thu, Feb 26, 2015 at 12:37 AM, Nico Schlömer
wrote:
> Kyle,
>
> you are absolutely correct, I used the wrong Git installation in my last
> email. With both your and Eric's patches applied, I'm getting
> ```
> [...]
> r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn)
> Unexpected result returned from git cat-file at
> /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 344.
> Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at
> /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 345, line
> 757.
>
> error closing pipe: Bad file descriptor at
> /home/nschloe/libexec/git-core/git-svn line 0.
> error closing pipe: Bad file descriptor at
> /home/nschloe/libexec/git-core/git-svn line 0.
> ```
> where line 344 is
> ```
> $size = $::_repository->cat_blob($fb->{blob}, $base);
> ```
> Adding the line in `perl/Git/SVN/Ra.pm` as per your suggestion does not
> change this.
>
> Cheers,
> Nico
>
>
> On Wed, Feb 25, 2015 at 9:34 PM Kyle J. McKay wrote:
>>
>> On Feb 25, 2015, at 07:07, Nico Schlömer wrote:
>>
>> > Thanks Kyle for the patch! I applied it and ran
>> > ```
>> > git svn clone https://geuz.org/svn/gmsh/trunk
>> > ```
>> > Unfortunately, I'm still getting
>> > ```
>> > [...]
>> > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn)
>> > error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/
>> > Fetcher.pm line 335.
>> > error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/
>> > Fetcher.pm line 335.
>> > out pipe went bad at /usr/share/perl5/Git.pm line 955.
>> > ```
>>
>> Are you certain you're running with the patch? I ask because earlier
>> you sent this:
>>
>> On Jan 31, 2015, at 04:51, Nico Schlömer wrote:
>>
>> > I tried the patch and I still get
>> > ```
>> > [...]
>> > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn)
>> > Unexpected result returned from git cat-file at
>> > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335.
>> > Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at
>> > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336,
>> > line 757.
>> >
>> > error closing pipe: Bad file descriptor at
>> > /home/nschloe/libexec/git-core/git-svn line 0.
>> > error closing pipe: Bad file descriptor at
>> > /home/nschloe/libexec/git-core/git-svn line 0.
>> > ```
>> > when
>> > ```
>> > git svn clone https://geuz.org/svn/gmsh/trunk
>> > ```
>>
>> And as the patch below applies to Fetcher.pm at line 322 and inserts 8
>> lines, I do not see how you could be getting the same error message at
>> line 335 if the patch was present. Even if you manually applied it by
>> only inserting the two lines of code, the line numbers would be at
>> least 337, not 335 as reported above. I also notice the path to
>> Fetcher.pm is different from your earlier report.
>>
>> That said, the fact that it happens right after r100 (which is when
>> SVN::Pool::clear is getting called) suggests there's another file
>> handle getting caught up in the SVN::Pool somehow. (When I try to
>> clone gmsh, I got to r4885 before a server disconnection.)
>>
>> It could be as simple as the open2 call FileHandle result in later
>> perl versions ends up in the SVN::Pool whereas in earlier Perl and/or
>> svn versions it does not.
>>
>> What happens if you add this change on top of the Fetcher.pm change:
>>
>> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
>> index 622535e2..96888228 100644
>> --- a/perl/Git/SVN/Ra.pm
>> +++ b/perl/Git/SVN/Ra.pm
>> @@ -391,6 +391,7 @@ sub longest_common_path {
>> sub gs_fetch_loop_common {
>> my ($self, $base, $head, $gsv, $globs) = @_;
>> return if ($base > $head);
>> + $::_repository->_open_cat_blob_if_needed;
>> my $gpool = SVN::Pool->new_default;
>> my $ra_url = $self->url;
>> my $reload_ra = sub {
>>
>> -Kyle
>>
>> > Cheers,
>> > Nico
>> >
>> > On Wed, Feb 25, 2015 at 11:20 AM Kyle J. McKay
>> > wrote:
>> > I believe I have been able to track down this problem and implement a
>> > fix. Please report back if this patch fixes the problem for you.
>> >
>> > -Kyle
>> >
>> > -- 8< --
>> > Subject: [PATCH] Git::SVN::Fetcher: avoid premature 'svn_hash' temp
>> > file closure
>> >
>> > Since b19138b (git-svn: Make it incrementally faster by minimizing
>> > temp
>> > files, v1.6.0), git-svn has been using the Git.pm temp_acquire and
>> >
Re: [RFC/PATCH 0/3] protocol v2
On Thu, Feb 26, 2015 at 2:31 PM, Stefan Beller wrote: > On Wed, Feb 25, 2015 at 10:04 AM, Junio C Hamano wrote: >> Duy Nguyen writes: >> >>> On Wed, Feb 25, 2015 at 6:37 AM, Stefan Beller wrote: I can understand, that we maybe want to just provide one generic "version 2" of the protocol which is an allrounder not doing bad in all of these aspects, but I can see usecases of having the desire to replace the wire protocol by your own implementation. To do so we could try to offer an API which makes implementing a new protocol somewhat easy. The current state of affairs is not providing this flexibility. >>> >>> I think we are quite flexible after initial ref advertisement. >> >> Yes, that is exactly where my "I am not convinced" comes from. >> > > We are not. (not really at least). We can tune some parameters or > change the behavior slightly, > but we cannot fix core assumptions made when creating v2 protocol. > This you can see when when talking about v1 as well: we cannot fix any > wrongdoings of v1 now by adding another capability. Step 1 then should be identifying these wrongdoings and assumptions. We can really go wild with these capabilities. The only thing that can't be changed is perhaps sending the first ref. I don't know whether we can accept a dummy first ref... After that point, you can turn the protocol upside down because both client and server know what it would be. > So from my point > of view we don't waste resources when having an advertisement of > possible protocols instead of a boolean flag indicating v2 is > supported. There is really not much overhead in coding nor bytes > exchanged on the wire, so why not accept stuff that comes for free > (nearly) ? You realize you're advertising v2 as a new capability, right? Instead of defining v2 feature set then advertise v2, people could simply add new features directly. I don't see v2 (at least with these patches) adds any value. > I mean how do we know all the core assumptions made for v2 hold in the > future? We don't. That's why I'd propose a plain and easy exchange at > first stating the version to talk. And we already does that, except that we don't state what version (as a number) exactly, but what feature that version supports. The focus should be the new protocol at daemon.c and maybe remote-curl.c where we do know the current protocol is not flexible enough. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] versionsort: support reorder prerelease suffixes
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Second round. Looking better. We can do
"1.0-pre12 < 1.0-rc0 < 1.0 < 1.0-post1" too but it relies on
config key's loading order, a bit iffy.
Documentation/config.txt | 7 +++
t/t7004-tag.sh | 28 +++
versioncmp.c | 50
3 files changed, 85 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 04e2a71..8e078df 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2539,6 +2539,13 @@ user.signingkey::
This option is passed unchanged to gpg's --local-user parameter,
so you may specify a key using any method that gpg supports.
+versionsort.prereleaseSuffix::
+ When version sort is used in linkgit:git-tag[1], prerelease
+ tags (e.g. "1.0-rc1") may appear after the main release
+ "1.0". By specifying the suffix "-rc" in this variable,
+ "1.0-rc1" will appear before "1.0". One variable assignment
+ per suffix.
+
web.browser::
Specify a web browser that may be used by some commands.
Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 35c805a..8bfeef9 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1459,6 +1459,34 @@ test_expect_success 'invalid sort parameter in
configuratoin' '
test_cmp expect actual
'
+test_expect_success 'version sort with prerelease reordering' '
+ git config --unset tag.sort &&
+ git config versionsort.prereleaseSuffix -rc &&
+ git tag foo1.6-rc1 &&
+ git tag foo1.6-rc2 &&
+ git tag -l --sort=version:refname "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6-rc1
+ foo1.6-rc2
+ foo1.6
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'reverse version sort with prerelease reordering' '
+ git tag -l --sort=-version:refname "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.6
+ foo1.6-rc2
+ foo1.6-rc1
+ foo1.3
+ EOF
+ test_cmp expect actual
+'
+
run_with_limited_stack () {
(ulimit -s 128 && "$@")
}
diff --git a/versioncmp.c b/versioncmp.c
index 7511e08..80bfd10 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "string-list.h"
/*
* versioncmp(): copied from string/strverscmp.c in glibc commit
@@ -20,6 +21,48 @@
#define CMP2
#define LEN3
+static const struct string_list *prereleases;
+static int initialized;
+
+/*
+ * p1 and p2 point to the first different character in two strings. If
+ * either p1 or p2 starts with a prerelease suffix, it will be forced
+ * to be on top.
+ *
+ * If both p1 and p2 start with (different) suffix, the order is
+ * determined by config file.
+ *
+ * Note that we don't have to deal with the situation when both p1 and
+ * p2 start with the same suffix because the common part is already
+ * consumed by the caller.
+ *
+ * Return non-zero if *diff contains the return value for versioncmp()
+ */
+static int swap_prereleases(const void *p1_,
+ const void *p2_,
+ int *diff)
+{
+ const char *p1 = p1_;
+ const char *p2 = p2_;
+ int i, i1 = -1, i2 = -1;
+
+ for (i = 0; i < prereleases->nr; i++) {
+ const char *suffix = prereleases->items[i].string;
+ if (i1 == -1 && starts_with(p1, suffix))
+ i1 = i;
+ if (i2 == -1 && starts_with(p2, suffix))
+ i2 = i;
+ }
+ if (i1 == -1 && i2 == -1)
+ return 0;
+ if (i1 >= 0 && i2 >= 0)
+ *diff = i1 - i2;
+ else if (i1 >= 0)
+ *diff = -1;
+ else /* if (i2 >= 0) */
+ *diff = 1;
+ return 1;
+}
/*
* Compare S1 and S2 as strings holding indices/version numbers,
@@ -74,6 +117,13 @@ int versioncmp(const char *s1, const char *s2)
state += (c1 == '0') + (isdigit (c1) != 0);
}
+ if (!initialized) {
+ initialized = 1;
+ prereleases =
git_config_get_value_multi("versionsort.prereleasesuffix");
+ }
+ if (prereleases && swap_prereleases(p1 - 1, p2 - 1, &diff))
+ return diff;
+
state = result_type[state * 3 + (((c2 == '0') + (isdigit (c2) != 0)))];
switch (state) {
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] index-pack: reduce object_entry size to save memory
For each object in the input pack, we need one struct object_entry. On
x86-64, this struct is 64 bytes long. Although:
- The 8 bytes for delta_depth and base_object_no are only useful when
show_stat is set. And it's never set unless someone is debugging.
- The three fields hdr_size, type and real_type take 4 bytes each
even though they never use more than 4 bits.
By moving delta_depth and base_object_no out of struct object_entry
and make the other 3 fields one byte long instead of 4, we shrink 25%
of this struct.
On a 3.4M object repo (*) that's about 53MB. The saving is less
impressive compared to index-pack memory use for basic bookkeeping (**),
about 16%.
(*) linux-2.6.git already has 4M objects as of v3.19-rc7 so this is
not an unrealistic number of objects that we have to deal with.
(**) 3.4M * (sizeof(object_entry) + sizeof(delta_entry)) = 311MB
Brought-up-by: Matthew Sporleder
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/index-pack.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4632117..9d854fb 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -18,9 +18,12 @@ static const char index_pack_usage[] =
struct object_entry {
struct pack_idx_entry idx;
unsigned long size;
- unsigned int hdr_size;
- enum object_type type;
- enum object_type real_type;
+ unsigned char hdr_size;
+ signed char type;
+ signed char real_type;
+};
+
+struct object_stat {
unsigned delta_depth;
int base_object_no;
};
@@ -64,6 +67,7 @@ struct delta_entry {
};
static struct object_entry *objects;
+static struct object_stat *obj_stat;
static struct delta_entry *deltas;
static struct thread_local nothread_data;
static int nr_objects;
@@ -873,13 +877,15 @@ static void resolve_delta(struct object_entry *delta_obj,
void *base_data, *delta_data;
if (show_stat) {
- delta_obj->delta_depth = base->obj->delta_depth + 1;
+ int i = delta_obj - objects;
+ int j = base->obj - objects;
+ obj_stat[i].delta_depth = obj_stat[j].delta_depth + 1;
deepest_delta_lock();
- if (deepest_delta < delta_obj->delta_depth)
- deepest_delta = delta_obj->delta_depth;
+ if (deepest_delta < obj_stat[i].delta_depth)
+ deepest_delta = obj_stat[i].delta_depth;
deepest_delta_unlock();
+ obj_stat[i].base_object_no = j;
}
- delta_obj->base_object_no = base->obj - objects;
delta_data = get_data_from_pack(delta_obj);
base_data = get_base_data(base);
result->obj = delta_obj;
@@ -902,7 +908,7 @@ static void resolve_delta(struct object_entry *delta_obj,
* "want"; if so, swap in "set" and return true. Otherwise, leave it untouched
* and return false.
*/
-static int compare_and_swap_type(enum object_type *type,
+static int compare_and_swap_type(signed char *type,
enum object_type want,
enum object_type set)
{
@@ -1499,7 +1505,7 @@ static void show_pack_info(int stat_only)
struct object_entry *obj = &objects[i];
if (is_delta_type(obj->type))
- chain_histogram[obj->delta_depth - 1]++;
+ chain_histogram[obj_stat[i].delta_depth - 1]++;
if (stat_only)
continue;
printf("%s %-6s %lu %lu %"PRIuMAX,
@@ -1508,8 +1514,8 @@ static void show_pack_info(int stat_only)
(unsigned long)(obj[1].idx.offset - obj->idx.offset),
(uintmax_t)obj->idx.offset);
if (is_delta_type(obj->type)) {
- struct object_entry *bobj =
&objects[obj->base_object_no];
- printf(" %u %s", obj->delta_depth,
sha1_to_hex(bobj->idx.sha1));
+ struct object_entry *bobj =
&objects[obj_stat[i].base_object_no];
+ printf(" %u %s", obj_stat[i].delta_depth,
sha1_to_hex(bobj->idx.sha1));
}
putchar('\n');
}
@@ -1672,6 +1678,8 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
curr_pack = open_pack_file(pack_name);
parse_pack_header();
objects = xcalloc(nr_objects + 1, sizeof(struct object_entry));
+ if (show_stat)
+ obj_stat = xcalloc(nr_objects + 1, sizeof(struct object_stat));
deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
parse_pack_objects(pack_sha1);
resolve_deltas();
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] nd/slim-index-pack-memory-usage updates
This fixes the "signed char" bug in 1/2: "char" alone could be either signed or unsigned but we do need signed char. One point to clang for detecting "obj->real_type != OBJ_BAD" on ARM where real_type becomes unsigned char and OBJ_BAD is -1. gcc just keeps quiet.. Nguyễn Thái Ngọc Duy (2): index-pack: reduce object_entry size to save memory index-pack: kill union delta_base to save memory builtin/index-pack.c | 290 +++ 1 file changed, 179 insertions(+), 111 deletions(-) -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] index-pack: kill union delta_base to save memory
Once we know the number of objects in the input pack, we allocate an
array of nr_objects of struct delta_entry. On x86-64, this struct is
32 bytes long. The union delta_base, which is part of struct
delta_entry, provides enough space to store either ofs-delta (8 bytes)
or ref-delta (20 bytes).
Notice that with "recent" Git versions, ofs-delta objects are
preferred over ref-delta objects and ref-delta objects have no reason
to be present in a clone pack. So in clone case we waste (20-8) *
nr_objects bytes because of this union. That's about 38MB out of 100MB
for deltas[] with 3.4M objects, or 38%. deltas[] would be around 62MB
without the waste.
This patch attempts to eliminate that. deltas[] array is split into
two: one for ofs-delta and one for ref-delta. Many functions are also
duplicated because of this split. With this patch, ofs_deltas[] array
takes 51MB. ref_deltas[] should remain unallocated in clone case (0
bytes). This array grows as we see ref-delta. We save about half in
clone case, or 25% of total bookkeeping.
The saving is more than the calculation above because some padding in
the old delta_entry struct is removed. ofs_delta_entry is 16 bytes,
including the 4 bytes padding. That's 13MB for padding, but packing
the struct could break platforms that do not support unaligned
access. If someone on 32-bit is really low on memory and only deals
with packs smaller than 2G, using 32-bit off_t would eliminate the
padding and save 27MB on top.
A note about ofs_deltas allocation. We could use ref_deltas memory
allocation strategy for ofs_deltas. But that probably just adds more
overhead on top. ofs-deltas are generally the majority (1/2 to 2/3) in
any pack. Incremental realloc may lead to too many memcpy. And if we
preallocate, say 1/2 or 2/3 of nr_objects initially, the growth rate
of ALLOC_GROW() could make this array larger than nr_objects, wasting
more memory.
Brought-up-by: Matthew Sporleder
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/index-pack.c | 260 +++
1 file changed, 160 insertions(+), 100 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 9d854fb..3ed53e3 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -28,11 +28,6 @@ struct object_stat {
int base_object_no;
};
-union delta_base {
- unsigned char sha1[20];
- off_t offset;
-};
-
struct base_data {
struct base_data *base;
struct base_data *child;
@@ -52,26 +47,28 @@ struct thread_local {
int pack_fd;
};
-/*
- * Even if sizeof(union delta_base) == 24 on 64-bit archs, we really want
- * to memcmp() only the first 20 bytes.
- */
-#define UNION_BASE_SZ 20
-
#define FLAG_LINK (1u<<20)
#define FLAG_CHECKED (1u<<21)
-struct delta_entry {
- union delta_base base;
+struct ofs_delta_entry {
+ off_t offset;
+ int obj_no;
+};
+
+struct ref_delta_entry {
+ unsigned char sha1[20];
int obj_no;
};
static struct object_entry *objects;
static struct object_stat *obj_stat;
-static struct delta_entry *deltas;
+static struct ofs_delta_entry *ofs_deltas;
+static struct ref_delta_entry *ref_deltas;
static struct thread_local nothread_data;
static int nr_objects;
-static int nr_deltas;
+static int nr_ofs_deltas;
+static int nr_ref_deltas;
+static int ref_deltas_alloc;
static int nr_resolved_deltas;
static int nr_threads;
@@ -480,7 +477,8 @@ static void *unpack_entry_data(unsigned long offset,
unsigned long size,
}
static void *unpack_raw_entry(struct object_entry *obj,
- union delta_base *delta_base,
+ off_t *ofs_offset,
+ unsigned char *ref_sha1,
unsigned char *sha1)
{
unsigned char *p;
@@ -509,11 +507,10 @@ static void *unpack_raw_entry(struct object_entry *obj,
switch (obj->type) {
case OBJ_REF_DELTA:
- hashcpy(delta_base->sha1, fill(20));
+ hashcpy(ref_sha1, fill(20));
use(20);
break;
case OBJ_OFS_DELTA:
- memset(delta_base, 0, sizeof(*delta_base));
p = fill(1);
c = *p;
use(1);
@@ -527,8 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
use(1);
base_offset = (base_offset << 7) + (c & 127);
}
- delta_base->offset = obj->idx.offset - base_offset;
- if (delta_base->offset <= 0 || delta_base->offset >=
obj->idx.offset)
+ *ofs_offset = obj->idx.offset - base_offset;
+ if (*ofs_offset <= 0 || *ofs_offset >= obj->idx.offset)
bad_object(obj->idx.offset, _("delta base offset is out
of bound"));
break;
case OBJ_COMMIT:
@@ -612,55 +609,108 @@ static void *get_data_from_pack(struct object_entry *obj)
return unpack_d
Re: [PATCH] sequencer: preserve commit messages
Junio C Hamano venit, vidit, dixit 25.02.2015 19:22: > Michael J Gruber writes: > >> Junio C Hamano venit, vidit, dixit 24.02.2015 19:29: >>> Michael J Gruber writes: >>> > Hmm, wouldn't it introduce a grave regression for users who > explicitly ask to clean crufty messages up (by setting their own > commit.cleanup configuration) if you unconditionally force > "--cleanup=verbatim" here? > That's what I meant by possible side-effects below. ... But git cherry-pick without conflict should no re-cleanup the commit message either, should it? >>> >>> Hmm, but if it does not, wouldn't that countermand the wish of the >>> user who explicitly asked to clean crufty messages up by setting >>> their own commit.cleanup configuration? >> >> Note that "verbatim" is not the default - we cleanup commits even >> without being asked to. And this makes sense for "git commit", of course. > > I am fine with the result of the updated code if the user does not > have anything in the config and uses the "default". Not touching in > "cherry-pick" would be more desirable than cleaning. We are in > agreement for that obvious case. I didn't know we were. It's clear now. > But your response is sidestepping my question, isn't it? I simply misunderstood it. > What does your change do to the user who wants that the clean-up to > always happen and expresses that desire by setting > commit.cleanup=strip in the config? Doesn't the internal invocation > of "commit --cleanup=verbatim" that is unconditional override it? > Yes, it obviously overrides it. I have to re-check which cleanups rebase does. I hope none. But I would think that to clean up a commit message according to the current config settings, a user should have to "commit --amend" or "rebase -i with reword" explicitly. I still think of rebase and cherry-picks as means to transplant a commit as unchanged as possible. Now, if there are conflicts and the user has to resolve them, they will use "git commit" themselves with their current config in effect. That is to be effected, and the user can use "git commit --cleanup=..." however they want. That leaves the case of "git cherry-pick --edit". I could easily catch that and not overrride config in this case. But the user cannot influence that other than by using "git -c commit.cleanup=... cherry-pick --edit". Hmm. With "--edit", current config being in effect should be expected, right? So how about: In case of no conflict: force cleanup=verbatim unless --edit is used? Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: feature request: excluding files/paths from "git grep"
Jeff King venit, vidit, dixit 25.02.2015 20:11: > On Wed, Feb 25, 2015 at 11:01:22AM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >>> So I think _if_ using "diff" attributes is enough for this purpose, then >>> there is no code to be written. But if somebody wants to draw a >>> distinction between the uses (I want to diff "foo" files, but never see >>> them in grep) then we could introduce a "grep" attribute (with the >>> fallback being the value of the "diff" attribute for that path). >> >> That is all true. >> >> If we were to have a new 'grep' attribute that can be used to >> express 'It is OK to diff two versions of this path, but hits by >> grep in this path is useless' (and verse versa), the built-in macro >> attribute 'binary' should also be updated with it. A path being >> 'binary' currently means '-diff -merge -text' but it should also >> mean '-grep' in the new world, if we were to go in that direction. > > I think it would do so automatically. There is no "grep" attribute > given, so we fall back to the "-diff" attribute. But I do not mind > modifying the macro to be more explicit. > > Note also that I am not volunteering to work on this, nor am I convinced > it's actually worth pursuing. I've yet to see a useful case where you > would want text diffs but not greps (or vice versa), and if we can avoid > cluttering the attribute space, we should. I was mostly pointing it out > that it is not logically inconsistent to want such a thing. :) > > If somebody does look into it, I suspect the place to start is modifying > userdiff_find_by_path to optionally prefer "grep" to "diff". > > -Peff > So, as a summary of the discussion, it seems it's time to switch the default to --textconv for git grep? Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Salvaging borked project history
Junio C Hamano wrote: But I personally think "git am -3" may be easier to handle. Thanks! At least now, I see the light at the end of the tunnel. I fetched linux-stable.git inside our repo. I created ~300 patches using git format-patch -1 in a loop. I can now run 'git am --3way $IGNORE *.patch' IGNORE is used to --exclude the directories I'm not interested in. Note: it seems --exclude=arch/mips and --exclude=arch/mips/ are not sufficient, I need to write --exclude=arch/mips/* for git-apply to ignore changes to files inside arch/mips. Is that expected behavior? Another nit: if a patch contains only changes to files inside arch/mips then git-apply will create an "empty commit" (one with no diff). Is there an option to say "skip empty patches"? One more thing: "regular" diff -q returns 0 when the files are identical, and 1 when they differ. It seems git diff -s does not have that behavior. Is that by design? If there is no option to skip empty patches, I'm thinking I can script a fixup step to squash all empty commits. What do you think? Regards. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: feature request: excluding files/paths from "git grep"
On Thu, Feb 26, 2015 at 6:16 PM, Michael J Gruber wrote: > So, as a summary of the discussion, it seems it's time to switch the > default to --textconv for git grep? Either that or make it clearer in git-grep.txt about this diff attribute. It takes me some time to make the connection after reading both git-grep.txt and gitattributes.txt -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/git-clean.txt: document that -f may need to be given twice
This is needed in build automation where the tree really needs to be reset to known state. Signed-off-by: Mikko Rapeli --- Documentation/git-clean.txt |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 94b6d19..872ab45 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -34,8 +34,12 @@ OPTIONS -f:: --force:: If the Git configuration variable clean.requireForce is not set - to false, 'git clean' will refuse to run unless given -f, -n or - -i. + to false, 'git clean' will refuse to delete files or directories + unless given -f, -n or -i. Git will refuse to delete directories + with .git sub directory or file unless a second -f + is given. This affects also git submodules where the storage area of + of the removed submodule under .git/modules/ is not removed until + -f is give twice. -i:: --interactive:: -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/git-clean.txt: document that -f may need to be given twice
This is needed in build automation where the tree really needs to be reset to known state. Signed-off-by: Mikko Rapeli --- Documentation/git-clean.txt |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 94b6d19..bd4e93d 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -34,8 +34,12 @@ OPTIONS -f:: --force:: If the Git configuration variable clean.requireForce is not set - to false, 'git clean' will refuse to run unless given -f, -n or - -i. + to false, 'git clean' will refuse to delete files or directories + unless given -f, -n or -i. Git will refuse to delete directories + with .git sub directory or file unless a second -f + is given. This affects also git submodules where the storage area + of the removed submodule under .git/modules/ is not removed until + -f is give twice. -i:: --interactive:: -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-clean.txt: document that -f may need to be given twice
On Thu, Feb 26, 2015 at 02:56:40PM +0200, Mikko Rapeli wrote: > This is needed in build automation where the tree really needs to > be reset to known state. > > Signed-off-by: Mikko Rapeli > --- > Documentation/git-clean.txt |8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt > index 94b6d19..872ab45 100644 > --- a/Documentation/git-clean.txt > +++ b/Documentation/git-clean.txt > @@ -34,8 +34,12 @@ OPTIONS > -f:: > --force:: > If the Git configuration variable clean.requireForce is not set > - to false, 'git clean' will refuse to run unless given -f, -n or > - -i. > + to false, 'git clean' will refuse to delete files or directories > + unless given -f, -n or -i. Git will refuse to delete directories > + with .git sub directory or file unless a second -f > + is given. This affects also git submodules where the storage area of Oops, "of" is here twice. > + of the removed submodule under .git/modules/ is not removed until > + -f is give twice. > > -i:: > --interactive:: > -- > 1.7.10.4 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] GSoC 2015 application
On Thu, Feb 19, 2015 at 2:14 AM, Jeff King wrote: > Where I really need help now is in the "ideas" page: > > http://git.github.io/SoC-2015-Ideas.html Is this too ambitious for a summer? I suspect the answer is yes, but anyway.. Due to http limitations and stateless decision, a lot of data is sent back and forth during have/want negotiation for smart-http. I wonder if we could implement the "long polling" scheme in a CGI program. The program terminates HTTP requests and recreates a full duplex connection for upload-pack to talk to the client. upload-pack falls back to the normal mode, used by git:// and ssh://. An example of this is BOSH [1]. From a quick glance it does not seem to require any special thing, so it's unlikely to cause problems with firewalls, proxies.. If this is implemented as cgi (instead of http server), we'll need to save session infos somewhere. I suppose shm with proper locking is enough. [1] http://xmpp.org/extensions/xep-0124.html -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/git-clean.txt: document that -f may need to be given twice
This is needed in build automation where the tree really needs to be reset to known state. Signed-off-by: Mikko Rapeli --- Documentation/git-clean.txt |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 94b6d19..641681f 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -34,8 +34,12 @@ OPTIONS -f:: --force:: If the Git configuration variable clean.requireForce is not set - to false, 'git clean' will refuse to run unless given -f, -n or - -i. + to false, 'git clean' will refuse to delete files or directories + unless given -f, -n or -i. Git will refuse to delete directories + with .git sub directory or file unless a second -f + is given. This affects also git submodules where the storage area + of the removed submodule under .git/modules/ is not removed until + -f is given twice. -i:: --interactive:: -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git describe --contains doesn't work properly for a commit
Hi, I have just encountered an old kernel git commit: commit c854363e80b49dd04a4de18ebc379eb8c8806674 Author: Dave Chinner Date: Sat Feb 6 12:39:36 2010 +1100 xfs: Use delayed write for inodes rather than async V2 [...] which cannot be described properly: $ git describe --contains c854363e80b49dd04a4de18ebc379eb8c8806674 fatal: cannot describe 'c854363e80b49dd04a4de18ebc379eb8c8806674' but it seems to find a tag on which the commit is based: $ git describe c854363e80b49dd04a4de18ebc379eb8c8806674 v2.6.33-rc4-49-gc854363e80b4 if I follow parents sha=c854363e80b49dd04a4de18ebc379eb8c8806674; while true do parent=$(git show --format=%P $sha | head -1) echo $sha $parent git describe --contains $parent && break sha=$parent done c854363e80b49dd04a4de18ebc379eb8c8806674 777df5afdb26c71634edd60582be620ff94e87a0 fatal: cannot describe '777df5afdb26c71634edd60582be620ff94e87a0' 777df5afdb26c71634edd60582be620ff94e87a0 d5db0f97fbbeff11c88dec1aaf1536a975afbaeb fatal: cannot describe 'd5db0f97fbbeff11c88dec1aaf1536a975afbaeb' d5db0f97fbbeff11c88dec1aaf1536a975afbaeb 388f1f0c346b533b06d8bc792f7204ebc3e4b7da v2.6.34-rc1~278^2~14 I am using: $ git --version git version 2.1.4 but the same seems to be the case with older git version (1.8.5.6). $ git rev-list c854363e80b49dd04a4de18ebc379eb8c8806674..v2.6.34 | wc -l 11648 So there seems to be a line between the two commits AFAIU. Is the history somehow broken or is it a bug in git? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor
On Feb 26, 2015, at 01:09, Nico Schl=F6mer wrote:
> All,
>
> I applied Kyle's latest patch
>
>> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
>> index 622535e2..96888228 100644
>> --- a/perl/Git/SVN/Ra.pm
>> +++ b/perl/Git/SVN/Ra.pm
>> @@ -391,6 +391,7 @@ sub longest_common_path {
>> sub gs_fetch_loop_common {
>>my ($self, $base, $head, $gsv, $globs) =3D @_;
>>return if ($base > $head);
>> + $::_repository->_open_cat_blob_if_needed;
>>my $gpool =3D SVN::Pool->new_default;
>>my $ra_url =3D $self->url;
>>my $reload_ra =3D sub {
>
> alone and this fixes the bug for me. Thanks a lot, Kyle!
The updated patch with the additional fix is below.
There are two symptoms it addresses:
1) the 'Git_svn_hash_... bad file descriptor' failure
2) the 'out pipe went bad' failure
While (1) could probably also be addressed by Eric's alternate
"destroy all tempfiles when reloading RA" patch, that would require re-
opening all the temp files every 100 revisions (or whatever the log
window size is changed to).
I noticed that with the default log window size of 100 revisions, each
time the connection was reset at the 100-revision boundary (to reduce
the overall memory usage) it seemed to take approx. 3 seconds to start
up again processing revisions on that gmsh repository. If you're
cloning 3 revisions, that adds 15 minutes to the total clone time
already. Admittedly opening new temp files will be a lot faster and
hardly noticeable, but doing that 300 times over the course of 3
revisions will probably add at least a little extra delay and since
blowing away all the temp files does not seem to be necessary, why
incur the extra delay?
Also the "destroy all tempfiles when reloading RA" patch isn't going
to fix the cat_blob 'out pipe went bad' problem since that has nothing
to do with the temp files so another change will still be required for
that.
-Kyle
-- 8< --
Subject: [PATCH v2] Git::SVN::*: avoid premature FileHandle closure
Since b19138b (git-svn: Make it incrementally faster by minimizing temp
files, v1.6.0), git-svn has been using the Git.pm temp_acquire and
temp_release mechanism to avoid unnecessary temp file churn and provide
a speed boost.
However, that change introduced a call to temp_acquire inside the
Git::SVN::Fetcher::close_file function for an 'svn_hash' temp file.
Because an SVN::Pool is active at the time this function is called, if
the Git::temp_acquire function ends up actually creating a new
FileHandle for the temp file (which it will the first time it's called
with the name 'svn_hash') that FileHandle will end up in the SVN::Pool
and should that pool have SVN::Pool::clear called on it that FileHandle
will be closed out from under Git::temp_acquire.
Since the only call site to Git::temp_acquire with the name 'svn_hash'
is inside the close_file function, if an 'svn_hash' temp file is ever
created its FileHandle is guaranteed to be created in the active
SVN::Pool.
This has not been a problem in the past because the SVN::Pool was not
being cleared. However, since dfa72fdb (git-svn: reload RA every
log-window-size, v2.2.0) the pool has been getting cleared periodically
at which point the FileHandle for the 'svn_hash' temp file gets closed.
Any subsequent calls to Git::temp_acquire for 'svn_hash', however,
succeed without creating/opening a new temporary file since it still has
the now invalid FileHandle in its cache. Callers that then attempt to
use that FileHandle fail with an error.
We avoid this problem by making sure the 'svn_hash' temp file is created
in the same place the 'svn_delta_...' and 'git_blob_...' temp files are
(and then temp_release'd) so that it can be safely used inside the
close_file function without having its FileHandle end up in an SVN::Pool
that gets cleared.
Additionally the Git.pm cat_blob function creates a bidirectional pipe
FileHandle using the IPC::Open2::open2 function. If that handle is
created too late, it also gets caught up in the SVN::Pool and incorrectly
closed by the SVN::Pool::clear call. But this only seems to happen with
more recent versions of Perl and svn.
To avoid this problem we add an explicit call to _open_cat_blob_if_needed
before the first call to SVN::Pool->new_default to make sure the open2
handle does not end up in the SVN::Pool.
Signed-off-by: Kyle J. McKay
---
perl/Git/SVN/Fetcher.pm | 8
perl/Git/SVN/Ra.pm | 3 +++
2 files changed, 11 insertions(+)
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index 10edb277..613055a3 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -322,6 +322,14 @@ sub apply_textdelta {
# (but $base does not,) so dup() it for reading in close_file
open my $dup, '<&', $fh or croak $!;
my $base = $::_repository->temp_acquire("git_blob_${$}_$suffix");
+ # close_file may call temp_acquire on 'svn_hash', but because of the
+ # call chain, if the temp_acquire call from close_f
Re: git describe --contains doesn't work properly for a commit
On Thu 26-02-15 14:35:34, Michal Hocko wrote: > Hi, > I have just encountered an old kernel git commit: > commit c854363e80b49dd04a4de18ebc379eb8c8806674 > Author: Dave Chinner > Date: Sat Feb 6 12:39:36 2010 +1100 > > xfs: Use delayed write for inodes rather than async V2 > [...] OK, I've managed to recreate this in a simple repo with 3 commits: $ git log --format="%H %cd" ab0efec2b697f2f9f864bb0e2cd77308d1f04561 Thu Feb 26 15:18:36 2015 +0100 d63972e4e4e7eda0444e56739ad09bfbc476b9bd Wed Feb 26 15:18:30 2014 +0100 108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3 Thu Feb 26 14:57:29 2015 +0100 The commit in the middle was ammended to have committer date in the past. $ git describe --contains d63972e4e4e7eda0444e56739ad09bfbc476b9bd tag~1 but $ git describe --contains 108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3 fatal: cannot describe '108a0d5972fd2e5f25b2f38cfd2fee73031ff9d3' I guess this is the same issue reported previously here: http://git.661346.n2.nabble.com/git-describe-contains-fails-on-given-tree-td5448286.html Can this be fixed somehow or it would lead to other kind of issues? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
weird behaviour in git
Hi! I've played around with git and found that 'git mv' does not honor what I tell it to do: wiz@yt:~> mkdir a wiz@yt:~> cd a wiz@yt:~/a> git init . Initialized empty Git repository in /home/wiz/a/.git/ wiz@yt:~/a> touch a wiz@yt:~/a> git add a wiz@yt:~/a> git commit -m 'add a' [master (root-commit) 99d0ee7] add a 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 a wiz@yt:~/a> git mv a b wiz@yt:~/a> touch Makefile wiz@yt:~/a> git add Makefile wiz@yt:~/a> git commit # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # On branch master # Changes to be committed: # renamed:a -> Makefile # new file: b # This is reproducible for me with "git version 2.3.0" on NetBSD-7.99.5/amd64. I guess this happens because the checksums of the files are the same and 'Makefile' is earlier when sorting, but since I explicitly told "git mv" old and new name, I think that's a bug nevertheless. Thomas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: weird behaviour in git
Thomas Klausner venit, vidit, dixit 26.02.2015 15:12: > Hi! > > I've played around with git and found that 'git mv' does not honor > what I tell it to do: > > wiz@yt:~> mkdir a > wiz@yt:~> cd a > wiz@yt:~/a> git init . > Initialized empty Git repository in /home/wiz/a/.git/ > wiz@yt:~/a> touch a > wiz@yt:~/a> git add a > wiz@yt:~/a> git commit -m 'add a' > [master (root-commit) 99d0ee7] add a > 1 file changed, 0 insertions(+), 0 deletions(-) > create mode 100644 a > wiz@yt:~/a> git mv a b > wiz@yt:~/a> touch Makefile > wiz@yt:~/a> git add Makefile > wiz@yt:~/a> git commit > > > # Please enter the commit message for your changes. Lines starting > # with '#' will be ignored, and an empty message aborts the commit. > # On branch master > # Changes to be committed: > # renamed:a -> Makefile > # new file: b > # > > This is reproducible for me with "git version 2.3.0" on > NetBSD-7.99.5/amd64. > > I guess this happens because the checksums of the files are the same > and 'Makefile' is earlier when sorting, but since I explicitly told > "git mv" old and new name, I think that's a bug nevertheless. > Thomas > git tracks content, not paths. It does record the path at which the tracked content is, of course. But it tracks the history of content, not that of paths. What you see in the diff above is merely one way to interpret the history of the content. Saying renamed: a -> b new file: Makefile leads to the same content at the same paths (with the proper new file content). By default, diff tries to interpret content history in terms of renames and copies when possible, in order to help users. Sometimes this fails - while still being correct, it confuses them ;) Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: weird behaviour in git
On Thu, Feb 26, 2015 at 03:45:13PM +0100, Michael J Gruber wrote: > Thomas Klausner venit, vidit, dixit 26.02.2015 15:12: > > Hi! > > > > I've played around with git and found that 'git mv' does not honor > > what I tell it to do: > > > > wiz@yt:~> mkdir a > > wiz@yt:~> cd a > > wiz@yt:~/a> git init . > > Initialized empty Git repository in /home/wiz/a/.git/ > > wiz@yt:~/a> touch a > > wiz@yt:~/a> git add a > > wiz@yt:~/a> git commit -m 'add a' > > [master (root-commit) 99d0ee7] add a > > 1 file changed, 0 insertions(+), 0 deletions(-) > > create mode 100644 a > > wiz@yt:~/a> git mv a b > > wiz@yt:~/a> touch Makefile > > wiz@yt:~/a> git add Makefile > > wiz@yt:~/a> git commit > > > > > > # Please enter the commit message for your changes. Lines starting > > # with '#' will be ignored, and an empty message aborts the commit. > > # On branch master > > # Changes to be committed: > > # renamed:a -> Makefile > > # new file: b > > # > > > > This is reproducible for me with "git version 2.3.0" on > > NetBSD-7.99.5/amd64. > > > > I guess this happens because the checksums of the files are the same > > and 'Makefile' is earlier when sorting, but since I explicitly told > > "git mv" old and new name, I think that's a bug nevertheless. > > Thomas > > > > git tracks content, not paths. > > It does record the path at which the tracked content is, of course. But > it tracks the history of content, not that of paths. > > What you see in the diff above is merely one way to interpret the > history of the content. Saying > > renamed: a -> b > new file: Makefile > > leads to the same content at the same paths (with the proper new file > content). > > By default, diff tries to interpret content history in terms of renames > and copies when possible, in order to help users. Sometimes this fails - > while still being correct, it confuses them ;) Sure, that's one way to look at it, but I disagree. You give the user the way to tell the system the intention of which file moves where, but internally this information is lost and "guessed" incorrectly. hg seems to do this correctly, the same commands with 'hg diff --git' at the end show: diff --git a/Makefile b/Makefile new file mode 100644 diff --git a/a b/b rename from a rename to b Thomas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sha1_file: Add sha1_object_type_literally and export it.
On Wed, 2015-02-25 at 16:55 -0500, Eric Sunshine wrote:
> I had written a longer review but was interrupted for a several hours,
> and upon returning found that David and Junio covered many of the same
> issues or overrode comments I was making, so the below review is pared
> down quite a bit. Junio's proposed approach negates all of the below
> review comments, but they may still be meaningful if kept in mind for
> future submissions.
>
> On Wed, Feb 25, 2015 at 6:07 AM, Karthik Nayak wrote:
> > sha1_file: Add sha1_object_type_literally and export it.
>
> Style: downcase "Add"; drop terminating period.
>
> > sha1_object_type_literally takes a sha value and
> > gives the type of the given loose object, used by
> > git cat-file -t --literally.
> >
> > Signed-off-by: Karthik Nayak
> > ---
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -2635,6 +2635,33 @@ int sha1_object_info(const unsigned char *sha1,
> > unsigned long *sizep)
> > return type;
> > }
> >
> > +int sha1_object_type_literally(const unsigned char *sha1, char *type)
>
> This functionality is very specific to the --literally option you're
> adding to cat-file, so it would make more sense to make it private to
> builtin/cat-file.c rather than publishing it globally.
>
> Also, this is an unsafe contract. The caller does not know how many
> bytes to allocate for 'type', and this new function may write past the
> end of the buffer. It is more common to also pass in the size of the
> 'type' buffer and ensure that you do not write beyond that. Or, if
> this is intended for wider consumption, pass in a strbuf instead.
>
> > +{
> > + int status = 0;
> > + unsigned long mapsize;
> > + void *map;
> > + git_zstream stream;
> > + char hdr[32];
> > + int i;
> > +
> > + map = map_sha1_file(sha1, &mapsize);
> > + if (!map)
> > + return -1;
> > + if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
> > + status = error("unable to unpack %s header",
> > + sha1_to_hex(sha1));
>
> Since 'hdr' unpacking failed, shouldn't you be returning at this point
> rather than continuing to the 'hdr' processing loop?
>
> > + for (i = 0; i < 32; i++) {
> > + if (hdr[i] == ' ') {
> > + type[i] = '\0';
> > + break;
> > + }
> > + type[i] = hdr[i];
> > + }
>
> David already mentioned that this loop is suspect. Perhaps take a look
> at, sha1_file.c:parse_sha1_header() for an example of cleaner logic.
>
> > +
> > + return status;
> > +}
> > +
> > static void *read_packed_sha1(const unsigned char *sha1,
> > enum object_type *type, unsigned long *size)
> > {
> > --
> > 2.3.1.129.g11acff1.dirty
Thanks for all your inputs, I will work on the points you've mentioned
considering what David and Junio also have mentioned.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: weird behaviour in git
Thomas Klausner venit, vidit, dixit 26.02.2015 15:58: > On Thu, Feb 26, 2015 at 03:45:13PM +0100, Michael J Gruber wrote: >> Thomas Klausner venit, vidit, dixit 26.02.2015 15:12: >>> Hi! >>> >>> I've played around with git and found that 'git mv' does not honor >>> what I tell it to do: >>> >>> wiz@yt:~> mkdir a >>> wiz@yt:~> cd a >>> wiz@yt:~/a> git init . >>> Initialized empty Git repository in /home/wiz/a/.git/ >>> wiz@yt:~/a> touch a >>> wiz@yt:~/a> git add a >>> wiz@yt:~/a> git commit -m 'add a' >>> [master (root-commit) 99d0ee7] add a >>> 1 file changed, 0 insertions(+), 0 deletions(-) >>> create mode 100644 a >>> wiz@yt:~/a> git mv a b >>> wiz@yt:~/a> touch Makefile >>> wiz@yt:~/a> git add Makefile >>> wiz@yt:~/a> git commit >>> >>> >>> # Please enter the commit message for your changes. Lines starting >>> # with '#' will be ignored, and an empty message aborts the commit. >>> # On branch master >>> # Changes to be committed: >>> # renamed:a -> Makefile >>> # new file: b >>> # >>> >>> This is reproducible for me with "git version 2.3.0" on >>> NetBSD-7.99.5/amd64. >>> >>> I guess this happens because the checksums of the files are the same >>> and 'Makefile' is earlier when sorting, but since I explicitly told >>> "git mv" old and new name, I think that's a bug nevertheless. >>> Thomas >>> >> >> git tracks content, not paths. >> >> It does record the path at which the tracked content is, of course. But >> it tracks the history of content, not that of paths. >> >> What you see in the diff above is merely one way to interpret the >> history of the content. Saying >> >> renamed: a -> b >> new file: Makefile >> >> leads to the same content at the same paths (with the proper new file >> content). >> >> By default, diff tries to interpret content history in terms of renames >> and copies when possible, in order to help users. Sometimes this fails - >> while still being correct, it confuses them ;) > > Sure, that's one way to look at it, but I disagree. You give the user > the way to tell the system the intention of which file moves where, > but internally this information is lost and "guessed" incorrectly. > > hg seems to do this correctly, the same commands with 'hg diff --git' > at the end show: > > diff --git a/Makefile b/Makefile > new file mode 100644 > diff --git a/a b/b > rename from a > rename to b > > Thomas > Maybe you can re-read what I wrote above, keeping in mind the first line: git tracks content, not paths. That explains everything, really. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: weird behaviour in git
Thomas Klausner writes: > I've played around with git and found that 'git mv' does not honor > what I tell it to do: > > wiz@yt:~> mkdir a > wiz@yt:~> cd a > wiz@yt:~/a> git init . > Initialized empty Git repository in /home/wiz/a/.git/ > wiz@yt:~/a> touch a > wiz@yt:~/a> git add a > wiz@yt:~/a> git commit -m 'add a' > [master (root-commit) 99d0ee7] add a > 1 file changed, 0 insertions(+), 0 deletions(-) > create mode 100644 a > wiz@yt:~/a> git mv a b > wiz@yt:~/a> touch Makefile > wiz@yt:~/a> git add Makefile > wiz@yt:~/a> git commit > > > # Please enter the commit message for your changes. Lines starting > # with '#' will be ignored, and an empty message aborts the commit. > # On branch master > # Changes to be committed: > # renamed:a -> Makefile > # new file: b > # git mv was tasked with removing file a and creating file b with the same content and permissions. It did so successfully. "Changes to be committed" states an interpretation consistent with that. Now it is entirely silly in my book to describe files as "renamed" that are actually empty and thus do not contain a single common byte. I would call that change description a bug or at least a "misfeature". git mv, however, did exactly what it was tasked to do and could not possibly do anything better since Git does, by design, not ever track file operations. > This is reproducible for me with "git version 2.3.0" on > NetBSD-7.99.5/amd64. > > I guess this happens because the checksums of the files are the same > and 'Makefile' is earlier when sorting, but since I explicitly told > "git mv" old and new name, I think that's a bug nevertheless. No. Git mv is just a convenience command for deleting one file and creating another one with the same contents. Git has no concept of file renames in its repository, so git mv cannot record anything there that could not be interpreted exactly like the commit info interpreted it. It's nonsensical and should in my opinion rather be stated as # Changes to be committed: # removed:a # new file: Makefile # new file: b But that's not the fault of Git mv. -- David Kastrup -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Feb 2015, #07; Thu, 26)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Fifth batch of topics have been merged to 'master', including both fixes and enhancements. First maintenance release for v2.3 was cut with many fixes that have already been merged to 'master'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * ak/git-pm-typofix (2015-02-18) 1 commit (merged to 'next' on 2015-02-20 at 0fa233a) + Git.pm: two minor typo fixes Typofix in comments. * dp/remove-duplicated-header-inclusion (2015-02-13) 1 commit (merged to 'next' on 2015-02-18 at a1bf108) + do not include the same header twice Code clean-up. * jc/max-io-size-and-ssize-max (2015-02-12) 1 commit (merged to 'next' on 2015-02-18 at 0c8a4da) + xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX Our default I/O size (8 MiB) for large files was too large for some platforms with smaller SSIZE_MAX, leading to read(2)/write(2) failures. * jc/send-email-sensible-encoding (2015-02-13) 1 commit (merged to 'next' on 2015-02-18 at 7457655) + send-email: ask confirmation if given encoding name is very short "git send-email" used to accept a mistaken "y" (or "yes") as an answer to "What encoding do you want to use [UTF-8]? " without questioning. Now it asks for confirmation when the answer looks too short to be a valid encoding name. * jk/fast-import-die-nicely-fix (2015-02-10) 1 commit (merged to 'next' on 2015-02-18 at e249425) + fast-import: avoid running end_packfile recursively "git fast-import" used to crash when it could not close and conclude the resulting packfile cleanly. * jk/sanity (2015-02-15) 3 commits (merged to 'next' on 2015-02-18 at 5c54b53) + test-lib.sh: set prerequisite SANITY by testing what we really need + tests: correct misuses of POSIXPERM + t/lib-httpd: switch SANITY check for NOT_ROOT The tests that wanted to see that file becomes unreadable after running "chmod a-r file", and the tests that wanted to make sure it is not run as root, we used "can we write into the / directory?" as a cheap substitute, but on some platforms that is not a good heuristics. The tests and their prerequisites have been updated to check what they really require. * jk/strbuf-doc-to-header (2015-01-16) 7 commits (merged to 'next' on 2015-02-18 at 0482c65) + strbuf.h: group documentation for trim functions + strbuf.h: drop boilerplate descriptions of strbuf_split_* + strbuf.h: reorganize api function grouping headers + strbuf.h: format asciidoc code blocks as 4-space indent + strbuf.h: drop asciidoc list formatting from API docs + strbuf.h: unify documentation comments beginnings + strbuf.h: integrate api-strbuf.txt documentation The strbuf API was explained between the API documentation and in the header file. Move missing bits to strbuf.h so that programmers can check only one place for all necessary information. * jn/doc-api-errors (2014-12-04) 1 commit (merged to 'next' on 2015-02-18 at f60eda6) + doc: document error handling functions and conventions The error handling functions and conventions are now documented in the API manual. * mh/transport-capabilities (2015-02-13) 2 commits (merged to 'next' on 2015-02-18 at 87e8fcc) + transport-helper: ask the helper to set the same options for import as for fetch + transport-helper: ask the helper to set progress and verbosity options after asking for its capabilities The transport-helper did not give transport options such as verbosity, progress, cloning, etc. to import and export based helpers, like it did for fetch and push based helpers, robbing them the chance to honor the wish of the end-users better. * nd/attr-optim (2014-12-29) 3 commits (merged to 'next' on 2015-02-18 at 598e68a) + attr: avoid heavy work when we know the specified attr is not defined + attr: do not attempt to expand when we know it's not a macro + attr.c: rename arg name attr_nr to avoid shadowing the global one Optimize attribute look-up, mostly useful in "git grep" on a project that does not use many attributes, by avoiding it when we (should) know that the attributes are not defined in the first place. * sb/hex-object-name-is-at-most-41-bytes-long (2015-02-13) 1 commit (merged to 'next' on 2015-02-18 at 53d522b) + hex.c: reduce memory footprint of sha1_to_hex static buffers Code clean-up. * sb/plug-leak-in-make-cache-entry (2015-02-17) 1 commit (merged to 'next' on 2015-02-18 at e637f65) + read-cache.c: free cache entry when refreshing fails "update-index --refresh" used to leak when an entry cannot be refreshed for whatever reason. * tc/missing-http-proxyauth (2015-02-03) 1 commit (merged to 'next' on 2015-02-18 at 8ff01ad) + http: support curl < 7
Re: Salvaging borked project history
Mason writes: > Thanks! At least now, I see the light at the end of the tunnel. > > I fetched linux-stable.git inside our repo. > I created ~300 patches using git format-patch -1 in a loop. > I can now run 'git am --3way $IGNORE *.patch' > > IGNORE is used to --exclude the directories I'm not interested in. > > Note: it seems --exclude=arch/mips and --exclude=arch/mips/ are > not sufficient, I need to write --exclude=arch/mips/* for git-apply > to ignore changes to files inside arch/mips. > > Is that expected behavior? I have no idea; at least to me, "--exclude" option to "git apply" was invented to name individual paths, not patterns, and I wouldn't be surprised if glob working were merely by accident not by design. > Another nit: if a patch contains only changes to files inside arch/mips > then git-apply will create an "empty commit" (one with no diff). Is there > an option to say "skip empty patches"? "git am --skip" perhaps? "git am" may pass the "--exclude/--include" options to "git apply" but I wouldn't be surprised if that support was added without thinking. Perhaps the reason why you discovered that it needed a lot more thinking to properly integrate these options to "git am" only now is because hardly anybody uses it ;-). Not just passing these options, the code in "git am" to react to the result of patch application to avoid the issue you observed when these options are passed need to be adjusted by changes that started passing them, but I do not think they did, cf. 77e9e496 (am: pass exclude down to apply, 2011-08-03). > One more thing: "regular" diff -q returns 0 when the files are identical, > and 1 when they differ. It seems git diff -s does not have that behavior. > Is that by design? "diff -s" may be accepted but it is an idiotic thing for a user to say. The "-s" option is to squelch output from "log" and friends, and it is exposed to "diff" only because these two families of commands share the command line parser. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sequencer: preserve commit messages
Michael J Gruber writes: > Hmm. With "--edit", current config being in effect should be expected, > right? So how about: > > In case of no conflict: force cleanup=verbatim unless --edit is used? Perhaps something like that. Stepping back a bit and imagine a world where the sole purpose of cherry-pick were to recreate the original commit as faithfully as possible. The commit log message would not be cleaned up in such a case by default, and the users need cherrypick.cleanup setting if they do not like that default. The implementation of cherry-pick that does not spawn the editor in that world would look like this: - read the cleanup mode from cherrypick.cleanup config; if there is none, read the cleanup mode from commit.cleanup config; if neither is defined, then use 'verbatim' as the default; - invoke "commit --cleanup=" + that mode from the command line to force the mode chosen by the above. Thanks to the falling back to commit.cleanup, the above logic would be usable even before we invent cherrypick.cleanup configuration, i.e. in today's world. If there is no commit.cleanup defined by the user, the above logic would still use 'verbatim' as the default for 'cherry-pick', while using the 'default' for 'commit'. When cherry-pick invokes the editor, then the first part would be different. So my conclusion would be something like: #if IN_THE_FUTURE if (config_exists(cherrypick.cleanup)) mode = config_value(cherrypick.cleanup); else #endif if (config_exists(commit.cleanup)) mode = config_value(commit.cleanup); else mode = editing ? 'verbatim' : 'default'; invoke "commit --cleanup=" + mode; perhaps? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] protocol v2
On Thu, Feb 26, 2015 at 2:15 AM, Duy Nguyen wrote:
> On Thu, Feb 26, 2015 at 2:31 PM, Stefan Beller wrote:
>> On Wed, Feb 25, 2015 at 10:04 AM, Junio C Hamano wrote:
>>> Duy Nguyen writes:
>>>
On Wed, Feb 25, 2015 at 6:37 AM, Stefan Beller wrote:
> I can understand, that we maybe want to just provide one generic
> "version 2" of the protocol which is an allrounder not doing bad in
> all of these aspects, but I can see usecases of having the desire to
> replace the wire protocol by your own implementation. To do so
> we could try to offer an API which makes implementing a new
> protocol somewhat easy. The current state of affairs is not providing
> this flexibility.
I think we are quite flexible after initial ref advertisement.
>>>
>>> Yes, that is exactly where my "I am not convinced" comes from.
>>>
>>
>> We are not. (not really at least). We can tune some parameters or
>> change the behavior slightly,
>> but we cannot fix core assumptions made when creating v2 protocol.
>> This you can see when when talking about v1 as well: we cannot fix any
>> wrongdoings of v1 now by adding another capability.
>
> Step 1 then should be identifying these wrongdoings and assumptions.
So I think one of the key assumption was to not have many refs to advertise,
and advertising the refs is fine under that assumption.
So from my point of view it is hard to change the general
>
> We can really go wild with these capabilities. The only thing that
> can't be changed is perhaps sending the first ref. I don't know
> whether we can accept a dummy first ref... After that point, you can
> turn the protocol upside down because both client and server know what
> it would be.
So the way I currently envision (the transition to and) version 2 of
the protocol:
First connection (using the protocol as of now):
Server: Here are all the refs and capabilities I can offer. The capabilities
include "not-send-refs-first" (aka version2)
Client: Ok, I'll store not-send-refs-first for next time. Now we will
continue with
these options: <...>. For now we continue using the current
protocol and I want
to update the "master" branch.
Server: Ok here is a pack file, and then master advances $SHA1..$SHA1
Client: ok, thanks, bye
For the next connection I have different ideas:
Client thinks v2 is supported, so it talks first: Last time we talked
your capabilities
hashed to "$SHA1", is that still correct?
Server: yes it is
# In the above roundtrip we would have a new key assumption that the
capabilities
# don't change often. Having push-certs enabled, this is invalid of
today. Hover this
# could be implemented with very low bandwidth usage
# The alternative path would be:
# Server: No my new capabilities are: <>
Client: Ok I want to update all of refs/heads/{master,next,pu}. My
last fetch was
yesterday at noon.
Server: Let me check the ref logs for these refs. Here is a packfile of length
1000 bytes:
{master, next} did not update since yesterday noon, pu updates from A..B
Client: ok, thanks, bye
Another approach would be this:
Client thinks v2 is supported, so it talks first: Last time we talked
you sent me
refs advertisement including capabilities which hash to $SHA1.
Server: I see, I have stored that. Now that time has advanced there are a few
differences, here is a diff of the refs advertisement:
* b3a551adf53c224b04c40f05b72a8790807b3138 HEAD\0
* b3a551adf53c224b04c40f05b72a8790807b3138 refs/heads/master
- 24ca137a384aa1ac5a776eddaf35bb820fc6f6e6 refs/heads/tmp-fix
+ 1da8335ad5d0e46062a929ba6481bbbe35c8eef0 refs/pull/123/head
Note that I do not include changed lines as +one line and - one
line as you know
what the line was by your given $SHA1, so changed lines are marked
with *, while
lines starting with '-' indicate deleted refs and '+' indicate new refs.
Client: I see, I can reconstruct the refs advertisement. Now we can
continue talking
as we always talked using v1 protocol.
>
>> So from my point
>> of view we don't waste resources when having an advertisement of
>> possible protocols instead of a boolean flag indicating v2 is
>> supported. There is really not much overhead in coding nor bytes
>> exchanged on the wire, so why not accept stuff that comes for free
>> (nearly) ?
>
> You realize you're advertising v2 as a new capability, right? Instead
> of defining v2 feature set then advertise v2, people could simply add
> new features directly. I don't see v2 (at least with these patches)
> adds any value.
Yes, we can go wild after the refs advertisement, but that is not the
critical problem as it works ok-ish? The problem I see for now is the huge
refs advertisement even before the capabilities are exchanged. So maybe
I do not want to talk about v2 but about changing the current protocol to first
talk about the capabilities in the first round trip, not sure if we
ever want to attach
data into the first RT as it may explode as soon as that inf
git probably bug
Hello. Thanks for the git, it is powerful, fast and nice to use. I've probably found a bug. When checking out into some different state, if there is no permissions to unlink files (if some files in the current state doesn't exist in the new) warnings are outputing, but then git says that checking out is successful, shows that we are in the new state and the non-unlinked files are marked as untracked. In my case. there was whole directories only, i don't know about this error when we cannot unlink different files or when the denied operation is not file removing (but it seems like the error is about unlinking only). My git version is 2.1.0. I've found your email at https://github.com/git/git/blob/master/README#L45 and don't know about any other bugtrackers. Best regards, Vitaly Bormotov aka Quilfe.
Re: [RFC/PATCH 0/3] protocol v2
Duy Nguyen writes: > Step 1 then should be identifying these wrongdoings and assumptions. > > We can really go wild with these capabilities. The only thing that > can't be changed is perhaps sending the first ref. I don't know > whether we can accept a dummy first ref... After that point, you can > turn the protocol upside down because both client and server know what > it would be. Yes, exactly. To up/down/side-grade from v1 is technically possible, but being technically possible is different from being sensible. The capability-based sidegrade does not solve the problem when the problem to be solved is that the server side needs to spend a lot of cycles and the network needs to carry megabytes of data before capability exchange happens. Yes, the newer server and the newer client can notice that the counterparty is new and start talking in new protocol (which may or may not benefit from already knowing the result of ref advertisement), but by the time that happens, the resource has already been spent and wasted. I do not think v1 can be fixed by "send one ref with capability, newer client may respond immediately so we can stop enumerating remaining refs and older one will get stuck so we can have a timeout to see if the connection is from the newer one, and send the rest for the older client", because anything that involves such a timeout would not reliably work over WAN. > You realize you're advertising v2 as a new capability, right? Instead > of defining v2 feature set then advertise v2, people could simply add > new features directly. I don't see v2 (at least with these patches) > adds any value. I agree with the value assessment of these patches 98%, but these bits can be taken as the "we have v2 server availble for you on the side, by the way" hint you mentioned in the older thread, I think. > And we already does that, except that we don't state what version (as > a number) exactly, but what feature that version supports. The focus > should be the new protocol at daemon.c and maybe remote-curl.c where > we do know the current protocol is not flexible enough. The "first" thing the client tells the server is what service it requests. A request over git:// protocol is read by "git daemon" to choose which service to run, and it is read directly by the login shell if it comes over ssh:// protocol. There is nothing that prevents us from defining that service to be a generic "git service", not "upload-pack", "archive", "receive-pack". And the early protocol exchange, once "git service" is spawned, with the client can be "what real services does the server end support?" capability list responded with "wow, you are new enough to support the 'trickle-pack' service---please connect me to it" request. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Salvaging borked project history
Junio C Hamano wrote: > Mason wrote: > >> I fetched linux-stable.git inside our repo. >> I created ~300 patches using git format-patch -1 in a loop. >> I can now run 'git am --3way $IGNORE *.patch' >> >> IGNORE is used to --exclude the directories I'm not interested in. >> >> Note: it seems --exclude=arch/mips and --exclude=arch/mips/ are >> not sufficient, I need to write --exclude=arch/mips/* for git-apply >> to ignore changes to files inside arch/mips. >> >> Is that expected behavior? > > I have no idea; at least to me, "--exclude" option to "git apply" > was invented to name individual paths, not patterns, and I wouldn't > be surprised if glob working were merely by accident not by design. According to the docs, IIUC globbing is by design: http://git-scm.com/docs/git-apply --exclude= Don't apply changes to files matching the given path pattern. This can be useful when importing patchsets, where you want to exclude certain files or directories. This lead me to believe that --exclude=arch/mips should work. >> Another nit: if a patch contains only changes to files inside arch/mips >> then git-apply will create an "empty commit" (one with no diff). Is there >> an option to say "skip empty patches"? > > "git am --skip" perhaps? Nah, as the doc states, --skip "is only meaningful when restarting an aborted patch." > "git am" may pass the "--exclude/--include" options to "git apply" > but I wouldn't be surprised if that support was added without > thinking. Perhaps the reason why you discovered that it needed a > lot more thinking to properly integrate these options to "git am" > only now is because hardly anybody uses it ;-). I do have a rather obscure use-case (and if I do it right, I will never have to do it again). > Not just passing > these options, the code in "git am" to react to the result of patch > application to avoid the issue you observed when these options are > passed need to be adjusted by changes that started passing them, but > I do not think they did, cf. 77e9e496 (am: pass exclude down to > apply, 2011-08-03). Sorry, I could not parse that paragraph :-) >> One more thing: "regular" diff -q returns 0 when the files are identical, >> and 1 when they differ. It seems git diff -s does not have that behavior. >> Is that by design? > > "diff -s" may be accepted but it is an idiotic thing for a user to > say. The "-s" option is to squelch output from "log" and friends, > and it is exposed to "diff" only because these two families of > commands share the command line parser. Here is the use-case: if diff -q A B; then do_X; else do_Y; fi It makes sense to prevent diff from writing to stdout. I was planning to write 'git diff -q commit^ commit' to test for empty commits. Looks like I'll be needing 'git diff commit^ commit | wc -l' instead? Regards. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
compiling master fails
On Post 2.3 cyle (batch #5) (v2.3.1-167-g7f4ba4b) CC http.o http.c: In function 'get_preferred_languages': http.c:1021:2: warning: implicit declaration of function 'setlocale' [-Wimplicit-function-declaration] retval = setlocale(LC_MESSAGES, NULL); ^ http.c:1021:21: error: 'LC_MESSAGES' undeclared (first use in this function) retval = setlocale(LC_MESSAGES, NULL); ^ http.c:1021:21: note: each undeclared identifier is reported only once for each function it appears in make: *** [http.o] Error 1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: compiling master fails
Removing config.mak makes it compile again. config.mak contained: CFLAGS += -Wall -g -rdynamic -O0 # CFLAGS += -Wextra # CFLAGS += -Werror CFLAGS += -Wno-format-zero-length CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wpointer-arith CFLAGS += -Wstrict-prototypes CFLAGS += -Wold-style-declaration # CFLAGS += -flto # CFLAGS += -fwhole-program CC = /usr/lib/gcc-snapshot/bin/gcc Mind that compiling did work with this exact version of config.mak, (I did not touch it for a few weeks now), so I suspect new changes in changes to break the compilation. On Thu, Feb 26, 2015 at 12:48 PM, Stefan Beller wrote: > On Post 2.3 cyle (batch #5) (v2.3.1-167-g7f4ba4b) > > CC http.o > http.c: In function 'get_preferred_languages': > http.c:1021:2: warning: implicit declaration of function 'setlocale' > [-Wimplicit-function-declaration] > retval = setlocale(LC_MESSAGES, NULL); > ^ > http.c:1021:21: error: 'LC_MESSAGES' undeclared (first use in this function) > retval = setlocale(LC_MESSAGES, NULL); > ^ > http.c:1021:21: note: each undeclared identifier is reported only once > for each function it appears in > make: *** [http.o] Error 1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Any chance for a Git v2.1.5 release?
"Kyle J. McKay" writes: > I would like to better understand how the various heads are > maintained. I've read MaintNotes and I've got the concepts, but I'm > still a little fuzzy on some details. It looks to me like all topics > still only in pu after master has been updated are then rebased onto > the new master and then pu is rebuilt. Topics in 'pu' not yet in 'next' _can_ be rebased, but unless there is a good reason to do so, I wouldn't spend extra cycles necessary to do such thing. I even try to keep the same base when replacing the contents of a branch with a rerolled series. For example, today I have this: $ git config alias.lgf log --oneline --boundary --first-parent $ git lgf master..nd/slim-index-pack-memory-usage 3538291 index-pack: kill union delta_base to save memory 7b4ff41 FIXUP 45b6b71 index-pack: reduce memory footprint a bit - 9874fca Git 2.3 and Duy has a newer iteration of it starting at $gmane/264429. What I would do, after saving these patches in a mbox +nd-index-pack, would be to $ git checkout 9874fca $ git am -s3c ./+nd-index-pack $ git show-branch HEAD nd/slim-index-pack-memory-usage ... compare corresponding changes between the old and the new ... until I am satisified; otherwise I may tweak the new one $ git rebase -i 9874fca ... and then finally $ git branch -f nd/slim-index-pack-memory-usage HEAD to update the topic. Of course, it would be necessary to rebuild 'pu' by merging all the topics that are in it on top of 'master'. https://github.com/gitster/git.git has these topic branches broken out and you can observe how they change over time from your local reflog for refs/remotes//. > I vaguely recall you may have explained some of this in more detail in > the context of explaining how you used the scripts in todo to put > everything together when someone asked a question about it on the > list. But I can't seem to find that message anymore. :( There may be a copy in Documentaiton/howto/ somewhere. > I think I mostly understand how master, next and pu are maintained. > But MaintNotes says "Whenever a feature release is made, 'maint' > branch is forked off from 'master' at that point." What happens to > the previous maint at that time? Is it just renamed to maint-X.X? After finishing 2.3.0 release, at some point while 'master' is still at 2.3.0, something like this would happen: $ git branch -m maint maint-2.2 $ git branch maint master > Also, how do you handle a down merge to maint when you have this: > > * (master) > * merge topic foo > |\ > | * topic foo > |/ > * c > * b > * a > * (tag: vX.X.X, maint) > * I don't, and this is done by making sure I do not get into such a situation in the first place. A general rule of thumb when applying a set of patches that are fixes eventually worth having in older maintenance tracks is to find the oldest maintenance branch and apply them there. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: compiling master fails
> On Thu, Feb 26, 2015 at 12:48 PM, Stefan Beller wrote: >> On Post 2.3 cyle (batch #5) (v2.3.1-167-g7f4ba4b) >> >> CC http.o >> http.c: In function 'get_preferred_languages': >> http.c:1021:2: warning: implicit declaration of function 'setlocale' >> [-Wimplicit-function-declaration] >> retval = setlocale(LC_MESSAGES, NULL); >> ^ >> http.c:1021:21: error: 'LC_MESSAGES' undeclared (first use in this function) >> retval = setlocale(LC_MESSAGES, NULL); >> ^ >> http.c:1021:21: note: each undeclared identifier is reported only once >> for each function it appears in >> make: *** [http.o] Error 1 On Thu, Feb 26, 2015 at 12:52 PM, Stefan Beller wrote: > Removing config.mak makes it compile again. > config.mak contained: > CFLAGS += -Wall -g -rdynamic -O0 > # CFLAGS += -Wextra > # CFLAGS += -Werror > CFLAGS += -Wno-format-zero-length > CFLAGS += -Wdeclaration-after-statement > CFLAGS += -Wpointer-arith > CFLAGS += -Wstrict-prototypes > CFLAGS += -Wold-style-declaration > # CFLAGS += -flto > # CFLAGS += -fwhole-program > CC = /usr/lib/gcc-snapshot/bin/gcc > > Mind that compiling did work with this exact version of > config.mak, (I did not touch it for a few weeks now), > so I suspect new changes in changes to break the > compilation. > > "git bisect run make" shows f18604bbf2c391c689a41fca14cbaeff5e106255 is the first bad commit commit f18604bbf2c391c689a41fca14cbaeff5e106255 Author: Yi EungJun Date: Wed Jan 28 21:04:37 2015 +0900 Is there an easy way to fix it or is my config.mak bogus now? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: feature request: excluding files/paths from "git grep"
Michael J Gruber writes: > So, as a summary of the discussion, it seems it's time to switch the > default to --textconv for git grep? Hmmm, why? Nobody seems to be asking for such a change in this thread. The original issue IIRC was that the grep output was unnecessary for some paths and the repository did not mark these paths as such. Once they are marked as "-diff", there is no reason why you want to trigger textconv to squelch the hits from grep. So that does not sound to me a summary of the discussion at all. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: compiling master fails
Hi, Stefan Beller wrote: > On Post 2.3 cyle (batch #5) (v2.3.1-167-g7f4ba4b) > > CC http.o > http.c: In function 'get_preferred_languages': > http.c:1021:2: warning: implicit declaration of function 'setlocale' > [-Wimplicit-function-declaration] > retval = setlocale(LC_MESSAGES, NULL); > ^ See http://thread.gmane.org/gmane.comp.version-control.git/253901/focus=264421 Thanks for reporting, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
Jeff King writes: > Perhaps it would be less risky to stick get_preferred_languages() into > gettext.c, like the patch below. Then we do not have to worry about > locale.h introducing other disruptive includes. The function is not > technically about gettext, but it seems reasonable to me to stuff all of > the i18n code together. Yeah, I like that a lot better. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor
"Kyle J. McKay" wrote: > The updated patch with the additional fix is below. Thanks, signed-off and pushed to master on git://bogomips.org/git-svn I've dropped my "destroy all tempfiles..." patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Salvaging borked project history
Mason writes: >> Not just passing >> these options, the code in "git am" to react to the result of patch >> application to avoid the issue you observed when these options are >> passed need to be adjusted by changes that started passing them, but >> I do not think they did, cf. 77e9e496 (am: pass exclude down to >> apply, 2011-08-03). > > Sorry, I could not parse that paragraph :-) Heh, paraphrasing. 77e9e496 and others tried to teach --exclude/--include to "git am". For "git am" to be able to claim that these options are properly supported, you need two things: - The options can be given from the command line and they are passed to the underlying "git apply", instead of complaining with "no such option". - After calling "git mailsplit", "git mailinfo", and "git apply" and then these helper programs return, "git am" needs to inspect what happened and react. The patch application may have failed due to conflicts, in which case it may have to ask the user to resolve. A patch application may have resulted in no-change, which often is a sign that there is something wrong and "am" would want to stop and ask the user for confirmation. If use of --include/--exclude introduces a new failure mode (e.g. mailsplit and mailinfo may find a patch text and happy without complaint, but passing --exclude to apply may cause the remaining patch to become essentially empty, which never happens before "am" started accepting these options), that codepath needs to be updated to cope with the new failure mode it has introduced. And I think 77e9e496 and the other one that added --include only did the former without doing the latter. > Here is the use-case: > > if diff -q A B; then do_X; else do_Y; fi > ... > I was planning to write 'git diff -q commit^ commit' > to test for empty commits. s/-q/--quiet/ and all is well, no? "git diff --quiet" doesn't abbreviate down to "git diff -q" primarily because "-q" for the underlying "git diff-files" means something different from "--quiet". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 12:59:56PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > Perhaps it would be less risky to stick get_preferred_languages() into > > gettext.c, like the patch below. Then we do not have to worry about > > locale.h introducing other disruptive includes. The function is not > > technically about gettext, but it seems reasonable to me to stuff all of > > the i18n code together. > > Yeah, I like that a lot better. Thanks. Are you just using it for inspiration, or did you want me to wrap it up with a commit message? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] git-svn updates for master
Hi Junio, mainly bugfixes but a tiny amount of progress on lazy-loading. The bugfixes from Kyle and Ryuichi should also be applied to maint. Thanks. The following changes since commit 7f4ba4b6e3ba7075ca6b379ba23fd3088cbe69a8: Post 2.3 cyle (batch #5) (2015-02-25 15:44:04 -0800) are available in the git repository at: git://bogomips.org/git-svn.git master for you to fetch changes up to e0b4cad4fd77e4cd787c3ed26e7650fc4de8bcd2: Git::SVN::*: avoid premature FileHandle closure (2015-02-26 20:19:41 +) Eric Wong (1): git-svn: lazy load some modules Kyle J. McKay (1): Git::SVN::*: avoid premature FileHandle closure Ryuichi Kokubo (1): git-svn: fix localtime=true on non-glibc environments git-svn.perl| 13 +++-- perl/Git/SVN.pm | 25 +++-- perl/Git/SVN/Editor.pm | 3 +-- perl/Git/SVN/Fetcher.pm | 11 +-- perl/Git/SVN/Ra.pm | 8 +++- 5 files changed, 39 insertions(+), 21 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git probably bug
Виталий Бормотов writes: > When checking out into some different > state, if there is no permissions to unlink files (if some files in > the current state doesn't exist in the new) warnings are outputing, > but then git says that checking out is successful, shows that we are > in the new state and the non-unlinked files are marked as > untracked. This is done by reasonably old part of the codebase, and I think it is exactly working as designed. Unlinking of an existing path in the filesystem is done for one of the two reasons: - The old tree has the path, the new tree does not. - The old tree has the path, the new tree has it but with a different content. In both cases, we first attempt to unlink (and warn if we cannot) and then attempt to create the path with the new contents in the latter case (and die if we cannot). The checkout itself succeeds, because having untracked cruft in your working tree is not an error, but warnings are given to let you know that you may want to remove them after fixing the screwy permission problem yourself. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
Jeff King writes:
> On Thu, Feb 26, 2015 at 12:59:56PM -0800, Junio C Hamano wrote:
>
>> Jeff King writes:
>>
>> > Perhaps it would be less risky to stick get_preferred_languages() into
>> > gettext.c, like the patch below. Then we do not have to worry about
>> > locale.h introducing other disruptive includes. The function is not
>> > technically about gettext, but it seems reasonable to me to stuff all of
>> > the i18n code together.
>>
>> Yeah, I like that a lot better. Thanks.
>
> Are you just using it for inspiration, or did you want me to wrap it up
> with a commit message?
Here is what I queued. Thanks.
-- >8 --
From: Jeff King
Date: Wed, 25 Feb 2015 22:04:16 -0500
Subject: [PATCH] gettext.c: move get_preferred_languages() from http.c
Calling setlocale(LC_MESSAGES, ...) directly from http.c, without
including , was causing compilation warnings. Move the
helper function to gettext.c that already includes the header and
where locale-related issues are handled.
Signed-off-by: Jeff King
Signed-off-by: Junio C Hamano
---
gettext.c | 25 +
gettext.h | 2 ++
http.c| 1 +
3 files changed, 28 insertions(+)
diff --git a/gettext.c b/gettext.c
index 8b2da46..7378ba2 100644
--- a/gettext.c
+++ b/gettext.c
@@ -18,6 +18,31 @@
# endif
#endif
+/*
+ * Guess the user's preferred languages from the value in LANGUAGE environment
+ * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
+ *
+ * The result can be a colon-separated list like "ko:ja:en".
+ */
+const char *get_preferred_languages(void)
+{
+ const char *retval;
+
+ retval = getenv("LANGUAGE");
+ if (retval && *retval)
+ return retval;
+
+#ifndef NO_GETTEXT
+ retval = setlocale(LC_MESSAGES, NULL);
+ if (retval && *retval &&
+ strcmp(retval, "C") &&
+ strcmp(retval, "POSIX"))
+ return retval;
+#endif
+
+ return NULL;
+}
+
#ifdef GETTEXT_POISON
int use_gettext_poison(void)
{
diff --git a/gettext.h b/gettext.h
index 7671d09..e539482 100644
--- a/gettext.h
+++ b/gettext.h
@@ -65,4 +65,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned
long n)
/* Mark msgid for translation but do not translate it. */
#define N_(msgid) msgid
+const char *get_preferred_languages(void);
+
#endif
diff --git a/http.c b/http.c
index 8b659b6..71ed418 100644
--- a/http.c
+++ b/http.c
@@ -8,6 +8,7 @@
#include "credential.h"
#include "version.h"
#include "pkt-line.h"
+#include "gettext.h"
int active_requests;
int http_is_verbose;
--
2.3.1-280-g2531f2d
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano wrote:>
> Here is what I queued. Thanks.
I did not follow the thread if there are any intermediate patches,
though it applied cleanly.
Applying this on top of f18604bbf2c391c689a41fca14cbaeff5e106255
(http: add Accept-Language header if possible) still doesn't compile for me.
http.c:1001:20: error: static declaration of 'get_preferred_languages'
follows non-static declaration
static const char *get_preferred_languages(void)
^
In file included from cache.h:8:0,
from http.h:4,
from http.c:2:
gettext.h:68:13: note: previous declaration of
'get_preferred_languages' was here
const char *get_preferred_languages(void);
^
http.c: In function 'get_preferred_languages':
http.c:1010:2: warning: implicit declaration of function 'setlocale'
[-Wimplicit-function-declaration]
retval = setlocale(LC_MESSAGES, NULL);
^
http.c:1010:21: error: 'LC_MESSAGES' undeclared (first use in this function)
retval = setlocale(LC_MESSAGES, NULL);
^
http.c:1010:21: note: each undeclared identifier is reported only once
for each function it appears in
Rebasing this on top of current master (Post 2.3 cyle (batch #5)) also fails:
http.c:1013:20: error: static declaration of 'get_preferred_languages'
follows non-static declaration
static const char *get_preferred_languages(void)
^
In file included from cache.h:8:0,
from http.h:4,
from http.c:2:
gettext.h:92:13: note: previous declaration of
'get_preferred_languages' was here
const char *get_preferred_languages(void);
^
http.c: In function 'get_preferred_languages':
http.c:1022:2: warning: implicit declaration of function 'setlocale'
[-Wimplicit-function-declaration]
retval = setlocale(LC_MESSAGES, NULL);
^
http.c:1022:21: error: 'LC_MESSAGES' undeclared (first use in this function)
retval = setlocale(LC_MESSAGES, NULL);
^
http.c:1022:21: note: each undeclared identifier is reported only once
for each function it appears in
>
> -- >8 --
> From: Jeff King
> Date: Wed, 25 Feb 2015 22:04:16 -0500
> Subject: [PATCH] gettext.c: move get_preferred_languages() from http.c
>
> Calling setlocale(LC_MESSAGES, ...) directly from http.c, without
> including , was causing compilation warnings. Move the
> helper function to gettext.c that already includes the header and
> where locale-related issues are handled.
>
> Signed-off-by: Jeff King
> Signed-off-by: Junio C Hamano
> ---
> gettext.c | 25 +
> gettext.h | 2 ++
> http.c| 1 +
> 3 files changed, 28 insertions(+)
>
> diff --git a/gettext.c b/gettext.c
> index 8b2da46..7378ba2 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -18,6 +18,31 @@
> # endif
> #endif
>
> +/*
> + * Guess the user's preferred languages from the value in LANGUAGE
> environment
> + * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
> + *
> + * The result can be a colon-separated list like "ko:ja:en".
> + */
> +const char *get_preferred_languages(void)
> +{
> + const char *retval;
> +
> + retval = getenv("LANGUAGE");
> + if (retval && *retval)
> + return retval;
> +
> +#ifndef NO_GETTEXT
> + retval = setlocale(LC_MESSAGES, NULL);
> + if (retval && *retval &&
> + strcmp(retval, "C") &&
> + strcmp(retval, "POSIX"))
> + return retval;
> +#endif
> +
> + return NULL;
> +}
> +
> #ifdef GETTEXT_POISON
> int use_gettext_poison(void)
> {
> diff --git a/gettext.h b/gettext.h
> index 7671d09..e539482 100644
> --- a/gettext.h
> +++ b/gettext.h
> @@ -65,4 +65,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned
> long n)
> /* Mark msgid for translation but do not translate it. */
> #define N_(msgid) msgid
>
> +const char *get_preferred_languages(void);
> +
> #endif
> diff --git a/http.c b/http.c
> index 8b659b6..71ed418 100644
> --- a/http.c
> +++ b/http.c
> @@ -8,6 +8,7 @@
> #include "credential.h"
> #include "version.h"
> #include "pkt-line.h"
> +#include "gettext.h"
>
> int active_requests;
> int http_is_verbose;
> --
> 2.3.1-280-g2531f2d
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 01:47:34PM -0800, Stefan Beller wrote: > On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano wrote:> > > Here is what I queued. Thanks. > > I did not follow the thread if there are any intermediate patches, > though it applied cleanly. What Junio posted is missing the hunk to drop the old static definition of get_preferred_languages from http.c. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] git-svn updates for master
Eric Wong writes: > Hi Junio, mainly bugfixes but a tiny amount of progress on lazy-loading. > The bugfixes from Kyle and Ryuichi should also be applied to maint. I think you mean "cherry-picked". Let me try to rearrange them by doing the following: 0. Grab your stuff. $ git fetch git://bogomips.org/git-svn.git master 1. Queue two fixes for maint-2.2 $ git checkout -b svn-maint-fixes maint-2.2 $ git cherry-pick -s -2 FETCH_HEAD 2. Queue the other one for master, together with the other two $ git co -b svn-fixes FETCH_HEAD~2 $ git merge --no-edit svn-maint-fixes 3. Make sure I got that right by comparing with yours $ git diff FETCH_HEAD ... no output ... Then I'll merge svn-fixes to my 'master' and hopefully we can later merge svn-maint-fixes to 'maint' and 'maint-2.2'. Thanks. > Thanks. > > The following changes since commit 7f4ba4b6e3ba7075ca6b379ba23fd3088cbe69a8: > > Post 2.3 cyle (batch #5) (2015-02-25 15:44:04 -0800) > > are available in the git repository at: > > git://bogomips.org/git-svn.git master > > for you to fetch changes up to e0b4cad4fd77e4cd787c3ed26e7650fc4de8bcd2: > > Git::SVN::*: avoid premature FileHandle closure (2015-02-26 20:19:41 +) > > > Eric Wong (1): > git-svn: lazy load some modules > > Kyle J. McKay (1): > Git::SVN::*: avoid premature FileHandle closure > > Ryuichi Kokubo (1): > git-svn: fix localtime=true on non-glibc environments > > git-svn.perl| 13 +++-- > perl/Git/SVN.pm | 25 +++-- > perl/Git/SVN/Editor.pm | 3 +-- > perl/Git/SVN/Fetcher.pm | 11 +-- > perl/Git/SVN/Ra.pm | 8 +++- > 5 files changed, 39 insertions(+), 21 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 05:06:10PM -0500, Jeff King wrote:
> On Thu, Feb 26, 2015 at 01:47:34PM -0800, Stefan Beller wrote:
>
> > On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano wrote:>
> > > Here is what I queued. Thanks.
> >
> > I did not follow the thread if there are any intermediate patches,
> > though it applied cleanly.
>
> What Junio posted is missing the hunk to drop the old static definition
> of get_preferred_languages from http.c.
Here it is, with the commit message and the missing hunk. This works for
me both with and without NO_GETTEXT defined.
-- >8 --
Subject: [PATCH] gettext.c: move get_preferred_languages() from http.c
Calling setlocale(LC_MESSAGES, ...) directly from http.c,
without including , was causing compilation
warnings. Move the helper function to gettext.c that
already includes the header and where locale-related issues
are handled.
Signed-off-by: Jeff King
---
gettext.c | 25 +
gettext.h | 2 ++
http.c| 27 +--
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/gettext.c b/gettext.c
index 8b2da46..7378ba2 100644
--- a/gettext.c
+++ b/gettext.c
@@ -18,6 +18,31 @@
# endif
#endif
+/*
+ * Guess the user's preferred languages from the value in LANGUAGE environment
+ * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
+ *
+ * The result can be a colon-separated list like "ko:ja:en".
+ */
+const char *get_preferred_languages(void)
+{
+ const char *retval;
+
+ retval = getenv("LANGUAGE");
+ if (retval && *retval)
+ return retval;
+
+#ifndef NO_GETTEXT
+ retval = setlocale(LC_MESSAGES, NULL);
+ if (retval && *retval &&
+ strcmp(retval, "C") &&
+ strcmp(retval, "POSIX"))
+ return retval;
+#endif
+
+ return NULL;
+}
+
#ifdef GETTEXT_POISON
int use_gettext_poison(void)
{
diff --git a/gettext.h b/gettext.h
index dc1722d..5d8d2df 100644
--- a/gettext.h
+++ b/gettext.h
@@ -89,4 +89,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned
long n)
#define N_(msgid) (msgid)
#endif
+const char *get_preferred_languages();
+
#endif
diff --git a/http.c b/http.c
index 0153fb0..9c825af 100644
--- a/http.c
+++ b/http.c
@@ -8,6 +8,7 @@
#include "credential.h"
#include "version.h"
#include "pkt-line.h"
+#include "gettext.h"
int active_requests;
int http_is_verbose;
@@ -1002,32 +1003,6 @@ static void extract_content_type(struct strbuf *raw,
struct strbuf *type,
strbuf_addstr(charset, "ISO-8859-1");
}
-
-/*
- * Guess the user's preferred languages from the value in LANGUAGE environment
- * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
- *
- * The result can be a colon-separated list like "ko:ja:en".
- */
-static const char *get_preferred_languages(void)
-{
- const char *retval;
-
- retval = getenv("LANGUAGE");
- if (retval && *retval)
- return retval;
-
-#ifndef NO_GETTEXT
- retval = setlocale(LC_MESSAGES, NULL);
- if (retval && *retval &&
- strcmp(retval, "C") &&
- strcmp(retval, "POSIX"))
- return retval;
-#endif
-
- return NULL;
-}
-
static void write_accept_language(struct strbuf *buf)
{
/*
--
2.3.0.449.g1690e78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
Jeff King writes: > On Thu, Feb 26, 2015 at 01:47:34PM -0800, Stefan Beller wrote: > >> On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano wrote:> >> > Here is what I queued. Thanks. >> >> I did not follow the thread if there are any intermediate patches, >> though it applied cleanly. > > What Junio posted is missing the hunk to drop the old static definition > of get_preferred_languages from http.c. I am still scratching my head to see how this happened, but I think when I did $ git checkout ye/http-accept-language $ git apply -3 $gmane/264422 I took the wrong side of the confict in http.c Thanks both for noticing. Now it is fixed up. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 2:07 PM, Jeff King wrote:
>
> Here it is, with the commit message and the missing hunk. This works for
> me both with and without NO_GETTEXT defined.
This compiles here though a warning is spit:
In file included from cache.h:8:0,
from userdiff.c:1:
gettext.h:92:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
const char *get_preferred_languages();
^
so I guess I can still add a
Tested-by: Stefan Beller
>
> -- >8 --
> Subject: [PATCH] gettext.c: move get_preferred_languages() from http.c
>
> Calling setlocale(LC_MESSAGES, ...) directly from http.c,
> without including , was causing compilation
> warnings. Move the helper function to gettext.c that
> already includes the header and where locale-related issues
> are handled.
>
> Signed-off-by: Jeff King
> ---
> gettext.c | 25 +
> gettext.h | 2 ++
> http.c| 27 +--
> 3 files changed, 28 insertions(+), 26 deletions(-)
>
> diff --git a/gettext.c b/gettext.c
> index 8b2da46..7378ba2 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -18,6 +18,31 @@
> # endif
> #endif
>
> +/*
> + * Guess the user's preferred languages from the value in LANGUAGE
> environment
> + * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
> + *
> + * The result can be a colon-separated list like "ko:ja:en".
> + */
> +const char *get_preferred_languages(void)
> +{
> + const char *retval;
> +
> + retval = getenv("LANGUAGE");
> + if (retval && *retval)
> + return retval;
> +
> +#ifndef NO_GETTEXT
> + retval = setlocale(LC_MESSAGES, NULL);
> + if (retval && *retval &&
> + strcmp(retval, "C") &&
> + strcmp(retval, "POSIX"))
> + return retval;
> +#endif
> +
> + return NULL;
> +}
> +
> #ifdef GETTEXT_POISON
> int use_gettext_poison(void)
> {
> diff --git a/gettext.h b/gettext.h
> index dc1722d..5d8d2df 100644
> --- a/gettext.h
> +++ b/gettext.h
> @@ -89,4 +89,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned
> long n)
> #define N_(msgid) (msgid)
> #endif
>
> +const char *get_preferred_languages();
> +
> #endif
> diff --git a/http.c b/http.c
> index 0153fb0..9c825af 100644
> --- a/http.c
> +++ b/http.c
> @@ -8,6 +8,7 @@
> #include "credential.h"
> #include "version.h"
> #include "pkt-line.h"
> +#include "gettext.h"
>
> int active_requests;
> int http_is_verbose;
> @@ -1002,32 +1003,6 @@ static void extract_content_type(struct strbuf *raw,
> struct strbuf *type,
> strbuf_addstr(charset, "ISO-8859-1");
> }
>
> -
> -/*
> - * Guess the user's preferred languages from the value in LANGUAGE
> environment
> - * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
> - *
> - * The result can be a colon-separated list like "ko:ja:en".
> - */
> -static const char *get_preferred_languages(void)
> -{
> - const char *retval;
> -
> - retval = getenv("LANGUAGE");
> - if (retval && *retval)
> - return retval;
> -
> -#ifndef NO_GETTEXT
> - retval = setlocale(LC_MESSAGES, NULL);
> - if (retval && *retval &&
> - strcmp(retval, "C") &&
> - strcmp(retval, "POSIX"))
> - return retval;
> -#endif
> -
> - return NULL;
> -}
> -
> static void write_accept_language(struct strbuf *buf)
> {
> /*
> --
> 2.3.0.449.g1690e78
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 02:26:05PM -0800, Stefan Beller wrote: > On Thu, Feb 26, 2015 at 2:07 PM, Jeff King wrote: > > > > Here it is, with the commit message and the missing hunk. This works for > > me both with and without NO_GETTEXT defined. > > This compiles here though a warning is spit: > In file included from cache.h:8:0, > from userdiff.c:1: > gettext.h:92:1: warning: function declaration isn't a prototype > [-Wstrict-prototypes] > const char *get_preferred_languages(); > ^ Hmph. The compiler is right that it should be: const char *get_preferred_languages(void); but my gcc (4.9.2, with -Wstrict_prototypes) does not seem to notice it! Weird. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
On Thu, Feb 26, 2015 at 05:36:03PM -0500, Jeff King wrote: > > [-Wstrict-prototypes] > > const char *get_preferred_languages(); > > ^ > > Hmph. The compiler is right that it should be: > > const char *get_preferred_languages(void); > > but my gcc (4.9.2, with -Wstrict_prototypes) does not seem to notice it! > Weird. Ugh. I have a snippet in my config.mak that relaxes the warnings on older versions of git, and it was accidentally triggering due to a typo. :( So that explains that. Junio, do you mind squashing in: diff --git a/gettext.h b/gettext.h index 5d8d2df..33696a4 100644 --- a/gettext.h +++ b/gettext.h @@ -89,6 +89,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n) #define N_(msgid) (msgid) #endif -const char *get_preferred_languages(); +const char *get_preferred_languages(void); #endif -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
does the git over ssh protocol tell the server the hostname?
Hey. Short question: I saw that when plain git (i.e. git://) is used, the client tells the server the hostname specified on the client side. For http one has the same automatically via http's Host: header. But after watching the git's over-ssh protocol, I couldn't find anything like that there? :-( Would be quite nice to have this in order to implement vhosting like things. Cheers, Chris. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v9 1/1] http: Add Accept-Language header if possible
Jeff King writes: > On Thu, Feb 26, 2015 at 05:36:03PM -0500, Jeff King wrote: > >> > [-Wstrict-prototypes] >> > const char *get_preferred_languages(); >> > ^ >> >> Hmph. The compiler is right that it should be: >> >> const char *get_preferred_languages(void); >> >> but my gcc (4.9.2, with -Wstrict_prototypes) does not seem to notice it! >> Weird. > > Ugh. I have a snippet in my config.mak that relaxes the warnings on older > versions of git, and it was accidentally triggering due to a typo. :( > > So that explains that. Junio, do you mind squashing in: Yup, I already did when I got the first one. Thanks. > > diff --git a/gettext.h b/gettext.h > index 5d8d2df..33696a4 100644 > --- a/gettext.h > +++ b/gettext.h > @@ -89,6 +89,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned > long n) > #define N_(msgid) (msgid) > #endif > > -const char *get_preferred_languages(); > +const char *get_preferred_languages(void); > > #endif -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Any chance for a Git v2.1.5 release?
On Feb 26, 2015, at 12:54, Junio C Hamano wrote: "Kyle J. McKay" writes: I would like to better understand how the various heads are maintained. I've read MaintNotes and I've got the concepts, but I'm still a little fuzzy on some details. It looks to me like all topics still only in pu after master has been updated are then rebased onto the new master and then pu is rebuilt. Topics in 'pu' not yet in 'next' _can_ be rebased, but unless there is a good reason to do so, I wouldn't spend extra cycles necessary to do such thing. I even try to keep the same base when replacing the contents of a branch with a rerolled series. For example, today I have this: $ git config alias.lgf log --oneline --boundary --first-parent $ git lgf master..nd/slim-index-pack-memory-usage 3538291 index-pack: kill union delta_base to save memory 7b4ff41 FIXUP 45b6b71 index-pack: reduce memory footprint a bit - 9874fca Git 2.3 and Duy has a newer iteration of it starting at $gmane/264429. What I would do, after saving these patches in a mbox +nd-index-pack, would be to $ git checkout 9874fca $ git am -s3c ./+nd-index-pack $ git show-branch HEAD nd/slim-index-pack-memory-usage ... compare corresponding changes between the old and the new ... until I am satisified; otherwise I may tweak the new one $ git rebase -i 9874fca ... and then finally $ git branch -f nd/slim-index-pack-memory-usage HEAD to update the topic. Of course, it would be necessary to rebuild 'pu' by merging all the topics that are in it on top of 'master'. Thanks. That's helpful. After finishing 2.3.0 release, at some point while 'master' is still at 2.3.0, something like this would happen: $ git branch -m maint maint-2.2 $ git branch maint master So the reason I don't notice force-updates to maint when this happens is because of the "Sync with maint" commits that make sure the new maint contains the old one. Also, how do you handle a down merge to maint when you have this: * (master) * merge topic foo |\ | * topic foo |/ * c * b * a * (tag: vX.X.X, maint) * I don't, and this is done by making sure I do not get into such a situation in the first place. A general rule of thumb when applying a set of patches that are fixes eventually worth having in older maintenance tracks is to find the oldest maintenance branch and apply them there. If I were to keep a maint-lts branch somewhere I would need to cherry- pick topics with desired fixes that fall into that situation. That's what I expected but when you mentioned down merging the fixes I wanted to make sure I wasn't overlooking something. I'll see about setting up a maint-lts in a local git repository clone and tracking LTS fixes. If I'm able to keep that going without it becoming a black-hole of temporal need that sucks the life right out of me ;) then perhaps we can have a discussion at a future date about what would be needed for you to consider pulling from it and issuing LTS releases off it. :) -Kyle -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Salvaging borked project history
Junio C Hamano wrote: > Mason wrote: > >> I was planning to write 'git diff -q commit^ commit' >> to test for empty commits. > > s/-q/--quiet/ and all is well, no? Doh! I've no idea how I missed these... --exit-code Make the program exit with codes similar to diff(1). That is, it exits with 1 if there were differences and 0 means no differences. --quiet Disable all output of the program. Implies --exit-code. Thanks for walking me through it. Regards. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Any chance for a Git v2.1.5 release?
"Kyle J. McKay" writes: >> After finishing 2.3.0 release, at some point while 'master' is still >> at 2.3.0, something like this would happen: >> >>$ git branch -m maint maint-2.2 >>$ git branch maint master > > So the reason I don't notice force-updates to maint when this happens > is because of the "Sync with maint" commits that make sure the new > maint contains the old one. I could also do git branch maint-2.2 maint git push . master:maint ;# not +master:maint to make sure that I won't rewind 'maint', but this works because 'master' is designed to be always a super-set of 'maint'. > If I were to keep a maint-lts branch somewhere I would need to cherry- > pick topics with desired fixes that fall into that situation. That's > what I expected but when you mentioned down merging the fixes I wanted > to make sure I wasn't overlooking something. Yup. I _can_ become sloppy especially late in the release cycle and end up doing something silly like this: - Fork km/svn-fix from somewhere it _could_ be merged to 'maint' (e.g. "the last big release", e.g. v2.3.0). - Merge km/svn-fix to 'master' early in a cycle; - A mistake is found in the topic late in the cycle; as the next release will happen soon _anyway_, and I do not happen to consider km/svn-fix is so important a fix to be merged to 'maint', I apply a fix-up patch directly on top of 'master'; - A release candidate is tagged from 'master'. Then even though you can easily grab a broken-out tree at github.com:gitster/git, km/svn-fix topic alone cannot be merged to 'maint' as it would lack the late fix. I've been trying to be careful but I cannot promise to be perfect X-<. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Salvaging borked project history
Mason writes: >> Mason wrote: >> >>> I was planning to write 'git diff -q commit^ commit' >>> to test for empty commits. >> >> s/-q/--quiet/ and all is well, no? > > Doh! I've no idea how I missed these... Yeah, this is one of the unfortunate corners of Git that I can apologize but cannot do much more than that about (in other words, making "git diff -q" as a short-hand for "git diff --quiet" is not acceptable) due to backward compatibility and consistency concerns. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git add to ignore whitespaces, some day?
Hey,
It would be nice if git-add could be told to ignore whitespace
changes, wouldn't it?
According to SO, I am not the one to think so:
http://stackoverflow.com/questions/3515597/git-add-only-non-whitespace-changes
A change to add--interactive would be as simple as adding the diff -b
or -w option like:
my @diff = run_cmd_pipe("git", @diff_cmd, "-w", "--", $path);
But I wonder if this should be configured in a diff.ignorews or passed
as an argument to git add. Opinions?
--
Marc-André Lureau
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] protocol v2
On Thu, Feb 26, 2015 at 12:13 PM, Junio C Hamano wrote:
> Duy Nguyen writes:
>
>> Step 1 then should be identifying these wrongdoings and assumptions.
>>
>> We can really go wild with these capabilities. The only thing that
>> can't be changed is perhaps sending the first ref. I don't know
>> whether we can accept a dummy first ref... After that point, you can
>> turn the protocol upside down because both client and server know what
>> it would be.
>
> Yes, exactly. To up/down/side-grade from v1 is technically
> possible, but being technically possible is different from being
> sensible. The capability-based sidegrade does not solve the problem
> when the problem to be solved is that the server side needs to spend
> a lot of cycles and the network needs to carry megabytes of data
> before capability exchange happens. Yes, the newer server and the
> newer client can notice that the counterparty is new and start
> talking in new protocol (which may or may not benefit from already
> knowing the result of ref advertisement), but by the time that
> happens, the resource has already been spent and wasted.
>
> I do not think v1 can be fixed by "send one ref with capability,
> newer client may respond immediately so we can stop enumerating
> remaining refs and older one will get stuck so we can have a timeout
> to see if the connection is from the newer one, and send the rest
> for the older client", because anything that involves such a timeout
> would not reliably work over WAN.
>
>> You realize you're advertising v2 as a new capability, right? Instead
>> of defining v2 feature set then advertise v2, people could simply add
>> new features directly. I don't see v2 (at least with these patches)
>> adds any value.
>
> I agree with the value assessment of these patches 98%, but these
> bits can be taken as the "we have v2 server availble for you on the
> side, by the way" hint you mentioned in the older thread, I think.
>
>> And we already does that, except that we don't state what version (as
>> a number) exactly, but what feature that version supports. The focus
>> should be the new protocol at daemon.c and maybe remote-curl.c where
>> we do know the current protocol is not flexible enough.
>
> The "first" thing the client tells the server is what service it
> requests. A request over git:// protocol is read by "git daemon" to
> choose which service to run, and it is read directly by the login
> shell if it comes over ssh:// protocol.
>
> There is nothing that prevents us from defining that service to be a
> generic "git service", not "upload-pack", "archive", "receive-pack".
> And the early protocol exchange, once "git service" is spawned, with
> the client can be "what real services does the server end support?"
> capability list responded with "wow, you are new enough to support
> the 'trickle-pack' service---please connect me to it" request.
>
So I am not quite sure how to understand this input.
I wonder if a high level test could look like the following,
which just tests the workflow with git fetch, but not the
internals.
(Note: patch formatting may be broken as it's send via gmail web ui)
---8<---
From: Stefan Beller
Date: Thu, 26 Feb 2015 17:19:30 -0800
Subject: [PATCH] Propose new tests for transitioning to the new option
transport.capabilitiesfirst
Signed-off-by: Stefan Beller
---
t/t5544-capability-handshake.sh | 81 +
1 file changed, 81 insertions(+)
create mode 100755 t/t5544-capability-handshake.sh
diff --git a/t/t5544-capability-handshake.sh b/t/t5544-capability-handshake.sh
new file mode 100755
index 000..aa2b52d
--- /dev/null
+++ b/t/t5544-capability-handshake.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+test_description='fetching from a repository using the "capabilities
first" push option'
+
+. ./test-lib.sh
+
+mk_repo_pair () {
+ rm -rf workbench upstream &&
+ test_create_repo upstream &&
+ test_create_repo workbench &&
+ (
+ cd upstream &&
+ git config receive.denyCurrentBranch warn
+ ) &&
+ (
+ cd workbench &&
+ git remote add origin ../upstream
+ )
+}
+
+generate_commits_upstream () {
+ (
+ cd upstream &&
+ echo "more content" >>file &&
+ git add file &&
+ git commit -a -m "create a commit"
+ )
+}
+
+# Compare the ref ($1) in upstream with a ref value from workbench ($2)
+# i.e. test_refs second HEAD@{2}
+test_refs () {
+ test $# = 2 &&
+ git -C upstream rev-parse --verify "$1" >expect &&
+ git -C workbench rev-parse --verify "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'transport.capabilitiesfirst is not overridden
when set already' '
+ mk_repo_pair &&
+ (
+ cd workbench &&
+ git config transport.capabilitiesfirst 0
+ git config --get transport.capabilitiesfirst 0 >expected
+ )
+ generate_commits_upstream &&
+ (
+ cd workbench &&
+ git fetch --all
+ git config --get transport.capabilitiesfirst >actual
+ test_cmp expected actual
+ )
+'
+
+test_expect_success 'enable transport by fetching from new server' '
+ mk_repo_pair &
[PATCH 0/2] diffcore-rename with duplicate tree entries can segfault
On Wed, Feb 25, 2015 at 01:50:38PM -0800, Junio C Hamano wrote: > > So to go forward, I'm happy to prepare a patch, but I'd like to know: > > > > 1. Does something like the above look reasonable to you (I'd probably > > refactor it to avoid the bizarre return value semantics from > > locate_rename_dst, though)? > > > > 2. If so, do you want something minimal like what's above, or do you > > mind if I build it on top of a hashmap conversion? I suspect the > > logic may also end up more clear with the hashmap (since inserting > > versus lookup will be more distinct in the callers). > > No, I don't mind. The diff-b-m topic seems to need a lot deeper > rethink than I originally anticipated anyway, and it can wait for a > clean-up to use hashmap to stabilize. I tried switching to a hashmap, but the diff_score code actually wants to index into the array using an int. In a hashmap, we'd use a pointer instead, but that increases the size of "struct diff_score", which is something that we have to allocate a lot of (src * dst, I think). So I punted on that and just cleaned up the locate_rename_dst interface a bit. Here's the result. [1/2]: diffcore-rename: split locate_rename_dst into two functions [2/2]: diffcore-rename: avoid processing duplicate destinations -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] diffcore-rename: split locate_rename_dst into two functions
This function manages the mapping of destination pathnames
to filepairs, and it handles both insertion and lookup. This
makes the return value a bit confusing, as we return a newly
created entry (even though no caller cares), and have no
room to indicate to the caller that an entry already
existed.
Instead, let's break this up into two distinct functions,
both backed by a common binary search. The binary search
will use our normal "return the index if we found something,
or negative index minus one to show where it would have
gone" semantics.
Signed-off-by: Jeff King
---
diffcore-rename.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 4e132f1..4250cc0 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -15,8 +15,7 @@ static struct diff_rename_dst {
} *rename_dst;
static int rename_dst_nr, rename_dst_alloc;
-static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
-int insert_ok)
+static int find_rename_dst(struct diff_filespec *two)
{
int first, last;
@@ -27,16 +26,33 @@ static struct diff_rename_dst *locate_rename_dst(struct
diff_filespec *two,
struct diff_rename_dst *dst = &(rename_dst[next]);
int cmp = strcmp(two->path, dst->two->path);
if (!cmp)
- return dst;
+ return next;
if (cmp < 0) {
last = next;
continue;
}
first = next+1;
}
- /* not found */
- if (!insert_ok)
- return NULL;
+ return -first - 1;
+}
+
+static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two)
+{
+ int ofs = find_rename_dst(two);
+ return ofs < 0 ? NULL : &rename_dst[ofs];
+}
+
+/*
+ * Returns 0 on success, -1 if we found a duplicate.
+ */
+static int add_rename_dst(struct diff_filespec *two)
+{
+ int first = find_rename_dst(two);
+
+ if (first >= 0)
+ return -1;
+ first = -first - 1;
+
/* insert to make it at "first" */
ALLOC_GROW(rename_dst, rename_dst_nr + 1, rename_dst_alloc);
rename_dst_nr++;
@@ -46,7 +62,7 @@ static struct diff_rename_dst *locate_rename_dst(struct
diff_filespec *two,
rename_dst[first].two = alloc_filespec(two->path);
fill_filespec(rename_dst[first].two, two->sha1, two->sha1_valid,
two->mode);
rename_dst[first].pair = NULL;
- return &(rename_dst[first]);
+ return 0;
}
/* Table of rename/copy src files */
@@ -451,7 +467,7 @@ void diffcore_rename(struct diff_options *options)
is_empty_blob_sha1(p->two->sha1))
continue;
else
- locate_rename_dst(p->two, 1);
+ add_rename_dst(p->two);
}
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
is_empty_blob_sha1(p->one->sha1))
@@ -582,8 +598,7 @@ void diffcore_rename(struct diff_options *options)
* We would output this create record if it has
* not been turned into a rename/copy already.
*/
- struct diff_rename_dst *dst =
- locate_rename_dst(p->two, 0);
+ struct diff_rename_dst *dst = locate_rename_dst(p->two);
if (dst && dst->pair) {
diff_q(&outq, dst->pair);
pair_to_free = p;
@@ -613,8 +628,7 @@ void diffcore_rename(struct diff_options *options)
*/
if (DIFF_PAIR_BROKEN(p)) {
/* broken delete */
- struct diff_rename_dst *dst =
- locate_rename_dst(p->one, 0);
+ struct diff_rename_dst *dst =
locate_rename_dst(p->one);
if (dst && dst->pair)
/* counterpart is now rename/copy */
pair_to_free = p;
--
2.3.0.449.g1690e78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] diffcore-rename: avoid processing duplicate destinations
The rename code cannot handle an input where we have
duplicate destinations (i.e., more than one diff_filepair in
the queue with the same string in its pair->two->path). We
end up allocating only one slot in the rename_dst mapping.
If we fill in the diff_filepair for that slot, when we
re-queue the results, we may queue that filepair multiple
times. When the diff is finally flushed, the filepair is
processed and free()d multiple times, leading to heap
corruption.
This situation should only happen when a tree diff sees
duplicates in one of the trees (see the added test for a
detailed example). Rather than handle it, the sanest thing
is just to turn off rename detection altogether for the
diff.
Signed-off-by: Jeff King
---
Like I mentioned before, I'm OK with not checking the actual diff output
in the test. It's not like it was planned, and is just what diff_tree()
happens to produce. It does make sense, though. We descend into the
first "outer/" of the "a/" side along with the sole "outer/" of the
"b/" side. We see that the entries from "b/" are all added. Then we come
back out, and see that "a/" has another "outer/", but that "b/" does
not. So all of those entries look like they were deleted.
diffcore-rename.c | 8 +++--
t/t4058-diff-duplicates.sh | 79 ++
2 files changed, 85 insertions(+), 2 deletions(-)
create mode 100755 t/t4058-diff-duplicates.sh
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 4250cc0..af1fe08 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -466,8 +466,12 @@ void diffcore_rename(struct diff_options *options)
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
is_empty_blob_sha1(p->two->sha1))
continue;
- else
- add_rename_dst(p->two);
+ else if (add_rename_dst(p->two) < 0) {
+ warning("skipping rename detection, detected"
+ " duplicate destination '%s'",
+ p->two->path);
+ goto cleanup;
+ }
}
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
is_empty_blob_sha1(p->one->sha1))
diff --git a/t/t4058-diff-duplicates.sh b/t/t4058-diff-duplicates.sh
new file mode 100755
index 000..0a23242
--- /dev/null
+++ b/t/t4058-diff-duplicates.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+test_description='test tree diff when trees have duplicate entries'
+. ./test-lib.sh
+
+# make_tree_entry
+#
+# We have to rely on perl here because not all printfs understand
+# hex escapes (only octal), and xxd is not portable.
+make_tree_entry () {
+ printf '%s %s\0' "$1" "$2" &&
+ perl -e 'print chr(hex($_)) for ($ARGV[0] =~ /../g)' "$3"
+}
+
+# Like git-mktree, but without all of the pesky sanity checking.
+# Arguments come in groups of three, each group specifying a single
+# tree entry (see make_tree_entry above).
+make_tree () {
+ while test $# -gt 2; do
+ make_tree_entry "$1" "$2" "$3"
+ shift; shift; shift
+ done |
+ git hash-object -w -t tree --stdin
+}
+
+# this is kind of a convoluted setup, but matches
+# a real-world case. Each tree contains four entries
+# for the given path, one with one sha1, and three with
+# the other. The first tree has them split across
+# two subtrees (which are themselves duplicate entries in
+# the root tree), and the second has them all in a single subtree.
+test_expect_success 'create trees with duplicate entries' '
+ blob_one=$(echo one | git hash-object -w --stdin) &&
+ blob_two=$(echo two | git hash-object -w --stdin) &&
+ inner_one_a=$(make_tree \
+ 100644 inner $blob_one
+ ) &&
+ inner_one_b=$(make_tree \
+ 100644 inner $blob_two \
+ 100644 inner $blob_two \
+ 100644 inner $blob_two
+ ) &&
+ outer_one=$(make_tree \
+ 04 outer $inner_one_a \
+ 04 outer $inner_one_b
+ ) &&
+ inner_two=$(make_tree \
+ 100644 inner $blob_one \
+ 100644 inner $blob_two \
+ 100644 inner $blob_two \
+ 100644 inner $blob_two
+ ) &&
+ outer_two=$(make_tree \
+ 04 outer $inner_two
+ ) &&
+ git tag one $outer_one &&
+ git tag two $outer_two
+'
+
+test_expect_success 'diff-tree between trees' '
+ {
+ printf ":00 100644 $_z40 $blob_two A\touter/inner\n" &&
+ printf ":00 100644 $_z40 $blob_two A\touter/inner\n" &&
+ printf ":00 100644 $_z40 $blob_two A\touter/inner\n" &&
+ printf ":100644 00 $blob_two $_z40 D\touter/inner\n" &&
+ printf ":100644 00 $blob_two $_z40 D
Re: [RFC/PATCH 0/3] protocol v2
On Thu, Feb 26, 2015 at 12:13 PM, Junio C Hamano wrote: > > I agree with the value assessment of these patches 98%, but these > bits can be taken as the "we have v2 server availble for you on the > side, by the way" hint you mentioned in the older thread, I think. The patches are not well polished (In fact they don't even compile :/), but I think they may demonstrate the ideas and though process. And as it turns out we'd not be following that spirit of ideas but rather want to have a dedicated v2. That said I did not want to spend lots of time to polish the patch for inclusion but rather to demonstrate ideas, which can be done with substantial less quality IMHO. Correct me if I am wrong here! Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
