[PATCH 7/6?] t4012: use 'printf' instead of 'dd' to generate a binary file

2012-07-12 Thread Johannes Sixt 
From: Johannes Sixt j...@kdbg.org

For some reason, 'echo X | dd bs=1k seek=1' creates a file with 2050 bytes
on Windows instead of the expected 1026 bytes, so that a test fails. Since
the actual contents of the file are irrelevant as long as there is at
least one zero byte so that the diff machinery recognizes it as binary,
use printf to generate it.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 While the focus is on t4012, maybe you can add this patch to the series.

 t/t4012-diff-binary.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 6cebb39..8c018ab 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -125,7 +125,7 @@ cat expect EOF
 EOF
 
 test_expect_success 'diff --stat with binary files and big change count' '
-   echo X | dd of=binfile bs=1k seek=1 
+   printf \01\00%1024d 1 binfile 
git add binfile 
i=0 
while test $i -lt 1; do
-- 
1.7.11.1.1304.g11834c6
--
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: Mini bug report origin/pu: t1512 failed on Mac OS X (commit 957d74062c1f0e ?)

2012-07-12 Thread Johannes Sixt 
Am 7/12/2012 1:30, schrieb Junio C Hamano:
 - test $(wc -l actual) = 16 
 + test $(wc -l actual) = 16 

We have a helper function for this:

test_line_count = 16 actual 

-- Hannes
--
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/4 v8] config: read (but not write) from $XDG_CONFIG_HOME/git/config file

2012-07-12 Thread Thomas Rast 
Hi all,

This patch causes valgrind warnings in t1300.81 (get --path copes with
unset $HOME) about passing NULL to access():

==25286== Syscall param access(pathname) points to unaddressable byte(s)
==25286==at 0x56E2227: access (in /lib64/libc-2.14.1.so)
==25286==by 0x48CA42: git_config_early (config.c:948)
==25286==by 0x4F0C20: check_repository_format_gently (setup.c:369)
==25286==by 0x4F1A52: setup_git_directory_gently_1 (setup.c:531)
==25286==by 0x4F24ED: setup_git_directory_gently (setup.c:725)
==25286==by 0x405D8C: run_builtin (git.c:287)
==25286==by 0x40548E: main (git.c:467)
==25286==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

AFAICT it is this bit of code:

Matthieu Moy matthieu@imag.fr writes:

 +void home_config_paths(char **global, char **xdg, char *file)
 +{
 + char *xdg_home = getenv(XDG_CONFIG_HOME);
 + char *home = getenv(HOME);
 + char *to_free = NULL;
 +
 + if (!home) {
 + if (global)
 + *global = NULL;
[...]
  int git_config_early(config_fn_t fn, void *data, const char *repo_config)
  {
   int ret = 0, found = 0;
 - const char *home = NULL;
 + char *xdg_config = NULL;
 + char *user_config = NULL;
 +
 + home_config_paths(user_config, xdg_config, config);
  
   if (git_config_system()  !access(git_etc_gitconfig(), R_OK)) {
   ret += git_config_from_file(fn, git_etc_gitconfig(),
 @@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const 
 char *repo_config)
   found += 1;
   }
  
 - home = getenv(HOME);
 - if (home) {
 - char buf[PATH_MAX];
 - char *user_config = mksnpath(buf, sizeof(buf), %s/.gitconfig, 
 home);
 - if (!access(user_config, R_OK)) {
 - ret += git_config_from_file(fn, user_config, data);
 - found += 1;
 - }
 + if (!access(xdg_config, R_OK)) {
 + ret += git_config_from_file(fn, xdg_config, data);
 + found += 1;
 + }
 +
 + if (!access(user_config, R_OK)) {
 + ret += git_config_from_file(fn, user_config, data);
 + found += 1;
   }

That is, while the old code was careful about home==NULL, the new code
checks this only in home_config_paths(); after that, it does no further
NULL check.

There's a similar instance in your changes to cmd_config(), found by
running 'unset HOME; valgrind git config --global --get foo.bar':

==27841== Syscall param access(pathname) points to unaddressable byte(s)
==27841==at 0x56E2227: access (in /lib64/libc-2.14.1.so)
==27841==by 0x425280: cmd_config (config.c:390)
==27841==by 0x405C34: run_builtin (git.c:306)
==27841==by 0x40548E: main (git.c:467)
==27841==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

around here:

 @@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char 
 *prefix)
[...]
 + char *user_config = NULL;
 + char *xdg_config = NULL;
 +
 + home_config_paths(user_config, xdg_config, config);
 +
 + if (access(user_config, R_OK)  !access(xdg_config, R_OK) 

Can you fix this?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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

How can I append authentication with git push ?

2012-07-12 Thread J. Bakshi 

Dear list,

Is there any option to add user-name and password with git push ?
Or any repo wise configuration file where I can save the info, so that
it doesn't ask the credential before every push ?

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 1/3] branch: introduce --set-upstream-to

2012-07-12 Thread Miles Bader 
Junio C Hamano gits...@pobox.com writes:
 is easier to understand, while I think

   git branch branch [start]
 git branch --set-upstream-to=upstream [branch]

Isn't one problem with this that even if a --set-upstream-to option
exists, inevitably some [and I'm guessing, many] people will not be
aware of it (after all, nobody reads documentation more than they have
to), and will attempt to use --set-upstream with an argument
(that's the natural thing to do, after all) -- which may succeed with
weird results ...?

-miles

-- 
One of the lessons of history is that nothing is often a good thing to
do, and always a clever thing to say.  -- Will Durant
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Garbage Collect Error.

2012-07-12 Thread Jeff King 
On Wed, Jun 13, 2012 at 11:27:04AM +0100, Thomas Lucas wrote:

 Hopefully this is the right place to send bug reports... The
 community page http://git-scm.com/community; suggests that it is.

It is the right place. Sorry that you did not get any response before
now.

 During garbage collection (git gc) it encountered the following error:
 
 git gc | git gc --prune :
 
Counting objects: 856758, done.
Delta compression using up to 2 threads.
fatal: Out of memory, malloc failed (tried to allocate 303237121 bytes)
error: failed to run repack

Packing can be memory hungry if you have a lot of large objects (we may
hold several large objects in memory while comparing them for deltas).
It is also worse with 2 threads, as they will be working simultaneously,
but in the same memory space.

 The compression gets over 90% of the way through before this error
 occurs, but I don't think any compression results are kept, because
 when you repeat it has the same amount of work to do.

Right. Nothing is written during compression; we are just coming up with
a list of deltas to perform during the writing phase.

 My system is XP64 2 core with 4Gb of memory and plenty of virtual memory.

Unfortunately, I believe that the msysgit build is 32-bit, which means
you are probably not even getting to use all 4Gb of your address space
(my impression is that without special flags, 32-bit Windows processes
are limited to 2Gb of address space).

I'd first try doing the pack single-threaded by setting the pack.threads
config option to 1. If that doesn't work, you might try setting
pack.windowMemory to limit the delta search based on available memory
(usually it is limited by number of objects). If the large blobs are
ones that do not delta well anyway (e.g., compressed media files), you
might also consider setting the -delta attribute for them to skip
delta compression entirely.

-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] config: fix several access(NULL) calls

2012-07-12 Thread Thomas Rast 
Matthieu Moy matthieu@imag.fr writes:

 When $HOME is unset, home_config_paths fails and returns NULL pointers
 for user_config and xdg_config. Valgrind complains with Syscall param
 access(pathname) points to unaddressable byte(s).

 Don't call blindly access() on these variables, but test them for
 NULL-ness before.

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 This patch causes valgrind warnings in t1300.81 (get --path copes with
 unset $HOME) about passing NULL to access():

 Indeed. The following patch should fix it.

Thanks!

Tested-by: Thomas Rast tr...@student.ethz.ch

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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

Question: git clone --no-checkout behavior

2012-07-12 Thread Bryan Turner 
I've witnessed the following behavior in both git 1.7.6 and 1.7.10.4.

Assume I have a bare clone, some-repo.git. If I run:
- git clone --shared --no-checkout /path/to/some-repo.git shared-repo
- cd shared-repo
- git status

I see that every file in the repository is _staged_ for deletion. I'm
not surprised every file shows deleted; I'm surprised that the
deletion starts out already staged. A git reset unstages all of the
deletions and I'm good to go. I'm just wondering why they're all
staged in the first place; it seems counter-intuitive.

Can anyone shed some light on this?
Bryan Turner
--
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: How can I append authentication with git push ?

2012-07-12 Thread Thiago Farina 
On Thu, Jul 12, 2012 at 5:18 AM, J. Bakshi
joydeep.bak...@infoservices.in wrote:
 Or any repo wise configuration file where I can save the info, so that
 it doesn't ask the credential before every push ?

I'd like to know how to do that too.

It's a pain to have to type username and password every time I need to
push to github. (Linux here btw).
--
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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Paul Gortmaker 
If git am wasn't run with --reject, we assume the end user
knows where to find the patch.  This is normally true for
a single patch, but if the user is processing a mbox with
many patches, they may not have a single broken out patch
handy.  So, provide a helpful hint as to where they can
find the patch to do the manual fixup before eventually
continuing with git add ... ; git am -r.

Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com

diff --git a/git-am.sh b/git-am.sh
index f8b7a0c..32e6ac0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -854,7 +854,10 @@ did you forget to use 'git add'?
fi
if test $apply_status != 0
then
-   eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
+   eval_gettextln Patch failed at $msgnum $FIRSTLINE
+You can try running the following command:
+   patch -p1 --dry-run  $dotest/patch
+in order to possibly get more information on why it failed.
stop_here_user_resolve $this
fi
 
-- 
1.7.9.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] branch: introduce --set-upstream-to

2012-07-12 Thread Junio C Hamano 
Miles Bader mi...@gnu.org writes:

 Junio C Hamano gits...@pobox.com writes:
 is easier to understand, while I think

  git branch branch [start]
 git branch --set-upstream-to=upstream [branch]

 Isn't one problem with this that even if a --set-upstream-to option
 exists, inevitably some [and I'm guessing, many] people will not be
 aware of it (after all, nobody reads documentation more than they have
 to), and will attempt to use --set-upstream with an argument
 (that's the natural thing to do, after all) -- which may succeed with
 weird results ...?

In the part you quoted in the message you are responding to in the
subthread between Jonathan and, I was expressing doubts about his
upon seeing a single argument for operations that need two pieces
of info, sometimes the first one is assumed to be missing and gets
the default, some other times the second one is assumed to be
missing and gets the default design, which I felt would be
unnecessarily confusing.

The issue of possible confusion you raised is real, was discussed in
the main thread of discussion of the earlier round, and has been
addressed in this round of the patch series, I think, with warnings
and/or advises.
--
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] submodules: don't stumble over symbolic links when cloning recursively

2012-07-12 Thread Johannes Sixt 
Am 11.07.2012 23:08, schrieb Jens Lehmann:
 Am 11.07.2012 22:39, schrieb Johannes Sixt:
 At this point we can be in a subdirectory of the worktree. With
 cd_to_toplevel we move up in the directory hierarchy (cd out). Then a
 relative $gitdir or $sm_path now points to the wrong directory. No?
 
 Nope, git submodule will refuse to run in anything but the root of
 the worktree. So we already are at the toplevel and use cd_to_toplevel
 only to resolve any symlinks present in $PWD. Looks like a comment
 explaining that above those lines would be a good idea ... will add one.

Ah, OK. I missed the lack of SUBDIRECTORY_OK=Yes, but noticed a few
cd_to_toplevel sprinkled throughout the script, so I thought it was OK
to be in a subdirectory.

-- Hannes
--
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] config: fix several access(NULL) calls

2012-07-12 Thread Junio C Hamano 
Matthieu Moy matthieu@imag.fr writes:

 When $HOME is unset, home_config_paths fails and returns NULL pointers
 for user_config and xdg_config. Valgrind complains with Syscall param
 access(pathname) points to unaddressable byte(s).

 Don't call blindly access() on these variables, but test them for
 NULL-ness before.

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 This patch causes valgrind warnings in t1300.81 (get --path copes with
 unset $HOME) about passing NULL to access():

 Indeed. The following patch should fix it.

  builtin/config.c | 3 ++-
  config.c | 4 ++--
  2 files changed, 4 insertions(+), 3 deletions(-)

 diff --git a/builtin/config.c b/builtin/config.c
 index e8e1c0a..67945b2 100644
 --- a/builtin/config.c
 +++ b/builtin/config.c
 @@ -387,7 +387,8 @@ int cmd_config(int argc, const char **argv, const char 
 *prefix)
  
   home_config_paths(user_config, xdg_config, config);
  
 - if (access(user_config, R_OK)  !access(xdg_config, R_OK))
 + if (user_config  access(user_config, R_OK) 
 + xdg_config  !access(xdg_config, R_OK))
   given_config_file = xdg_config;

Shouldn't we be using xdg_config, if user_config is NULL and
xdg_config is defined and accessible?

In other words, isn't the objective of this fix is to replace any
call to access(PATH, PERM) whose PATH can potentially be NULL with

(PATH ? access(PATH, PERM) : -1)

because we want to pretend access(PATH, PERM) returned an error,
saying The variable PATH holds a path to the file that is not
accessible, when PATH is NULL?

And that is equivalent to

(!PATH || access(PATH, PERM))

in the context of boolean.  The original

if (access(user_config, R_OK)  !access(xdg_config, R_OK))

becomes 

if ((!user_config || access(user_config, R_OK)) 
!(!xdg_config || access(xdg_config, R_OK)))

and the latter half of it is the same as

(xdg_config  !access(xdg_config, R_OK))

but the former half is not. Shouldn't it be

if ((!user_config || access(user_config, R_OK)) 
(xdg_config  !access(xdg_config, R_OK)))

Confused.

 diff --git a/config.c b/config.c
 index d28a499..6b97503 100644
 --- a/config.c
 +++ b/config.c
 @@ -940,12 +940,12 @@ int git_config_early(config_fn_t fn, void *data, const 
 char *repo_config)
   found += 1;
   }
  
 - if (!access(xdg_config, R_OK)) {
 + if (xdg_config  !access(xdg_config, R_OK)) {
   ret += git_config_from_file(fn, xdg_config, data);
   found += 1;
   }
  
 - if (!access(user_config, R_OK)) {
 + if (user_config  !access(user_config, R_OK)) {
   ret += git_config_from_file(fn, user_config, data);
   found += 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: How can I append authentication with git push ?

2012-07-12 Thread Junio C Hamano 
Thiago Farina tfrans...@gmail.com writes:

 On Thu, Jul 12, 2012 at 5:18 AM, J. Bakshi
 joydeep.bak...@infoservices.in wrote:
 Or any repo wise configuration file where I can save the info, so that
 it doesn't ask the credential before every push ?

 I'd like to know how to do that too.

 It's a pain to have to type username and password every time I need to
 push to github. (Linux here btw).

I never type either when pushing to github, and I've been pushing
there at least twice a day for quite some time (Linux here btw).

I have these in .git/config:

[remote github2]
url = https://github.com/git/git
fetch = +refs/heads/*:refs/remotes/github2/*
pushurl = github.com:git/git.git
push = refs/heads/maint:refs/heads/maint
push = refs/heads/master:refs/heads/master
push = refs/heads/next:refs/heads/next
push = +refs/heads/pu:refs/heads/pu

[remote github]
url = https://github.com/gitster/git
pushurl = github.com:gitster/git.git
mirror

and then this in $HOME/.ssh/config:

Host github.com
User git
IdentityFile ~/.ssh/github

--
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: Question: git clone --no-checkout behavior

2012-07-12 Thread Junio C Hamano 
Bryan Turner btur...@atlassian.com writes:

 I've witnessed the following behavior in both git 1.7.6 and 1.7.10.4.

 Assume I have a bare clone, some-repo.git. If I run:
 - git clone --shared --no-checkout /path/to/some-repo.git shared-repo
 - cd shared-repo
 - git status

I do not recall we *designed* it in such a way that you would commit
an empty tree if you run git commit immediately after making such
a clone.  But I do not think it is a bug, either.

I think the most likely reason nobody even noticed this is because
the expected use scenario for --no-checkout is when user does not
know (and does not care to find out) what branch is checked out (if
nonbare) or marked as the primary (if bare) in the repository she is
cloning from, and will checkout the branch she wants to work on
immediately after cloning, i.e.

git clone -n $over_there here
cd here
# she knows she wants to fork from 'nitfol'
git checkout -t -b frotz origin/nitfol

Not having anything in the $GIT_DIR/index (which is why you see
everything is removed from the index, you will commit an empty
tree in the status output) does not matter in this scenario,
because the first command she invokes will be git checkout.

If you populated $GIT_DIR/index from the tree of HEAD, you would see
everything is deleted in the working tree.  You can simulate it by
doing this:

git clone -n $over_there here
cd here
git read-tree HEAD
git status

But it would not help people who want to check another branch out
immediately after cloning with -n, which is the whole point of the
option, so...
--
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] submodules: don't stumble over symbolic links when cloning recursively

2012-07-12 Thread Jens Lehmann 
Since 69c3051 (submodules: refactor computation of relative gitdir path)
cloning a submodule recursively fails for nested submodules when a
symbolic link is part of the path to the work tree of the superproject.

This happens when module_clone() tries to find the relative paths between
the work tree and the git dir. When a symbolic link in current $PWD points
to a directory that is at a different level, then determining the number
of ../ needed to traverse to the superproject's work tree leads to a
wrong result.

As there is no portable way to say pwd -P, use cd_to_toplevel to remove
the link from $PWD, which fixes this problem.

A test for this issue has been added to t7406.

Reported-by: Bob Halley hal...@play-bow.org
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

The changes in this version are:
- the SYMLINKS prerequisite is used for the new test
- a comment explaining why cd_to_toplevel is needed has been added
- small updates to the commit message

Thanks to J6t for the input.

 git-submodule.sh|  6 --
 t/t7406-submodule-update.sh | 13 +
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 5629d87..dba4d39 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -186,8 +186,10 @@ module_clone()
die $(eval_gettext Clone of '\$url' into submodule path 
'\$sm_path' failed)
fi

-   a=$(cd $gitdir  pwd)/
-   b=$(cd $sm_path  pwd)/
+   # We already are at the root of the work tree but cd_to_toplevel will
+   # resolve any symlinks that might be present in $PWD
+   a=$(cd_to_toplevel  cd $gitdir  pwd)/
+   b=$(cd_to_toplevel  cd $sm_path  pwd)/
# normalize Windows-style absolute paths to POSIX-style absolute paths
case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac
case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dcb195b..ce61d4c 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -636,4 +636,17 @@ test_expect_success 'submodule update properly revives a 
moved submodule' '
)
 '

+test_expect_success SYMLINKS 'submodule update can handle symbolic links in 
pwd' '
+   mkdir -p linked/dir 
+   ln -s linked/dir linkto 
+   (
+   cd linkto 
+   git clone $TRASH_DIRECTORY/super_update_r2 super 
+   (
+   cd super 
+   git submodule update --init --recursive
+   )
+   )
+'
+
 test_done
-- 
1.7.11.1.166.gf56d108
--
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

Export from bzr / Import to git results in a deleted file re-appearing

2012-07-12 Thread Felix Natter 
hi,

I am trying to move freeplane's repository (GPL-project) from bzr to
git, but when I do this:

$ mkdir freeplane-git1
$ cd freeplane-git1
$ git init .
$ bzr fast-export --export-marks=../marks.bzr ../trunk/ | git fast-import 
--export-marks=../marks.git
$ git checkout

then there are no errors, but the resulting working index is broken:
 freeplane-git1/freeplane_plugin_formula/src/org/freeplane/plugin/formula
 contains SpreadSheetUtils.java which belongs to package
 'org.freeplane.plugin.spreadsheet' and which is no longer in the bzr
 trunk that I imported!

The freeplane repo is publically available, so you should easily be able
to reproduce this:
  bzr branch 
bzr://freeplane.bzr.sourceforge.net/bzrroot/freeplane/freeplane_program/trunk

I reported this as a possible bzr-fastimport bug, but didn't get a
response so far:
  https://bugs.launchpad.net/bzr-fastimport/+bug/1014291

Is there maybe a better way to convert a repo from bzr to git?
This seems to be the standard solution on the web.

I am using Bazaar (bzr) 2.5.0dev6, bzr-fastimport 0.13.0 and git 1.7.10.4.

Thanks and Best Regards!
-- 
Felix Natter

--
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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Paul Gortmaker 
On 12-07-12 01:45 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 If git am wasn't run with --reject, we assume the end user
 knows where to find the patch.  This is normally true for
 a single patch,
 
 Not at all.  Whether it is a single or broken, the patch is fed to
 underlying apply from an unadvertised place.

What I meant by this was the difference between:

git am 0001-some-standalone-single.patch
vs.
git am mbox

In the 1st, the standalone patch is 100% clear and easy to access,
because we really don't need/care about the unadvertised place.

Maybe I should have said knows how to get at the single patch?

 
 So, provide a helpful hint as to where they can
 find the patch ...
 
 This is OK, but you may want to give a way to squelch it once the
 user learns where it is by following the usual advice.* thing.
 
 ... to do the manual fixup before eventually
 continuing with git add ... ; git am -r.
 
 This is _NOT_ fine, especially if you suggest patch the user may
 not have, and more importantly does not have a clue why git apply
 rejected it (am does _not_ use patch at all).

I'm not 100% sure I'm following what part here is not OK.  If you
can help me understand that, I'll respin the change accordingly.

Is it the assumption that the user will have the patch
command in /usr/bin not OK, or that the message implies that
git is somehow using /usr/bin/patch is not OK?

In case it helps any, a brief summary of my workflow is this:

git am /tmp/mbox
some random fail halfway in the queue
patch -p1 --dry-run  .git/rebase-apply/patch
# gauge status.  Is patch really invalid, or already applied?
# already applied; git am --skip
# no, if valid, but with minor issues, apply what we can.
patch -p1  .git/rebase-apply/patch
# manually deal with rejects (typically with wiggle)
git add any_new_files
git add -u
git am -r

Paul.
--

 
 

--
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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Junio C Hamano 
Paul Gortmaker paul.gortma...@windriver.com writes:

 On 12-07-12 01:45 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 If git am wasn't run with --reject, we assume the end user
 knows where to find the patch.  This is normally true for
 a single patch,
 
 Not at all.  Whether it is a single or broken, the patch is fed to
 underlying apply from an unadvertised place.

 What I meant by this was the difference between:

   git am 0001-some-standalone-single.patch
 vs.
   git am mbox

 In the 1st, the standalone patch is 100% clear and easy to access,
 because we really don't need/care about the unadvertised place.

It does not matter at all that 0001-foo.patch only has a single
patch.  If you are going to fix up the patch after you saw git am
failed, you will be fixing .git/rebase-apply/patch with your editor
and re-run git am without arguments, at which point git am will
not look at your 0001-foo.patch file at all.

 This is _NOT_ fine, especially if you suggest patch the user may
 not have, and more importantly does not have a clue why git apply
 rejected it (am does _not_ use patch at all).

 I'm not 100% sure I'm following what part here is not OK.  If you
 can help me understand that, I'll respin the change accordingly.

Do not ever mention patch -p1.  It is not the command that git
am uses, and it is not what detected the breakage in the patch.

The command to guide the user to is git apply.
--
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] config: fix several access(NULL) calls

2012-07-12 Thread Matthieu Moy 
Junio C Hamano gits...@pobox.com writes:

 Matthieu Moy matthieu@imag.fr writes:

 When $HOME is unset, home_config_paths fails and returns NULL pointers
 for user_config and xdg_config. Valgrind complains with Syscall param
 access(pathname) points to unaddressable byte(s).

 Don't call blindly access() on these variables, but test them for
 NULL-ness before.

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 This patch causes valgrind warnings in t1300.81 (get --path copes with
 unset $HOME) about passing NULL to access():

 Indeed. The following patch should fix it.

  builtin/config.c | 3 ++-
  config.c | 4 ++--
  2 files changed, 4 insertions(+), 3 deletions(-)

 diff --git a/builtin/config.c b/builtin/config.c
 index e8e1c0a..67945b2 100644
 --- a/builtin/config.c
 +++ b/builtin/config.c
 @@ -387,7 +387,8 @@ int cmd_config(int argc, const char **argv, const char 
 *prefix)
  
  home_config_paths(user_config, xdg_config, config);
  
 -if (access(user_config, R_OK)  !access(xdg_config, R_OK))
 +if (user_config  access(user_config, R_OK) 
 +xdg_config  !access(xdg_config, R_OK))
  given_config_file = xdg_config;

 Shouldn't we be using xdg_config, if user_config is NULL and
 xdg_config is defined and accessible?

I don't think so. If user_config is NULL, it means something went wrong,
because $HOME is unset. In this case, I'd rather die than using some
other configuration file silently (which would be possible if
$XDG_CONFIG_HOME is set), and this is what the code does:

if (user_config  access(user_config, R_OK) 
xdg_config  !access(xdg_config, R_OK))
given_config_file = xdg_config;
else if (user_config)
given_config_file = user_config;
else
die($HOME not set);

Perhaps it could actually be made even clearer with

if (!user_config)
die($HOME not set);
else if (access(user_config, R_OK) 
 xdg_config  !access(xdg_config, R_OK))
given_config_file = xdg_config;
else
given_config_file = user_config;

That said, I don't care very strongly about it.

-- 
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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Paul Gortmaker 
On 12-07-12 02:53 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 On 12-07-12 01:45 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:

 If git am wasn't run with --reject, we assume the end user
 knows where to find the patch.  This is normally true for
 a single patch,

 Not at all.  Whether it is a single or broken, the patch is fed to
 underlying apply from an unadvertised place.

 What I meant by this was the difference between:

  git am 0001-some-standalone-single.patch
 vs.
  git am mbox

 In the 1st, the standalone patch is 100% clear and easy to access,
 because we really don't need/care about the unadvertised place.
 
 It does not matter at all that 0001-foo.patch only has a single
 patch.  If you are going to fix up the patch after you saw git am
 failed, you will be fixing .git/rebase-apply/patch with your editor
 and re-run git am without arguments, at which point git am will
 not look at your 0001-foo.patch file at all.

I think this is where our two thinking paths diverge.  You are
suggesting I edit and fix the patch.  Yes, occasionally I do
that, if it is a trivial context change.  But hand editing a
patch is not for Joe Average, and gets very complicated in all
but the trivial cases.  So, what happens _way_ more often, is that
I want to apply what can be applied, and deal with the rejects
on a one-by-one basis after that.  (BTW, this is not just me;
this patch came about from discussions with other kernel folks.)

 
 This is _NOT_ fine, especially if you suggest patch the user may
 not have, and more importantly does not have a clue why git apply
 rejected it (am does _not_ use patch at all).

 I'm not 100% sure I'm following what part here is not OK.  If you
 can help me understand that, I'll respin the change accordingly.
 
 Do not ever mention patch -p1.  It is not the command that git
 am uses, and it is not what detected the breakage in the patch.

This may be true, but it _is_ the command that I (and others) have
defaulted to using, if for no other reason than ignorance.

 
 The command to guide the user to is git apply.
 

OK.  But I don't see a --dry-run equivalent -- and git apply --check
just gives me a repeat of the same fail messages that git am did.

With patch -p1 --dry-run  I get information that immediately
lets me see whether the patch is viable or not.  Is there a way
to get a similar thing from git apply that I've overlooked?

Paul.
---
--
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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Junio C Hamano 
Paul Gortmaker paul.gortma...@windriver.com writes:

 This is _NOT_ fine, especially if you suggest patch the user may
 not have, and more importantly does not have a clue why git apply
 rejected it (am does _not_ use patch at all).

 I'm not 100% sure I'm following what part here is not OK.  If you
 can help me understand that, I'll respin the change accordingly.
 
 Do not ever mention patch -p1.  It is not the command that git
 am uses, and it is not what detected the breakage in the patch.

 This may be true, but it _is_ the command that I (and others) have
 defaulted to using, if for no other reason than ignorance.
 
 The command to guide the user to is git apply.

 OK.  But I don't see a --dry-run equivalent -- and git apply --check
 just gives me a repeat of the same fail messages that git am did.

 With patch -p1 --dry-run  I get information that immediately
 lets me see whether the patch is viable or not.

What do you mean by viable?  

Independent from the answer to that question...

Running git apply -p1 would by definition give you the same
failure without --dry-run (because you know it already failed), no?
Then you could ask for rejects or attempt to apply with reduced
contexts to git apply all without having to say --dry-run, as
unapplicable change will not be applied.
--
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] config: fix several access(NULL) calls

2012-07-12 Thread Junio C Hamano 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 +   if (user_config  access(user_config, R_OK) 
 +   xdg_config  !access(xdg_config, R_OK))
 given_config_file = xdg_config;

 Shouldn't we be using xdg_config, if user_config is NULL and
 xdg_config is defined and accessible?

 I don't think so. If user_config is NULL, it means something went wrong,
 because $HOME is unset. In this case, I'd rather die than using some
 other configuration file silently (which would be possible if
 $XDG_CONFIG_HOME is set), and this is what the code does:

   if (user_config  access(user_config, R_OK) 
   xdg_config  !access(xdg_config, R_OK))
   given_config_file = xdg_config;
   else if (user_config)
   given_config_file = user_config;
   else
   die($HOME not set);

 Perhaps it could actually be made even clearer with

   if (!user_config)
   die($HOME not set);
   else if (access(user_config, R_OK) 
xdg_config  !access(xdg_config, R_OK))
   given_config_file = xdg_config;
   else
   given_config_file = user_config;

At least the code needs to break lines at different point and a comment.

if (user_config 
access(user_config, R_OK) 
(xdg_config  !access(xdg_config, R_OK)))
/*
 * Even if we have usable XDG stuff, we want to
 * error out on missing HOME!!!
 */
use xdg config;
else if (user_config)
use user config;
else
unusable situation;

But is it really true that we want to error out on missing HOME if
we have usable XDG stuff?
--
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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Jeff King 
On Thu, Jul 12, 2012 at 02:32:01PM -0400, Paul Gortmaker wrote:

 In case it helps any, a brief summary of my workflow is this:
 
 git am /tmp/mbox
 some random fail halfway in the queue
 patch -p1 --dry-run  .git/rebase-apply/patch
 # gauge status.  Is patch really invalid, or already applied?
 # already applied; git am --skip
 # no, if valid, but with minor issues, apply what we can.
 patch -p1  .git/rebase-apply/patch
 # manually deal with rejects (typically with wiggle)
 git add any_new_files
 git add -u
 git am -r

This does not in any way address your patch, but you may find it
helpful. My usual next step after git am fails is to run git am -3
and have it do a 3-way merge. This can sometimes resolve issues
entirely, and when conflicts remain, they are placed in the file with
the usual conflict markers (and the conflicted files are marked in the
index, so you can even use git mergetool to help you run an external
tool like xxdiff).

-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] date.c: Fix off by one error in object-header date parsing

2012-07-12 Thread Junio C Hamano 
It is perfectly OK for a valid decimal integer to begin with '9' but
116eb3a (parse_date(): allow ancient git-timestamp, 2012-02-02) did
not express the range correctly.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 date.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/date.c b/date.c
index bf8e088..67b3d66 100644
--- a/date.c
+++ b/date.c
@@ -595,7 +595,7 @@ static int match_object_header_date(const char *date, 
unsigned long *timestamp,
unsigned long stamp;
int ofs;
 
-   if (*date  '0' || '9' = *date)
+   if (*date  '0' || '9'  *date)
return -1;
stamp = strtoul(date, end, 10);
if (*end != ' ' || stamp == ULONG_MAX || (end[1] != '+'  end[1] != 
'-'))
-- 
1.7.11.2

--
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: Export from bzr / Import to git results in a deleted file re-appearing

2012-07-12 Thread Jeff King 
On Thu, Jul 12, 2012 at 08:00:17PM +0200, Felix Natter wrote:

 I am trying to move freeplane's repository (GPL-project) from bzr to
 git, but when I do this:
 
 $ mkdir freeplane-git1
 $ cd freeplane-git1
 $ git init .
 $ bzr fast-export --export-marks=../marks.bzr ../trunk/ | git fast-import 
 --export-marks=../marks.git
 $ git checkout
 
 then there are no errors, but the resulting working index is broken:
  freeplane-git1/freeplane_plugin_formula/src/org/freeplane/plugin/formula
  contains SpreadSheetUtils.java which belongs to package
  'org.freeplane.plugin.spreadsheet' and which is no longer in the bzr
  trunk that I imported!

If you run only the bzr half of your command and inspect the output, you
will see that the file in question is mentioned twice.  Once in a commit
on refs/heads/master that renames into it from another file:

  R 
freeplane_plugin_spreadsheet/src/org/freeplane/plugin/spreadsheet/SpreadSheetUtils.java

freeplane_plugin_formula/src/org/freeplane/plugin/formula/SpreadSheetUtils.java

and another similar case creating a merge commit. But it is never
deleted. So from a cursory inspection, it looks like git is right to
include the file in the final output (but I didn't trace the complete
history graph, so it's possible that those commits are somehow not used
in the final result).

Which leads me to believe that the bug is in bzr.

-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] git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Junio C Hamano 
Paul Gortmaker paul.gortma...@windriver.com writes:

 I think this is where our two thinking paths diverge.  You are
 suggesting I edit and fix the patch.  Yes, occasionally I do
 that, if it is a trivial context change.  But hand editing a
 patch is not for Joe Average, and gets very complicated in all
 but the trivial cases.

In your patch, you do not special case and refrain from giving the
location of the patchfile when there is only one patch in the input,
so the above does not matter anyway.

The patch does two unrelated things: reveal the location of the
actual patchfile that failed to apply which was so far kept sekrit,
and tell the user what to do with it.

Because a user who _wants to_ use a patch, once she knows where it
is, would know her favorite way of working with it (be it by editing
it and reapplying, running git apply with --reject or reduced
context lines, or running patch), an advice on _what_ to do is of
secondary importance between the two.  Perhaps we can postpone the
discussion on that and first update the code to tell _where_ the
patch is to the user?  That would be an improvement from the current
codebase no matter what your faviourite workflow is.
--
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] Re: git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Nicolas Sebrecht 
The 12/07/12, Junio C Hamano wrote:

 It does not matter at all that 0001-foo.patch only has a single
 patch.  If you are going to fix up the patch after you saw git am
 failed, you will be fixing .git/rebase-apply/patch with your editor
 and re-run git am without arguments, at which point git am will
 not look at your 0001-foo.patch file at all.

Hugh! Didn't know that.

Is it actually expected from users to manually edit
.git/rebase-apply/patch path? I can't find any reference about that in
the documentation and it really sounds like interfering with the git
internals.

Shouldn't git-am/git-rebase expose this to the user (I'm thinking about
something like

  git am --edit-offending-patch
  git am --fix-patch

)?

-- 
Nicolas Sebrecht
--
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] Re: git-am: indicate where a failed patch is to be found.

2012-07-12 Thread Junio C Hamano 
Nicolas Sebrecht nicolas.s@gmx.fr writes:

 The 12/07/12, Junio C Hamano wrote:

 It does not matter at all that 0001-foo.patch only has a single
 patch.  If you are going to fix up the patch after you saw git am
 failed, you will be fixing .git/rebase-apply/patch with your editor
 and re-run git am without arguments, at which point git am will
 not look at your 0001-foo.patch file at all.

 Hugh! Didn't know that.

 Is it actually expected from users to manually edit
 .git/rebase-apply/patch path? I can't find any reference about that in
 the documentation and it really sounds like interfering with the git
 internals.

 Shouldn't git-am/git-rebase expose this to the user (I'm thinking about
 something like

   git am --edit-offending-patch
   git am --fix-patch

I doubt it would be very useful.  As Paul says, it is a powerful way
to work, but it is not for everybody, and more importantly, it is
not the only way to work with the patch, once the user knows where
it is.

The first problem before any of that is that we didn't tell the user
where the patch is.  You can re-run git am with different options
like reject, -3, and/or with a reduced context and many cases are
handled without having to know where the patch is at all, but if the
user starts wanting to know where the patch is because she wants to
do things beyond that, we should just tell her where it is, instead
of adding a yet another option to run an editor on it, still without
telling her where it is.
--
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: Question: git clone --no-checkout behavior

2012-07-12 Thread Junio C Hamano 
Bryan Turner btur...@atlassian.com writes:

 If you populated $GIT_DIR/index from the tree of HEAD, you would see
 everything is deleted in the working tree.  You can simulate it by
 doing this:

 git clone -n $over_there here
 cd here
 git read-tree HEAD
 git status

 But it would not help people who want to check another branch out
 immediately after cloning with -n, which is the whole point of the
 option, so...

 Is the reset call in my example in essence performing that same read-tree,
 when it unstages the changes?

git reset (without any other parameters) reads the HEAD tree into
the index without touching the working tree, so I think it is
probably equivalent.
--
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 cloning paths

2012-07-12 Thread Douglas Garstang 
All,

I'm a relative newcomer to git and I've just inherited a setup where
all of the company's code is in a single git repository. Within this
repository are multiple projects. It seems that git doesn't natively
allow cloning/checking out of individual paths within the repo (ie
projects), which would seem to make integrating git with a continuous
build system rather difficult. That is, the build system has to clone
the entire repo, and therefore a change to any project will result in
the entire contents of the repo being built.

Correct?

Doug.
--
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 7/6?] t4012: use 'printf' instead of 'dd' to generate a binary file

2012-07-12 Thread Alexander Strasser 
Hi, 

Johannes Sixt wrote:
 From: Johannes Sixt j...@kdbg.org
 
 For some reason, 'echo X | dd bs=1k seek=1' creates a file with 2050 bytes
 on Windows instead of the expected 1026 bytes, so that a test fails. Since
 the actual contents of the file are irrelevant as long as there is at
 least one zero byte so that the diff machinery recognizes it as binary,
 use printf to generate it.
 
 Signed-off-by: Johannes Sixt j...@kdbg.org
 ---
  While the focus is on t4012, maybe you can add this patch to the series.

  Your patch looks good to me and works here. If I hear no
objections I will include it as number 7 when resending this
series.

  Alexander

  t/t4012-diff-binary.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
 index 6cebb39..8c018ab 100755
 --- a/t/t4012-diff-binary.sh
 +++ b/t/t4012-diff-binary.sh
 @@ -125,7 +125,7 @@ cat expect EOF
  EOF
  
  test_expect_success 'diff --stat with binary files and big change count' '
 - echo X | dd of=binfile bs=1k seek=1 
 + printf \01\00%1024d 1 binfile 
   git add binfile 
   i=0 
   while test $i -lt 1; do
 -- 
 1.7.11.1.1304.g11834c6
--
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 3/6] t4012: Actually quote the sed script

2012-07-12 Thread Alexander Strasser 
Hi Zbigniew,

Zbigniew Jędrzejewski-Szmek wrote:
 On 07/12/2012 12:12 AM, Alexander Strasser wrote:
[...]
 I have some spelling corrections (minor, but since you intend to re-roll
 anyway, I'll post them), and one more thing which could be corrected
 (below).
 
 3/6: s/Never the less/Nevertheless/
 4/6: s/masquerading/masking/ (masquerade means to mask oneself)
[...]
   if git apply --stat --summary broken 2detected
   then
  echo unhappy - should have detected an error
 I think this can be changed to:
test_must_fail git apply --stat --summary broken 2detected

  Thank you for your sharp feedback. I will implement the requested
changes for the re-roll.

  Alexander
--
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

[PATCHv2 0/2] git p4: use move command for renames

2012-07-12 Thread Pete Wyckoff 
Difference from v1:
* Remove tee usage in t9814, just send output to a file
  and grep it directly.

Recent p4 supports a move command that records explicitly that
a file was moved from one place to another.  It can be changed a bit
during the move, too.  Use this feature, if it exists, when renames
are detected.

Gary sent these patches months ago, and I've been sitting on them
far too long.  Was hoping to move some other work out first, but it
was not ready.

These commits are on origin/next, as they depend on changes
in pw/git-p4-tests that was merged into next on Jul 3.  They
do not conflict with the other series in flight, notice Jobs: 


Gary Gibbons (2):
  git p4: refactor diffOpts calculation
  git p4: add support for 'p4 move' in P4Submit

 Documentation/git-p4.txt | 10 +++---
 git-p4.py| 86 
 t/t9814-git-p4-rename.sh | 16 -
 3 files changed, 72 insertions(+), 40 deletions(-)

-- 
1.7.11.1.72.g3344471

--
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

[PATCHv2 1/2] git p4: refactor diffOpts calculation

2012-07-12 Thread Pete Wyckoff 
From: Gary Gibbons ggibb...@perforce.com

P4Submit.applyCommit()

To avoid recalculating the same diffOpts for each commit, move it
out of applyCommit() and into the top-level run().  Also fix a bug
in that code which interpreted the value of detectRenames as a
string rather than as a boolean.

[pw: fix documentation, rearrange code a bit]

Signed-off-by: Gary Gibbons ggibb...@perforce.com
Signed-off-by: Pete Wyckoff p...@padd.com
---
 Documentation/git-p4.txt | 10 ++
 git-p4.py| 52 +---
 2 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index fe1f49b..8228f33 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -255,7 +255,7 @@ These options can be used to modify 'git p4 submit' 
behavior.
p4.  By default, this is the most recent p4 commit reachable
from 'HEAD'.
 
--M[n]::
+-M::
Detect renames.  See linkgit:git-diff[1].  Renames will be
represented in p4 using explicit 'move' operations.  There
is no corresponding option to detect copies, but there are
@@ -465,13 +465,15 @@ git-p4.useClientSpec::
 Submit variables
 
 git-p4.detectRenames::
-   Detect renames.  See linkgit:git-diff[1].
+   Detect renames.  See linkgit:git-diff[1].  This can be true,
+   false, or a score as expected by 'git diff -M'.
 
 git-p4.detectCopies::
-   Detect copies.  See linkgit:git-diff[1].
+   Detect copies.  See linkgit:git-diff[1].  This can be true,
+   false, or a score as expected by 'git diff -C'.
 
 git-p4.detectCopiesHarder::
-   Detect copies harder.  See linkgit:git-diff[1].
+   Detect copies harder.  See linkgit:git-diff[1].  A boolean.
 
 git-p4.preserveUser::
On submit, re-author changes to reflect the git author,
diff --git a/git-p4.py b/git-p4.py
index f895a24..5fe509f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1046,27 +1046,8 @@ class P4Submit(Command, P4UserMap):
 
 (p4User, gitEmail) = self.p4UserForCommit(id)
 
-if not self.detectRenames:
-# If not explicitly set check the config variable
-self.detectRenames = gitConfig(git-p4.detectRenames)
-
-if self.detectRenames.lower() == false or self.detectRenames == :
-diffOpts = 
-elif self.detectRenames.lower() == true:
-diffOpts = -M
-else:
-diffOpts = -M%s % self.detectRenames
-
-detectCopies = gitConfig(git-p4.detectCopies)
-if detectCopies.lower() == true:
-diffOpts +=  -C
-elif detectCopies !=  and detectCopies.lower() != false:
-diffOpts +=  -C%s % detectCopies
 
-if gitConfig(git-p4.detectCopiesHarder, --bool) == true:
-diffOpts +=  --find-copies-harder
-
-diff = read_pipe_lines(git diff-tree -r %s \%s^\ \%s\ % 
(diffOpts, id, id))
+diff = read_pipe_lines(git diff-tree -r %s \%s^\ \%s\ % 
(self.diffOpts, id, id))
 filesToAdd = set()
 filesToDelete = set()
 editedFiles = set()
@@ -1433,6 +1414,37 @@ class P4Submit(Command, P4UserMap):
 if self.preserveUser:
 self.checkValidP4Users(commits)
 
+#
+# Build up a set of options to be passed to diff when
+# submitting each commit to p4.
+#
+if self.detectRenames:
+# command-line -M arg
+self.diffOpts = -M
+else:
+# If not explicitly set check the config variable
+detectRenames = gitConfig(git-p4.detectRenames)
+
+if detectRenames.lower() == false or detectRenames == :
+self.diffOpts = 
+elif detectRenames.lower() == true:
+self.diffOpts = -M
+else:
+self.diffOpts = -M%s % detectRenames
+
+# no command-line arg for -C or --find-copies-harder, just
+# config variables
+detectCopies = gitConfig(git-p4.detectCopies)
+if detectCopies.lower() == false or detectCopies == :
+pass
+elif detectCopies.lower() == true:
+self.diffOpts +=  -C
+else:
+self.diffOpts +=  -C%s % detectCopies
+
+if gitConfig(git-p4.detectCopiesHarder, --bool) == true:
+self.diffOpts +=  --find-copies-harder
+
 while len(commits)  0:
 commit = commits[0]
 commits = commits[1:]
-- 
1.7.11.1.72.g3344471

--
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

[PATCHv2 2/2] git p4: add support for 'p4 move' in P4Submit

2012-07-12 Thread Pete Wyckoff 
From: Gary Gibbons ggibb...@perforce.com

For -M option (detectRenames) in P4Submit, use 'p4 move' rather
than 'p4 integrate'.  Check Perforce server for exisitence of
'p4 move' and use it if present, otherwise revert to 'p4 integrate'.

[pw: wildcard-encode src/dest, add/update tests, tweak code]

Signed-off-by: Gary Gibbons ggibb...@perforce.com
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py| 34 ++
 t/t9814-git-p4-rename.sh | 16 
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 5fe509f..b79e6f0 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -120,6 +120,15 @@ def p4_read_pipe_lines(c):
 real_cmd = p4_build_cmd(c)
 return read_pipe_lines(real_cmd)
 
+def p4_has_command(cmd):
+Ask p4 for help on this command.  If it returns an error, the
+   command does not exist in this version of p4.
+real_cmd = p4_build_cmd([help, cmd])
+p = subprocess.Popen(real_cmd, stdout=subprocess.PIPE,
+   stderr=subprocess.PIPE)
+p.communicate()
+return p.returncode == 0
+
 def system(cmd):
 expand = isinstance(cmd,basestring)
 if verbose:
@@ -157,6 +166,9 @@ def p4_revert(f):
 def p4_reopen(type, f):
 p4_system([reopen, -t, type, wildcard_encode(f)])
 
+def p4_move(src, dest):
+p4_system([move, -k, wildcard_encode(src), wildcard_encode(dest)])
+
 #
 # Canonicalize the p4 type and return a tuple of the
 # base type, plus any modifiers.  See p4 help filetypes
@@ -850,6 +862,7 @@ class P4Submit(Command, P4UserMap):
 self.preserveUser = gitConfig(git-p4.preserveUser).lower() == true
 self.isWindows = (platform.system() == Windows)
 self.exportLabels = False
+self.p4HasMoveCommand = p4_has_command(move) 
 
 def check(self):
 if len(p4CmdList(opened ...))  0:
@@ -1046,7 +1059,6 @@ class P4Submit(Command, P4UserMap):
 
 (p4User, gitEmail) = self.p4UserForCommit(id)
 
-
 diff = read_pipe_lines(git diff-tree -r %s \%s^\ \%s\ % 
(self.diffOpts, id, id))
 filesToAdd = set()
 filesToDelete = set()
@@ -1087,17 +1099,23 @@ class P4Submit(Command, P4UserMap):
 editedFiles.add(dest)
 elif modifier == R:
 src, dest = diff['src'], diff['dst']
-p4_integrate(src, dest)
-if diff['src_sha1'] != diff['dst_sha1']:
-p4_edit(dest)
+if self.p4HasMoveCommand:
+p4_edit(src)# src must be open before move
+p4_move(src, dest)  # opens for (move/delete, move/add)
 else:
-pureRenameCopy.add(dest)
+p4_integrate(src, dest)
+if diff['src_sha1'] != diff['dst_sha1']:
+p4_edit(dest)
+else:
+pureRenameCopy.add(dest)
 if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
-p4_edit(dest)
+if not self.p4HasMoveCommand:
+p4_edit(dest)   # with move: already open, writable
 filesToChangeExecBit[dest] = diff['dst_mode']
-os.unlink(dest)
+if not self.p4HasMoveCommand:
+os.unlink(dest)
+filesToDelete.add(src)
 editedFiles.add(dest)
-filesToDelete.add(src)
 else:
 die(unknown modifier %s for %s % (modifier, path))
 
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 84fffb3..3bf1224 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -77,16 +77,16 @@ test_expect_success 'detect renames' '
git commit -a -m Rename file1 to file4 
git diff-tree -r -M HEAD 
git p4 submit 
-   p4 filelog //depot/file4 
-   p4 filelog //depot/file4 | test_must_fail grep -q branch from 

+   p4 filelog //depot/file4 filelog 
+   ! grep  from //depot filelog 
 
git mv file4 file5 
git commit -a -m Rename file4 to file5 
git diff-tree -r -M HEAD 
git config git-p4.detectRenames true 
git p4 submit 
-   p4 filelog //depot/file5 
-   p4 filelog //depot/file5 | grep -q branch from //depot/file4 

+   p4 filelog //depot/file5 filelog 
+   grep  from //depot/file4 filelog 
 
git mv file5 file6 
echo update file6 
@@ -97,8 +97,8 @@ test_expect_success 'detect renames' '
test -n $level  test $level -gt 0  test $level -lt 98 

git config git-p4.detectRenames $(($level + 2)) 
git p4 submit 
-   p4 filelog //depot/file6 
-   p4 filelog //depot/file6 |