Re: problem with def of inet_ntop() in git-compat-util.h as well as other places

2014-08-28 Thread dev


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

2014-08-28 Thread Matthieu Moy
'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

2014-08-28 Thread Duy Nguyen
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

2014-08-28 Thread Jonas 'Sortie' Termansen
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

2014-08-28 Thread Stefan Beller
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Christian Couder
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

2014-08-28 Thread dev


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]

2014-08-28 Thread Coca-Cola/ Fifa Promotion


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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Jeff King
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()

2014-08-28 Thread Maxim Bublis
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

2014-08-28 Thread Maxim Bublis
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

2014-08-28 Thread Maxim Bublis
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

2014-08-28 Thread dev


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

2014-08-28 Thread Steffen Prohaska

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

2014-08-28 Thread Matthieu Moy
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

2014-08-28 Thread Steffen Prohaska

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

2014-08-28 Thread 李祐棠
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

2014-08-28 Thread Johannes Schindelin
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

2014-08-28 Thread Ramsay Jones
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

2014-08-28 Thread Maxim Bublis
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Matthieu Moy
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

2014-08-28 Thread Marc Branchaud
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

2014-08-28 Thread Beat Bolli
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Jonathan Nieder
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Heiko Voigt
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Keller, Jacob E
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

2014-08-28 Thread Keller, Jacob E
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread Jeff King
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

2014-08-28 Thread Junio C Hamano
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()

2014-08-28 Thread Junio C Hamano
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

2014-08-28 Thread brian m. carlson
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

2014-08-28 Thread Eric Sunshine
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

2014-08-28 Thread Jonas Fonseca
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 +