Re: problem with def of inet_ntop() in git-compat-util.h as well as other places
On August 27, 2014 at 6:28 PM Jonathan Nieder jrnie...@gmail.com wrote: Hi again, dev wrote: So I guess I have to create a config.mak file from somewhere. Sorry, let's take a step back. Actually I think we have some real progress to report here. After scanning through the various magic incantations in the Makefile and some trial and error I arrived at this gem : $ date Thu Aug 28 06:10:43 GMT 2014 $ ls $SRC/git* /usr/local/src/git-2.0.4.tar.gz $ gzip -dc /usr/local/src/git-2.0.4.tar.gz | tar -xf - $ mv git-2.0.4 git-2.0.4_SunOS5.10_sparcv9.005 $ cd git-2.0.4_SunOS5.10_sparcv9.005 $ gmake CFLAGS=$CFLAGS LDFLAGS=$LD_OPTIONS NEEDS_LIBICONV=Yes \ SHELL_PATH=/usr/local/bin/bash \ SANE_TOOL_PATH=/usr/local/bin \ USE_LIBPCRE=1 LIBPCREDIR=/usr/local CURLDIR=/usr/local \ EXPATDIR=/usr/local NEEDS_LIBINTL_BEFORE_LIBICONV=1 \ NEEDS_SOCKET=1 NEEDS_RESOLV=1 USE_NSEC=1 \ PERL_PATH=/usr/local/bin/perl \ NO_PYTHON=1 DEFAULT_PAGER=/usr/xpg4/bin/more \ DEFAULT_EDITOR=/usr/local/bin/vim DEFAULT_HELP_FORMAT=man \ prefix=/usr/local GIT_VERSION = 2.0.4 * new build flags CC credential-store.o * new link flags CC abspath.o CC advice.o CC alias.o . . . GEN bin-wrappers/test-wildmatch GEN git-remote-testgit $ A full build. Furthermore it looks like all the right bits are linked in and a test clone from a few open source projects works well. $ file git git: ELF 64-bit MSB executable, SPARC V9, total store ordering, version 1, dynamically linked (uses shared libs), not stripped $ ldd git libpcre.so.1 = /usr/local/lib/libpcre.so.1 libz.so.1 = /usr/local/lib/libz.so.1 libintl.so.8 = /usr/local/lib/libintl.so.8 libiconv.so.2 = /usr/local/lib/libiconv.so.2 libsocket.so.1 =/lib/64/libsocket.so.1 libnsl.so.1 = /lib/64/libnsl.so.1 libresolv.so.2 =/lib/64/libresolv.so.2 libcrypto.so.1.0.0 =/usr/local/ssl/lib/libcrypto.so.1.0.0 libpthread.so.1 = /lib/64/libpthread.so.1 libc.so.1 = /lib/64/libc.so.1 libmp.so.2 =/lib/64/libmp.so.2 libmd.so.1 =/lib/64/libmd.so.1 libscf.so.1 = /lib/64/libscf.so.1 libdl.so.1 =/lib/64/libdl.so.1 libz.so.1 (SUNW_1.1) = (version not found) libdoor.so.1 = /lib/64/libdoor.so.1 libuutil.so.1 = /lib/64/libuutil.so.1 libgen.so.1 = /lib/64/libgen.so.1 libm.so.2 = /lib/64/libm.so.2 /platform/SUNW,T5240/lib/sparcv9/libc_psr.so.1 /platform/SUNW,T5240/lib/sparcv9/libmd_psr.so.1 $ Ignore the misleading libz issue. I really don't think it is a problem however I may dig into it. That is libz.so.1 which is needed by libcrypto.so.1.0.0 from OpenSSL 1.0.1i and I just don't see an issue given that OpenSSL 1.0.1i passes all its tests. I did run a few clone tests : $ pwd /export/home/dev/git_test $ $BUILD/git-2.0.4_SunOS5.10_sparcv9.005/git clone --verbose git://cmake.org/cmake.git Cloning into 'cmake'... warning: templates not found /usr/local/share/git-core/templates remote: Counting objects: 162733, done. remote: Compressing objects: 100% (41726/41726), done. remote: Total 162733 (delta 124662), reused 157579 (delta 119831) Receiving objects: 100% (162733/162733), 37.31 MiB | 1001.00 KiB/s, done. Resolving deltas: 100% (124662/124662), done. Checking connectivity... done. Checking out files: 100% (7410/7410), done. $ $ cd cmake/ $ $BUILD/git-2.0.4_SunOS5.10_sparcv9.005/git status On branch master Your branch is up-to-date with 'origin/master'. nothing to commit, working directory clean $ cd .. $ $BUILD/git-2.0.4_SunOS5.10_sparcv9.005/git clone --verbose git://git.apache.org/httpd.git Cloning into 'httpd'... warning: templates not found /usr/local/share/git-core/templates remote: Counting objects: 391450, done. remote: Compressing objects: 100% (80188/80188), done. remote: Total 391450 (delta 331921), reused 367848 (delta 309308) Receiving objects: 100% (391450/391450), 111.46 MiB | 420.00 KiB/s, done. Resolving deltas: 100% (331921/331921), done. Checking connectivity... done. Checking out files: 100% (3495/3495), done. So that looks pretty good thus far. I must say thank you for the guidance. All I need to do now is figure out a way to run a test over SSH with a dummy repo of some sort. dev -- 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] merge, pull: stop advising 'commit -a' in case of conflict
'git commit -a' is rarely a good way to mark conflicts as resolved: the user anyway has to go manually through the list of conflicts to do the actual resolution, and it is usually better to use git add on each files after doing the resolution. On the other hand, using 'git commit -a' is potentially dangerous, as it makes it very easy to mistakenly commit conflict markers without noticing. While we're there, synchronize the 'git pull' and 'git merge' messages: the first was ending with '... and make a commit.', but not the later. Signed-off-by: Matthieu Moy matthieu@imag.fr --- - Hasty-and-careless new users will be incorrectly enticed to type the command given by or use 'git commit -a' at the end of this advice message without thinking. Perhaps it is safer to stop the sentence at ... and make a commit. and drop that last bit while there are conflicts still in the working tree files. We should use the current end-of-message only when all the conflicts have been resolved in the working tree. It was already on my todo-list, as a friend of mine semi-beginner with Git complained about the mis-advice the other day, and I had to agree. Eventually, git could detect that conflicts have been resolved, but then that would be a different message, as not only use git commit -a could be resurected, but Fix them up in the work tree should be dropped when it is the case. I am not doing this myself soon, though. Hint, hint... I guess I'm just taking the low hanging fruit here ;-). advice.c| 3 +-- git-pull.sh | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/advice.c b/advice.c index 9b42033..3b8bf3c 100644 --- a/advice.c +++ b/advice.c @@ -86,8 +86,7 @@ int error_resolve_conflict(const char *me) * other commands doing a merge do. */ advise(_(Fix them up in the work tree, and then use 'git add/rm file'\n -as appropriate to mark resolution and make a commit, or use\n -'git commit -a'.)); +as appropriate to mark resolution and make a commit.)); return -1; } diff --git a/git-pull.sh b/git-pull.sh index 18a394f..4d4fc77 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -20,7 +20,7 @@ die_conflict () { if [ $(git config --bool --get advice.resolveConflict || echo true) = true ]; then die $(gettext Pull is not possible because you have unmerged files. Please, fix them up in the work tree, and then use 'git add/rm file' -as appropriate to mark resolution, or use 'git commit -a'.) +as appropriate to mark resolution and make a commit.) else die $(gettext Pull is not possible because you have unmerged files.) fi -- 2.0.2.737.gfb43bde -- 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 v3] teach fast-export an --anonymize option
On Thu, Aug 28, 2014 at 12:01 AM, Jeff King p...@peff.net wrote: You can get an overview of what will be shared by running a command like: git fast-export --anonymize --all | perl -pe 's/\d+/X/g' | sort -u | less which will show every unique line we generate, modulo any numbers (each anonymized token is assigned a number, like User 0, and we replace it consistently in the output). I feel like this should be part of git-fast-export.txt, just to increase the user's confidence in the tool (and I don't expect most users to read this commit message). -- 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 2/9] autoconf: Check for timer_t
It would appear that Darwin does not have timer_t, at least from looking at the public libc and XNU headers online. A quick additional change is needed in config.mak.uname: ifeq ($(uname_S),Darwin) ... HAVE_DEV_TTY = YesPlease + NO_TIMER_T = UnfortunatelyYes COMPAT_OBJS += compat/precompose_utf8.o ... endif I'll include that proper in V2 of my patch series. Jonas -- 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] improving advice message from git commit during a merge
On 27.08.2014 20:23, Junio C Hamano wrote: I am not doing this myself soon, though. Hint, hint... I asked once upon a time, if there was a place, which collects such topics for casual contributors and new comers. Would you mind to update the leftover bits at http://git-blame.blogspot.de/search?q=leftoverby-date=true ? 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
Re: [PATCH v3] teach fast-export an --anonymize option
On Thu, Aug 28, 2014 at 05:30:44PM +0700, Duy Nguyen wrote: On Thu, Aug 28, 2014 at 12:01 AM, Jeff King p...@peff.net wrote: You can get an overview of what will be shared by running a command like: git fast-export --anonymize --all | perl -pe 's/\d+/X/g' | sort -u | less which will show every unique line we generate, modulo any numbers (each anonymized token is assigned a number, like User 0, and we replace it consistently in the output). I feel like this should be part of git-fast-export.txt, just to increase the user's confidence in the tool (and I don't expect most users to read this commit message). Hmph. Whenever I say I think this patch is done, suddenly the comments start pouring in. :) I think you are right, though, and we could stand to explain the feature a little more in the documentation in general. How about this patch on top (or squashed in): -- 8 -- Subject: docs/fast-export: explain --anonymize more completely The original commit made mention of this option, but not why one might want it or how they might use it. Let's try to be a little more thorough, and also explain how to confirm that the output really is anonymous. Signed-off-by: Jeff King p...@peff.net --- Documentation/git-fast-export.txt | 63 --- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt index 52831fa..dbe9a46 100644 --- a/Documentation/git-fast-export.txt +++ b/Documentation/git-fast-export.txt @@ -106,10 +106,9 @@ marks the same across runs. different from the commit's first parent). --anonymize:: - Replace all refnames, paths, blob contents, commit and tag - messages, names, and email addresses in the output with - anonymized data, while still retaining the shape of history and - of the stored tree. + Anonymize the contents of the repository while still retaining + the shape of the history and stored tree. See the section on + `ANONYMIZING` below. --refspec:: Apply the specified refspec to each ref exported. Multiple of them can @@ -147,6 +146,62 @@ referenced by that revision range contains the string 'refs/heads/master'. +ANONYMIZING +--- + +If the `--anonymize` option is given, git will attempt to remove all +identifying information from the repository while still retaining enough +of the original tree and history patterns to reproduce some bugs. The +goal is that a git bug which is found on a private repository will +persist in the anonymized repository, and the latter can be shared with +git developers to help solve the bug. + +With this option, git will replace all refnames, paths, blob contents, +commit and tag messages, names, and email addresses in the output with +anonymized data. Two instances of the same string will be replaced +equivalently (e.g., two commits with the same author will have the same +anonymized author in the output, but bear no resemblance to the original +author string). The relationship between commits, branches, and tags is +retained, as well as the commit timestamps (but the commit messages and +refnames bear no resemblance to the originals). The relative makeup of +the tree is retained (e.g., if you have a root tree with 10 files and 3 +trees, so will the output), but their names and the contents of the +files will be replaced. + +If you think you have found a git bug, you can start by exporting an +anonymized stream of the whole repository: + +--- +$ git fast-export --anonymize --all anon-stream +--- + +Then confirm that the bug persists in a repository created from that +stream (many bugs will not, as they really do depend on the exact +repository contents): + +--- +$ git init anon-repo +$ cd anon-repo +$ git fast-import ../anon-stream +$ ... test your bug ... +--- + +If the anonymized repository shows the bug, it may be worth sharing +`anon-stream` along with a regular bug report. Note that the anonymized +stream compresses very well, so gzipping it is encouraged. If you want +to examine the stream to see that it does not contain any private data, +you can peruse it directly before sending. You may also want to try: + +--- +$ perl -pe 's/\d+/X/g' anon-stream | sort -u | less +--- + +which shows all of the unique lines (with numbers converted to X, to +collapse User 0, User 1, etc into User X). This produces a much +smaller output, and it is usually easy to quickly confirm that there is +no private data in the stream. + + Limitations --- -- 2.1.0.346.ga0367b9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to
Re: Git Bug Report: bisect run failed to locate the right commit, detail testing
Hi, On Thu, Aug 28, 2014 at 3:49 AM, 李祐棠 r01942...@ntu.edu.tw wrote: Dear git developers: Allow me to describe the testing situation more detail: First the testing repository is in https://github.com/gawel/pyquery my git version is 1.9.2 running on Archlinux 3.14.2 I try to track issue #74(which is closed now) It give result False/True in version 1.2.8(good), and result False/False in version 1.2.9(bad). The testing script are manualscript.py and autoscript.py Both of them implement the test case describe in issue #74. Th only difference is that autoscript.py call sys.exit() to return the testing value. First we test with git-bisect manually with manualscript.py: The testing result is shown in 'bisectmanual_manualscript' Then we test with git-bisect manually with autoscript.py This time we echo $? every time we execute autoscript.py, and the testing result is shown in 'bisectmanual_autoscript' In both situation the script give the same result, and the return value of autoscript.py is correct, too. However, if we use git-bisect-run with autoscript.py, it will show a different result. The testing result is shown in 'bisectrun'. The log shows that autoscript.py output False/False all the way. As Mr. Couder said, there is some checkout commit that autoscript.py and manualscript.py give different result, for example commit d22159bb32510e9eacf6c5c2408a79792e99fe76. If I checkout this commit outside bisect state and run manualscript.py and autoscript.py, they both give False/True result. So, I guess there is some problem in the checkout procedure in bisect-run, so the commit didn't successfully checkout. I don't think so. I tried to run the following command many times in a row: for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done And here is the log: $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Note: checking out 'bc1b16509cec70de7a32354026443fca777f4d7d'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) HEAD is now at b81a9e8... fixed issue #9 False False Previous HEAD position was b81a9e8... fixed issue #9 HEAD is now at d22159b... test pseudo classes; documentation effort False True $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Previous HEAD position was d22159b... test pseudo classes; documentation effort HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) HEAD is now at b81a9e8... fixed issue #9 False False Previous HEAD position was b81a9e8... fixed issue #9 HEAD is now at d22159b... test pseudo classes; documentation effort False True $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Previous HEAD position was d22159b... test pseudo classes; documentation effort HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) HEAD is now at b81a9e8... fixed issue #9 False False Previous HEAD position was b81a9e8... fixed issue #9 HEAD is now at d22159b... test pseudo classes; documentation effort False False $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Previous HEAD position was d22159b... test pseudo classes; documentation effort HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was
make install fails because GNU tar needed
Well I am making progress in that I have what looks like a successful build. what fails next on the non-linux world is the next requirement for GNU tar for some reason : # gmake CFLAGS=$CFLAGS LDFLAGS=$LD_OPTIONS NEEDS_LIBICONV=Yes \ SHELL_PATH=/usr/local/bin/bash \ SANE_TOOL_PATH=/usr/local/bin \ USE_LIBPCRE=1 LIBPCREDIR=/usr/local CURLDIR=/usr/local \ EXPATDIR=/usr/local NEEDS_LIBINTL_BEFORE_LIBICONV=1 \ NEEDS_SOCKET=1 NEEDS_RESOLV=1 USE_NSEC=1 \ PERL_PATH=/usr/local/bin/perl \ NO_PYTHON=1 DEFAULT_PAGER=/usr/xpg4/bin/more \ DEFAULT_EDITOR=/usr/local/bin/vim DEFAULT_HELP_FORMAT=man \ prefix=/usr/local install ../git-2.0.4_SunOS5.10_sparcv9.006.install.log * new build flags read-cache.c, line 780: warning: statement not reached xdiff/xutils.c, line 180: warning: statement not reached Writing perl.mak for Git Writing MYMETA.yml and MYMETA.json /bin/sh: gtar: /bin/shnot found : gtar: not found gmake[1]: *** [install] Error 1 gmake: *** [install] Error 2 Well that is maddening. Is there some magic somewhere to use ordinary POSIX tar ? Also, what is shnot ? dev -- 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
[no subject]
Congratulations!!!Your email address has won $500,000 in Coca-Cola/Fifa Promotion. Ticket No:7PW1124. Contact us on e-Mail:coke.f...@outlook.com for your claim -- 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 6/6] Make sure that index-pack --strict fails upon invalid tag objects
One of the most important use cases for the strict tag object checking is when transfer.fsckobjects is set to true to catch invalid objects early on. This new regression test essentially tests the same code path by directly calling 'index-pack --strict' on a pack containing an invalid tag object. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- t/t5302-pack-index.sh | 19 +++ 1 file changed, 19 insertions(+) diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh index 4bbb718..80bd3ee 100755 --- a/t/t5302-pack-index.sh +++ b/t/t5302-pack-index.sh @@ -243,4 +243,23 @@ test_expect_success 'running index-pack in the object store' ' test -f .git/objects/pack/pack-${pack1}.idx ' +test_expect_success 'index-pack --strict fails upon invalid tag' ' +sha=$(git rev-parse HEAD) +cat wrong-tag EOF +object $sha +type commit +tag guten tag + +This is an invalid tag. +EOF + +tag=$(git hash-object -t tag -w --stdin wrong-tag) +pack1=$(echo $tag | git pack-objects tag-test) +echo remove tag object +thirtyeight=${tag#??} +rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight +test_must_fail git index-pack --strict tag-test-${pack1}.pack 2 err +grep invalid .tag. name err +' + test_done -- 2.0.0.rc3.9669.g840d1f9 -- 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 5/6] Add regression tests for stricter tag fsck'ing
The intent of the two new test cases is to catch general breakages in the fsck_tag() function, not so much to test it extensively, trying to strike the proper balance between thoroughness and speed. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- t/t1450-fsck.sh | 39 +++ 1 file changed, 39 insertions(+) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 8c739c9..16b3e4a 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -194,6 +194,45 @@ test_expect_success 'tag pointing to something else than its type' ' test_must_fail git fsck --tags ' +test_expect_success 'tag with incorrect tag name' ' + sha=$(git rev-parse HEAD) + cat wrong-tag -EOF + object $sha + type commit + tag wrong name format + tagger T A Gger tag...@example.com 1234567890 - + + This is an invalid tag. + EOF + + tag=$(git hash-object -t tag -w --stdin wrong-tag) + test_when_finished remove_object $tag + echo $tag .git/refs/tags/wrong + test_when_finished git update-ref -d refs/tags/wrong + git fsck --tags 2out + cat out + grep invalid .tag. name out +' + +test_expect_success 'tag with missing tagger' ' + sha=$(git rev-parse HEAD) + cat wrong-tag -EOF + object $sha + type commit + tag gutentag + + This is an invalid tag. + EOF + + tag=$(git hash-object -t tag -w --stdin wrong-tag) + test_when_finished remove_object $tag + echo $tag .git/refs/tags/wrong + test_when_finished git update-ref -d refs/tags/wrong + git fsck --tags 2out + cat out + grep expected .tagger. line out +' + test_expect_success 'cleaned up' ' git fsck actual 21 test_cmp empty actual -- 2.0.0.rc3.9669.g840d1f9 -- 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/6] Refactor type_from_string() to avoid die()ing in case of errors
In the next commits, we will enhance the fsck_tag() function to check tag objects more thoroughly. To this end, we need a function to verify that a given string is a valid object type, but that does not die() in the negative case. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- object.c | 13 - object.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/object.c b/object.c index a16b9f9..5eee592 100644 --- a/object.c +++ b/object.c @@ -33,13 +33,24 @@ const char *typename(unsigned int type) return object_type_strings[type]; } -int type_from_string(const char *str) +int type_from_string_gently(const char *str) { int i; for (i = 1; i ARRAY_SIZE(object_type_strings); i++) if (!strcmp(str, object_type_strings[i])) return i; + + return -1; +} + +int type_from_string(const char *str) +{ + int i = type_from_string_gently(str); + + if (i = 0) + return i; + die(invalid object type \%s\, str); } diff --git a/object.h b/object.h index 5e8d8ee..5c5d22f 100644 --- a/object.h +++ b/object.h @@ -54,6 +54,7 @@ struct object { extern const char *typename(unsigned int type); extern int type_from_string(const char *str); +extern int type_from_string_gently(const char *str); /* * Return the current number of buckets in the object hashmap. -- 2.0.0.rc3.9669.g840d1f9 -- 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/6] Accept object data in the fsck_object() function
When fsck'ing an incoming pack, we need to fsck objects that cannot be read via read_sha1_file() because they are not local yet (and might even be rejected if transfer.fsckobjects is set to 'true'). For commits, there is a hack in place: we basically cache commit objects' buffers anyway, but the same is not true, say, for tag objects. By refactoring fsck_object() to take the object buffer and size as optional arguments -- optional, because we still fall back to the previous method to look at the cached commit objects if the caller passes NULL -- we prepare the machinery for the upcoming handling of tag objects. The assumption that such buffers are inherently NUL terminated is now wrong, of course, hence we pass the size of the buffer so that we can add a sanity check later, to prevent running past the end of the buffer. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- builtin/fsck.c | 2 +- builtin/index-pack.c | 3 ++- builtin/unpack-objects.c | 14 ++ fsck.c | 24 +++- fsck.h | 4 +++- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index d42a27d..d9f4e6e 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -298,7 +298,7 @@ static int fsck_obj(struct object *obj) if (fsck_walk(obj, mark_used, NULL)) objerror(obj, broken links); - if (fsck_object(obj, check_strict, fsck_error_func)) + if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func)) return -1; if (obj-type == OBJ_TREE) { diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 5568a5b..f2465ff 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -773,7 +773,8 @@ static void sha1_object(const void *data, struct object_entry *obj_entry, if (!obj) die(_(invalid %s), typename(type)); if (do_fsck_object - fsck_object(obj, 1, fsck_error_function)) + fsck_object(obj, buf, size, 1, + fsck_error_function)) die(_(Error in object)); if (fsck_walk(obj, mark_link, NULL)) die(_(Not all child objects of %s are reachable), sha1_to_hex(obj-sha1)); diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 99cde45..855d94b 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -164,10 +164,10 @@ static unsigned nr_objects; * Called only from check_object() after it verified this object * is Ok. */ -static void write_cached_object(struct object *obj) +static void write_cached_object(struct object *obj, struct obj_buffer *obj_buf) { unsigned char sha1[20]; - struct obj_buffer *obj_buf = lookup_object_buffer(obj); + if (write_sha1_file(obj_buf-buffer, obj_buf-size, typename(obj-type), sha1) 0) die(failed to write object %s, sha1_to_hex(obj-sha1)); obj-flags |= FLAG_WRITTEN; @@ -180,6 +180,8 @@ static void write_cached_object(struct object *obj) */ static int check_object(struct object *obj, int type, void *data) { + struct obj_buffer *obj_buf; + if (!obj) return 1; @@ -198,11 +200,15 @@ static int check_object(struct object *obj, int type, void *data) return 0; } - if (fsck_object(obj, 1, fsck_error_function)) + obj_buf = lookup_object_buffer(obj); + if (!obj_buf) + die(Whoops! Cannot find object '%s', sha1_to_hex(obj-sha1)); + if (fsck_object(obj, obj_buf-buffer, obj_buf-size, 1, + fsck_error_function)) die(Error in object); if (fsck_walk(obj, check_object, NULL)) die(Error on reachable objects of %s, sha1_to_hex(obj-sha1)); - write_cached_object(obj); + write_cached_object(obj, obj_buf); return 0; } diff --git a/fsck.c b/fsck.c index 56156ff..dd77628 100644 --- a/fsck.c +++ b/fsck.c @@ -277,7 +277,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f } static int fsck_commit_buffer(struct commit *commit, const char *buffer, - fsck_error error_func) + unsigned long size, fsck_error error_func) { unsigned char tree_sha1[20], sha1[20]; struct commit_graft *graft; @@ -322,15 +322,18 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer, return 0; } -static int fsck_commit(struct commit *commit, fsck_error error_func) +static int fsck_commit(struct commit *commit, const char *data, + unsigned long size, fsck_error error_func) { - const char *buffer = get_commit_buffer(commit, NULL); - int ret = fsck_commit_buffer(commit, buffer, error_func); - unuse_commit_buffer(commit,
[PATCH 3/6] Make sure fsck_commit_buffer() does not run out of the buffer
So far, we assumed that the buffer is NUL terminated, but this is not a safe assumption, now that we opened the fsck_object() API to pass a buffer directly. So let's make sure that there is at least an empty line in the buffer. That way, our checks would fail if the empty line was encountered prematurely, and consequently we can get away with the current string comparisons even with non-NUL-terminated buffers are passed to fsck_object(). Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- fsck.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/fsck.c b/fsck.c index dd77628..db6aaa4 100644 --- a/fsck.c +++ b/fsck.c @@ -237,6 +237,26 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func) return retval; } +static int must_have_empty_line(const void *data, unsigned long size, + struct object *obj, fsck_error error_func) +{ + const char *buffer = (const char *)data; + int i; + + for (i = 0; i size; i++) { + switch (buffer[i]) { + case '\0': + return error_func(obj, FSCK_ERROR, + invalid message: NUL at offset %d, i); + case '\n': + if (i + 1 size buffer[i + 1] == '\n') + return 0; + } + } + + return error_func(obj, FSCK_ERROR, invalid buffer: missing empty line); +} + static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func) { char *end; @@ -284,6 +304,9 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer, unsigned parent_count, parent_line_count = 0; int err; + if (must_have_empty_line(buffer, size, commit-object, error_func)) + return -1; + if (!skip_prefix(buffer, tree , buffer)) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'tree' line); if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n') -- 2.0.0.rc3.9669.g840d1f9 -- 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 4/6] fsck: check tag objects' headers
We inspect commit objects pretty much in detail in git-fsck, but we just glanced over the tag objects. Let's be stricter. This work was sponsored by GitHub Inc. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- fsck.c | 88 +- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/fsck.c b/fsck.c index db6aaa4..d30946b 100644 --- a/fsck.c +++ b/fsck.c @@ -6,6 +6,7 @@ #include commit.h #include tag.h #include fsck.h +#include refs.h static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data) { @@ -355,6 +356,90 @@ static int fsck_commit(struct commit *commit, const char *data, return ret; } +static int fsck_tag_buffer(struct tag *tag, const char *data, + unsigned long size, fsck_error error_func) +{ + unsigned char commit_sha1[20]; + int ret = 0; + const char *buffer; + char *tmp = NULL, *eol; + + if (data) + buffer = data; + else { + enum object_type type; + + buffer = tmp = read_sha1_file(tag-object.sha1, type, size); + if (!buffer) + return error_func(tag-object, FSCK_ERROR, + cannot read tag object); + + if (type != OBJ_TAG) { + ret = error_func(tag-object, FSCK_ERROR, + expected tag got %s, + typename(type)); + goto done; + } + } + + if (must_have_empty_line(buffer, size, tag-object, error_func)) + goto done; + + if (!skip_prefix(buffer, object , buffer)) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - expected 'object' line); + goto done; + } + if (get_sha1_hex(buffer, commit_sha1) || buffer[40] != '\n') { + ret = error_func(tag-object, FSCK_ERROR, invalid 'object' line format - bad sha1); + goto done; + } + buffer += 41; + + if (!skip_prefix(buffer, type , buffer)) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - expected 'type' line); + goto done; + } + eol = strchr(buffer, '\n'); + if (!eol) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - unexpected end after 'type' line); + goto done; + } + *eol = '\0'; + if (type_from_string_gently(buffer) 0) + ret = error_func(tag-object, FSCK_ERROR, invalid 'type' value); + *eol = '\n'; + if (ret) + goto done; + buffer = eol + 1; + + if (!skip_prefix(buffer, tag , buffer)) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - expected 'tag' line); + goto done; + } + eol = strchr(buffer, '\n'); + if (!eol) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - unexpected end after 'type' line); + goto done; + } + *eol = '\0'; + if (check_refname_format(buffer, REFNAME_ALLOW_ONELEVEL)) + ret = error_func(tag-object, FSCK_ERROR, invalid 'tag' name: %s, buffer); + *eol = '\n'; + if (ret) + goto done; + buffer = eol + 1; + + if (!skip_prefix(buffer, tagger , buffer)) { + /* early tags do not contain 'tagger' lines; warn only */ + error_func(tag-object, FSCK_WARN, invalid format - expected 'tagger' line); + } + ret = fsck_ident(buffer, tag-object, error_func); + +done: + free(tmp); + return ret; +} + static int fsck_tag(struct tag *tag, const char *data, unsigned long size, fsck_error error_func) { @@ -362,7 +447,8 @@ static int fsck_tag(struct tag *tag, const char *data, if (!tagged) return error_func(tag-object, FSCK_ERROR, could not load tagged object); - return 0; + + return fsck_tag_buffer(tag, data, size, error_func); } int fsck_object(struct object *obj, void *data, unsigned long size, -- 2.0.0.rc3.9669.g840d1f9 -- 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: make install fails because GNU tar needed
On Thu, Aug 28, 2014 at 10:16:48AM -0400, dev wrote: # gmake CFLAGS=$CFLAGS LDFLAGS=$LD_OPTIONS NEEDS_LIBICONV=Yes \ SHELL_PATH=/usr/local/bin/bash \ SANE_TOOL_PATH=/usr/local/bin \ USE_LIBPCRE=1 LIBPCREDIR=/usr/local CURLDIR=/usr/local \ EXPATDIR=/usr/local NEEDS_LIBINTL_BEFORE_LIBICONV=1 \ NEEDS_SOCKET=1 NEEDS_RESOLV=1 USE_NSEC=1 \ As an aside, you may be able to drop some of these defines. For example, we set NEEDS_SOCKET automatically on Solaris. See the SunOS section of config.mak.uname for the complete set of defaults. Is there some magic somewhere to use ordinary POSIX tar ? gmake TAR=tar ? The default of gtar for Solaris dates back to 2005. There may have been a reason then that is no longer valid now, or there may be something besides make install which uses a more advanced feature. /bin/sh: gtar: /bin/shnot found : gtar: not found gmake[1]: *** [install] Error 1 gmake: *** [install] Error 2 [...] Also, what is shnot ? Two messages stepping on each other's toes? -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 2/2] fast-import: fix segfault in store_tree()
Branch tree is NULLified by filedelete command if we are trying to delete root tree. Add sanity check and use load_tree() in that case. Signed-off-by: Maxim Bublis sat...@yandex-team.ru --- fast-import.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fast-import.c b/fast-import.c index d73f58c..b77f12c 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1419,7 +1419,7 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b) static void store_tree(struct tree_entry *root) { - struct tree_content *t = root-tree; + struct tree_content *t; unsigned int i, j, del; struct last_object lo = { STRBUF_INIT, 0, 0, /* no_swap */ 1 }; struct object_entry *le = NULL; @@ -1427,6 +1427,10 @@ static void store_tree(struct tree_entry *root) if (!is_null_sha1(root-versions[1].sha1)) return; + if (!root-tree) + load_tree(root) + t = root-tree; + for (i = 0; i t-entry_count; i++) { if (t-entries[i]-tree) store_tree(t-entries[i]); -- 1.8.5.2 (Apple Git-48) -- 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] t9300: test filedelete root
Add new fast-import test series for filedelete command. Signed-off-by: Maxim Bublis sat...@yandex-team.ru --- t/t9300-fast-import.sh | 46 ++ 1 file changed, 46 insertions(+) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 5fc9ef2..3d557b3 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -3017,4 +3017,50 @@ test_expect_success 'T: empty reset doesnt delete branch' ' git rev-parse --verify refs/heads/not-to-delete ' +### +### series U (filedelete) +### + +cat input INPUT_END +commit refs/heads/U +committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE +data COMMIT +test setup +COMMIT +M 100644 inline hello.c +data BLOB +blob 1 +BLOB + +INPUT_END + +test_expect_success 'U: initialize for U tests' ' + git fast-import input +' + +cat input INPUT_END +commit refs/heads/U +committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE +data COMMIT +must succeed +COMMIT +from refs/heads/U^0 +D + +INPUT_END + +test_expect_success 'U: filedelete root succeeds' ' +git fast-import input +' + +cat expect EOF +:100644 00 c18147dc648481eeb65dc5e66628429a64843327 D hello.c +EOF + +git diff-tree -M -r U^1 U actual + +test_expect_success 'U: validate filedelete result' ' + compare_diff_raw expect actual +' + test_done -- 1.8.5.2 (Apple Git-48) -- 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 0/2] fast-import: fix segfault and tests
Removing root tree with filedelete command would lead to segmentation fault in store_tree(). First patch from patch series adds test to show incorrect behaviour and second one fixes bug by sanity check and load_tree() usage. Maxim Bublis (2): t9300: test filedelete root fast-import: fix segfault in store_tree() fast-import.c | 6 +- t/t9300-fast-import.sh | 46 ++ 2 files changed, 51 insertions(+), 1 deletion(-) -- 1.8.5.2 (Apple Git-48) -- 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: make install fails because GNU tar needed
On August 28, 2014 at 10:50 AM Jeff King p...@peff.net wrote: On Thu, Aug 28, 2014 at 10:16:48AM -0400, dev wrote: # gmake CFLAGS=$CFLAGS LDFLAGS=$LD_OPTIONS NEEDS_LIBICONV=Yes \ SHELL_PATH=/usr/local/bin/bash \ SANE_TOOL_PATH=/usr/local/bin \ USE_LIBPCRE=1 LIBPCREDIR=/usr/local CURLDIR=/usr/local \ EXPATDIR=/usr/local NEEDS_LIBINTL_BEFORE_LIBICONV=1 \ NEEDS_SOCKET=1 NEEDS_RESOLV=1 USE_NSEC=1 \ As an aside, you may be able to drop some of these defines. For example, we set NEEDS_SOCKET automatically on Solaris. See the SunOS section of config.mak.uname for the complete set of defaults. I figured as much but for the moment I am flailing along towards a nice working build first and then pray to the gods of complication for some kindness and simplification. :-) Thus far the build process seems to work fine. I have no idea if I can use SSH protocol as I would need to set up a dummy to test it. Everything else seems to work. I think. :-\ Is there some magic somewhere to use ordinary POSIX tar ? gmake TAR=tar ? ha .. yeah I guess. Actually I found a file called GIT-BUILD-OPTIONS : # cat GIT-BUILD-OPTIONS SHELL_PATH='/usr/local/bin/bash' PERL_PATH='/usr/local/bin/perl' DIFF='diff' PYTHON_PATH='/usr/bin/python' TAR='tar' NO_CURL='' USE_LIBPCRE='1' NO_PERL='' NO_PYTHON='1' NO_UNIX_SOCKETS='' NO_GETTEXT='' GETTEXT_POISON='' Funny looking options for NO_foo where I would think that a null string indicates that in fact I have foo? Because I do have curl and perl and most likely UNIX_SOCKETS. Regardless, I simply edited that file and three others to stop the search for gtar. The default of gtar for Solaris dates back to 2005. There may have been a reason then that is no longer valid now, or there may be something besides make install which uses a more advanced feature. Yes, I seem to recall that long long ago there were problems with old tar on Solaris 2.5.1 back in the 90's and then it carried forwards up to Solaris 7 or 8. The ultimate POSIX tar as well as tar that can archive or extract anything from anything is Joerg Schilling's star. http://sourceforge.net/projects/s-tar/ I use that nearly everywhere that I must ensure all metadata and data gets taken care of correctly and cross platform. However, hell will freeze over before we ever see it included in a distro or UNIX anywhere so for now tar will suffice. /bin/sh: gtar: /bin/shnot found : gtar: not found gmake[1]: *** [install] Error 1 gmake: *** [install] Error 2 [...] Also, what is shnot ? Two messages stepping on each other's toes? Yeah .. I saw that after I sent the email. dev -- 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 v6 2/6] Add git_env_ulong() to parse environment variable
On Aug 27, 2014, at 4:47 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: On Tue, Aug 26, 2014 at 02:54:11PM -0700, Junio C Hamano wrote: A worse position is to have git_env_bool() that says empty is false and add a new git_env_ulong() that says empty is unset. We should pick one or the other and use it for both. Yeah, I agree they should probably behave the same. The middle ground would be to die(). That does not seem super-friendly, but then we would also die with GIT_SMART_HTTP=foobar, so perhaps it is not unreasonable to just consider it a syntax error. Hmm, I am not sure if dying is better. Unless we decide to make empty string no longer false everywhere and warn now and then later die as part of a 3.0 transition plan or something, that is. I think it is better in the sense that while it may be unexpected, it does not unexpectedly do something that the user cannot easily undo. I really do not think this topic is worth the effort of a long-term deprecation scheme (which I agree _is_ required for a change to the config behavior). Let's just leave it as-is. We've seen zero real-world complaints, only my own surprise after reading the code (and Steffen's patch should be tweaked to match). OK, then let's do that at least for now and move on. Ok. I saw that you tweaked my patch on pu. Maybe remove the outdated comment above the function completely: diff --git a/config.c b/config.c index 87db755..010bcd0 100644 --- a/config.c +++ b/config.c @@ -1122,9 +1122,6 @@ int git_env_bool(const char *k, int def) return v ? git_config_bool(k, v) : def; } -/* - * Use default if environment variable is unset or empty string. - */ unsigned long git_env_ulong(const char *k, unsigned long val) { const char *v = getenv(k); Steffen-- 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: make install fails because GNU tar needed
dev d...@cor0.com writes: Actually I found a file called GIT-BUILD-OPTIONS : That's a generated file (not included by the Makefile, but by some shell scripts later), don't edit it. Use config.mak for your build configuration. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v6 5/6] Change copy_fd() to not close input fd
On Aug 26, 2014, at 8:29 PM, Jeff King p...@peff.net wrote: On Tue, Aug 26, 2014 at 05:23:24PM +0200, Steffen Prohaska wrote: The caller opened the fd, so it should be responsible for closing it. Signed-off-by: Steffen Prohaska proha...@zib.de --- copy.c | 5 + lockfile.c | 3 +++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/copy.c b/copy.c index a7f58fd..d0a1d82 100644 --- a/copy.c +++ b/copy.c @@ -10,7 +10,6 @@ int copy_fd(int ifd, int ofd) break; if (len 0) { int read_error = errno; -close(ifd); return error(copy-fd: read returned %s, strerror(read_error)); } This saved errno is not necessary anymore (the problem was that close() clobbered the error in the original code). It can go away, and we can even drop the curly braces. @@ -21,17 +20,14 @@ int copy_fd(int ifd, int ofd) len -= written; } else if (!written) { -close(ifd); return error(copy-fd: write returned 0); } else { int write_error = errno; -close(ifd); return error(copy-fd: write returned %s, strerror(write_error)); } } Ditto here. Actually, isn't this whole write just a reimplementation of write_in_full? The latter treats a return of 0 as ENOSPC rather than using a custom message, but I think that is sane. All together: Makes all sense, and seems sane to me, too. Junio, I saw that you have the changes on pu with 'SQUASH???...'. Will you squash it, or shall I send another complete update of the patch series? Steffen --- copy.c | 28 +--- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/copy.c b/copy.c index a7f58fd..53a9ece 100644 --- a/copy.c +++ b/copy.c @@ -4,34 +4,16 @@ int copy_fd(int ifd, int ofd) { while (1) { char buffer[8192]; - char *buf = buffer; ssize_t len = xread(ifd, buffer, sizeof(buffer)); if (!len) break; - if (len 0) { - int read_error = errno; - close(ifd); + if (len 0) return error(copy-fd: read returned %s, - strerror(read_error)); - } - while (len) { - int written = xwrite(ofd, buf, len); - if (written 0) { - buf += written; - len -= written; - } - else if (!written) { - close(ifd); - return error(copy-fd: write returned 0); - } else { - int write_error = errno; - close(ifd); - return error(copy-fd: write returned %s, - strerror(write_error)); - } - } + strerror(errno)); + if (write_in_full(ofd, buffer, len) 0) + return error(copy-fd: write returned %s, + strerror(errno)); } - close(ifd); return 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
Git Bug Resolve: bisect run failed to locate the right commit, not git bug
Dear git developer: On August 27, I report a bug on git bisect: bisect run failed to locate the right commit Today, my friend told me that the problem may be caused by the python compiled file *.pyc. Since if python see the pyc is a new compiled, it will directly run *pyc file but update it. When we use bisect-run, it quickly checkout different version and run the script to test it. It's so fast that the *pyc file are still new files, so python run *.pyc and get same result. That's the wrong result came from. There are two way to avoid this problem: First is delete all *.pyc before python run, add a test.sh with following content: find . -name *.pyc -exec rm {} \; ./autoscript.py Second is let python run slower, adding a delay in autoscript.py: import time time.sleep(1) Both method let git-bisect-run give the right result. Sorry about the wrong bug report, it prove that git-bisect-run is very very fast -- too fast to cause some mistake lol Sincerely YodaLee 20140828
[PATCH 0/6] Improve tag checking in fsck and with transfer.fsckobjects
This patch series introduces detailed checking of tag objects when calling git fsck, and also when transfer.fsckobjects is set to true. To this end, the fsck machinery is reworked to accept the buffer and size of the object to check, and for commit and tag objects, we verify that the buffers end in a NUL. This work was sponsored by GitHub. Johannes Schindelin (5): Refactor type_from_string() to avoid die()ing in case of errors fsck: inspect tag objects more closely Add regression tests for stricter tag fsck'ing Refactor out fsck_object_buffer() accepting the object data Make sure that index-pack --strict fails upon invalid tag objects builtin/index-pack.c | 3 +- builtin/unpack-objects.c | 14 -- fsck.c | 109 --- fsck.h | 3 ++ object.c | 13 +- object.h | 1 + t/t1450-fsck.sh | 39 + t/t5302-pack-index.sh| 19 + 8 files changed, 188 insertions(+), 13 deletions(-) -- 2.0.0.rc3.9669.g840d1f9 -- 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 v3] teach fast-export an --anonymize option
On 28/08/14 13:32, Jeff King wrote: On Thu, Aug 28, 2014 at 05:30:44PM +0700, Duy Nguyen wrote: On Thu, Aug 28, 2014 at 12:01 AM, Jeff King p...@peff.net wrote: You can get an overview of what will be shared by running a command like: git fast-export --anonymize --all | perl -pe 's/\d+/X/g' | sort -u | less which will show every unique line we generate, modulo any numbers (each anonymized token is assigned a number, like User 0, and we replace it consistently in the output). I feel like this should be part of git-fast-export.txt, just to increase the user's confidence in the tool (and I don't expect most users to read this commit message). Hmph. Whenever I say I think this patch is done, suddenly the comments start pouring in. :) :-D I think you are right, though, and we could stand to explain the feature a little more in the documentation in general. How about this patch on top (or squashed in): -- 8 -- Subject: docs/fast-export: explain --anonymize more completely The original commit made mention of this option, but not why one might want it or how they might use it. Let's try to be a little more thorough, and also explain how to confirm that the output really is anonymous. Signed-off-by: Jeff King p...@peff.net --- Documentation/git-fast-export.txt | 63 --- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt index 52831fa..dbe9a46 100644 --- a/Documentation/git-fast-export.txt +++ b/Documentation/git-fast-export.txt @@ -106,10 +106,9 @@ marks the same across runs. different from the commit's first parent). --anonymize:: - Replace all refnames, paths, blob contents, commit and tag - messages, names, and email addresses in the output with - anonymized data, while still retaining the shape of history and - of the stored tree. + Anonymize the contents of the repository while still retaining + the shape of the history and stored tree. See the section on + `ANONYMIZING` below. --refspec:: Apply the specified refspec to each ref exported. Multiple of them can @@ -147,6 +146,62 @@ referenced by that revision range contains the string 'refs/heads/master'. +ANONYMIZING +--- + +If the `--anonymize` option is given, git will attempt to remove all +identifying information from the repository while still retaining enough +of the original tree and history patterns to reproduce some bugs. The +goal is that a git bug which is found on a private repository will s/goal/hope/ ;-) +persist in the anonymized repository, and the latter can be shared with +git developers to help solve the bug. + +With this option, git will replace all refnames, paths, blob contents, +commit and tag messages, names, and email addresses in the output with +anonymized data. Two instances of the same string will be replaced +equivalently (e.g., two commits with the same author will have the same +anonymized author in the output, but bear no resemblance to the original +author string). The relationship between commits, branches, and tags is +retained, as well as the commit timestamps (but the commit messages and +refnames bear no resemblance to the originals). The relative makeup of +the tree is retained (e.g., if you have a root tree with 10 files and 3 +trees, so will the output), but their names and the contents of the +files will be replaced. + +If you think you have found a git bug, you can start by exporting an +anonymized stream of the whole repository: + +--- +$ git fast-export --anonymize --all anon-stream +--- + +Then confirm that the bug persists in a repository created from that +stream (many bugs will not, as they really do depend on the exact +repository contents): Dumb question (I have not even read the patch, so please just ignore me if this is indeed dumb!): Is the map of original-name, anonymized-name available to the user while he attempts to confirm that the bug is still present? For example, if I anonymized git.git, and did 'git branch -v' (say), how easy would it be for me to recognise which branch was 'next'? ATB, Ramsay Jones -- 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] contrib/svn-fe: fix Makefile
Fixes several problems: * include config.mak.uname, config.mak.autogen and config.mak in order to use settings for prefix and other such things; * link xdiff/lib.a as it is a requirement for libgit.a; * fix CFLAGS, LDFLAGS and EXTLIBS for Linux and Mac OS X. Signed-off-by: Maxim Bublis sat...@yandex-team.ru --- Changes from previous version: * added possibility to disable MacPorts and Fink; * respecting NEEDS_CRYPTO_WITH_SSL. contrib/svn-fe/Makefile | 60 + 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile index 360d8da..e8651aa 100644 --- a/contrib/svn-fe/Makefile +++ b/contrib/svn-fe/Makefile @@ -1,18 +1,58 @@ all:: svn-fe$X -CC = gcc +CC = cc RM = rm -f MV = mv CFLAGS = -g -O2 -Wall LDFLAGS = -ALL_CFLAGS = $(CFLAGS) -ALL_LDFLAGS = $(LDFLAGS) -EXTLIBS = +EXTLIBS = -lz + +include ../../config.mak.uname +-include ../../config.mak.autogen +-include ../../config.mak + +ifeq ($(uname_S),Darwin) + ifndef NO_FINK + ifeq ($(shell test -d /sw/lib echo y),y) + CFLAGS += -I/sw/include + LDFLAGS += -L/sw/lib + endif + endif + ifndef NO_DARWIN_PORTS + ifeq ($(shell test -d /opt/local/lib echo y),y) + CFLAGS += -I/opt/local/include + LDFLAGS += -L/opt/local/lib + endif + endif +endif + +ifndef NO_OPENSSL + EXTLIBS += -lssl + ifdef NEEDS_CRYPTO_WITH_SSL + EXTLIBS += -lcrypto + endif +endif + +ifndef NO_PTHREADS + CFLAGS += $(PTHREADS_CFLAGS) + EXTLIBS += $(PTHREAD_LIBS) +endif + +ifdef HAVE_CLOCK_GETTIME + CFLAGS += -DHAVE_CLOCK_GETTIME + EXTLIBS += -lrt +endif + +ifdef NEEDS_LIBICONV + EXTLIBS += -liconv +endif GIT_LIB = ../../libgit.a VCSSVN_LIB = ../../vcs-svn/lib.a -LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(EXTLIBS) +XDIFF_LIB = ../../xdiff/lib.a + +LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(XDIFF_LIB) QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir QUIET_SUBDIR1 = @@ -33,12 +73,11 @@ ifndef V endif endif -svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \ - $(ALL_LDFLAGS) $(LIBS) +svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(XDIFF_LIB) $(GIT_LIB) + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(EXTLIBS) -o $@ svn-fe.o $(LIBS) svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h - $(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $ + $(QUIET_CC)$(CC) $(CFLAGS) -I../../vcs-svn -o $*.o -c $ svn-fe.html: svn-fe.txt $(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \ @@ -54,6 +93,9 @@ svn-fe.1: svn-fe.txt ../../vcs-svn/lib.a: FORCE $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) vcs-svn/lib.a +../../xdiff/lib.a: FORCE + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) xdiff/lib.a + ../../libgit.a: FORCE $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a -- 1.8.5.2 (Apple Git-48) -- 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] improving advice message from git commit during a merge
Stefan Beller stefanbel...@gmail.com writes: On 27.08.2014 20:23, Junio C Hamano wrote: I am not doing this myself soon, though. Hint, hint... I asked once upon a time, if there was a place, which collects such topics for casual contributors and new comers. Would you mind to update the leftover bits at http://git-blame.blogspot.de/search?q=leftoverby-date=true ? They live in a bit more permanent home at http://git-blame.blogspot.com/p/leftover-bits.html these days. It is too early to tell this particular one will make an entry there yet ;-) -- 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 v6 2/6] Add git_env_ulong() to parse environment variable
Steffen Prohaska proha...@zib.de writes: OK, then let's do that at least for now and move on. Ok. I saw that you tweaked my patch on pu. Maybe remove the outdated comment above the function completely: diff --git a/config.c b/config.c index 87db755..010bcd0 100644 --- a/config.c +++ b/config.c @@ -1122,9 +1122,6 @@ int git_env_bool(const char *k, int def) return v ? git_config_bool(k, v) : def; } -/* - * Use default if environment variable is unset or empty string. - */ Thanks, will do. unsigned long git_env_ulong(const char *k, unsigned long val) { const char *v = getenv(k); Steffen-- 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 v6 5/6] Change copy_fd() to not close input fd
Steffen Prohaska proha...@zib.de writes: On Aug 26, 2014, at 8:29 PM, Jeff King p...@peff.net wrote: ... Makes all sense, and seems sane to me, too. Junio, I saw that you have the changes on pu with 'SQUASH???...'. Will you squash it, or shall I send another complete update of the patch series? If I let you reroll, I will risk that you will forget to propagate the log message fixes I did here while queuing the v6 iteration in it, so let me try doing the squashing and push out the result first. 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: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict
Matthieu Moy matthieu@imag.fr writes: 'git commit -a' is rarely a good way to mark conflicts as resolved: the user anyway has to go manually through the list of conflicts to do the actual resolution, and it is usually better to use git add on each files after doing the resolution. On the other hand, using 'git commit -a' is potentially dangerous, as it makes it very easy to mistakenly commit conflict markers without noticing. While we're there, synchronize the 'git pull' and 'git merge' messages: the first was ending with '... and make a commit.', but not the later. Signed-off-by: Matthieu Moy matthieu@imag.fr --- - Hasty-and-careless new users will be incorrectly enticed to type the command given by or use 'git commit -a' at the end of this advice message without thinking. Perhaps it is safer to stop the sentence at ... and make a commit. and drop that last bit while there are conflicts still in the working tree files. We should use the current end-of-message only when all the conflicts have been resolved in the working tree. It was already on my todo-list, as a friend of mine semi-beginner with Git complained about the mis-advice the other day, and I had to agree. Eventually, git could detect that conflicts have been resolved, but then that would be a different message, as not only use git commit -a could be resurected, but Fix them up in the work tree should be dropped when it is the case. This paragraph should be in the log message, shouldn't it, probably with s/could/should/? I guess I'm just taking the low hanging fruit here ;-). I'd say it is more like scooping a fruit lying on the ground before it rots, but thanks anyway ;-) advice.c| 3 +-- git-pull.sh | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/advice.c b/advice.c index 9b42033..3b8bf3c 100644 --- a/advice.c +++ b/advice.c @@ -86,8 +86,7 @@ int error_resolve_conflict(const char *me) * other commands doing a merge do. */ advise(_(Fix them up in the work tree, and then use 'git add/rm file'\n - as appropriate to mark resolution and make a commit, or use\n - 'git commit -a'.)); + as appropriate to mark resolution and make a commit.)); return -1; } diff --git a/git-pull.sh b/git-pull.sh index 18a394f..4d4fc77 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -20,7 +20,7 @@ die_conflict () { if [ $(git config --bool --get advice.resolveConflict || echo true) = true ]; then die $(gettext Pull is not possible because you have unmerged files. Please, fix them up in the work tree, and then use 'git add/rm file' -as appropriate to mark resolution, or use 'git commit -a'.) +as appropriate to mark resolution and make a commit.) else die $(gettext Pull is not possible because you have unmerged files.) fi -- 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] merge, pull: stop advising 'commit -a' in case of conflict
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: 'git commit -a' is rarely a good way to mark conflicts as resolved: the user anyway has to go manually through the list of conflicts to do the actual resolution, and it is usually better to use git add on each files after doing the resolution. On the other hand, using 'git commit -a' is potentially dangerous, as it makes it very easy to mistakenly commit conflict markers without noticing. While we're there, synchronize the 'git pull' and 'git merge' messages: the first was ending with '... and make a commit.', but not the later. Signed-off-by: Matthieu Moy matthieu@imag.fr --- - Hasty-and-careless new users will be incorrectly enticed to type the command given by or use 'git commit -a' at the end of this advice message without thinking. Perhaps it is safer to stop the sentence at ... and make a commit. and drop that last bit while there are conflicts still in the working tree files. We should use the current end-of-message only when all the conflicts have been resolved in the working tree. It was already on my todo-list, as a friend of mine semi-beginner with Git complained about the mis-advice the other day, and I had to agree. Eventually, git could detect that conflicts have been resolved, but then that would be a different message, as not only use git commit -a could be resurected, but Fix them up in the work tree should be dropped when it is the case. This paragraph should be in the log message, shouldn't it, probably with s/could/should/? I think the commit message explains well enough why the change is good. The additional paragraph explains why I did not do it the way your message suggests (which has to do with the current discussion, but does not have to be recorded in history IHMO). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: Relative submodule URLs
Sorry for dropping out of the conversation; the last few days were a bit hectic. Regarding recursive branching, I agree that a super-repo's branch names are not necessarily appropriate for its submodules, and that Heiko's simple workflow is a workable base to build upon. More thought is needed here, but that's for another day. Regarding remote.default, Robert please understand that the feature doesn't exist, and the idea is to only serve as a fallback when the current methods for remote selection end up resorting to the hardcoded origin name. More thought is also needed here, but not today. Both Heiko and Robert took issue with this statement of mine: On 14-08-22 12:00 PM, Marc Branchaud wrote: A branch should fork the entire repo, including its submodules. The implication is that if you want to push that branch somewhere, that somewhere needs to be able to accept the forks of the submodules *even if those submodules aren't changed in your branch* because at the very least the branch ref has to exist in the submodules' repositories. Heiko said: It should be easy to work on a repository that is forked in its entirety, but it should also be possible (and properly supported) to only fork some submodules. You're right, I overstated it when I said that the branch ref has to exist in the unchanged submodules. The super-repo branch records which submodules it updates, and when pushing the branch somewhere only those submodules' changes need to be pushed. Robert asked: How will this impact *creating* branches? What about forking? Do you expect submodule forking branching to be automatic as well? ... This seems difficult to do, especially the forking part since you would need an API for this (Github, Atlassian Stash, etc), unless you are thinking of something clever like local/relative forks. I meant fork in the local-branch sense: The branch represents a topic in the repository, and it should encompass the entire repository including its submodules (just like the branch encompasses all the files in the repository, even though the branch's commits only change a subset of those files). I think you're talking about fork in the sense of setting up a mirror of a repository. I agree that there aren't really any tools for automatically doing that with repositories that contain relative-path submodules. I think git clone could learn to do it, though. Heiko also said this: On Fri, Aug 22, 2014 at 12:00:07PM -0400, Marc Branchaud wrote: With relative-path submodules, the push's target repo *must* also have the submodules in their proper places, so that they can get updated. Furthermore, if you clone a repo that has relative-path submodules you *must* also clone the submodules. That is not true. You can have relative submodules and just clone/fetch some from a different remote. Its just a question of how to specifiy/transport this information. I meant that more as a general guideline than some kind of physical law. Sure, it's possible to scatter the submodules across all sorts of hosts, but it's not a good idea. When it comes to relative-path submodules, pushing and fetching submodule changes in the super-repo should just involve the one remote host (whatever way that's determined). This keeps things tractable, because otherwise your branch's changes are scattered among many different hosts and you end up considering weird things like this part of the branch's changes are on host A but this other part are on host B, so let's record that somewhere, oh but what if host B is down when I'm trying to fetch, but I know that host C has the changes too so why don't I just fetch what I want from there. It's a nightmare. It's infinitely better to treat a repository and its relative-path submodules as an atomic unit, so that any remote that hosts the repository also hosts the submodules. When pushing a branch with submodule changes, expect to find those submodules on the target remote and update them. Regardless of how the target remote is determined. Same thing for fetching. It's just so much simpler to work this way. So please, let's not try to specify submodule remotes per-branch or make that info pushable. It's enough for a branch's local configuration to say that it tracks fetch/pull refs on different remotes. The rest should flow from that. M. -- 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] pretty: Provide a strict ISO8601 date format
It uses the '%aI' and '%cI' format specifiers or the '--date=iso-strict' date format name. See http://article.gmane.org/gmane.comp.version-control.git/255879 for discussion. Signed-off-by: Beat Bolli bbo...@ewanet.ch --- Documentation/git-rev-list.txt | 2 +- Documentation/pretty-formats.txt | 6 -- Documentation/rev-list-options.txt | 13 +++-- cache.h| 1 + date.c | 10 ++ pretty.c | 5 - t/t4205-log-pretty-formats.sh | 7 +++ 7 files changed, 38 insertions(+), 6 deletions(-) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index 7a1585d..fd7f8b5 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -45,7 +45,7 @@ SYNOPSIS [ \--regexp-ignore-case | -i ] [ \--extended-regexp | -E ] [ \--fixed-strings | -F ] -[ \--date=(local|relative|default|iso|rfc|short) ] +[ \--date=(local|relative|default|iso|iso-strict|rfc|short) ] [ [\--objects | \--objects-edge] [ \--unpacked ] ] [ \--pretty | \--header ] [ \--bisect ] diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 85d6353..50a2c30 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -115,7 +115,8 @@ The placeholders are: - '%aD': author date, RFC2822 style - '%ar': author date, relative - '%at': author date, UNIX timestamp -- '%ai': author date, ISO 8601 format +- '%ai': author date, ISO 8601-like format +- '%aI': author date, strict ISO 8601 format - '%cn': committer name - '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) @@ -126,7 +127,8 @@ The placeholders are: - '%cD': committer date, RFC2822 style - '%cr': committer date, relative - '%ct': committer date, UNIX timestamp -- '%ci': committer date, ISO 8601 format +- '%ci': committer date, ISO 8601-like format +- '%cI': committer date, strict ISO 8601 format - '%d': ref names, like the --decorate option of linkgit:git-log[1] - '%e': encoding - '%s': subject diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index deb8cca..5d311b8 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -677,7 +677,7 @@ include::pretty-options.txt[] --relative-date:: Synonym for `--date=relative`. ---date=(relative|local|default|iso|rfc|short|raw):: +--date=(relative|local|default|iso|iso-strict|rfc|short|raw):: Only takes effect for dates shown in human-readable format, such as when using `--pretty`. `log.date` config variable sets a default value for the log command's `--date` option. @@ -687,7 +687,16 @@ e.g. ``2 hours ago''. + `--date=local` shows timestamps in user's local time zone. + -`--date=iso` (or `--date=iso8601`) shows timestamps in ISO 8601 format. +`--date=iso` (or `--date=iso8601`) shows timestamps in a ISO 8601-like format. +The differences to the strict ISO 8601 format are: + + - a space instead of the `T` date/time delimiter + - a space between time and time zone + - no colon between hours and minutes of the time zone + ++ +`--date=iso-strict` (or `--date=iso8601-strict`) shows timestamps in strict +ISO 8601 format. + `--date=rfc` (or `--date=rfc2822`) shows timestamps in RFC 2822 format, often found in email messages. diff --git a/cache.h b/cache.h index fcb511d..fa92aaf 100644 --- a/cache.h +++ b/cache.h @@ -1037,6 +1037,7 @@ enum date_mode { DATE_SHORT, DATE_LOCAL, DATE_ISO8601, + DATE_ISO8601_STRICT, DATE_RFC2822, DATE_RAW }; diff --git a/date.c b/date.c index 782de95..d545ee6 100644 --- a/date.c +++ b/date.c @@ -200,6 +200,13 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode) tm-tm_mday, tm-tm_hour, tm-tm_min, tm-tm_sec, tz); + else if (mode == DATE_ISO8601_STRICT) + strbuf_addf(timebuf, %04d-%02d-%02dT%02d:%02d:%02d%+03d:%02d, + tm-tm_year + 1900, + tm-tm_mon + 1, + tm-tm_mday, + tm-tm_hour, tm-tm_min, tm-tm_sec, + tz / 100, abs(tz % 100)); else if (mode == DATE_RFC2822) strbuf_addf(timebuf, %.3s, %d %.3s %d %02d:%02d:%02d %+05d, weekday_names[tm-tm_wday], tm-tm_mday, @@ -751,6 +758,9 @@ enum date_mode parse_date_format(const char *format) else if (!strcmp(format, iso8601) || !strcmp(format, iso)) return DATE_ISO8601; + else if (!strcmp(format, iso8601-strict) || +!strcmp(format, iso-strict)) + return
Re: [PATCH v3] teach fast-export an --anonymize option
Jeff King p...@peff.net writes: Subject: docs/fast-export: explain --anonymize more completely The original commit made mention of this option, but not why one might want it or how they might use it. Let's try to be a little more thorough, and also explain how to confirm that the output really is anonymous. Signed-off-by: Jeff King p...@peff.net --- Documentation/git-fast-export.txt | 63 --- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt index 52831fa..dbe9a46 100644 --- a/Documentation/git-fast-export.txt +++ b/Documentation/git-fast-export.txt @@ -106,10 +106,9 @@ marks the same across runs. different from the commit's first parent). --anonymize:: - Replace all refnames, paths, blob contents, commit and tag - messages, names, and email addresses in the output with - anonymized data, while still retaining the shape of history and - of the stored tree. + Anonymize the contents of the repository while still retaining + the shape of the history and stored tree. See the section on + `ANONYMIZING` below. Technically s/tree/trees/, I would think. For a repository with multiple branches, perhaps s/history/histories/, too, but I would not insist on that ;-). +ANONYMIZING +--- + +If the `--anonymize` option is given, git will attempt to remove all +identifying information from the repository while still retaining enough +of the original tree and history patterns to reproduce some bugs. The +goal is that a git bug which is found on a private repository will +persist in the anonymized repository, and the latter can be shared with +git developers to help solve the bug. + +With this option, git will replace all refnames, paths, blob contents, +commit and tag messages, names, and email addresses in the output with +anonymized data. Two instances of the same string will be replaced +equivalently (e.g., two commits with the same author will have the same +anonymized author in the output, but bear no resemblance to the original +author string). The relationship between commits, branches, and tags is +retained, as well as the commit timestamps (but the commit messages and +refnames bear no resemblance to the originals). The relative makeup of +the tree is retained (e.g., if you have a root tree with 10 files and 3 +trees, so will the output), but their names and the contents of the +files will be replaced. While I do not think I or anybody who would ask other people to use this option would be confused, the phrase the same string may risk unnecessary worries from those who are asked to trust this option. I am not yet convinced that it is unlikely for the reader to read the above and imagine that the anonymiser may go word by word, replacing the same string with the same anonymised gibberish (which would be susceptible to old-school cryptoanalysis techniques). Among the ones that listed, refnames, blob contents, commit messages and tag messages are converted as a single string and I wish I could think of phrasing to stress that point somehow. Each path component in paths is converted as a single string, so we can read from two anonymised paths if they refer to blobs in the same directory in the original. This is a good thing, of course, but it shows that among those listed in refnames, paths, blob contents, ... in a flat sentence, some are treated as a single token for replacement but not others, and it is hard to tell for a reader which one is which, unless the reader knows the internals of Git, i.e. what kind of things we as the debuggers-of-Git would want to preserve. Isn't the unit for human identity anonymisation even more coarse? If it is not should it? In other words, do Junio C Hamano ju...@pobox.com and Junio C Hamano gits...@pobox.com map to one gibberish human readable name with two gibberish e-mail addresses, or 2 User$n user$n? Is the fact that this organization seems to allocate two e-mails to each developer something this organization may want to hide from the public (and something we as the Git debuggers would not benefit from knowing)? -- 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] merge, pull: stop advising 'commit -a' in case of conflict
Matthieu Moy wrote: Signed-off-by: Matthieu Moy matthieu@imag.fr [...] --- advice.c| 3 +-- git-pull.sh | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) Thanks for taking it on. Reviewed-by: Jonathan Nieder jrnie...@gmail.com [...] It was already on my todo-list, as a friend of mine semi-beginner with Git complained about the mis-advice the other day, and I had to agree. That's a useful sort of thing to put in a commit message. :) Eventually, git could detect that conflicts have been resolved, but then that would be a different message, as not only use git commit -a could be resurected, but Fix them up in the work tree should be dropped when it is the case. As is this --- when I wonder why code isn't a certain way, ideas for future work found in the description for the blamed commit are often helpful in explaining the current state and saving me from blind alleys in changing it. Anyway, this is already a very good change as-is. Actually, I'd be nervous about suggesting use git commit -a without at least also saying inspect the result or run tests in the no-conflict-markers-found case. Rerere sometimes makes mistakes, and the result of picking one side when merging binary files can be even worse. Thanks, 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 v3] teach fast-export an --anonymize option
Ramsay Jones ram...@ramsay1.demon.co.uk writes: Dumb question (I have not even read the patch, so please just ignore me if this is indeed dumb!): Is the map of original-name, anonymized-name available to the user while he attempts to confirm that the bug is still present? For example, if I anonymized git.git, and did 'git branch -v' (say), how easy would it be for me to recognise which branch was 'next'? It is not dumb but actually is a very good point. There needs an easy way for the reporting user to turn an observation such as When I do 'git log master..next' I see this one extraneous commit shown into a corresponding statement to accompany the anonymised output. The user needs it to make sure that the symptom reproduces in the anonymised repository in order to decide if it is even worthwhile to send the output for analysis in the first place. -- 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] merge, pull: stop advising 'commit -a' in case of conflict
Jonathan Nieder jrnie...@gmail.com writes: It was already on my todo-list, as a friend of mine semi-beginner with Git complained about the mis-advice the other day, and I had to agree. That's a useful sort of thing to put in a commit message. :) Eventually, git could detect that conflicts have been resolved, but then that would be a different message, as not only use git commit -a could be resurected, but Fix them up in the work tree should be dropped when it is the case. As is this --- when I wonder why code isn't a certain way, ideas for future work found in the description for the blamed commit are often helpful in explaining the current state and saving me from blind alleys in changing it. Yes. Anyway, this is already a very good change as-is. Actually, I'd be nervous about suggesting use git commit -a without at least also saying inspect the result or run tests in the no-conflict-markers-found case. Rerere sometimes makes mistakes, and the result of picking one side when merging binary files can be even worse. Here is how I phrased in the one queued tentatively. -- 8 -- From: Matthieu Moy matthieu@imag.fr Date: Thu, 28 Aug 2014 11:46:58 +0200 Subject: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict 'git commit -a' is rarely a good way to mark conflicts as resolved: the user anyway has to go manually through the list of conflicts to do the actual resolution, and it is usually better to use git add on each files after doing the resolution. On the other hand, using 'git commit -a' is potentially dangerous, as it makes it very easy to mistakenly commit conflict markers without noticing, and even worse, the user may have started a merge while having local changes that do not overlap with it in the working tree. While we're there, synchronize the 'git pull' and 'git merge' messages: the first was ending with '... and make a commit.', but not the latter. Eventually, git should detect that conflicts have been resolved in the working tree and tailor these messages further. Not only use git commit -a could be resurected, but Fix them up in the work tree should be dropped when it happens. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com diff --git a/advice.c b/advice.c index 9b42033..3b8bf3c 100644 --- a/advice.c +++ b/advice.c @@ -86,8 +86,7 @@ int error_resolve_conflict(const char *me) * other commands doing a merge do. */ advise(_(Fix them up in the work tree, and then use 'git add/rm file'\n -as appropriate to mark resolution and make a commit, or use\n -'git commit -a'.)); +as appropriate to mark resolution and make a commit.)); return -1; } diff --git a/git-pull.sh b/git-pull.sh index 18a394f..4d4fc77 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -20,7 +20,7 @@ die_conflict () { if [ $(git config --bool --get advice.resolveConflict || echo true) = true ]; then die $(gettext Pull is not possible because you have unmerged files. Please, fix them up in the work tree, and then use 'git add/rm file' -as appropriate to mark resolution, or use 'git commit -a'.) +as appropriate to mark resolution and make a commit.) else die $(gettext Pull is not possible because you have unmerged files.) fi -- 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 v3] teach fast-export an --anonymize option
On Thu, Aug 28, 2014 at 05:46:15PM +0100, Ramsay Jones wrote: Dumb question (I have not even read the patch, so please just ignore me if this is indeed dumb!): Is the map of original-name, anonymized-name available to the user while he attempts to confirm that the bug is still present? No, it's not. For example, if I anonymized git.git, and did 'git branch -v' (say), how easy would it be for me to recognise which branch was 'next'? You can't, really. The simplest thing would be to pare down your repository to the minimum number of branches before anonymizing. It might make sense to have an option to dump the maps we've stored to a separate file (in theory, you could even load them back in and do an incremental anonymized export[1]). I think I'd rather wait on implementing that until we see more real-world use cases (but as always, I'm happy to review if somebody wants to pick it up). -Peff [1] Incremental anonymization is not something I think is worth supporting by itself. However, there may be some value in being able to anonymize two similar repositories using the same mappings. For instance, a repository and its clone. -- 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 v3] teach fast-export an --anonymize option
On Thu, Aug 28, 2014 at 11:11:47AM -0700, Junio C Hamano wrote: + Anonymize the contents of the repository while still retaining + the shape of the history and stored tree. See the section on + `ANONYMIZING` below. Technically s/tree/trees/, I would think. For a repository with multiple branches, perhaps s/history/histories/, too, but I would not insist on that ;-). Sure, I think both of those are fine (I meant tree here to refer to the general notion of a set of paths over time, not a particular tree object). +With this option, git will replace all refnames, paths, blob contents, +commit and tag messages, names, and email addresses in the output with +anonymized data. Two instances of the same string will be replaced +equivalently (e.g., two commits with the same author will have the same +anonymized author in the output, but bear no resemblance to the original +author string). The relationship between commits, branches, and tags is +retained, as well as the commit timestamps (but the commit messages and +refnames bear no resemblance to the originals). The relative makeup of +the tree is retained (e.g., if you have a root tree with 10 files and 3 +trees, so will the output), but their names and the contents of the +files will be replaced. While I do not think I or anybody who would ask other people to use this option would be confused, the phrase the same string may risk unnecessary worries from those who are asked to trust this option. I am not yet convinced that it is unlikely for the reader to read the above and imagine that the anonymiser may go word by word, replacing the same string with the same anonymised gibberish (which would be susceptible to old-school cryptoanalysis techniques). I tried to use phrases like bears no resemblance to indicate that the mapping was not leaking information. Does it bear a separate paragraph explaining the transformation (I was trying to avoid that because it is necessarily intimately linked with the particular implementation chosen). Among the ones that listed, refnames, blob contents, commit messages and tag messages are converted as a single string and I wish I could think of phrasing to stress that point somehow. Maybe a separate paragraph like: Note that the replacement strings are chosen with no input from the original strings. There is no cryptography or other tricks involved, but rather we make up a new string like message 123, replace a particular commit message with it, and then use the mapping between the two for the rest of the output. Thus, no information about the original commit message is leaked, and only the internal mapping (which is not part of the output stream) could reverse the transformation. Each path component in paths is converted as a single string, so we can read from two anonymised paths if they refer to blobs in the same directory in the original. This is a good thing, of course, but it shows that among those listed in refnames, paths, blob contents, ... in a flat sentence, some are treated as a single token for replacement but not others, and it is hard to tell for a reader which one is which, unless the reader knows the internals of Git, i.e. what kind of things we as the debuggers-of-Git would want to preserve. Yes, I was really trying not to get into those details, because I do not think they matter to most callers and are subject to change as we come up with better heuristics. I do not even want to promise an implementation like no tricky cryptography above, because we may think of a more interesting way to transform components. Isn't the unit for human identity anonymisation even more coarse? If it is not should it? In other words, do Junio C Hamano ju...@pobox.com and Junio C Hamano gits...@pobox.com map to one gibberish human readable name with two gibberish e-mail addresses, or 2 User$n user$n? Is the fact that this organization seems to allocate two e-mails to each developer something this organization may want to hide from the public (and something we as the Git debuggers would not benefit from knowing)? The ident mapping takes a single Name email string and converts it into a User X us...@example.com string. So no, we are not leaking the fact that one name has multiple emails. I actually started down that path, but gave it up, as it could produce entries like User 3 ema...@example.com which were downright confusing. Plus I did not think that would be a useful thing for debuggers to know, and replacing the whole string is simpler (I also entertained the idea of just blanking _all_ idents; what I expect to be of primary use here is the history shape, and I doubt that a bug would be triggered by the pattern of usernames but not their actual content). -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
Re: [PATCH 1/2] t0027: Tests for core.eol=native, eol=lf, eol=crlf
Torsten Bögershausen tbo...@web.de writes: Add test cases for core.eol native and (unset). (MINGW uses CRLF, all other systems LF as native line endings) Add test cases for the attributes eol=lf and eol=crlf Other minor changes: - Use the more portable 'tr' instead of 'od -c' to convert '\n' into 'Q' and '\0' into 'N' - Style fixes for shell functions according to the coding guide lines - Replace txtbin with attr Signed-off-by: Torsten Bögershausen tbo...@web.de --- It appears that I missed this patch? You seem to have rerolled the corresponding 2/2, to which I responded ($gmane/255507). Is this one still viable/necessary? If you are doing the whole-sale style fixes for this script, can you also fix the case/esac statement in create_gitattributes? The case arm labels are indented one level too deep, making it harder to spot them than necessary. Test files setup master step creates appear to end in an incomplete line. Is this intended or by mistake? Making sure things work even on files that end in an incomplete line is a good thing, but it looks somewhat strange not to test normal cases (in other words, it makes it appear as if normal cases work OK but incomplete lines cause corner case bugs and these tests are meant to check them, or something). 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: Re: Relative submodule URLs
On Thu, Aug 28, 2014 at 01:44:18PM -0400, Marc Branchaud wrote: Heiko also said this: On Fri, Aug 22, 2014 at 12:00:07PM -0400, Marc Branchaud wrote: With relative-path submodules, the push's target repo *must* also have the submodules in their proper places, so that they can get updated. Furthermore, if you clone a repo that has relative-path submodules you *must* also clone the submodules. That is not true. You can have relative submodules and just clone/fetch some from a different remote. Its just a question of how to specifiy/transport this information. I meant that more as a general guideline than some kind of physical law. Sure, it's possible to scatter the submodules across all sorts of hosts, but it's not a good idea. When it comes to relative-path submodules, pushing and fetching submodule changes in the super-repo should just involve the one remote host (whatever way that's determined). This keeps things tractable, because otherwise your branch's changes are scattered among many different hosts and you end up considering weird things like this part of the branch's changes are on host A but this other part are on host B, so let's record that somewhere, oh but what if host B is down when I'm trying to fetch, but I know that host C has the changes too so why don't I just fetch what I want from there. It's a nightmare. It's infinitely better to treat a repository and its relative-path submodules as an atomic unit, so that any remote that hosts the repository also hosts the submodules. When pushing a branch with submodule changes, expect to find those submodules on the target remote and update them. Regardless of how the target remote is determined. Same thing for fetching. It's just so much simpler to work this way. You are right, its simpler. But I would not say better. Depending on your project it might be better to just fork some submodules. So please, let's not try to specify submodule remotes per-branch or make that info pushable. It's enough for a branch's local configuration to say that it tracks fetch/pull refs on different remotes. The rest should flow from that. Why not? Git is all about flexibility. Of course if you organise your submodules in chaos you will get chaos. But consider this: You have this big project which consists of submodule (e.g. like Android with hundreds of submodules). Now you want to develop on something that involves just a subset of submodules, lets say two submodules. Now if someone just wants to publish a small change to some submodules you are demanding to setup a mirror of *all* submodules that are in this big project. That might not even be feasible depending on the projects size and the remote quota. Not to speak about having to first create a fork of hundreds of repositories. So in this situation we should support just referring some submodules to other places. Regarding transporting this information. If you ask someone to try out your change it should be as simple as possible. It should be enough to say. clone from there and checkout that branch (once recursive checkout and fetch for submodules is in place). So here we need a way to transport this configuration for a fork. Yes for a small project where its feasible to simply clone all submodules you can just say: please fork everything. But for bigger projects thats not necessarily an option. So we should at least give the users that option. Then its a matter of policy how you work with a project. I am not saying that everything for this should be implemented in the first steps but we should keep it in mind and design everything in such a way that it is still possible to implement such a kind of workflow later. Cheers Heiko -- 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 5/9] autoconf: Check for struct itimerval
Jonas 'Sortie' Termansen sor...@maxsi.org writes: The makefile has provisions for this case, so let's detect it in the configure script as well. Signed-off-by: Jonas 'Sortie' Termansen sor...@maxsi.org --- This, 1/9 and later 7/9, are independently good changes to the current codebase, unlike all the other patches that become only necessary if/when we want to migrate to timer_settime(). As such, we would prefer to have these fixes to the current system at the beginning of the series before enhancements to the current system patches. Thanks. configure.ac | 7 +++ 1 file changed, 7 insertions(+) diff --git a/configure.ac b/configure.ac index 31b3218..00842ae 100644 --- a/configure.ac +++ b/configure.ac @@ -761,6 +761,13 @@ AC_CHECK_TYPES([struct timespec], [#include sys/time.h]) GIT_CONF_SUBST([NO_STRUCT_TIMESPEC]) # +# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval. +AC_CHECK_TYPES([struct itimerval], +[NO_STRUCT_ITIMERVAL=], +[NO_STRUCT_ITIMERVAL=UnfortunatelyYes], +[#include sys/time.h]) +GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL]) +# # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent. AC_CHECK_MEMBER(struct dirent.d_ino, [NO_D_INO_IN_DIRENT=], -- 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 9/9] Use timer_settime for new platforms
Jonas 'Sortie' Termansen sor...@maxsi.org writes: setitimer() is an obsolescent XSI interface and may be removed in a future standard. Applications should use the core POSIX timer_settime() instead. This patch cleans up the progress reporting and changes it to try using timer_settime, or if that fails, setitimer. If either function is not provided by the system, then git-compat-util.h provides replacements that always fail with ENOSYS. It's important that code doesn't simply check if timer_settime is available as it can give false positives. Some systems like contemporary OpenBSD provides the function, but it unconditionally fails with ENOSYS at runtime. This approach allows the code using timer_settime() and setitimer() to be simple and readable. My first attempt used #ifdef around each use of timer_settime(), this quickly turned a into unmaintainable maze of preprocessor conditionals. Signed-off-by: Jonas 'Sortie' Termansen sor...@maxsi.org --- builtin/log.c | 47 --- progress.c| 34 +++--- 2 files changed, 67 insertions(+), 14 deletions(-) Yuck. I didn't look at the change very carefully, but are the two interface so vastly different that you cannot emulate one in terms of the other, and use a single API at the callsites, isolating the knowledge of which kind of API is used to interact with the system timer in one place (perhaps in compat/itimer.c)? Having to sprinkle if (is_using_timer_settime) around means we need to support two APIs at each and every callsite that wants timer interrupt actions. -- 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: revert top most commit
On Wed, 2014-08-27 at 17:22 -0700, Jonathan Nieder wrote: Keller, Jacob E wrote: On Wed, 2014-08-27 at 21:14 +, Keller, Jacob E wrote: I am having trouble using revert. If I attempt to $ git revert sha1id where sha1id is the same as the HEAD commit, I instead get the output of what looks like git status. [...] It's actually not my repository, I was helping a co-worker, and thought I'd ask if anyone here knew if it was expected behavior or not. More details about the output would help --- otherwise people have to guess[*]. I'm guessing your co-worker's working tree is not clean. Commiting or stashing their changes first might get things working. Hope that helps, Jonathan [*] Another nice thing about quoting output is that it becomes more obvious what about it wasn't helpful, which sometimes leads to patches from kind people on the list to fix it. -- 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 I will see if I can get the actual output. Thanks, Jake
Re: revert top most commit
On Wed, 2014-08-27 at 17:22 -0700, Jonathan Nieder wrote: Keller, Jacob E wrote: On Wed, 2014-08-27 at 21:14 +, Keller, Jacob E wrote: I am having trouble using revert. If I attempt to $ git revert sha1id where sha1id is the same as the HEAD commit, I instead get the output of what looks like git status. [...] It's actually not my repository, I was helping a co-worker, and thought I'd ask if anyone here knew if it was expected behavior or not. More details about the output would help --- otherwise people have to guess[*]. I'm guessing your co-worker's working tree is not clean. Commiting or stashing their changes first might get things working. Hope that helps, Jonathan [*] Another nice thing about quoting output is that it becomes more obvious what about it wasn't helpful, which sometimes leads to patches from kind people on the list to fix it. -- 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 # Command, where $git revert b718c4d508204b7f46b147c7c47c51fe7a2c7e31On # Output branch master Your branch is up-to-date with 'origin/master'. nothing to commit, working directory clean I also just got one key piece of information regarding this, the patch itself was blank! For backstory, the issue is that he pushed to a tree which does not allow non-fastforward updates (so he can't rewind). He was using stacked git, and accidentally pushed an empty patch, and wanted to revert it so that at least the log showed that we had removed it (even though the patch itself was empty). At minimum, this output from git revert could be made more clear about why it's failing on an empty patch. Regards, Jake
Re: [PATCH 1/6] Refactor type_from_string() to avoid die()ing in case of errors
Johannes Schindelin johannes.schinde...@gmx.de writes: In the next commits, we will enhance the fsck_tag() function to check tag objects more thoroughly. To this end, we need a function to verify that a given string is a valid object type, but that does not die() in the negative case. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- object.c | 13 - object.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/object.c b/object.c index a16b9f9..5eee592 100644 --- a/object.c +++ b/object.c @@ -33,13 +33,24 @@ const char *typename(unsigned int type) return object_type_strings[type]; } -int type_from_string(const char *str) +int type_from_string_gently(const char *str) { int i; for (i = 1; i ARRAY_SIZE(object_type_strings); i++) if (!strcmp(str, object_type_strings[i])) return i; + + return -1; +} + +int type_from_string(const char *str) +{ + int i = type_from_string_gently(str); + + if (i = 0) + return i; + die(invalid object type \%s\, str); } Hmph. The above is not wrong per-se, but I would have done int type_from_string_gently(const char *str, int gentle); #define type_from_string(str) type_from_string_gently((str), 1) and added two lines to renamed type_from_string() function + if (gently) + return -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: [PATCH 2/6] Accept object data in the fsck_object() function
Johannes Schindelin johannes.schinde...@gmx.de writes: When fsck'ing an incoming pack, we need to fsck objects that cannot be read via read_sha1_file() because they are not local yet (and might even be rejected if transfer.fsckobjects is set to 'true'). For commits, there is a hack in place: we basically cache commit objects' buffers anyway, but the same is not true, say, for tag objects. By refactoring fsck_object() to take the object buffer and size as optional arguments -- optional, because we still fall back to the previous method to look at the cached commit objects if the caller passes NULL -- we prepare the machinery for the upcoming handling of tag objects. The assumption that such buffers are inherently NUL terminated is now wrong, of course, hence we pass the size of the buffer so that we can add a sanity check later, to prevent running past the end of the buffer. A nice side effect may be that we can now check (and perhaps warn) a commit buffer with a NUL inside, perhaps? I am not suggesting to add such a check to this series, but mentioning the possibilty here may have a merit. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- builtin/fsck.c | 2 +- builtin/index-pack.c | 3 ++- builtin/unpack-objects.c | 14 ++ fsck.c | 24 +++- fsck.h | 4 +++- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index d42a27d..d9f4e6e 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -298,7 +298,7 @@ static int fsck_obj(struct object *obj) if (fsck_walk(obj, mark_used, NULL)) objerror(obj, broken links); - if (fsck_object(obj, check_strict, fsck_error_func)) + if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func)) return -1; if (obj-type == OBJ_TREE) { diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 5568a5b..f2465ff 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -773,7 +773,8 @@ static void sha1_object(const void *data, struct object_entry *obj_entry, if (!obj) die(_(invalid %s), typename(type)); if (do_fsck_object - fsck_object(obj, 1, fsck_error_function)) + fsck_object(obj, buf, size, 1, + fsck_error_function)) die(_(Error in object)); if (fsck_walk(obj, mark_link, NULL)) die(_(Not all child objects of %s are reachable), sha1_to_hex(obj-sha1)); diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 99cde45..855d94b 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -164,10 +164,10 @@ static unsigned nr_objects; * Called only from check_object() after it verified this object * is Ok. */ -static void write_cached_object(struct object *obj) +static void write_cached_object(struct object *obj, struct obj_buffer *obj_buf) { unsigned char sha1[20]; - struct obj_buffer *obj_buf = lookup_object_buffer(obj); + if (write_sha1_file(obj_buf-buffer, obj_buf-size, typename(obj-type), sha1) 0) die(failed to write object %s, sha1_to_hex(obj-sha1)); obj-flags |= FLAG_WRITTEN; @@ -180,6 +180,8 @@ static void write_cached_object(struct object *obj) */ static int check_object(struct object *obj, int type, void *data) { + struct obj_buffer *obj_buf; + if (!obj) return 1; @@ -198,11 +200,15 @@ static int check_object(struct object *obj, int type, void *data) return 0; } - if (fsck_object(obj, 1, fsck_error_function)) + obj_buf = lookup_object_buffer(obj); + if (!obj_buf) + die(Whoops! Cannot find object '%s', sha1_to_hex(obj-sha1)); + if (fsck_object(obj, obj_buf-buffer, obj_buf-size, 1, + fsck_error_function)) die(Error in object); if (fsck_walk(obj, check_object, NULL)) die(Error on reachable objects of %s, sha1_to_hex(obj-sha1)); - write_cached_object(obj); + write_cached_object(obj, obj_buf); return 0; } diff --git a/fsck.c b/fsck.c index 56156ff..dd77628 100644 --- a/fsck.c +++ b/fsck.c @@ -277,7 +277,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f } static int fsck_commit_buffer(struct commit *commit, const char *buffer, - fsck_error error_func) + unsigned long size, fsck_error error_func) { unsigned char tree_sha1[20], sha1[20]; struct commit_graft *graft; @@ -322,15 +322,18 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer, return 0; } -static int fsck_commit(struct commit
Re: [PATCH 3/6] Make sure fsck_commit_buffer() does not run out of the buffer
Johannes Schindelin johannes.schinde...@gmx.de writes: So far, we assumed that the buffer is NUL terminated, but this is not a safe assumption, now that we opened the fsck_object() API to pass a buffer directly. So let's make sure that there is at least an empty line in the buffer. That way, our checks would fail if the empty line was encountered prematurely, and consequently we can get away with the current string comparisons even with non-NUL-terminated buffers are passed to fsck_object(). Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- Heh, I probably should have read this one before commenting on 2/6. It makes me feel somewhat uneasy to insist that there must be a blank line in the commit object, even though from the very first implementation of commit-tree I think we have always had a blank there at the end of the header, even when you feed nothing as the message to it. I think the new check is OK, but the message should be s/empty line/end of header/ or something. It is not like we require an empty line in the log message proper. fsck.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/fsck.c b/fsck.c index dd77628..db6aaa4 100644 --- a/fsck.c +++ b/fsck.c @@ -237,6 +237,26 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func) return retval; } +static int must_have_empty_line(const void *data, unsigned long size, + struct object *obj, fsck_error error_func) +{ + const char *buffer = (const char *)data; + int i; + + for (i = 0; i size; i++) { + switch (buffer[i]) { + case '\0': + return error_func(obj, FSCK_ERROR, + invalid message: NUL at offset %d, i); + case '\n': + if (i + 1 size buffer[i + 1] == '\n') + return 0; + } + } + + return error_func(obj, FSCK_ERROR, invalid buffer: missing empty line); +} + static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func) { char *end; @@ -284,6 +304,9 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer, unsigned parent_count, parent_line_count = 0; int err; + if (must_have_empty_line(buffer, size, commit-object, error_func)) + return -1; + if (!skip_prefix(buffer, tree , buffer)) return error_func(commit-object, FSCK_ERROR, invalid format - expected 'tree' line); if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n') -- 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 4/6] fsck: check tag objects' headers
Johannes Schindelin johannes.schinde...@gmx.de writes: We inspect commit objects pretty much in detail in git-fsck, but we just glanced over the tag objects. Let's be stricter. This work was sponsored by GitHub Inc. Is it only this commit, or all of these patches in the series? Does GitHub want their name sprinkled over all changes they sponsor? Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- fsck.c | 88 +- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/fsck.c b/fsck.c index db6aaa4..d30946b 100644 --- a/fsck.c +++ b/fsck.c @@ -6,6 +6,7 @@ #include commit.h #include tag.h #include fsck.h +#include refs.h static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data) { @@ -355,6 +356,90 @@ static int fsck_commit(struct commit *commit, const char *data, return ret; } +static int fsck_tag_buffer(struct tag *tag, const char *data, + unsigned long size, fsck_error error_func) +{ + unsigned char commit_sha1[20]; + int ret = 0; + const char *buffer; + char *tmp = NULL, *eol; Call it to_free or something; naming it 'tmp' would tempt people who later touch this code to reuse it temporarily to hold something unrelated (I know they will notice that mistake later, but noticing mistake after wasting time is too late). + if (data) + buffer = data; + else { + enum object_type type; + + buffer = tmp = read_sha1_file(tag-object.sha1, type, size); + if (!buffer) + return error_func(tag-object, FSCK_ERROR, + cannot read tag object); + + if (type != OBJ_TAG) { + ret = error_func(tag-object, FSCK_ERROR, + expected tag got %s, + typename(type)); + goto done; + } + } + + if (must_have_empty_line(buffer, size, tag-object, error_func)) + goto done; + + if (!skip_prefix(buffer, object , buffer)) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - expected 'object' line); + goto done; + } + if (get_sha1_hex(buffer, commit_sha1) || buffer[40] != '\n') { This code is not making the mistake to assume that tagged objects are always commits, so let's not call it commit_sha1; I'd suggest just calling it sha1[] (or even tmp or junk, as the parsed value is not used). + *eol = '\0'; + if (type_from_string_gently(buffer) 0) + ret = error_func(tag-object, FSCK_ERROR, invalid 'type' value); + *eol = '\n'; + if (ret) + goto done; I can see that it is reverted back to '\n' immediately after calling type_from_string_gently(), but it is a bit unfortunate that const char *data needs to be touched this way. Since the callee is introduced in this series, perhaps it can be made to take a counted string? + if (!skip_prefix(buffer, tag , buffer)) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - expected 'tag' line); + goto done; + } + eol = strchr(buffer, '\n'); + if (!eol) { + ret = error_func(tag-object, FSCK_ERROR, invalid format - unexpected end after 'type' line); + goto done; + } + *eol = '\0'; + if (check_refname_format(buffer, REFNAME_ALLOW_ONELEVEL)) + ret = error_func(tag-object, FSCK_ERROR, invalid 'tag' name: %s, buffer); + *eol = '\n'; I actually think this check is harmful. It often matches the name of the ref, but there is nothing inherently refname like in the tag name proper. And I think it is unnecessary. Don't we already warn if it does not match the name of the ref we use to point at it? It would mean that anything that does not conform to the check-refname-format will get a warning one way or the other, either it is pointed at by a ref whose name is kosher and does not match, or a ref itself has a name that does not pass check-refname-format. (goes and looks) Yikes. I assumed too much. We do not seem to do much checking on refs in that (1) After git rev-parse HEAD .git/refs/heads/ee..oo fsck does not complain about the malformed ee..oo branch; (2) After git tag -a foo -m foo cp .git/refs/tags/foo .git/refs/tags/bar fsck does not complain that refs/tags/bar talks about foo But these two are something we would want to fix in a larger context within git fack anyway, so my comments above still stand. + if (!skip_prefix(buffer, tagger , buffer)) { + /* early tags do not contain 'tagger' lines; warn only */ + error_func(tag-object, FSCK_WARN, invalid format - expected 'tagger' line); Nice. Even nicer that this explains why people should not touch 'ret' here. + } + ret =
Re: [PATCH 4/6] fsck: check tag objects' headers
Junio C Hamano gits...@pobox.com writes: +if (check_refname_format(buffer, REFNAME_ALLOW_ONELEVEL)) +ret = error_func(tag-object, FSCK_ERROR, invalid 'tag' name: %s, buffer); +*eol = '\n'; I actually think this check is harmful. Let me take this one back; we do a moral equivalent when we create a tag, like this: strbuf_addf(sb, refs/tags/%s, name); return check_refname_format(sb-buf, 0); So validating using check_refname_format() is indeed a very good thing to do. As you have length and buffer here, I would suggest updating this part of your patch to print into a strbuf strbuf_addf(sb, refs/tags/%.*s, (eol - buffer), buffer); if (check_refname_format(sb.buf)) ret = ... and keep the constness of the incoming data. -- 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: [BUG] resolved deltas
On Mon, Aug 25, 2014 at 06:39:45PM +0200, René Scharfe wrote: Thanks, that looks good. But while preparing the patch I noticed that the added test sometimes fails. Helgrind pointed outet a race condition. It is not caused by the patch to turn the asserts into regular ifs, however -- here's a Helgrind report for the original code with the new test: Interesting. I couldn't convince Helgrind to catch such a case, but it makes sense. We split the delta-resolving work by dividing up the base objects. We then find any deltas that need that base object (which is read-only). If there's only one instances of the base, then we'll be the only thread working on those delta. But if there are two such bases, they're going to look at the same deltas. So we need some kind of mutual exclusion so that only one thread proceeds with resolving the delta. The real_type check sort-of functions in that way (except of course it is not actually thread safe). So one obvious option is just a coarse-grained global lock to modify or check real_type fields. That would probably perform badly (lots of useless lock contention on unrelated objects), but at least it would work. If we accept pushing a lock into _each_ object_entry, then we would get a lot less lock contention (i.e., none at all in the common case of no duplicates). The cost is storing one lock per object, though. Not great, but probably OK. Is there some way we can make real_type itself more atomic? I.e., use it as the mutex with the rule that we do not claim the object_entry as ours to work on until we atomically set delta_obj-real_type. I think doing a compare-and-swap looking for OBJ_REF_DELTA and replacing it with base-real_type would be enough there (and substitute OBJ_OFS_DELTA for the second conditional, of course). However, I'm not sure we can portably rely on having a compare-and-swap primitive (or that we want to go through the hassle of conditionally using it). -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: [BUG] resolved deltas
On Thu, Aug 28, 2014 at 06:08:21PM -0400, Jeff King wrote: So we need some kind of mutual exclusion so that only one thread proceeds with resolving the delta. The real_type check sort-of functions in that way (except of course it is not actually thread safe). Here's a patch which implements that. Since I couldn't replicate the original problem with helgrind, I am just guessing at whether it properly fixes it (well, it is more than just a guess; I used my brain to analyze it, but that is far from foolproof). It uses a single lock. I did a best-of-five timing of git index-pack --verify on the kernel repo, both before and after. The results ended up quite similar (both about 57s), though the run-to-run numbers are all over the place (up to about 65s in the worst case). So maybe it is not so bad. As I implemented, I realized that even with the mutex, I really was just implementing compare_and_swap (and I wrote it that way to make it more obvious). So if we wanted to, it would be trivial to replace the claim_delta function with a true compare-and-swap instruction if the compiler and processor support it. --- diff --git a/builtin/index-pack.c b/builtin/index-pack.c index f7dc5b0..ed9e253 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -112,6 +112,10 @@ static pthread_mutex_t deepest_delta_mutex; #define deepest_delta_lock() lock_mutex(deepest_delta_mutex) #define deepest_delta_unlock() unlock_mutex(deepest_delta_mutex) +static pthread_mutex_t object_entry_mutex; +#define object_entry_lock()lock_mutex(object_entry_mutex) +#define object_entry_unlock() unlock_mutex(object_entry_mutex) + static pthread_key_t key; static inline void lock_mutex(pthread_mutex_t *mutex) @@ -135,6 +139,7 @@ static void init_thread(void) init_recursive_mutex(read_mutex); pthread_mutex_init(counter_mutex, NULL); pthread_mutex_init(work_mutex, NULL); + pthread_mutex_init(object_entry_mutex, NULL); if (show_stat) pthread_mutex_init(deepest_delta_mutex, NULL); pthread_key_create(key, NULL); @@ -157,6 +162,7 @@ static void cleanup_thread(void) pthread_mutex_destroy(read_mutex); pthread_mutex_destroy(counter_mutex); pthread_mutex_destroy(work_mutex); + pthread_mutex_destroy(object_entry_mutex); if (show_stat) pthread_mutex_destroy(deepest_delta_mutex); for (i = 0; i nr_threads; i++) @@ -862,7 +868,6 @@ static void resolve_delta(struct object_entry *delta_obj, { void *base_data, *delta_data; - delta_obj-real_type = base-obj-real_type; if (show_stat) { delta_obj-delta_depth = base-obj-delta_depth + 1; deepest_delta_lock(); @@ -888,6 +893,21 @@ static void resolve_delta(struct object_entry *delta_obj, counter_unlock(); } +static int claim_delta(struct object_entry *delta_obj, + enum object_type delta_type, + enum object_type base_type) +{ + enum object_type old_type; + + object_entry_lock(); + old_type = delta_obj-real_type; + if (old_type == delta_type) + delta_obj-real_type = base_type; + object_entry_unlock(); + + return old_type == delta_type; +} + static struct base_data *find_unresolved_deltas_1(struct base_data *base, struct base_data *prev_base) { @@ -914,7 +934,7 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base, if (base-ref_first = base-ref_last) { struct object_entry *child = objects + deltas[base-ref_first].obj_no; - if (child-real_type == OBJ_REF_DELTA) { + if (claim_delta(child, OBJ_REF_DELTA, base-obj-real_type)) { struct base_data *result = alloc_base_data(); resolve_delta(child, base, result); @@ -930,7 +950,7 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base, if (base-ofs_first = base-ofs_last) { struct object_entry *child = objects + deltas[base-ofs_first].obj_no; - if (child-real_type == OBJ_OFS_DELTA) { + if (claim_delta(child, OBJ_OFS_DELTA, base-obj-real_type)) { struct base_data *result = alloc_base_data(); resolve_delta(child, base, result); -- 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: [BUG] resolved deltas
On Thu, Aug 28, 2014 at 06:08:21PM -0400, Jeff King wrote: Interesting. I couldn't convince Helgrind to catch such a case... Ugh. It helps if you actually helgrind the git binary, and not the shell-script from bin-wrappers. I can easily replicate the problem now. The patch I just posted seems to fix it (at least it has been running in a loop for over a minute with no failures, whereas before it took only a few seconds to provoke a failure). -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 1/2] t9300: test filedelete root
Maxim Bublis sat...@yandex-team.ru writes: Add new fast-import test series for filedelete command. Signed-off-by: Maxim Bublis sat...@yandex-team.ru --- You may have been concentrating on the delete root case, but as long as you claim We add a series to test filedelete command, it would be sensible to test more typical cases of deleting files, not the entire tree as well, no? Perhaps add three paths in the initial commit e.g. hello.c, good/night.txt and good/bye.txt, first remove good/night.txt and validate the result, then remove good/ directory and validate the result, and finally remove the whole thing and validate the result, or something like that? In a patch series that introduces a demonstration of existing breakage and then fixes the breakage in a separate patch, mark the test that shows the known breakage with test_expect_failure and then turn that line into test_expect_success in the later patch that fixes the breakage. What the test checks and the fix in 2/2 both looked sensible from a cursory read. Thanks. t/t9300-fast-import.sh | 46 ++ 1 file changed, 46 insertions(+) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 5fc9ef2..3d557b3 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -3017,4 +3017,50 @@ test_expect_success 'T: empty reset doesnt delete branch' ' git rev-parse --verify refs/heads/not-to-delete ' +### +### series U (filedelete) +### + +cat input INPUT_END +commit refs/heads/U +committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE +data COMMIT +test setup +COMMIT +M 100644 inline hello.c +data BLOB +blob 1 +BLOB + +INPUT_END + +test_expect_success 'U: initialize for U tests' ' + git fast-import input +' + +cat input INPUT_END +commit refs/heads/U +committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE +data COMMIT +must succeed +COMMIT +from refs/heads/U^0 +D + +INPUT_END + +test_expect_success 'U: filedelete root succeeds' ' +git fast-import input +' + +cat expect EOF +:100644 00 c18147dc648481eeb65dc5e66628429a64843327 D hello.c +EOF + +git diff-tree -M -r U^1 U actual + +test_expect_success 'U: validate filedelete result' ' + compare_diff_raw expect actual +' + test_done -- 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] pretty: Provide a strict ISO8601 date format
Beat Bolli bbo...@ewanet.ch writes: It uses the '%aI' and '%cI' format specifiers or the '--date=iso-strict' date format name. OK. See http://article.gmane.org/gmane.comp.version-control.git/255879 for discussion. Please think of a way to explain/justify your changes better before forcing readers to go online. In this case, I think what you wrote in the updates to the documentation would serve as a good basis for it (describe it backwards). +The differences to the strict ISO 8601 format are: + + - a space instead of the `T` date/time delimiter + - a space between time and time zone + - no colon between hours and minutes of the time zone + ... -`--date=iso` (or `--date=iso8601`) shows timestamps in ISO 8601 format. +`--date=iso` (or `--date=iso8601`) shows timestamps in a ISO 8601-like format. Should it be s/a ISO/an ISO/? + else if (mode == DATE_ISO8601_STRICT) + strbuf_addf(timebuf, %04d-%02d-%02dT%02d:%02d:%02d%+03d:%02d, + tm-tm_year + 1900, + tm-tm_mon + 1, + tm-tm_mday, + tm-tm_hour, tm-tm_min, tm-tm_sec, + tz / 100, abs(tz % 100)); Wouldn't this misidentify a zone that is 30 minutes off of GMT, i.e. tz == -30? tz/100 would not be negative and %+03d: would happily show +00:, no? -- 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: [BUG] resolved deltas
On Thu, Aug 28, 2014 at 06:15:18PM -0400, Jeff King wrote: As I implemented, I realized that even with the mutex, I really was just implementing compare_and_swap (and I wrote it that way to make it more obvious). So if we wanted to, it would be trivial to replace the claim_delta function with a true compare-and-swap instruction if the compiler and processor support it. That's something like this on top: diff --git a/builtin/index-pack.c b/builtin/index-pack.c index ed9e253..1782a46 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -897,6 +897,10 @@ static int claim_delta(struct object_entry *delta_obj, enum object_type delta_type, enum object_type base_type) { +#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 + return __sync_bool_compare_and_swap(delta_obj-real_type, delta_type, + base_type); +#else enum object_type old_type; object_entry_lock(); @@ -906,6 +910,7 @@ static int claim_delta(struct object_entry *delta_obj, object_entry_unlock(); return old_type == delta_type; +#endif } static struct base_data *find_unresolved_deltas_1(struct base_data *base, Though I guess gcc's __sync stuff is old, and the new C11 thing is stdatomic.h. I guess we could support either if we can find the right preprocessor macros (I am not even sure if the above is correct; the 4 is for the size of the data type, but this is an enum, so we don't even know what the right size is...). However, the above doesn't seem to be any faster for me, so it may not be worth the hassle. -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: [BUG] resolved deltas
Jeff King p...@peff.net writes: On Thu, Aug 28, 2014 at 06:08:21PM -0400, Jeff King wrote: Interesting. I couldn't convince Helgrind to catch such a case... Ugh. It helps if you actually helgrind the git binary, and not the shell-script from bin-wrappers. I can easily replicate the problem now. The patch I just posted seems to fix it (at least it has been running in a loop for over a minute with no failures, whereas before it took only a few seconds to provoke a failure). Thanks for digging. I'll pick it up and may comment on it in tomorrow's cycle (it is a bit too late for today's cycle, unfortunately). -- 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 2/2] fast-import: fix segfault in store_tree()
Maxim Bublis sat...@yandex-team.ru writes: Branch tree is NULLified by filedelete command if we are trying to delete root tree. Add sanity check and use load_tree() in that case. Signed-off-by: Maxim Bublis sat...@yandex-team.ru --- fast-import.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fast-import.c b/fast-import.c index d73f58c..b77f12c 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1419,7 +1419,7 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b) static void store_tree(struct tree_entry *root) { - struct tree_content *t = root-tree; + struct tree_content *t; unsigned int i, j, del; struct last_object lo = { STRBUF_INIT, 0, 0, /* no_swap */ 1 }; struct object_entry *le = NULL; @@ -1427,6 +1427,10 @@ static void store_tree(struct tree_entry *root) if (!is_null_sha1(root-versions[1].sha1)) return; + if (!root-tree) + load_tree(root) Missing ';' + t = root-tree; + for (i = 0; i t-entry_count; i++) { if (t-entries[i]-tree) store_tree(t-entries[i]); -- 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: problem with def of inet_ntop() in git-compat-util.h as well as other places
On Thu, Aug 28, 2014 at 12:54:30AM -0400, dev wrote: Funny I don't see libcurl anywhere. Thought that was needed? Also the RUNPATH and RPATH look duplicated and slightly borked but the initial data there is correct enough to locate all the libs except for some strange libz issue. The main git binary is not linked with libcurl, only the HTTP and FTP programs. You'd want to check git-remote-http, for instance. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 1/2] read_index_from(): catch out of order entries when reading an index file
On Wed, Aug 27, 2014 at 3:48 PM, Jaime Soriano Pastor jsorianopas...@gmail.com wrote: Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com --- read-cache.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/read-cache.c b/read-cache.c index 7f5645e..1cdb762 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1438,6 +1438,21 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk, return ce; } +static void check_ce_order(struct cache_entry *ce, struct cache_entry *next_ce) +{ + int name_compare = strcmp(ce-name, next_ce-name); + if (0 name_compare) + die(Unordered stage entries in index); + if (!name_compare) { + if (!ce_stage(ce)) + die(Multiple stage entries for merged file '%s', + ce-name); + if (ce_stage(ce) ce_stage(next_ce)) + die(Unordered stage entries for '%s', + ce-name); Perhaps consider dropping capitalization from error messages [1] (despite existing code in read-cache.c having a mix of the two styles). See Error Messages in Documentation/CodingGuidelines. [1]: http://thread.gmane.org/gmane.comp.version-control.git/251715/focus=253209 + } +} + /* remember to discard_cache() before reading a different cache! */ int read_index_from(struct index_state *istate, const char *path) { @@ -1499,6 +1514,9 @@ int read_index_from(struct index_state *istate, const char *path) ce = create_from_disk(disk_ce, consumed, previous_name); set_index_entry(istate, i, ce); + if (i 0) + check_ce_order(istate-cache[i - 1], ce); + src_offset += consumed; } strbuf_release(previous_name_buf); -- 2.0.4.2.g7bc378e.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
[ANNOUNCE] tig-2.0.3
Hello, I am very happy to announce that another bug fix release of Tig is now available. Among the most prominent fixes are readline completion and srefreshing of view after returning from commands when refresh-mode is set to after-command. This release also improves testing so that Tig now has an actual test suite, which although being far from complete has already help to ensure more consistency and avoid regressions. For a complete list of fixes and improvements see the release notes below. With this maintainance release out of the way I plan to merge some of the experimental features such as support for key combos that didn't get finalized for the 2.0 release. What is Tig? Tig is an ncurses-based text-mode interface for git. It functions mainly as a Git repository browser, but can also assist in staging changes for commit at chunk level and act as a pager for output from various Git commands. Resources - - Homepage: http://jonas.nitro.dk/tig/ - Manual: http://jonas.nitro.dk/tig/manual.html - Tarballs: http://jonas.nitro.dk/tig/releases/ - Git URL: git://github.com/jonas/tig.git - Gitweb: http://repo.or.cz/w/tig.git - QA: http://stackoverflow.com/questions/tagged/tig Release notes - Improvement: - Add `:save-display file` prompt command to save the current display. - Add `:script file` prompt command for scripting the Tig UI. - Add test framework and convert existing tests to use it. - Add command-line option for starting in refs view: `tig refs`. (GH #309) - Make blame commit ID colors stable across reloads. (GH #303) - Increase blame ID and graph rendering color palette to 14 colors. - New setting 'split-view-width' controls the width for vertical splits. It takes the width of the right-most view either as a number or a percentage. - Expose settings holding command line argument lists: `file-args`, `rev-args`, and `cmdline-args`. They are mainly intended for testing purposes but also allows to change the filtering arguments dynamically. (GH #306) - Add `log-options` setting for specifying default log view options. Example: `set log-options = --pretty=fuller`. - Use option specific view flags to reload view after `:set` commands. Bug fixes: - Refresh the current view when returning from an external command and `refresh-mode=after-command`. (GH #289) - Fix readline completion. - Fix '/' to `find-next` when readline support is enabled. (GH #302) - Fix readline prompt to correctly handle UTF-8 characters. - Add warnings for more obsolete actions and colors. - Fix passing of commit IDS via stdin to the main view. - Fix commit title overflow drawing for multibyte text. (GH #307) - Fix installation directory permissions. - Handle binary files matches reported by git-grep. - Toggling of args-typed options without any arguments will clear the current arguments. Example: `:toggle blame-options`. - Detect custom `pretty.format` settings that break the log view and fallback to use the `medium` format. (GH #225) - Fix invocation of git-diff for the blame view's line tracking. (GH #316) - Fix blame completion of directory names. (GH #317) - Fix display of conflicts in the main view when 'show-changes' is enabled. - Fix off-by-one error when displaying line numbers in the grep view. - When showing the commit graph ensure that either topo, date or author-date commit order is used. (Debian #757692) (GH #238) Change summary -- The diffstat and log summary for changes made in this release. .gitignore| 3 +- .travis.yml | 4 +- Makefile |30 +- NEWS.adoc |43 + contrib/tig-completion.bash |38 +- doc/manual.adoc | 2 +- doc/tig.1.adoc|19 + doc/tigrc.5.adoc |74 +- include/tig/display.h |18 +- include/tig/graph.h | 2 +- include/tig/io.h | 4 +- include/tig/keys.h| 1 + include/tig/line.h| 7 + include/tig/options.h |15 +- include/tig/prompt.h | 8 + include/tig/refdb.h | 1 + include/tig/watch.h | 2 +- src/argv.c|22 +- src/blame.c |16 +- src/diff.c| 4 +- src/display.c | 206 +- src/draw.c|56 +- src/grep.c|36 +- src/keys.c| 1 - src/line.c| 7 +