trace.c: struct timeval tv not portable
The following printout gives a warning: (trace.c, arounf line 105) strbuf_addf(buf, "%02d:%02d:%02d.%06ld ", tm.tm_hour, tm.tm_min, tm.tm_sec, tv.tv_usec); trace.c:105: warning: format ‘%06ld’ expects type ‘long int’, but argument 6 has type ‘__darwin_suseconds_t’ A format string of "%06ld" for is not always good for tv.tv_usec http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/time.h.html A better solution would be: strbuf_addf(buf, "%02d:%02d:%02d.%06ld ", tm.tm_hour, tm.tm_min, tm.tm_sec, (long int)tv.tv_usec); -- 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] t0027: combinations of core.autocrlf, core.eol and text
Historically there are 3 different parameters controlling how line endings
are handled by Git:
- core.autocrlf
- core.eol
- the "text" attribute in .gitattributes
There are different types of content:
- (1) Files with only LF
- (2) Files with only CRLF
- (3) Files with mixed LF and CRLF
- (4) Files with LF and/or CRLF with CR not followed by LF
- (5) Files which are binary (e.g. have NUL bytes)
Recently the question came up, how files with mixed EOLs are handled by Git
(and libgit2) when they are checked out and core.autocrlf=true.
See
http://git.661346.n2.nabble.com/The-different-EOL-behavior-between-libgit2-based-software-and-official-Git-td7613670.html#a7613801
Add the EXPENSIVE t0027-auto-crlf.sh to test all combination of files
and parameters for both "git add/commit" and "git checkout".
Signed-off-by: Torsten Bögershausen
---
t/t0027-auto-crlf.sh | 265 +++
1 file changed, 265 insertions(+)
create mode 100755 t/t0027-auto-crlf.sh
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
new file mode 100755
index 000..72dd3e8
--- /dev/null
+++ b/t/t0027-auto-crlf.sh
@@ -0,0 +1,265 @@
+#!/bin/sh
+
+test_description='CRLF conversion all combinations'
+
+. ./test-lib.sh
+
+if ! test_have_prereq EXPENSIVE
+then
+ skip_all="EXPENSIVE not set"
+ test_done
+fi
+
+
+compare_files()
+{
+ od -c <"$1" >"$1".expect &&
+ od -c <"$2" >"$2".actual &&
+ test_cmp "$1".expect "$2".actual &&
+ rm "$1".expect "$2".actual
+}
+
+compare_ws_file()
+{
+ pfx=$1
+ exp=$2.expect
+ act=$pfx.actual.$3
+ od -c <"$2" >"$exp" &&
+ od -c <"$3" >"$act" &&
+ test_cmp $exp $act &&
+ rm $exp $act
+}
+
+create_gitattributes()
+{
+ txtbin=$1
+ case "$txtbin" in
+ auto)
+ echo "*.txt text=auto" >.gitattributes
+ ;;
+ text)
+ echo "*.txt text" >.gitattributes
+ ;;
+ -text)
+ echo "*.txt -text" >.gitattributes
+ ;;
+ *)
+ echo >.gitattributes
+ ;;
+ esac
+}
+
+create_file_in_repo()
+{
+ crlf=$1
+ txtbin=$2
+ create_gitattributes "$txtbin" &&
+ for f in LF CRLF LF_mix_CR CRLF_mix_LF CRLF_nul
+ do
+ pfx=crlf_${crlf}_attr_${txtbin}_$f.txt &&
+ cp $f $pfx && git -c core.autocrlf=$crlf add $pfx
+ done &&
+ git commit -m "core.autocrlf $crlf"
+}
+
+check_files_in_repo()
+{
+ crlf=$1
+ txtbin=$2
+ lfname=$3
+ crlfname=$4
+ lfmixcrlf=$5
+ lfmixcr=$6
+ crlfnul=$7
+ pfx=crlf_${crlf}_attr_${txtbin}_ &&
+ compare_files $lfname ${pfx}LF.txt &&
+ compare_files $crlfname ${pfx}CRLF.txt &&
+ compare_files $lfmixcrlf ${pfx}CRLF_mix_LF.txt &&
+ compare_files $lfmixcr ${pfx}LF_mix_CR.txt &&
+ compare_files $crlfnul ${pfx}CRLF_nul.txt
+}
+
+
+check_files_in_ws()
+{
+ eol=$1
+ crlf=$2
+ txtbin=$3
+ lfname=$4
+ crlfname=$5
+ lfmixcrlf=$6
+ lfmixcr=$7
+ crlfnul=$8
+ create_gitattributes $txtbin &&
+ git config core.autocrlf $crlf &&
+ pfx=eol_${eol}_crlf_${crlf}_attr_${txtbin}_ &&
+ src=crlf_false_attr__ &&
+ for f in LF CRLF LF_mix_CR CRLF_mix_LF CRLF_nul
+ do
+ rm $src$f.txt &&
+ if test -z "$eol"; then
+ git checkout $src$f.txt
+ else
+ git -c core.eol=$eol checkout $src$f.txt
+ fi
+ done
+
+
+ test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf
gitattributes=$txtbin file=LF" "
+ compare_ws_file $pfx $lfname${src}LF.txt
+ "
+ test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf
gitattributes=$txtbin file=CRLF" "
+ compare_ws_file $pfx $crlfname ${src}CRLF.txt
+ "
+ test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf
gitattributes=$txtbin file=CRLF_mix_LF" "
+ compare_ws_file $pfx $lfmixcrlf ${src}CRLF_mix_LF.txt
+ "
+ test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf
gitattributes=$txtbin file=LF_mix_CR" "
+ compare_ws_file $pfx $lfmixcr ${src}LF_mix_CR.txt
+ "
+ test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf
gitattributes=$txtbin file=CRLF_nul" "
+ compare_ws_file $pfx $crlfnul ${src}CRLF_nul.txt
+ "
+}
+
+###
+(
+ type od >/dev/null &&
+ printf "line1Q\r\nline2\r\nline3" | q_to_nul >CRLF_nul &&
+ cat >expect <<-EOF &&
+ 000 l i n e 1 \0 \r \n l i n e 2 \r \n l
+ 020 i n e 3
+ 024
+EOF
+ od -c CRLF_nul | sed -e "s/[][ ]*/ /g" -e "s/ *$//" >actual
+ test_cmp expect actual &&
+ rm expect actual
+) || {
+ skip_all="
Re: [PATCH v3 1/4] replace: add --graft option
On Sun, Jun 8, 2014 at 10:18 AM, Junio C Hamano wrote: > > On Sat, Jun 7, 2014 at 11:49 PM, Christian Couder > wrote: >> >> On Fri, Jun 6, 2014 at 5:44 PM, Christian Couder >> wrote: >> > >> > /* find existing parents */ >> > strbuf_addstr(&buf, commit->buffer); >> >> Unfortunately, it looks like the above will not work if the commit->buffer >> contains an embedded NUL. I wonder if it is a real problem or not. > > Yes, it is a real problem (there was another thread on this regarding the > code path that verifies GPG signature on the commit itself), which > incidentally reminds us to another thing to think about in your patch as > well. I *think* you shoud drop the GPG signature on the commit itself, and > you also should drop the merge-tag of a parent you are not going to keep, > but should keep the merge-tag of a parent you are keeping. In the v5 of the patch series, I now drop the GPG signature on the commit itself. Now, after having read the recent thread about "git verify-commit", I understand that you also want me to drop the signature of a tag that was merged, because such signatures are added to the commit message. But I wonder how far should we go in this path. For example merge commits have a title like "Merge branch 'dev'" or "Merge tag 'stuff'", but this does not make sense any more if the replacement commit drops the parent corresponding to 'dev' or 'stuff'. And I don't think we should change the commit title. Thanks, Christian. -- 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] t0025: Rename the test files
The current test files are named one, two and three.
Make it clearer what the tests do and rename them into
LFonly, CRLFonly and LFwithNUL.
After the renaming we can see easier that we may want more test cases
for 2 types of files:
- files which have mixed LF and CRLF line endings,
- files which have mixed LF and CR line endings.
See commit fd6cce9e, "Add per-repository eol normalization" and
"the new safer autocrlf handling" in convert.c
Signed-off-by: Torsten Bögershausen
---
t/t0025-crlf-auto.sh | 122 +--
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
index 28102de..c164b46 100755
--- a/t/t0025-crlf-auto.sh
+++ b/t/t0025-crlf-auto.sh
@@ -12,144 +12,144 @@ test_expect_success setup '
git config core.autocrlf false &&
- for w in Hello world how are you; do echo $w; done >one &&
- for w in I am very very fine thank you; do echo ${w}Q; done | q_to_cr
>two &&
- for w in Oh here is a QNUL byte how alarming; do echo ${w}; done |
q_to_nul >three &&
+ for w in Hello world how are you; do echo $w; done >LFonly &&
+ for w in I am very very fine thank you; do echo ${w}Q; done | q_to_cr
>CRLFonly &&
+ for w in Oh here is a QNUL byte how alarming; do echo ${w}; done |
q_to_nul >LFwithNUL &&
git add . &&
git commit -m initial &&
- one=$(git rev-parse HEAD:one) &&
- two=$(git rev-parse HEAD:two) &&
- three=$(git rev-parse HEAD:three) &&
+ LFonly=$(git rev-parse HEAD:LFonly) &&
+ CRLFonly=$(git rev-parse HEAD:CRLFonly) &&
+ LFwithNUL=$(git rev-parse HEAD:LFwithNUL) &&
echo happy.
'
test_expect_success 'default settings cause no changes' '
- rm -f .gitattributes tmp one two three &&
+ rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
git read-tree --reset -u HEAD &&
- ! has_cr one &&
- has_cr two &&
- onediff=$(git diff one) &&
- twodiff=$(git diff two) &&
- threediff=$(git diff three) &&
- test -z "$onediff" && test -z "$twodiff" && test -z "$threediff"
+ ! has_cr LFonly &&
+ has_cr CRLFonly &&
+ LFonlydiff=$(git diff LFonly) &&
+ CRLFonlydiff=$(git diff CRLFonly) &&
+ LFwithNULdiff=$(git diff LFwithNUL) &&
+ test -z "$LFonlydiff" -a -z "$CRLFonlydiff" -a -z "$LFwithNULdiff"
'
test_expect_success 'crlf=true causes a CRLF file to be normalized' '
# Backwards compatibility check
- rm -f .gitattributes tmp one two three &&
- echo "two crlf" > .gitattributes &&
+ rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
+ echo "CRLFonly crlf" > .gitattributes &&
git read-tree --reset -u HEAD &&
# Note, "normalized" means that git will normalize it if added
- has_cr two &&
- twodiff=$(git diff two) &&
- test -n "$twodiff"
+ has_cr CRLFonly &&
+ CRLFonlydiff=$(git diff CRLFonly) &&
+ test -n "$CRLFonlydiff"
'
test_expect_success 'text=true causes a CRLF file to be normalized' '
- rm -f .gitattributes tmp one two three &&
- echo "two text" > .gitattributes &&
+ rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
+ echo "CRLFonly text" > .gitattributes &&
git read-tree --reset -u HEAD &&
# Note, "normalized" means that git will normalize it if added
- has_cr two &&
- twodiff=$(git diff two) &&
- test -n "$twodiff"
+ has_cr CRLFonly &&
+ CRLFonlydiff=$(git diff CRLFonly) &&
+ test -n "$CRLFonlydiff"
'
test_expect_success 'eol=crlf gives a normalized file CRLFs with
autocrlf=false' '
- rm -f .gitattributes tmp one two three &&
+ rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
git config core.autocrlf false &&
- echo "one eol=crlf" > .gitattributes &&
+ echo "LFonly eol=crlf" > .gitattributes &&
git read-tree --reset -u HEAD &&
- has_cr one &&
- onediff=$(git diff one) &&
- test -z "$onediff"
+ has_cr LFonly &&
+ LFonlydiff=$(git diff LFonly) &&
+ test -z "$LFonlydiff"
'
test_expect_success 'eol=crlf gives a normalized file CRLFs with
autocrlf=input' '
- rm -f .gitattributes tmp one two three &&
+ rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
git config core.autocrlf input &&
- echo "one eol=crlf" > .gitattributes &&
+ echo "LFonly eol=crlf" > .gitattributes &&
git read-tree --reset -u HEAD &&
- has_cr one &&
- onediff=$(git diff one) &&
- test -z "$onediff"
+ has_cr LFonly &&
+ LFonlydiff=$(git diff LFonly) &&
+ test -z "$LFonlydiff"
'
test_expect_success 'eol=lf gives a normalized file LFs with autocrlf=true' '
- rm -f .gitattributes tmp one two three &&
+ rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
git config c
[PATCH v2 2/2] sha1_file: use strncmp for string comparison
Avoid overrunning the existing pack name (p->pack_name, a C string) in
the case that the new path is longer by using strncmp instead of memcmp
for comparing. While at it, replace the magic constant 4 with a
strlen call to document its meaning.
Signed-off-by: Rene Scharfe
---
No changes from intial round.
sha1_file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index 394fa45..8adab14 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1207,7 +1207,8 @@ static void prepare_packed_git_one(char *objdir, int
local)
if (has_extension(de->d_name, ".idx")) {
/* Don't reopen a pack we already have. */
for (p = packed_git; p; p = p->next) {
- if (!memcmp(path.buf, p->pack_name, path.len -
4))
+ if (!strncmp(path.buf, p->pack_name,
+path.len - strlen(".idx")))
break;
}
if (p == NULL &&
--
2.0.0
--
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] sha1_file: replace PATH_MAX buffer wirh strbuf in, prepare_packed_git_one()
Instead of using strbuf to create a message string in case a path is
too long for our fixed-size buffer, replace that buffer with a strbuf
and thus get rid of the limitation.
Helped-by: Duy Nguyen
Signed-off-by: Rene Scharfe
---
@Duy: Thanks for catching the missing strbuf_release in the opendir
error case.
sha1_file.c | 42 --
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 34d527f..394fa45 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1177,48 +1177,37 @@ static void report_pack_garbage(struct string_list
*list)
static void prepare_packed_git_one(char *objdir, int local)
{
- /* Ensure that this buffer is large enough so that we can
- append "/pack/" without clobbering the stack even if
- strlen(objdir) were PATH_MAX. */
- char path[PATH_MAX + 1 + 4 + 1 + 1];
- int len;
+ struct strbuf path = STRBUF_INIT;
+ size_t dirnamelen;
DIR *dir;
struct dirent *de;
struct string_list garbage = STRING_LIST_INIT_DUP;
- sprintf(path, "%s/pack", objdir);
- len = strlen(path);
- dir = opendir(path);
+ strbuf_addstr(&path, objdir);
+ strbuf_addstr(&path, "/pack");
+ dir = opendir(path.buf);
if (!dir) {
if (errno != ENOENT)
error("unable to open object pack directory: %s: %s",
- path, strerror(errno));
+ path.buf, strerror(errno));
+ strbuf_release(&path);
return;
}
- path[len++] = '/';
+ strbuf_addch(&path, '/');
+ dirnamelen = path.len;
while ((de = readdir(dir)) != NULL) {
- int namelen = strlen(de->d_name);
struct packed_git *p;
- if (len + namelen + 1 > sizeof(path)) {
- if (report_garbage) {
- struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, "%.*s/%s", len - 1, path,
de->d_name);
- report_garbage("path too long", sb.buf);
- strbuf_release(&sb);
- }
- continue;
- }
-
if (is_dot_or_dotdot(de->d_name))
continue;
- strcpy(path + len, de->d_name);
+ strbuf_setlen(&path, dirnamelen);
+ strbuf_addstr(&path, de->d_name);
if (has_extension(de->d_name, ".idx")) {
/* Don't reopen a pack we already have. */
for (p = packed_git; p; p = p->next) {
- if (!memcmp(path, p->pack_name, len + namelen -
4))
+ if (!memcmp(path.buf, p->pack_name, path.len -
4))
break;
}
if (p == NULL &&
@@ -1226,7 +1215,7 @@ static void prepare_packed_git_one(char *objdir, int
local)
* See if it really is a valid .idx file with
* corresponding .pack file that we can map.
*/
- (p = add_packed_git(path, len + namelen, local)) !=
NULL)
+ (p = add_packed_git(path.buf, path.len, local)) !=
NULL)
install_packed_git(p);
}
@@ -1237,13 +1226,14 @@ static void prepare_packed_git_one(char *objdir, int
local)
has_extension(de->d_name, ".pack") ||
has_extension(de->d_name, ".bitmap") ||
has_extension(de->d_name, ".keep"))
- string_list_append(&garbage, path);
+ string_list_append(&garbage, path.buf);
else
- report_garbage("garbage found", path);
+ report_garbage("garbage found", path.buf);
}
closedir(dir);
report_pack_garbage(&garbage);
string_list_clear(&garbage, 0);
+ strbuf_release(&path);
}
static int sort_pack(const void *a_, const void *b_)
--
2.0.0
--
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/3] cache-tree: Create/update cache-tree on checkout
On Sat, Jun 28, 2014 at 7:20 AM, David Turner wrote:
> When git checkout checks out a branch, create or update the
> cache-tree so that subsequent operations are faster.
>
> Signed-off-by: David Turner
> ---
> builtin/checkout.c| 4
> cache-tree.c | 22 --
> cache-tree.h | 1 +
> t/t0090-cache-tree.sh | 15 ++-
> 4 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 07cf555..df791e8 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -553,6 +553,10 @@ static int merge_working_tree(const struct checkout_opts
> *opts,
> }
> }
>
> + if (write_cache_as_tree(NULL, WRITE_TREE_DO_NOT_WRITE, "")) {
> + warn("Unable to write cache_tree");
> + }
> +
I wonder if we should do this in !opts->force code path only. In the
opts->force code path we could use prime_cache_tree() (like
read-tree), which is supposedly faster (but may need some tests to be
sure). prime_cache_tree() could be made a bit faster by doing it
during tree traversal in unpack_trees() so we don't have to unpack any
tree objects twice.
--
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
Re: [PATCH 1/2] sha1_file: replace PATH_MAX buffer wirh strbuf in prepare_packed_git_one()
On Sat, Jun 28, 2014 at 9:47 PM, René Scharfe wrote:
> - sprintf(path, "%s/pack", objdir);
> - len = strlen(path);
> - dir = opendir(path);
> + strbuf_addstr(&path, objdir);
> + strbuf_addstr(&path, "/pack");
> + dir = opendir(path.buf);
> if (!dir) {
> if (errno != ENOENT)
> error("unable to open object pack directory: %s: %s",
> - path, strerror(errno));
> + path.buf, strerror(errno));
> return;
Memory leak. The rest looks good.
--
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
Re: [PATCH v2 4/4] diff: mark any file larger than core.bigfilethreshold binary
On Sat, Jun 28, 2014 at 1:56 AM, Thomas Braun
wrote:
>> The name is misleading and forced me to read it twice before I
>> realized that this is "populating the is-binary bit". It might make
>> it a bit better if you renamed it to DIFF_POPULATE_IS_BINARY_BIT or
>> perhaps DIFF_POPULATE_CHECK_BINARY or something. For consistency,
>> the other bit may want to be also renamed from SIZE_ONLY to either
>>
>> (1) CHECK_SIZE_ONLY
>>
>> (2) One bit for CHECK_SIZE, another for NO_CONTENTS, and optionally
>> make SIZE_ONLY the union of two
>>
>> I do not have strong preference either way; the latter may be more
>> logical in that "not loading contents" and "check size" are sort of
>> orthogonal in that you can later choose to check, without loading
>> contents, only the binary-ness without checking size, but no calles
>> that passes a non-zero flag to the populate-filespec function will
>> want to slurp in the contents in practice, so in that sense we could
>> declare that the NO_CONENTS bit is implied.
Will do (and probably go with (1) as I still prefer zero as "good defaults")
>> But more importantly, would this patch actually help?
Well yes as demonstrated by the new test ;-) Unfortunately the scope
of help is limited to --stat.. I should have done more thorough
testing.
>> For one
>> thing, this wouldn't (and shouldn't) help if the user wants --binary
>> diff out of us anyway, I suspect, but I wonder what the following
>> codepath in the builtin_diff() function would do:
>>
>> ...
>> } else if (!DIFF_OPT_TST(o, TEXT) &&
>> ( (!textconv_one && diff_filespec_is_binary(one)) ||
>> (!textconv_two && diff_filespec_is_binary(two)) )) {
>> if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
>> die("unable to read files to diff");
>> /* Quite common confusing case */
>> if (mf1.size == mf2.size &&
>> !memcmp(mf1.ptr, mf2.ptr, mf1.size)) {
>> if (must_show_header)
>> fprintf(o->file, "%s", header.buf);
>> goto free_ab_and_return;
>> }
>> fprintf(o->file, "%s", header.buf);
>> strbuf_reset(&header);
>> if (DIFF_OPT_TST(o, BINARY))
>> emit_binary_diff(o->file, &mf1, &mf2, line_prefix);
>> else
>> fprintf(o->file, "%sBinary files %s and %s differ\n",
>> line_prefix, lbl[0], lbl[1]);
>> o->found_changes = 1;
>> } else {
>> ...
>>
>> If we weren't told with --text/-a to force textual output, and
>> at least one of the sides is marked as binary (and this patch marks
>> a large blob as binary unless attributes says otherwise), we still
>> call fill_mmfile() on them to slurp the contents to be compared, no?
>>
>> And before you get to the DIFF_OPT_TST(o, BINARY), you memcmp(3) to
>> check if the sides are identical, so...
>
> Good point. So how about an additional change roughly sketched as
>
> @@ -2223,6 +2223,14 @@ struct userdiff_driver *get_textconv(struct
> diff_filespec *one)
> return userdiff_get_textconv(one->driver);
> }
>
> +/* read the files in small chunks into memory and compare them */
> +static int filecmp_chunked(struct diff_filespec *one,
> + struct diff_filespec *two)
> +{
> + // TODO add implementation
> + return 0;
> +}
> +
We have object streaming interface to do similar like this. In fact
index-pack already does large file memcmp() for hash collision test. I
think I can move some code around and support large file in this code
path..
--
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
Re: [PATCH v2 2/4] fsck: do not die when not enough memory to examine a pack entry
On Fri, Jun 27, 2014 at 1:09 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> fsck is a tool that error() is more preferred than die(), but many > > "more preferred" without justifying why it is "more preferred" is > not quite a justification, is it? Also, an object failing to load > in-core is not a missing object, so if your aim is to let "fsck" > diagnose a too-large-to-load object as missing and let it continue, > I do not know if it is "more preferred" in the first place. Adding > a "too large--cannot check" bin of objects may be needed for it to > be useful. Also, we might need to give at the end "oh by the way, > because we couldn't read some objects to even determine its type, > the unreachable report from this fsck run is totally useless." Fair enough. I think avoiding dying in xmalloc() in this code path is still a good thing. At least "failed to read object %s" is more informative than simply "Out of memory". The error cascading effect in fsck is something I think we already have. I'll try to rephrase the commit message. But if you think this is not a good direction, dropping it is not so bad. I'm going to look at xmalloc() in unpack-objects. That's where we really should not abort because of memory shortage as the user may try to get as many objects as possible out of the pack. > The log message tries to justify that this may be a good thing for > "fsck", but the patch actually tries to change the behaviour of all > code paths that try to load an object in-core without considering > the ramifications of such a change. I _think_ all callers should be > prepared to receive NULL when we encounter a corrupt object (and > otherwise we should fix them), but it is unclear how much audit of > the callers (if any) was done to prepare this change. -- 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
Re: What's cooking in git.git (Jun 2014, #06; Thu, 26)
On Fri, Jun 27, 2014 at 5:02 AM, Junio C Hamano wrote: > * nd/split-index (2014-06-13) 32 commits > - t1700: new tests for split-index mode > - t2104: make sure split index mode is off for the version test > - read-cache: force split index mode with GIT_TEST_SPLIT_INDEX > - read-tree: note about dropping split-index mode or index version > - read-tree: force split-index mode off on --index-output > - rev-parse: add --shared-index-path to get shared index path > - update-index --split-index: do not split if $GIT_DIR is read only > - update-index: new options to enable/disable split index mode > - split-index: strip pathname of on-disk replaced entries > - split-index: do not invalidate cache-tree at read time > - split-index: the reading part > - split-index: the writing part > - read-cache: mark updated entries for split index > - read-cache: save deleted entries in split index > - read-cache: mark new entries for split index > - read-cache: split-index mode > - read-cache: save index SHA-1 after reading > - entry.c: update cache_changed if refresh_cache is set in checkout_entry() > - cache-tree: mark istate->cache_changed on prime_cache_tree() > - cache-tree: mark istate->cache_changed on cache tree update > - cache-tree: mark istate->cache_changed on cache tree invalidation > - unpack-trees: be specific what part of the index has changed > - resolve-undo: be specific what part of the index has changed > - update-index: be specific what part of the index has changed > - read-cache: be specific what part of the index has changed > - read-cache: be strict about "changed" in remove_marked_cache_entries() > - read-cache: store in-memory flags in the first 12 bits of ce_flags > - read-cache: relocate and unexport commit_locked_index() > - read-cache: new API write_locked_index instead of write_index/write_cache > - sequencer: do not update/refresh index if the lock cannot be held > - ewah: delete unused ewah_read_mmap_native declaration > - ewah: fix constness of ewah_read_mmap > > What's the doneness of this one? It's done, at least for the extension format and core algorithm. Later we may want to automatically re-split the index when too many changes are accumulated in the main index. -- 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 v2 1/2] t6023-merge-file.sh: fix and mark as broken invalid tests
Tests "merge without conflict (missing LF at EOF" and "merge result added missing LF" are meaningless - the first one is identical to "merge without conflict" and the second compares results of those identical tests, which are always same. This has been so since their addition in ba1f5f3537. Probably "new4.txt" was meant to be used instead of "new2.txt". Unfortunately, the current merge-file breaks with new4 - conflict is reported. They also fail at that revision if fixed. Fix the file reference to "new4.txt" and mark the tests as failing - they look like legitimate expectations, just not satisfied at time being. Signed-off-by: Max Kirillov --- t/t6023-merge-file.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index d9f3439..6da921c 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -77,10 +77,10 @@ test_expect_success "merge without conflict (--quiet)" \ "git merge-file --quiet test.txt orig.txt new2.txt" cp new1.txt test2.txt -test_expect_success "merge without conflict (missing LF at EOF)" \ - "git merge-file test2.txt orig.txt new2.txt" +test_expect_failure "merge without conflict (missing LF at EOF)" \ + "git merge-file test2.txt orig.txt new4.txt" -test_expect_success "merge result added missing LF" \ +test_expect_failure "merge result added missing LF" \ "test_cmp test.txt test2.txt" cp test.txt backup.txt -- 2.0.0.526.g5318336 -- 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] git-merge-file: do not add LF at EOF while applying unrelated change
If 'current-file' does not contain LF at EOF, and change between 'base-file' and 'other-file' does not change any line close to EOF, the 3-way merge should not add LF to EOF. This is what 'diff3 -m' does, and seems to be a reasonable expectation. The change which introduced the behavior is cd1d61c44f. It always calls function xdl_recs_copy() for sides with add_nl == 1. In fact, it looks like the only case when this is needed is when 2 files are being union-merged, and they do not have LF at EOF (strictly speaking, the first of them). Add tests: * "merge without conflict (missing LF at EOF, away from change in the other file)" and "merge does not add LF away of change", to demonstrate the changed behavior. * "conflict at EOF without LF resolved by --union", to verify that the union-merge at the end inerts newline between versions. * some more tests which I felt like not covering the functionality well Signed-off-by: Max Kirillov --- t/t6023-merge-file.sh | 85 +++ xdiff/xmerge.c| 4 +-- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index 6da921c..59ae712 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -83,6 +83,23 @@ test_expect_failure "merge without conflict (missing LF at EOF)" \ test_expect_failure "merge result added missing LF" \ "test_cmp test.txt test2.txt" +cp new4.txt test3.txt +test_expect_success "merge without conflict (missing LF at EOF, away from change in the other file)" \ + "git merge-file --quiet test3.txt new2.txt new3.txt" + +cat > expect.txt << EOF +DOMINUS regit me, +et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +EOF +printf "propter nomen suum." >> expect.txt + +test_expect_success "merge does not add LF away of change" \ + "test_cmp test3.txt expect.txt" + cp test.txt backup.txt test_expect_success "merge with conflicts" \ "test_must_fail git merge-file test.txt orig.txt new3.txt" @@ -107,6 +124,55 @@ EOF test_expect_success "expected conflict markers" "test_cmp test.txt expect.txt" cp backup.txt test.txt + +cat > expect.txt << EOF +Dominus regit me, et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +propter nomen suum. +Nam et si ambulavero in medio umbrae mortis, +non timebo mala, quoniam tu mecum es: +virga tua et baculus tuus ipsa me consolata sunt. +EOF +test_expect_success "merge conflicting with --ours" \ + "git merge-file --ours test.txt orig.txt new3.txt && test_cmp test.txt expect.txt" +cp backup.txt test.txt + +cat > expect.txt << EOF +DOMINUS regit me, +et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +propter nomen suum. +Nam et si ambulavero in medio umbrae mortis, +non timebo mala, quoniam tu mecum es: +virga tua et baculus tuus ipsa me consolata sunt. +EOF +test_expect_success "merge conflicting with --theirs" \ + "git merge-file --theirs test.txt orig.txt new3.txt && test_cmp test.txt expect.txt" +cp backup.txt test.txt + +cat > expect.txt << EOF +Dominus regit me, et nihil mihi deerit. +DOMINUS regit me, +et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +propter nomen suum. +Nam et si ambulavero in medio umbrae mortis, +non timebo mala, quoniam tu mecum es: +virga tua et baculus tuus ipsa me consolata sunt. +EOF +test_expect_success "merge conflicting with --union" \ + "git merge-file --union test.txt orig.txt new3.txt && test_cmp test.txt expect.txt" +cp backup.txt test.txt + test_expect_success "merge with conflicts, using -L" \ "test_must_fail git merge-file -L 1 -L 2 test.txt orig.txt new3.txt" @@ -260,4 +326,23 @@ test_expect_success 'marker size' ' test_cmp expect actual ' +printf "line1\nline2\nline3" >nolf-orig.txt +printf "line1\nline2\nline3x" >nolf-diff1.txt +printf "line1\nline2\nline3y" >nolf-diff2.txt + +test_expect_success 'conflict at EOF without LF resolved by --ours' \ + 'git merge-file -p --ours nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && +printf "line1\nline2\nline3x" >expect.txt && +test_cmp expect.txt output.txt' + +test_expect_success 'conflict at EOF without LF resolved by --theirs' \ + 'git merge-file -p --theirs nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && +printf "line1\nline2\nline3y" >expect.txt && +test_cmp expect.txt output.txt' + +test_expect_success 'conflict at EOF without LF resolved by --union' \ + 'git merge-file -p --union nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && +printf "line1
[PATCH v2 0/2] git-merge-file: do not add LF at EOF while applying unrelated change
I realized the case when the newline adding can be needed. The version 2 have this case (union-merge of changes at EOF without LF) fixed, with adding corresponding tests. Max Kirillov (2): t6023-merge-file.sh: fix and mark as broken invalid tests git-merge-file: do not add LF at EOF while applying unrelated change t/t6023-merge-file.sh | 91 +-- xdiff/xmerge.c| 4 +-- 2 files changed, 90 insertions(+), 5 deletions(-) -- 2.0.0.526.g5318336 -- 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] t6023-merge-file.sh: fix and mark as broken invalid tests
Tests "merge without conflict (missing LF at EOF" and "merge result added missing LF" are meaningless - the first one is identical to "merge without conflict" and the second compares results of those identical tests, which are always same. This has been so since their addition in ba1f5f3537. Probably "new4.txt" was meant to be used instead of "new2.txt". Unfortunately, the current merge-file breaks with new4 - conflict is reported. They also fail at that revision if fixed. Fix the file reference to "new4.txt" and mark the tests as failing - they look like legitimate expectations, just not satisfied at time being. Signed-off-by: Max Kirillov --- t/t6023-merge-file.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index d9f3439..6da921c 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -77,10 +77,10 @@ test_expect_success "merge without conflict (--quiet)" \ "git merge-file --quiet test.txt orig.txt new2.txt" cp new1.txt test2.txt -test_expect_success "merge without conflict (missing LF at EOF)" \ - "git merge-file test2.txt orig.txt new2.txt" +test_expect_failure "merge without conflict (missing LF at EOF)" \ + "git merge-file test2.txt orig.txt new4.txt" -test_expect_success "merge result added missing LF" \ +test_expect_failure "merge result added missing LF" \ "test_cmp test.txt test2.txt" cp test.txt backup.txt -- 2.0.0.526.g5318336 -- 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] git-merge-file: do not add LF at EOF while applying unrelated change
If 'current-file' does not contain LF at EOF, and change between 'base-file' and 'other-file' does not change any line close to EOF, the 3-way merge should not add LF to EOF. This is what 'diff3 -m' does, and seems to be a reasonable expectation. The change which introduced the behavior is cd1d61c44f and was about union merge. The union merges are tested in the t6026-merge-attr, and merge-file in general are tested in t6023-merge-file. Add tests "merge conflicting with --.." there, to verify that the fix does not break anything. Also add tests "merge without conflict (missing LF at EOF, away from change in the other file)" and "merge does not add LF away of change", to demonstrate the changed behavior. Signed-off-by: Max Kirillov --- t/t6023-merge-file.sh | 66 +++ xdiff/xmerge.c| 4 ++-- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index 6da921c..21d1b85 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -83,6 +83,23 @@ test_expect_failure "merge without conflict (missing LF at EOF)" \ test_expect_failure "merge result added missing LF" \ "test_cmp test.txt test2.txt" +cp new4.txt test3.txt +test_expect_success "merge without conflict (missing LF at EOF, away from change in the other file)" \ + "git merge-file --quiet test3.txt new2.txt new3.txt" + +cat > expect.txt << EOF +DOMINUS regit me, +et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +EOF +printf "propter nomen suum." >> expect.txt + +test_expect_success "merge does not add LF away of change" \ + "test_cmp test3.txt expect.txt" + cp test.txt backup.txt test_expect_success "merge with conflicts" \ "test_must_fail git merge-file test.txt orig.txt new3.txt" @@ -107,6 +124,55 @@ EOF test_expect_success "expected conflict markers" "test_cmp test.txt expect.txt" cp backup.txt test.txt + +cat > expect.txt << EOF +Dominus regit me, et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +propter nomen suum. +Nam et si ambulavero in medio umbrae mortis, +non timebo mala, quoniam tu mecum es: +virga tua et baculus tuus ipsa me consolata sunt. +EOF +test_expect_success "merge conflicting with --ours" \ + "git merge-file --ours test.txt orig.txt new3.txt && test_cmp test.txt expect.txt" +cp backup.txt test.txt + +cat > expect.txt << EOF +DOMINUS regit me, +et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +propter nomen suum. +Nam et si ambulavero in medio umbrae mortis, +non timebo mala, quoniam tu mecum es: +virga tua et baculus tuus ipsa me consolata sunt. +EOF +test_expect_success "merge conflicting with --theirs" \ + "git merge-file --theirs test.txt orig.txt new3.txt && test_cmp test.txt expect.txt" +cp backup.txt test.txt + +cat > expect.txt << EOF +Dominus regit me, et nihil mihi deerit. +DOMINUS regit me, +et nihil mihi deerit. +In loco pascuae ibi me collocavit, +super aquam refectionis educavit me; +animam meam convertit, +deduxit me super semitas jusitiae, +propter nomen suum. +Nam et si ambulavero in medio umbrae mortis, +non timebo mala, quoniam tu mecum es: +virga tua et baculus tuus ipsa me consolata sunt. +EOF +test_expect_success "merge conflicting with --union" \ + "git merge-file --union test.txt orig.txt new3.txt && test_cmp test.txt expect.txt" +cp backup.txt test.txt + test_expect_success "merge with conflicts, using -L" \ "test_must_fail git merge-file -L 1 -L 2 test.txt orig.txt new3.txt" diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c index 9e13b25..5f7a95a 100644 --- a/xdiff/xmerge.c +++ b/xdiff/xmerge.c @@ -245,11 +245,11 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1, dest ? dest + size : NULL); /* Postimage from side #1 */ if (m->mode & 1) - size += xdl_recs_copy(xe1, m->i1, m->chg1, 1, + size += xdl_recs_copy(xe1, m->i1, m->chg1, 0, dest ? dest + size : NULL); /* Postimage from side #2 */ if (m->mode & 2) - size += xdl_recs_copy(xe2, m->i2, m->chg2, 1, + size += xdl_recs_copy(xe2, m->i2, m->chg2, 0, dest ? dest + size : NULL); } else continue; -- 2.0.0.526.g5318336 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More
[PATCH 0/2] git-merge-file: do not add LF at EOF while applying unrelated change
Hi. I have noticed that cherry-pick adds trailing newlines when it is not expected to - the change does not contain its addition. Here is the fix for it. The fix is quite debugging-driven, without detailed analysis of how exactly this "add_nl" parameter works in all cases. But it passes all tests. I have added some more where I felt there were not enough. Also I have noticed that some tests in the t6023, which related to the behavior contain a mistake which makes them meaningless. I have fixed it and marked the tests as expected failure, because they are failing after that. Hopely they will be fixed some day. Max Kirillov (2): t6023-merge-file.sh: fix and mark as broken invalid tests git-merge-file: do not add LF at EOF while applying unrelated change t/t6023-merge-file.sh | 72 --- xdiff/xmerge.c| 4 +-- 2 files changed, 71 insertions(+), 5 deletions(-) -- 2.0.0.526.g5318336 -- 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 v5 5/7] replace: refactor replacing parents
Signed-off-by: Christian Couder
---
builtin/replace.c | 42 +-
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 3515979..ad47237 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -295,27 +295,14 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, "replacement", new, force);
}
-static int create_graft(int argc, const char **argv, int force)
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
{
- unsigned char old[20], new[20];
- const char *old_ref = argv[0];
- struct commit *commit;
- struct strbuf buf = STRBUF_INIT;
struct strbuf new_parents = STRBUF_INIT;
const char *parent_start, *parent_end;
- const char *buffer;
- unsigned long size;
int i;
- if (get_sha1(old_ref, old) < 0)
- die(_("Not a valid object name: '%s'"), old_ref);
- commit = lookup_commit_or_die(old, old_ref);
-
/* find existing parents */
- buffer = get_commit_buffer(commit, &size);
- strbuf_add(&buf, buffer, size);
- unuse_commit_buffer(commit, buffer);
- parent_start = buf.buf;
+ parent_start = buf->buf;
parent_start += 46; /* "tree " + "hex sha1" + "\n" */
parent_end = parent_start;
@@ -332,13 +319,34 @@ static int create_graft(int argc, const char **argv, int
force)
}
/* replace existing parents with new ones */
- strbuf_splice(&buf, parent_start - buf.buf, parent_end - parent_start,
+ strbuf_splice(buf, parent_start - buf->buf, parent_end - parent_start,
new_parents.buf, new_parents.len);
+ strbuf_release(&new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ const char *buffer;
+ unsigned long size;
+
+ if (get_sha1(old_ref, old) < 0)
+ die(_("Not a valid object name: '%s'"), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ buffer = get_commit_buffer(commit, &size);
+ strbuf_add(&buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+
+ replace_parents(&buf, argc, argv);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_("could not write replacement commit for: '%s'"), old_ref);
- strbuf_release(&new_parents);
strbuf_release(&buf);
if (!hashcmp(old, new))
--
2.0.0.421.g786a89d.dirty
--
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 v5 3/7] Documentation: replace: add --graft option
Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] 'git replace' [-f] --edit +'git replace' [-f] --graft [...] 'git replace' -d ... 'git replace' [--format=] [-l []] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft [...]:: + Create a graft commit. A new commit is created with the same + content as except that its parents will be + [...] instead of 's parents. A replacement ref + is then created to replace with the newly created + commit. + -l :: --list :: List replace refs for objects that match the given pattern (or -- 2.0.0.421.g786a89d.dirty -- 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 v5 7/7] replace: add test for --graft with signed commit
Signed-off-by: Christian Couder --- t/t6050-replace.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index ca45a84..80b85e3 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -7,6 +7,7 @@ test_description='Tests replace refs functionality' exec > hello && + echo "line 18" >> hello && + git add hello && + test_tick && + git commit --quiet -S -m "hello: 2 more lines in a signed commit" && + HASH8=$(git rev-parse --verify HEAD) && + git verify-commit $HASH8 +' + +test_expect_success GPG '--graft with a signed commit' ' + git cat-file commit $HASH8 >orig && + git replace --graft $HASH8 && + git cat-file commit $HASH8 >repl && + test_must_fail grep gpgsig repl && + diff -u orig repl | grep "^-parent $HASH7" && + diff -u orig repl | grep "^-gpgsig -BEGIN PGP SIGNATURE-" && + test_must_fail git verify-commit $HASH8 && + git replace -d $HASH8 +' + test_done -- 2.0.0.421.g786a89d.dirty -- 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 v5 4/7] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of "git replace".
While at it let's mention this new script in the
"git replace" documentation for the --graft option.
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 28
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as except that its parents will be
[...] instead of 's parents. A replacement ref
is then created to replace with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l ::
--list ::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE="${GIT_DIR:-.git}/info/grafts"
+
+. $(git --exec-path)/git-sh-setup
+
+test -f "$GRAFTS_FILE" || die "Could not find graft file: '$GRAFTS_FILE'"
+
+grep '^[^# ]' "$GRAFTS_FILE" |
+while read definition
+do
+ if test -n "$definition"
+ then
+ echo "Converting: $definition"
+ git replace --graft $definition ||
+ die "Conversion failed for: $definition"
+ fi
+done
+
+mv "$GRAFTS_FILE" "$GRAFTS_FILE.bak" ||
+ die "Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'"
+
+echo "Success!"
+echo "All the grafts in '$GRAFTS_FILE' have been converted to replace refs!"
+echo "The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'"
--
2.0.0.421.g786a89d.dirty
--
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 v5 6/7] replace: remove signature when using --graft
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Signed-off-by: Christian Couder
---
builtin/replace.c | 5 +
commit.c | 34 ++
commit.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index ad47237..000db65 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int
force)
replace_parents(&buf, argc, argv);
+ if (remove_signature(&buf))
+ warning(_("the original commit '%s' has a gpg signature.\n"
+ "It will be removed in the replacement commit!"),
+ old_ref);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_("could not write replacement commit for: '%s'"), old_ref);
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf->buf;
+ const char *tail = buf->buf + buf->len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line < tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
+
+ if (in_signature && line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header) &&
+line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);
+
+ return sig_start != NULL;
+}
+
static void handle_signed_tag(struct commit *parent, struct
commit_extra_header ***tail)
{
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index 2e1492a..4234dae 100644
--- a/commit.h
+++ b/commit.h
@@ -327,6 +327,8 @@ struct commit *get_merge_parent(const char *name);
extern int parse_signed_commit(const struct commit *commit,
struct strbuf *message, struct strbuf
*signature);
+extern int remove_signature(struct strbuf *buf);
+
extern void print_commit_list(struct commit_list *list,
const char *format_cur,
const char *format_last);
--
2.0.0.421.g786a89d.dirty
--
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 v5 1/7] replace: add --graft option
The usage string for this option is:
git replace [-f] --graft [...]
First we create a new commit that is the same as
except that its parents are [...]
Then we create a replace ref that replace with
the commit we just created.
With this new option, it should be straightforward to
convert grafts to replace refs.
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
builtin/replace.c | 66 ++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..3515979 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -17,6 +17,7 @@
static const char * const git_replace_usage[] = {
N_("git replace [-f] "),
N_("git replace [-f] --edit "),
+ N_("git replace [-f] --graft [...]"),
N_("git replace -d ..."),
N_("git replace [--format=] [-l []]"),
NULL
@@ -294,6 +295,58 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, "replacement", new, force);
}
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ const char *buffer;
+ unsigned long size;
+ int i;
+
+ if (get_sha1(old_ref, old) < 0)
+ die(_("Not a valid object name: '%s'"), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ /* find existing parents */
+ buffer = get_commit_buffer(commit, &size);
+ strbuf_add(&buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+ parent_start = buf.buf;
+ parent_start += 46; /* "tree " + "hex sha1" + "\n" */
+ parent_end = parent_start;
+
+ while (starts_with(parent_end, "parent "))
+ parent_end += 48; /* "parent " + "hex sha1" + "\n" */
+
+ /* prepare new parents */
+ for (i = 1; i < argc; i++) {
+ unsigned char sha1[20];
+ if (get_sha1(argv[i], sha1) < 0)
+ die(_("Not a valid object name: '%s'"), argv[i]);
+ lookup_commit_or_die(sha1, argv[i]);
+ strbuf_addf(&new_parents, "parent %s\n", sha1_to_hex(sha1));
+ }
+
+ /* replace existing parents with new ones */
+ strbuf_splice(&buf, parent_start - buf.buf, parent_end - parent_start,
+ new_parents.buf, new_parents.len);
+
+ if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+ die(_("could not write replacement commit for: '%s'"), old_ref);
+
+ strbuf_release(&new_parents);
+ strbuf_release(&buf);
+
+ if (!hashcmp(old, new))
+ return error("new commit is the same as the old one: '%s'",
sha1_to_hex(old));
+
+ return replace_object_sha1(old_ref, old, "replacement", new, force);
+}
+
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
@@ -303,12 +356,14 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
MODE_LIST,
MODE_DELETE,
MODE_EDIT,
+ MODE_GRAFT,
MODE_REPLACE
} cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list replace refs"),
MODE_LIST),
OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"),
MODE_DELETE),
OPT_CMDMODE('e', "edit", &cmdmode, N_("edit existing object"),
MODE_EDIT),
+ OPT_CMDMODE('g', "graft", &cmdmode, N_("change a commit's
parents"), MODE_GRAFT),
OPT_BOOL('f', "force", &force, N_("replace the ref if it
exists")),
OPT_STRING(0, "format", &format, N_("format"), N_("use this
format")),
OPT_END()
@@ -325,7 +380,10 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
usage_msg_opt("--format cannot be used when not listing",
git_replace_usage, options);
- if (force && cmdmode != MODE_REPLACE && cmdmode != MODE_EDIT)
+ if (force &&
+ cmdmode != MODE_REPLACE &&
+ cmdmode != MODE_EDIT &&
+ cmdmode != MODE_GRAFT)
usage_msg_opt("-f only makes sense when writing a replacement",
git_replace_usage, options);
@@ -348,6 +406,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return edit_and_replace(argv[0], force);
+ case MODE_GRAFT:
+ if (argc < 1)
+ usage_msg_opt("-g needs at least one argument",
+ git_replace_usag
[PATCH v5 2/7] replace: add test for --graft
Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' ' test -z "$(git replace)" ' +test_expect_success '--graft with and without already replaced object' ' + test $(git log --oneline | wc -l) = 7 && + git replace --graft $HASH5 && + test $(git log --oneline | wc -l) = 3 && + git cat-file -p $HASH5 | test_must_fail grep parent && + test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 && + git replace --force -g $HASH5 $HASH4 $HASH3 && + git cat-file -p $HASH5 | grep "parent $HASH4" && + git cat-file -p $HASH5 | grep "parent $HASH3" && + git replace -d $HASH5 +' + test_done -- 2.0.0.421.g786a89d.dirty -- 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 v5 0/7] Add --graft option to git replace
Here is a small patch series to implement: git replace [-f] --graft [...] This patch series goes on top of the patch series that implements --edit. The changes since v4, thanks to Junio and Peff, are: - The series has been rebased on top of Peff's series to store the commit buffer length (jk/commit-buffer-length). This changes patch 1/7 and fixes the problem that we lost everything after a NUL byte in the commit buffer. - Patches 5/7, 6/7 and 7/7 have been added to lose a gpg signature in the original commit buffer. Notes: - Maybe patches could be reordered or squashed, but I prefered not to do it in this round so that changes compared to previous version are easier to spot. - Junio suggested to drop the merge-tag of a parent we are not going to keep, but to keep the merge-tag of a parent we are keeping. This has not yet been done. It is unfortunate that merge-tags don't use a format like the trailer format, because we could have factorised droping a merge-tag in the "git interpret-trailers" command. Christian Couder (7): replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh replace: refactor replacing parents replace: remove signature when using --graft replace: add test for --graft with signed commit Documentation/git-replace.txt | 10 builtin/replace.c | 79 ++- commit.c | 34 + commit.h | 2 + contrib/convert-grafts-to-replace-refs.sh | 28 +++ t/t6050-replace.sh| 34 + 6 files changed, 186 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.421.g786a89d.dirty -- 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/4] sha1_file: use strncmp for string comparison
Avoid overrunning the existing pack name (p->pack_name, a C string) in
the case that the new path is longer by using strncmp instead of memcmp
for comparing. While at it, replace the magic constant 4 with a
strlen call to document its meaning.
Signed-off-by: Rene Scharfe
---
sha1_file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index 0c3cada..72b8fcb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1206,7 +1206,8 @@ static void prepare_packed_git_one(char *objdir, int
local)
if (has_extension(de->d_name, ".idx")) {
/* Don't reopen a pack we already have. */
for (p = packed_git; p; p = p->next) {
- if (!memcmp(path.buf, p->pack_name, path.len -
4))
+ if (!strncmp(path.buf, p->pack_name,
+path.len - strlen(".idx")))
break;
}
if (p == NULL &&
--
2.0.0
--
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] sha1_file: replace PATH_MAX buffer wirh strbuf in prepare_packed_git_one()
Instead of using strbuf to create a message string in case a path is
too long for our fixed-size buffer, replace that buffer with a strbuf
and thus get rid of the limitation.
Signed-off-by: Rene Scharfe
---
sha1_file.c | 41 +++--
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 34d527f..0c3cada 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1177,48 +1177,36 @@ static void report_pack_garbage(struct string_list
*list)
static void prepare_packed_git_one(char *objdir, int local)
{
- /* Ensure that this buffer is large enough so that we can
- append "/pack/" without clobbering the stack even if
- strlen(objdir) were PATH_MAX. */
- char path[PATH_MAX + 1 + 4 + 1 + 1];
- int len;
+ struct strbuf path = STRBUF_INIT;
+ size_t dirnamelen;
DIR *dir;
struct dirent *de;
struct string_list garbage = STRING_LIST_INIT_DUP;
- sprintf(path, "%s/pack", objdir);
- len = strlen(path);
- dir = opendir(path);
+ strbuf_addstr(&path, objdir);
+ strbuf_addstr(&path, "/pack");
+ dir = opendir(path.buf);
if (!dir) {
if (errno != ENOENT)
error("unable to open object pack directory: %s: %s",
- path, strerror(errno));
+ path.buf, strerror(errno));
return;
}
- path[len++] = '/';
+ strbuf_addch(&path, '/');
+ dirnamelen = path.len;
while ((de = readdir(dir)) != NULL) {
- int namelen = strlen(de->d_name);
struct packed_git *p;
- if (len + namelen + 1 > sizeof(path)) {
- if (report_garbage) {
- struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, "%.*s/%s", len - 1, path,
de->d_name);
- report_garbage("path too long", sb.buf);
- strbuf_release(&sb);
- }
- continue;
- }
-
if (is_dot_or_dotdot(de->d_name))
continue;
- strcpy(path + len, de->d_name);
+ strbuf_setlen(&path, dirnamelen);
+ strbuf_addstr(&path, de->d_name);
if (has_extension(de->d_name, ".idx")) {
/* Don't reopen a pack we already have. */
for (p = packed_git; p; p = p->next) {
- if (!memcmp(path, p->pack_name, len + namelen -
4))
+ if (!memcmp(path.buf, p->pack_name, path.len -
4))
break;
}
if (p == NULL &&
@@ -1226,7 +1214,7 @@ static void prepare_packed_git_one(char *objdir, int
local)
* See if it really is a valid .idx file with
* corresponding .pack file that we can map.
*/
- (p = add_packed_git(path, len + namelen, local)) !=
NULL)
+ (p = add_packed_git(path.buf, path.len, local)) !=
NULL)
install_packed_git(p);
}
@@ -1237,13 +1225,14 @@ static void prepare_packed_git_one(char *objdir, int
local)
has_extension(de->d_name, ".pack") ||
has_extension(de->d_name, ".bitmap") ||
has_extension(de->d_name, ".keep"))
- string_list_append(&garbage, path);
+ string_list_append(&garbage, path.buf);
else
- report_garbage("garbage found", path);
+ report_garbage("garbage found", path.buf);
}
closedir(dir);
report_pack_garbage(&garbage);
string_list_clear(&garbage, 0);
+ strbuf_release(&path);
}
static int sort_pack(const void *a_, const void *b_)
--
2.0.0
--
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] pager.c: replace git_config with git_config_get_string
Am 28.06.2014 08:01, schrieb Matthieu Moy:
> Karsten Blees writes:
>
>> I still don't like that the invalidation is done in git_config_set, though,
>> as
>> this is also used to write completely unrelated files.
>
> I don't get it. It is used to write the config files. Yes, we trigger a
> complete reload instead of just changing this particular value in the
> hashmap, but I do not see "unrelated files" in the picture.
>
git-cherry-pick, revert: writes '.git/sequencer/opts' (sequencer.c:save_opts)
git-mv : writes '.gitmodules' (submodule.c:update_path_in_gitmodules)
...and the submodule's '.git/config'
(submodule.c:connect_work_tree_and_git_dir)
Note that the typical configuration commands git-config/remote/branch seem
to call git_config_set* immediately before exit, so no need to invalidate
the config cache here either.
Which leaves clone, init and push (transport.c:set_upstreams, seems to be
just before exit as well).
I didn't look further after finding the example in cmd_clone, so with the
statistics above, it may well be the only instance of {git_config;
git_config_set; git_config}, I don't know.
I've attached the reverse call hierarchy as generated by Eclipse - quite
useful for things like this.
>> Wouldn't it be better to have a 'git_config_refresh()' that could be
>> used in place of (or before) current 'git_config(callback)' calls? The
>> initial implementation could just invalidate the config cache. If
>> there's time and energy to spare, a more advanced version could first
>> check if any of the involved config files has changed.
>
> That would not change the "xstrdup" vs "no xstrdup" issue, right?
>
Right. (Unless it turns out invalidation isn't needed after all. But even
then using interned strings for the config would be a Good Thing IMO.)
>> The xstrdup() problem could be solved by interning strings (see the
>> attached patch for a trivial implementation). I.e. allocate each distinct
>> string only once (and keep it allocated).
>
> That's an option. We need to be carefull not to modify any string
> in-place,
Hopefully an illegal non-const cast will be caught in the review process.
git_config_set_multivar_in_file(const char *, const char *, const char *, const
char *, int) : int - /git/config.c
cmd_config(int, const char * *, const char *) : int -
/git/builtin/config.c (5 matches)
git_config_set_in_file(const char *, const char *, const char *) : int
- /git/config.c
cmd_config(int, const char * *, const char *) : int -
/git/builtin/config.c (2 matches)
connect_work_tree_and_git_dir(const char *, const char *) :
void - /git/submodule.c
cmd_mv(int, const char * *, const char *) : int -
/git/builtin/mv.c
save_opts(replay_opts *) : void - /git/sequencer.c (8 matches)
sequencer_pick_revisions(replay_opts *) : int -
/git/sequencer.c
cmd_cherry_pick(int, const char * *, const char
*) : int - /git/builtin/revert.c
cmd_revert(int, const char * *, const char *) :
int - /git/builtin/revert.c
update_path_in_gitmodules(const char *, const char *) : int -
/git/submodule.c
cmd_mv(int, const char * *, const char *) : int -
/git/builtin/mv.c
git_config_set_multivar(const char *, const char *, const char *, int)
: int - /git/config.c
add_branch(const char *, const char *, const char *, int,
strbuf *) : int - /git/builtin/remote.c
add_branches(remote *, const char * *, const char *) :
int - /git/builtin/remote.c
set_remote_branches(const char *, const char *
*, int) : int - /git/builtin/remote.c
set_branches(int, const char * *) : int
- /git/builtin/remote.c
cmd_remote(int, const char * *,
const char *) : int - /git/builtin/remote.c
add(int, const char * *) : int - /git/builtin/remote.c
cmd_remote(int, const char * *, const char *) :
int - /git/builtin/remote.c
cmd_branch(int, const char * *, const char *) : int -
/git/builtin/branch.c (2 matches)
git_config_set(const char *, const char *) : int - /git/config.c
add(int, const char * *) : int - /git/builtin/remote.c
(3 matches)
cmd_remote(int, const char * *, const char *) :
int - /git/builtin/remote.c
cmd_clone(int, const char * *, const char *) : int -
/git/builtin/clone.c (2 matches)
create_default_files(const char *) : int -
/git/builtin/init-db.c (8 matches)
init_db(const char *, unsigned int) : int -
/git/builtin/init-db.c
cmd_clone(int, const char * *, const
char *) : int -
[PATCH v2 4/4] do not die on error of parsing fetchrecursesubmodules option
We should not die when reading the submodule config cache since the user
might not be able to get out of that situation when the configuration is
part of the history.
We should handle this condition later when the value is about to be
used.
Signed-off-by: Heiko Voigt
---
builtin/fetch.c | 1 +
submodule-config.c | 29 -
submodule-config.h | 1 +
submodule.c | 15 ---
submodule.h | 2 +-
t/t7411-submodule-config.sh | 35 +++
6 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..706326f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -12,6 +12,7 @@
#include "parse-options.h"
#include "sigchain.h"
#include "transport.h"
+#include "submodule-config.h"
#include "submodule.h"
#include "connected.h"
#include "argv-array.h"
diff --git a/submodule-config.c b/submodule-config.c
index 4046233..96623ad 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -199,6 +199,30 @@ static struct submodule *lookup_or_create_by_name(struct
submodule_cache *cache,
return submodule;
}
+static int parse_fetch_recurse(const char *opt, const char *arg,
+ int die_on_error)
+{
+ switch (git_config_maybe_bool(opt, arg)) {
+ case 1:
+ return RECURSE_SUBMODULES_ON;
+ case 0:
+ return RECURSE_SUBMODULES_OFF;
+ default:
+ if (!strcmp(arg, "on-demand"))
+ return RECURSE_SUBMODULES_ON_DEMAND;
+
+ if (die_on_error)
+ die("bad %s argument: %s", opt, arg);
+ else
+ return RECURSE_SUBMODULES_ERROR;
+ }
+}
+
+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
+{
+ return parse_fetch_recurse(opt, arg, 1);
+}
+
static void warn_multiple_config(const unsigned char *commit_sha1,
const char *name, const char *option)
{
@@ -250,6 +274,8 @@ static int parse_config(const char *var, const char *value,
void *data)
submodule->path = strbuf_detach(&path, NULL);
cache_put_path(me->cache, submodule);
} else if (!strcmp(item.buf, "fetchrecursesubmodules")) {
+ /* when parsing worktree configurations we can die early */
+ int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
submodule->fetch_recurse != RECURSE_SUBMODULES_NONE) {
warn_multiple_config(me->commit_sha1, submodule->name,
@@ -257,7 +283,8 @@ static int parse_config(const char *var, const char *value,
void *data)
goto release_return;
}
- submodule->fetch_recurse =
parse_fetch_recurse_submodules_arg(var, value);
+ submodule->fetch_recurse = parse_fetch_recurse(var, value,
+ die_on_error);
} else if (!strcmp(item.buf, "ignore")) {
struct strbuf ignore = STRBUF_INIT;
if (!me->overwrite && submodule->ignore != NULL) {
diff --git a/submodule-config.h b/submodule-config.h
index 2083cb9..58afc83 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,7 @@ struct submodule {
unsigned char gitmodules_sha1[20];
};
+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
int parse_submodule_config_option(const char *var, const char *value);
const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name);
diff --git a/submodule.c b/submodule.c
index 188b4d2..75f502f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -288,21 +288,6 @@ static void print_submodule_summary(struct rev_info *rev,
FILE *f,
strbuf_release(&sb);
}
-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
-{
- switch (git_config_maybe_bool(opt, arg)) {
- case 1:
- return RECURSE_SUBMODULES_ON;
- case 0:
- return RECURSE_SUBMODULES_OFF;
- default:
- if (!strcmp(arg, "on-demand"))
- return RECURSE_SUBMODULES_ON_DEMAND;
- /* TODO: remove the die for history parsing here */
- die("bad %s argument: %s", opt, arg);
- }
-}
-
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
diff --git a/submodule.h b/submodule.h
index 547219d..5507c3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;
struct argv_array;
enum {
+ RECURSE_SUBMODULES_ERROR = -3,
RECURSE_SUBMODULES_NONE = -2,
RECURSE_SUBMODULES_ON_DEMAND = -1,
RECURSE_SUBMODULES_OFF = 0,
@@ -21,7 +22,6
[PATCH v2 3/4] use new config API for worktree configurations of submodules
We remove the extracted functions and directly parse into and read out
of the cache. This allows us to have one unified way of accessing
submodule configuration values specific to single submodules. Regardless
whether we need to access a configuration from history or from the
worktree.
Signed-off-by: Heiko Voigt
---
Documentation/technical/api-submodule-config.txt | 19 ++-
builtin/checkout.c | 1 +
diff.c | 1 +
submodule-config.c | 12 ++
submodule-config.h | 1 +
submodule.c | 160 ---
submodule.h | 1 -
t/t7411-submodule-config.sh | 37 +-
test-submodule-config.c | 10 ++
9 files changed, 104 insertions(+), 138 deletions(-)
diff --git a/Documentation/technical/api-submodule-config.txt
b/Documentation/technical/api-submodule-config.txt
index 2ff4907..2ea520a 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -10,10 +10,18 @@ submodule path or name.
Usage
-
+To initialize the cache with configurations from the worktree the caller
+typically first calls `gitmodules_config()` to read values from the
+worktree .gitmodules and then to overlay the local git config values
+`parse_submodule_config_option()` from the config parsing
+infrastructure.
+
The caller can look up information about submodules by using the
`submodule_from_path()` or `submodule_from_name()` functions. They return
a `struct submodule` which contains the values. The API automatically
-initializes and allocates the needed infrastructure on-demand.
+initializes and allocates the needed infrastructure on-demand. If the
+caller does only want to lookup values from revisions the initialization
+can be skipped.
If the internal cache might grow too big or when the caller is done with
the API, all internally cached values can be freed with submodule_free().
@@ -34,6 +42,11 @@ Functions
Use these to free the internally cached values.
+`int parse_submodule_config_option(const char *var, const char *value)`::
+
+ Can be passed to the config parsing infrastructure to parse
+ local (worktree) submodule configurations.
+
`const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path)`::
Lookup values for one submodule by its commit_sha1 and path or
@@ -43,4 +56,8 @@ Functions
The same as above but lookup by name.
+If given the null_sha1 as commit_sha1 the local configuration of a
+submodule will be returned (e.g. consolidated values from local git
+configuration and the .gitmodules file in the worktree).
+
For an example usage see test-submodule-config.c.
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..03ea20d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -18,6 +18,7 @@
#include "xdiff-interface.h"
#include "ll-merge.h"
#include "resolve-undo.h"
+#include "submodule-config.h"
#include "submodule.h"
#include "argv-array.h"
diff --git a/diff.c b/diff.c
index f72769a..f692a3c 100644
--- a/diff.c
+++ b/diff.c
@@ -13,6 +13,7 @@
#include "utf8.h"
#include "userdiff.h"
#include "sigchain.h"
+#include "submodule-config.h"
#include "submodule.h"
#include "ll-merge.h"
#include "string-list.h"
diff --git a/submodule-config.c b/submodule-config.c
index 57cc1da..4046233 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -375,6 +375,18 @@ static void ensure_cache_init(void)
is_cache_init = 1;
}
+int parse_submodule_config_option(const char *var, const char *value)
+{
+ struct parse_config_parameter parameter;
+ parameter.cache = &cache;
+ parameter.commit_sha1 = NULL;
+ parameter.gitmodules_sha1 = null_sha1;
+ parameter.overwrite = 1;
+
+ ensure_cache_init();
+ return parse_config(var, value, ¶meter);
+}
+
const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name)
{
diff --git a/submodule-config.h b/submodule-config.h
index 972496d..2083cb9 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,7 @@ struct submodule {
unsigned char gitmodules_sha1[20];
};
+int parse_submodule_config_option(const char *var, const char *value);
const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name);
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
diff --git a/submodule.c b/submodule.c
index 86ec2e3..188b4d2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "submodule-config.h"
#include "submodule.h"
#include "dir.h"
#include "diff.h"
@@ -12,9 +13,6 @@
#include "argv-array.h"
#include "blob.h"
-static struct string_lis
[PATCH v2 2/4] extract functions for submodule config set and lookup
This is one step towards using the new configuration API. We just
extract these functions to make replacing the actual code easier.
Signed-off-by: Heiko Voigt
---
submodule.c | 142 +---
1 file changed, 97 insertions(+), 45 deletions(-)
diff --git a/submodule.c b/submodule.c
index 85e2b12..86ec2e3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -41,6 +41,76 @@ static int gitmodules_is_unmerged;
*/
static int gitmodules_is_modified;
+static const char *get_name_for_path(const char *path)
+{
+ struct string_list_item *path_option;
+ if (path == NULL) {
+ if (config_name_for_path.nr > 0)
+ return config_name_for_path.items[0].util;
+ else
+ return NULL;
+ }
+ path_option = unsorted_string_list_lookup(&config_name_for_path, path);
+ if (!path_option)
+ return NULL;
+ return path_option->util;
+}
+
+static void set_name_for_path(const char *path, const char *name, int namelen)
+{
+ struct string_list_item *config;
+ config = unsorted_string_list_lookup(&config_name_for_path, path);
+ if (config)
+ free(config->util);
+ else
+ config = string_list_append(&config_name_for_path,
xstrdup(path));
+ config->util = xmemdupz(name, namelen);
+}
+
+static const char *get_ignore_for_name(const char *name)
+{
+ struct string_list_item *ignore_option;
+ ignore_option = unsorted_string_list_lookup(&config_ignore_for_name,
name);
+ if (!ignore_option)
+ return NULL;
+
+ return ignore_option->util;
+}
+
+static void set_ignore_for_name(const char *name, int namelen, const char
*ignore)
+{
+ struct string_list_item *config;
+ char *name_cstr = xmemdupz(name, namelen);
+ config = unsorted_string_list_lookup(&config_ignore_for_name,
name_cstr);
+ if (config) {
+ free(config->util);
+ free(name_cstr);
+ } else
+ config = string_list_append(&config_ignore_for_name, name_cstr);
+ config->util = xstrdup(ignore);
+}
+
+static int get_fetch_recurse_for_name(const char *name)
+{
+ struct string_list_item *fetch_recurse;
+ fetch_recurse =
unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);
+ if (!fetch_recurse)
+ return RECURSE_SUBMODULES_NONE;
+
+ return (intptr_t) fetch_recurse->util;
+}
+
+static void set_fetch_recurse_for_name(const char *name, int namelen, int
fetch_recurse)
+{
+ struct string_list_item *config;
+ char *name_cstr = xmemdupz(name, namelen);
+ config =
unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name,
name_cstr);
+ if (!config)
+ config =
string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);
+ else
+ free(name_cstr);
+ config->util = (void *)(intptr_t) fetch_recurse;
+}
int is_staging_gitmodules_ok(void)
{
@@ -55,7 +125,7 @@ int is_staging_gitmodules_ok(void)
int update_path_in_gitmodules(const char *oldpath, const char *newpath)
{
struct strbuf entry = STRBUF_INIT;
- struct string_list_item *path_option;
+ const char *path;
if (!file_exists(".gitmodules")) /* Do nothing without .gitmodules */
return -1;
@@ -63,13 +133,13 @@ int update_path_in_gitmodules(const char *oldpath, const
char *newpath)
if (gitmodules_is_unmerged)
die(_("Cannot change unmerged .gitmodules, resolve merge
conflicts first"));
- path_option = unsorted_string_list_lookup(&config_name_for_path,
oldpath);
- if (!path_option) {
+ path = get_name_for_path(oldpath);
+ if (!path) {
warning(_("Could not find section in .gitmodules where
path=%s"), oldpath);
return -1;
}
strbuf_addstr(&entry, "submodule.");
- strbuf_addstr(&entry, path_option->util);
+ strbuf_addstr(&entry, path);
strbuf_addstr(&entry, ".path");
if (git_config_set_in_file(".gitmodules", entry.buf, newpath) < 0) {
/* Maybe the user already did that, don't error out here */
@@ -89,7 +159,7 @@ int update_path_in_gitmodules(const char *oldpath, const
char *newpath)
int remove_path_from_gitmodules(const char *path)
{
struct strbuf sect = STRBUF_INIT;
- struct string_list_item *path_option;
+ const char *path_option;
if (!file_exists(".gitmodules")) /* Do nothing without .gitmodules */
return -1;
@@ -97,13 +167,13 @@ int remove_path_from_gitmodules(const char *path)
if (gitmodules_is_unmerged)
die(_("Cannot change unmerged .gitmodules, resolve merge
conflicts first"));
- path_option = unsorted_string_list_lookup(&config_name_for_path, path);
+ path_option = get_name_for_path(path);
[PATCH v2 1/4] implement submodule config cache for lookup of submodule names
This submodule configuration cache allows us to lazily read .gitmodules configurations by commit into a runtime cache which can then be used to easily lookup values from it. Currently only the values for path or name are stored but it can be extended for any value needed. It is expected that .gitmodules files do not change often between commits. Thats why we lookup the .gitmodules sha1 from a commit and then either lookup an already parsed configuration or parse and cache an unknown one for each sha1. The cache is lazily build on demand for each requested commit. This cache can be used for all purposes which need knowledge about submodule configurations. Example use cases are: * Recursive submodule checkout needs lookup a submodule name from its path when a submodule first appears. This needs be done before this configuration exists in the worktree. * The implementation of submodule support for 'git archive' needs to lookup the submodule name to generate the archive when given a revision that is not checked out. * 'git fetch' when given the --recurse-submodules=on-demand option (or configuration) needs to lookup submodule names by path from the database rather than reading from the worktree. For new submodule it needs to lookup the name from its path to allow cloning new submodules into the .git folder so they can be checked out without any network interaction when the user does a checkout of that revision. Signed-off-by: Heiko Voigt --- .gitignore | 1 + Documentation/technical/api-submodule-config.txt | 46 +++ Makefile | 2 + submodule-config.c | 396 +++ submodule-config.h | 27 ++ submodule.c | 1 + submodule.h | 1 + t/t7411-submodule-config.sh | 73 + test-submodule-config.c | 60 9 files changed, 607 insertions(+) create mode 100644 Documentation/technical/api-submodule-config.txt create mode 100644 submodule-config.c create mode 100644 submodule-config.h create mode 100755 t/t7411-submodule-config.sh create mode 100644 test-submodule-config.c diff --git a/.gitignore b/.gitignore index dc600f9..9e3352a 100644 --- a/.gitignore +++ b/.gitignore @@ -198,6 +198,7 @@ /test-sha1 /test-sigchain /test-string-list +/test-submodule-config /test-subprocess /test-svn-fe /test-urlmatch-normalization diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt new file mode 100644 index 000..2ff4907 --- /dev/null +++ b/Documentation/technical/api-submodule-config.txt @@ -0,0 +1,46 @@ +submodule config cache API +== + +The submodule config cache API allows to read submodule +configurations/information from specified revisions. Internally +information is lazily read into a cache that is used to avoid +unnecessary parsing of the same .gitmodule files. Lookups can be done by +submodule path or name. + +Usage +- + +The caller can look up information about submodules by using the +`submodule_from_path()` or `submodule_from_name()` functions. They return +a `struct submodule` which contains the values. The API automatically +initializes and allocates the needed infrastructure on-demand. + +If the internal cache might grow too big or when the caller is done with +the API, all internally cached values can be freed with submodule_free(). + +Data Structures +--- + +`struct submodule`:: + + This structure is used to return the information about one + submodule for a certain revision. It is returned by the lookup + functions. + +Functions +- + +`void submodule_free()`:: + + Use these to free the internally cached values. + +`const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path)`:: + + Lookup values for one submodule by its commit_sha1 and path or + name. + +`const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const char *name)`:: + + The same as above but lookup by name. + +For an example usage see test-submodule-config.c. diff --git a/Makefile b/Makefile index a53f3a8..5c0a652 100644 --- a/Makefile +++ b/Makefile @@ -571,6 +571,7 @@ TEST_PROGRAMS_NEED_X += test-scrap-cache-tree TEST_PROGRAMS_NEED_X += test-sha1 TEST_PROGRAMS_NEED_X += test-sigchain TEST_PROGRAMS_NEED_X += test-string-list +TEST_PROGRAMS_NEED_X += test-submodule-config TEST_PROGRAMS_NEED_X += test-subprocess TEST_PROGRAMS_NEED_X += test-svn-fe TEST_PROGRAMS_NEED_X += test-urlmatch-normalization @@ -879,6 +880,7 @@ LIB_OBJS += strbuf.o LIB_OBJS += streaming.o LIB_OBJS += string-list.o LIB_OBJS += submodule.o +LIB_OBJS += submodule-config.o LIB_OBJS += symlinks.o LIB_OBJS += tag.o LIB_OBJS
[PATCH v2 0/4] submodule config lookup API
Here another iteration with all the comments incorporated: * Dropped the hashmap enum parameter patch * Renamed the test to t7411 * Compilation fixes and style Gmane seems offline for me, so I can not check but this should be the last iteration: http://mid.gmane.org/20140605060425.GA23874@sandbox-ub Heiko Voigt (4): implement submodule config cache for lookup of submodule names extract functions for submodule config set and lookup use new config API for worktree configurations of submodules do not die on error of parsing fetchrecursesubmodules option .gitignore | 1 + Documentation/technical/api-submodule-config.txt | 63 Makefile | 2 + builtin/checkout.c | 1 + builtin/fetch.c | 1 + diff.c | 1 + submodule-config.c | 435 +++ submodule-config.h | 29 ++ submodule.c | 122 ++- submodule.h | 4 +- t/t7411-submodule-config.sh | 141 test-submodule-config.c | 70 12 files changed, 772 insertions(+), 98 deletions(-) create mode 100644 Documentation/technical/api-submodule-config.txt create mode 100644 submodule-config.c create mode 100644 submodule-config.h create mode 100755 t/t7411-submodule-config.sh create mode 100644 test-submodule-config.c -- 1.9.2.464.g1bbf329 -- 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: Trouble merging renamed but identical files - CONFLICT (rename/rename)
> -Original Message- > From: Jason Pyeron > Sent: Friday, June 27, 2014 20:42 > To: 'git' > Cc: 'Phil Hord' > Subject: RE: Trouble merging renamed but identical files - > CONFLICT (rename/rename) > > > -Original Message- > > From: Jason Pyeron > > Sent: Friday, June 27, 2014 18:39 > > > > > -Original Message- > > > From: Phil Hord [mailto:phil.h...@gmail.com] > > > Sent: Friday, June 27, 2014 17:46 > > > To: Jason Pyeron > > > Cc: git > > > Subject: Re: Trouble merging renamed but identical files - > > > CONFLICT (rename/rename) > > > > > > On Fri, Jun 27, 2014 at 4:47 PM, Jason Pyeron > > > wrote: > > > > There are two identical files from the same original > > > parent, but both were > > > > renamed in their own branches. One branch moved the file to > > > a new folder, the > > > > other renamed the file in the same folder. > > > > > > You have not stated what you think the issue is. You have > > only stated > > > the setup. > > > > Thanks, I could have said it better. > > > > I think that git should understand that I have moved a file > > in path only (the tree object containing the file's entry > > change, but not the entry it self) and that the branch from > > which I want to merge back (with common ancestry) has renamed > > the file in the same path ( the tree object is unchanged, but > > the entry is) such that the object is re-parented and renamed > > in that path. > > > > How can this be done in git or if it cannot what are the > > chalenges to patching git for this issue. > > > > git cat-file -p b60070f4d0879e277f44d174a163bbb292325fea # > > tree d8df83fc6714aab1fc1df061fcb03410e1dab1e5 > > git cat-file -p d8df83fc6714aab1fc1df061fcb03410e1dab1e5 # > > 04 tree 68bb8a223284e0f5057421217a5965128bf1d51asrc > > git cat-file -p 68bb8a223284e0f5057421217a5965128bf1d51a # > > 100644 blob 25c7d3b12bced67046359ba1e7945f82a2640147 > TrueCrypt.sln > > > > git cat-file -p a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68 # > > tree 7f82a6c46f19931c3c40d44dc196cbfab7feaa72 > > git cat-file -p 7f82a6c46f19931c3c40d44dc196cbfab7feaa72 # > > 100644 blob 25c7d3b12bced67046359ba1e7945f82a2640147 > CipherShed.sln > > > > > > > > > > > I suppose you want Git to merge without conflict in the > end, though, > > > based on your script. Is that right? > > > > > > > > > > Steps to reproduce the issue: > > > > git init > > > > git fetch https://github.com/pdinc-oss/CipherShed.git > > > > git fetch https://github.com/srguglielmo/CipherShed.git > > > > git checkout -b test b60070f4d0879e277f44d174a163bbb292325fea > > > > git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68 > > > > > > > > CONFLICT (rename/rename): Rename > > > "TrueCrypt.sln"->"src/TrueCrypt.sln" in branch > > > > "HEAD" rename "TrueCrypt.sln"->"CipherShed.sln" in > > > > "a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68" > > > > > > Git seems to be doing the correct thing here. > > > > > > > > > > git reset --hard b60070f4d0879e277f44d174a163bbb292325fea > > > > git mv src/TrueCrypt.sln src/CipherShed.sln > > > > git commit -m 'renamed to be congruent with a0c84ff' > > > > git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68 > > > > > > > > Sill get a CONFLICT (rename/rename): Rename > > > > "TrueCrypt.sln"->"src/CipherShed.sln" in branch "HEAD" rename > > > > "TrueCrypt.sln"->"CipherShed.sln" in > > > "a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68" > > > > > > Git seems to be doing the correct thing here, too. > > > > > > > I will have many more to come, any suggestions? > > > > > > Maybe you meant to move the renamed file to the same > folder where it > > > exists in the merge target. I do not get a conflict when > I do that. > > > > Are you saying I should git mv src/TrueCrypt.sln CipherShed.sln ? > > > > Then it will be in the wrong path as intended. > > > > > > > >git reset --hard b60070f4d0879e277f44d174a163bbb292325fea > > >git mv src/TrueCrypt.sln CipherShed.sln > > >git commit -m 'renamed to be congruent with a0c84ff' > > >git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68 > > > > > > No conflict (on that file, anyway). > > > > Agreed, but not the desired end state. > > Sorry for the http://pastebin.com/1R68v6jt (changes the merge to > 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually > reconciles the merge), > but it was too long to be readable in the email. > > git blame HEAD -- src/Main/Forms/CipherShed.fbp | cut -c 1-8 > | sort -u > > Gives: > ac812aa3 > b50a2fb1 > > git blame b60070f4d0879e277f44d174a163bbb292325fea -- > src/Main/Forms/TrueCrypt.fbp | cut -c 1-8 | sort -u > > Gives: > 07b2176f > 0eb8b4fa > 12c94add > a17c95a3 > a757b4d4 > cac6cd14 > d0a9dfa8 > d94128a9 > e6b1437a > f1bb489c > > If I use cherry pick (vs merge), I can maintain the big > history in b60070f, but > loose the small history in 1ca13ed > > [test] > / \ > / \ > [b60070f] [1ca13ed] > | | > | | > [65efd37] | >
