Re: [PATCH] test-lib: avoid full path to store test results

2012-10-30 Thread Elia Pinto
The shell word splitting done in base is a bashism, iow not portable.

Best

2012/10/30, Felipe Contreras felipe.contre...@gmail.com:
 No reason to use the full path in case this is used externally.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  t/test-lib.sh | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/t/test-lib.sh b/t/test-lib.sh
 index 514282c..5a3d665 100644
 --- a/t/test-lib.sh
 +++ b/t/test-lib.sh
 @@ -389,7 +389,8 @@ test_done () {
   then
   test_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
   mkdir -p $test_results_dir
 - test_results_path=$test_results_dir/${0%.sh}-$$.counts
 + base=${0##*/}
 + test_results_path=$test_results_dir/${base%.sh}-$$.counts

   cat $test_results_path -EOF
   total $test_count
 --
 1.8.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


-- 
Inviato dal mio dispositivo mobile
--
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-users] Git clone fails with bad pack header, how to get remote log

2012-10-30 Thread kevin molcard

Hi Konstantin,

thanks for the reply.
The versions of git are:
- on remote: 1.5.6.5
- on windows build machine: 1.7.11.msysgit.1
- on mac build machine: 1.7.3.4

I will try to install latest git version on my remote server and get 
back to you.


thanks again
Kevin

On 10/29/12 6:18 PM, Konstantin Khomoutov wrote:

On Mon, 29 Oct 2012 09:52:54 -0700 (PDT)
Kevin Molcard kev2...@gmail.com wrote:


I have a problem with my build system.

I have a remote server with a relatively large repository (around 12
GB, each branch having a size of 3 GB).

I have also 2 build servers (Mac, Windows) that are cloning the repo
from the remote.

Sometimes (very often when several git clone are sent at the same
time), I have the following error:
 
 remote: internal server error

 fatal: protocol error: bad pack header

I know that it happens when the remote is compressing objects (thanks
to `--progress -v` flags) because the last line of the log before the
erro is:
 remote: Compressing objects:  93% (17959/19284)   [K

  * So I have 2 questions, does anybody what is the problem and what
should I do?
  * Is there a way to get a more precise log from the remote to debug
this problem?

This reminds me of a bug fixed in 1.7.12.1 [1]:

* When git push triggered the automatic gc on the receiving end, a
   message from git prune that said it was removing cruft leaked to
   the standard output, breaking the communication protocol.

In any case, bugs should be reported to the main Git list (which is
git at vger.kernel.org), not here.
I'm Cc'ing the main Git list so you'll get any responses from there, if
any.

Kevin, please answer to this message (keeping all the Ccs -- use Reply
to group or Reply to all in your MUA) and describe exactly what Git
versions on which platforms your have.

1. https://raw.github.com/git/git/master/Documentation/RelNotes/1.7.12.1.txt



--
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] test-lib: avoid full path to store test results

2012-10-30 Thread Jonathan Nieder
Elia Pinto wrote:

 The shell word splitting done in base is a bashism, iow not portable.

No, ${varname##glob} is in POSIX and we already use it here and there.
See Documentation/CodingGuidelines:

   - We use ${parameter#word} and its [#%] siblings, and their
 doubled longest matching form.

Thanks for looking the patch over.
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: Links broken in ref docs.

2012-10-30 Thread Kevin
Any follow-up on this?

On Tue, Oct 23, 2012 at 7:11 AM, Scott Chacon scha...@gmail.com wrote:

 So, this is due to the major AWS outage today.  git-scm.com is hosted
 on Heroku and thus on AWS.  Heroku is continuing to bring up their
 database systems in the wake of the massive AWS outage.  Once that is
 back online, git-scm.com will also be back online.

 As for the git-fetch issue, we'll look into it once Heroku is back online.

 Scott
--
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: Links broken in ref docs.

2012-10-30 Thread Mike Norman
Not seen any recently. I'm guessing the dev is in the path of
hurricane Sandy? (Not sarcasm, btw.)

On Tue, Oct 30, 2012 at 1:04 AM, Kevin i...@ikke.info wrote:
 Any follow-up on this?

 On Tue, Oct 23, 2012 at 7:11 AM, Scott Chacon scha...@gmail.com wrote:

 So, this is due to the major AWS outage today.  git-scm.com is hosted
 on Heroku and thus on AWS.  Heroku is continuing to bring up their
 database systems in the wake of the massive AWS outage.  Once that is
 back online, git-scm.com will also be back online.

 As for the git-fetch issue, we'll look into it once Heroku is back online.

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


RE: [PATCH v2] fix 'make test' for HP NonStop

2012-10-30 Thread Joachim Schmitz
 From: Jeff King [mailto:p...@peff.net]
 Sent: Monday, October 29, 2012 8:07 AM
 To: Joachim Schmitz
 Cc: git@vger.kernel.org
 Subject: Re: [PATCH v2] fix 'make test' for HP NonStop
 
 On Thu, Oct 25, 2012 at 12:57:10PM +0200, Joachim Schmitz wrote:
 
  diff --git a/Makefile b/Makefile
  index f69979e..35380dd 100644
  --- a/Makefile
  +++ b/Makefile
  @@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
  MKDIR_WO_TRAILING_SLASH = YesPlease
  # RFE 10-120912-4693 submitted to HP NonStop development.
  NO_SETITIMER = UnfortunatelyYes
  +
  +   # for 'make test'
  +   # some test don't work with /bin/diff, some fail with /bin/tar
  +   # some need bash, and some need /usr/local/bin in PATH first
  +   SHELL_PATH=/usr/local/bin/bash
  +   SANE_TOOL_PATH=/usr/local/bin
 
 I think we can drop these comments, as the reasoning really should just
 go in the commit message.

OK by me.
 
  +   # as of H06.25/J06.14, we might better use this
  +   #SHELL_PATH=/usr/coreutils/bin/bash
  +   #SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin
 
 Is there any reason not to put both into the default SANE_TOOL_PATH? If
 /usr/coreutils/bin does not exist on older versions, it will be a
 harmless no-op. I guess we arestuck with picking one $SHELL_PATH,
 though.

And because of that have to modify something anyway...
But I don't really mind about an extended SANE_TOOL_PATH
 
 -Peff

Bye, Jojo

-- 8 --
This fixes the vast majority of test failures on HP NonStop.
Some test don't work with /bin/diff, some fail with /bin/tar,
so let's put /usr/local/bin in PATH first. 
Some tests fail with /bin/sh (link to /bin/ksh) so use bash instead

Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---

Makefile | 9 +
1 file changed, 9 insertions(+)

diff --git a/Makefile b/Makefile
index f69979e..35380dd 100644
--- a/Makefile
+++ b/Makefile
@@ -1381,6 +1381,10 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
MKDIR_WO_TRAILING_SLASH = YesPlease
# RFE 10-120912-4693 submitted to HP NonStop development.
NO_SETITIMER = UnfortunatelyYes
+   SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin 
+   SHELL_PATH=/usr/local/bin/bash
+   # as of H06.25/J06.14, we might better use this
+   #SHELL_PATH=/usr/coreutils/bin/bash
endif
ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;

--
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 objects/info/alternates doesn't work on remote SMB repo

2012-10-30 Thread Orgad Shaneh
On Thu, Aug 30, 2012 at 3:34 PM, Orgad and Raizel Shaneh
org...@gmail.com wrote:

 On Thu, Aug 30, 2012 at 4:22 PM, Nguyen Thai Ngoc Duy pclo...@gmail.com
 wrote:
  On Thu, Aug 30, 2012 at 8:12 PM, Orgad and Raizel Shaneh
  org...@gmail.com wrote:
  Could be path normalization. What does git rev-parse --git-dir say?
  Try to run it at top working directory and a subdirectory as well.
 
  If you set GIT_OBJECT_DIRECTORY environment variable to
  //server/share/foo/repo/.git/objects, does it work?
 
  git rev-parse --git-dir in a subdirectory has //server
 
  Hmm where is your git repository? That does not look like a git
  repository's path.
 

 Let me try to explain again.
 I have /d/share/bare, which is a bare repository, and /d/share/repo
 which is a clone with a relative path to bare/.git/objects in its
 .git/objects/info/alternates

 D:\share is configured as a SMB shared folder. It is accessed using
 //server/share.
 I do not clone from this directory, but work directly in it using 'cd
 //server/share', then performing git operations.

  setting GIT_OBJECT_DIRECTORY prints fatal: bad object HEAD on git
  status.
 
  I guessed you put your repo in .../repo/.git, but I was probably
  wrong. Try setting again, pointing GIT_OBJECT_DIRECTORY to the
  objects directory inside your repository. I just want to make see if
  it's because git miscalculates this path. If setting the env variable
  works, then it probably does.
  --
  Duy

 Same result. fatal: bad object HEAD. Tried even using a full (local)
 path to the objects dir.

 - Orgad

Any news? This still doesn't work with 1.8.0.

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


Password parsing fix on windows

2012-10-30 Thread Aleksey Vasenev
 

patch.diff
Description: Binary data


[PATCH] update-index/diff-index: use core.preloadindex to improve performance

2012-10-30 Thread karsten . blees
'update-index --refresh' and 'diff-index' (without --cached) don't honor
the core.preloadindex setting yet. Porcelain commands using these (such as
git [svn] rebase) suffer from this, especially on Windows.

Use read_cache_preload to improve performance.

Additionally, in builtin/diff.c, don't preload index status if we don't
access the working copy (--cached).

Results with msysgit on WebKit repo (2GB in 200k files):

| update-index | diff-index | rebase
+--++-
msysgit-v1.8.0  |   9.157s |10.536s | 42.791s
+ preloadindex  |   9.157s |10.536s | 28.725s
+ this patch|   2.329s | 2.752s | 15.152s
+ fscache [1]   |   0.731s | 1.171s |  8.877s

[1] https://github.com/kblees/git/tree/kb/fscache-v3

Thanks-to: Albert Krawczyk pro-lo...@optusnet.com.au
Signed-off-by: Karsten Blees bl...@dcon.de
---

Can also be pulled from: 
https://github.com/kblees/git/tree/kb/update-diff-index-preload-upstream

I thought I might send this upstream directly, as its not msysgit related. 
More performance figures (for msysgit) can be found in this discussion: 
https://github.com/pro-logic/git/commit/32c03dd8

Ciao,
Karsten

 builtin/diff-index.c   |  8 ++--
 builtin/diff.c | 12 
 builtin/update-index.c |  1 +
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 2eb32bd..1c737f7 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -41,9 +41,13 @@ int cmd_diff_index(int argc, const char **argv, const 
char *prefix)
if (rev.pending.nr != 1 ||
rev.max_count != -1 || rev.min_age != -1 || rev.max_age != -1)
usage(diff_cache_usage);
-   if (!cached)
+   if (!cached) {
setup_work_tree();
-   if (read_cache()  0) {
+   if (read_cache_preload(rev.diffopt.pathspec.raw)  0) {
+   perror(read_cache_preload);
+   return -1;
+   }
+   } else if (read_cache()  0) {
perror(read_cache);
return -1;
}
diff --git a/builtin/diff.c b/builtin/diff.c
index 9650be2..198b921 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -130,8 +130,6 @@ static int builtin_diff_index(struct rev_info *revs,
usage(builtin_diff_usage);
argv++; argc--;
}
-   if (!cached)
-   setup_work_tree();
/*
 * Make sure there is one revision (i.e. pending object),
 * and there is no revision filtering parameters.
@@ -140,8 +138,14 @@ static int builtin_diff_index(struct rev_info *revs,
revs-max_count != -1 || revs-min_age != -1 ||
revs-max_age != -1)
usage(builtin_diff_usage);
-   if (read_cache_preload(revs-diffopt.pathspec.raw)  0) {
-   perror(read_cache_preload);
+   if (!cached) {
+   setup_work_tree();
+   if (read_cache_preload(revs-diffopt.pathspec.raw)  0) {
+   perror(read_cache_preload);
+   return -1;
+   }
+   } else if (read_cache()  0) {
+   perror(read_cache);
return -1;
}
return run_diff_index(revs, cached);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 74986bf..ada1dff 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -593,6 +593,7 @@ struct refresh_params {
 static int refresh(struct refresh_params *o, unsigned int flag)
 {
setup_work_tree();
+   read_cache_preload(NULL);
*o-has_errors |= refresh_cache(o-flags | flag);
return 0;
 }
-- 
1.8.0.msysgit.0.3.g7d9d98c
--
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] update-index/diff-index: use core.preloadindex to improve performance

2012-10-30 Thread Erik Faye-Lund
On Tue, Oct 30, 2012 at 10:50 AM,  karsten.bl...@dcon.de wrote:
 'update-index --refresh' and 'diff-index' (without --cached) don't honor
 the core.preloadindex setting yet. Porcelain commands using these (such as
 git [svn] rebase) suffer from this, especially on Windows.

 Use read_cache_preload to improve performance.

 Additionally, in builtin/diff.c, don't preload index status if we don't
 access the working copy (--cached).

 Results with msysgit on WebKit repo (2GB in 200k files):

 | update-index | diff-index | rebase
 +--++-
 msysgit-v1.8.0  |   9.157s |10.536s | 42.791s
 + preloadindex  |   9.157s |10.536s | 28.725s
 + this patch|   2.329s | 2.752s | 15.152s
 + fscache [1]   |   0.731s | 1.171s |  8.877s


Wow, awesome results :)

This also makes me want to play around with the fscache stuff a bit;
about an order of magnitude improvement is quite noticeable :)
--
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 v5 00/14] New remote-hg helper

2012-10-30 Thread Chris Webb
Hi. I routinely work with projects in both hg and git, so I'm really
interested in this. Thanks for working on it! I grabbed the latest version
from

  
https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg

and have been trying it out. For the most part, it seems to work very nicely
for the hg repos I have access to and can test against. I've spotted a couple
of issues along the way that I thought would be worth reporting.

The first is really a symptom of a general difference between hg and git: an hg
repository can have multiple heads, whereas a git repo has exactly one head. To
demonstrate:

  $ hg init hgtest  cd hgtest
  $ echo zero foo  hg add foo  hg commit -m zero
  $ echo one foo  hg commit -m one
  $ hg checkout -r 0
  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
  $ echo two foo  hg commit -m two
  created new head
  $ hg log --graph
  @  changeset:   2:ca09651009cb
  |  tag: tip
  |  parent:  0:9f552c53d116
  |  user:Chris Webb ch...@arachsys.com
  |  date:Tue Oct 30 09:33:38 2012 +
  |  summary: two
  |
  | o  changeset:   1:58fad8998339
  |/   user:Chris Webb ch...@arachsys.com
  |date:Tue Oct 30 09:33:25 2012 +
  |summary: one
  |
  o  changeset:   0:9f552c53d116
 user:Chris Webb ch...@arachsys.com
 date:Tue Oct 30 09:33:08 2012 +
 summary: zero

  $ cd ..

Now if I try to convert this:

  $ git clone hg::$PWD/hgtest gittest
  Cloning into 'gittest'...
  WARNING: Branch 'default' has more than one head, consider merging
  Traceback (most recent call last):
File /home/chris/bin/git-remote-hg, line 773, in module
  sys.exit(main(sys.argv))
File /home/chris/bin/git-remote-hg, line 759, in main
  do_list(parser)
File /home/chris/bin/git-remote-hg, line 463, in do_list
  list_branch_head(repo, cur)
File /home/chris/bin/git-remote-hg, line 425, in list_branch_head
  tip = get_branch_tip(repo, cur)
File /home/chris/bin/git-remote-hg, line 418, in get_branch_tip
  return repo.branchtip(branch)
  AttributeError: 'mqrepo' object has no attribute 'branchtip'

Strip the second head and it's fine:

  $ hg -R hgtest strip 2
  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
  saved backup bundle to 
/tmp/hgtest/hgtest/.hg/strip-backup/ca09651009cb-backup.hg
  $ git clone hg::$PWD/hgtest gittest
  Cloning into 'gittest'...
  $

Not sure what the most friendly thing to do here is. Perhaps refuse to
clone/pull from a repo with multiple heads unless you name the specific head
you want?


The second thing I spotted is the behaviour of bookmarks on push:

  $ hg init hgtest  cd hgtest
  $ echo zero foo  hg add foo  hg commit -m zero
  $ hg bookmark development
  $ cd ..
  $ git clone hg::$PWD/hgtest gittest  cd gittest
  Cloning into 'gittest'...
  $ git checkout development
  Branch development set up to track remote branch development from origin.
  Switched to a new branch 'development'
  $ echo one foo  git add foo  git commit -m one
  [development 9f67dc4] one
   1 file changed, 1 insertion(+), 1 deletion(-)
  $ git status
  # On branch development
  # Your branch is ahead of 'origin/development' by 1 commit.
  #
  nothing to commit
  $ git push
  warning: helper reported unexpected status of 
refs/hg/origin/bookmarks/development
  To hg::/tmp/hgtest/hgtest
   * [new branch]  branches/default - branches/default
   * [new branch]  development - development
  $ hg log -R ../hgtest
  changeset:   1:1c0714d93864
  tag: tip
  user:Chris Webb ch...@arachsys.com
  date:Tue Oct 30 09:51:51 2012 +
  summary: one

  changeset:   0:f56c463398ea
  bookmark:development
  user:Chris Webb ch...@arachsys.com
  date:Tue Oct 30 09:50:53 2012 +
  summary: zero

i.e. the development bookmark hasn't been updated by the push. This might be
connected to the warning message

  warning: helper reported unexpected status of 
refs/hg/origin/bookmarks/development

I'm testing with hg 2.2.2 and current git master, so I expect this could be a
python api change in the more recent versions of hg if you don't see the same
behaviour.

Best wishes,

Chris.
--
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 v5 00/14] New remote-hg helper

2012-10-30 Thread Chris Webb
Chris Webb ch...@arachsys.com writes:

 The first is really a symptom of a general difference between hg and git: an 
 hg
 repository can have multiple heads, whereas a git repo has exactly one head.

By this I mean an hg repository without bookmarks or branches can still have
multiple heads, whereas a git branch points at exactly one place. Sorry,
very vague description there!

Cheers,

Chris.
--
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-p4 clone @all error

2012-10-30 Thread Arthur
Hi,

(I am a French student, sorry for my English.)

So, i want import my perforce projet on my server git.

perforce my project tree :

depot
  dev_data
  mainline
  release_1.0
  release_1.0.0

my command is :

git-p4 clone -v --detect-branches //depot@all /home/user/projets/deport

The problem :

Importing revision 7727 (100%)Traceback (most recent call last):
  File /usr/bin/git-p4, line 3183, in module
main()
  File /usr/bin/git-p4, line 3177, in main
if not cmd.run(args):
  File /usr/bin/git-p4, line 3048, in run
if not P4Sync.run(self, depotPaths):
  File /usr/bin/git-p4, line 2911, in run
self.importChanges(changes)
  File /usr/bin/git-p4, line 2618, in importChanges
self.initialParent)
  File /usr/bin/git-p4, line 2198, in commit
epoch = details[time]
KeyError: 'time'

if i make a p4 sync //depot/...#head on my perforce server i've this error : 
Librarian checkout
depot/mainline/xxx/api429decryption.txt failed.
open for read: depot/mainline/xx/api429decryption.txt,v: Le
fichier spcifi est introuvable.

My p4 clone can't checking out files after importing revision..

I hope I was clear ...

Thanks for your help.



--
View this message in context: 
http://git.661346.n2.nabble.com/git-p4-clone-all-error-tp7570219.html
Sent from the git mailing list archive at Nabble.com.
--
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: Links broken in ref docs.

2012-10-30 Thread Holger Hellmuth (IKS)

Am 30.10.2012 09:07, schrieb Mike Norman:

Not seen any recently. I'm guessing the dev is in the path of
hurricane Sandy? (Not sarcasm, btw.)


Do you still see failures? I checked out the website just now and it 
seemed to work flawlessly (at least the links I tried, could not find 
any Sharing or Updating section). New design since I last visited, more 
end-user polish.



--
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: Why does git-commit --template want the template to be modified ?

2012-10-30 Thread Tomas Carnecky
On Tue, 30 Oct 2012 11:53:08 +0100, Francis Moreau francis.m...@gmail.com 
wrote:
 Hi,
 
 I'm using git-commit with the --template option. The template I'm
 given is self sufficient for my purpose but as stated in the
 documentation, git-commit wants the template to be edited otherwise it
 aborts the operation.
 
 Is it possible to change this ?

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


Re: [PATCH v2] fix 'make test' for HP NonStop

2012-10-30 Thread Jeff King
On Tue, Oct 30, 2012 at 10:21:40AM +0100, Joachim Schmitz wrote:

 This fixes the vast majority of test failures on HP NonStop.
 Some test don't work with /bin/diff, some fail with /bin/tar,
 so let's put /usr/local/bin in PATH first. 
 Some tests fail with /bin/sh (link to /bin/ksh) so use bash instead
 
 Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
 ---
 
 Makefile | 9 +
 1 file changed, 9 insertions(+)
 
 diff --git a/Makefile b/Makefile
 index f69979e..35380dd 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1381,6 +1381,10 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
   MKDIR_WO_TRAILING_SLASH = YesPlease
   # RFE 10-120912-4693 submitted to HP NonStop development.
   NO_SETITIMER = UnfortunatelyYes
 + SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin 
 + SHELL_PATH=/usr/local/bin/bash
 + # as of H06.25/J06.14, we might better use this
 + #SHELL_PATH=/usr/coreutils/bin/bash
 endif
 ifneq (,$(findstring MINGW,$(uname_S)))
   pathsep = ;

Your patch was whitespace damaged, but I was able to fix it up. Thanks.

-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: Why does git-commit --template want the template to be modified ?

2012-10-30 Thread Johannes Sixt
Am 10/30/2012 11:53, schrieb Francis Moreau:
 I'm using git-commit with the --template option. The template I'm
 given is self sufficient for my purpose but as stated in the
 documentation, git-commit wants the template to be edited otherwise it
 aborts the operation.
 
 Is it possible to change this ?

Perhaps you wanted to use --file instead of --template?

-- 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: crash on git diff-tree -Ganything tree for new files with textconv filter

2012-10-30 Thread Jeff King
On Mon, Oct 29, 2012 at 06:47:05PM -0400, Jeff King wrote:

 On Mon, Oct 29, 2012 at 06:35:21PM -0400, Jeff King wrote:
 
  The patch below fixes it, but it's terribly inefficient (it just detects
  the situation and reallocates). It would be much better to disable the
  reuse_worktree_file mmap when we populate the filespec, but it is too
  late to pass an option; we may have already populated from an earlier
  diffcore stage.
  
  I guess if we teach the whole diff code that -G (and --pickaxe-regex)
  is brittle, we can disable the optimization from the beginning based on
  the diff options. I'll take a look.
 
 Hmm. That is problematic for two reasons.
 
   1. The whole diff call chain will have to be modified to pass the
  options around, so they can make it down to the
  diff_populate_filespec level. Alternatively, we could do some kind
  of global hack, which is ugly but would work OK in practice.
 
   2. Reusing a working tree file is only half of the reason a filespec
  might be mmap'd. It might also be because we are literally diffing
  the working tree. -G was meant to be used to limit log traversal,
  but it also works to reduce the diff output for something like git
  diff HEAD^.
 
 I really wish there were an alternate regexec interface we could use
 that took a pointer/size pair. Bleh.

Thinking on it more, my patch, hacky thought it seems, may not be the
worst solution. Here are the options that I see:

  1. Use a regex library that does not require NUL termination. If we
 are bound by the regular regexec interface, this is not feasible.
 But the GNU implementation works on arbitrary-length buffers (you
 just have to use a slightly different interface), and we already
 carry it in compat. It would mean platforms which provide a working
 but non-GNU regexec would have to start defining NO_REGEX.

  2. Figure out a way to get one extra zero byte via mmap. If the
 requested size does not fall on a page boundary, you get extra
 zero-ed bytes. Unfortunately, requesting an extra byte does not
 do what we want; you get SIGBUS accessing it.

  3. Copy mmap'd data at point-of-use into a NUL-terminated buffer. That
 way we only incur the cost when we need it.

  4. Avoid mmap-ing in the first place when we are using -G or
 --pickaxe-regex (e.g., by doing a big read()). At first glance,
 this sounds more efficient than loading the data one way and then
 making another copy. But mmap+memcpy, aside from the momentary
 doubled memory requirement, is probably just as fast or faster than
 calling read() repeatedly.

I am really tempted by (1).

Given that (2) does not work, unless somebody comes up with something
clever there, that would make (3) the next best choice.

-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: crash on git diff-tree -Ganything tree for new files with textconv filter

2012-10-30 Thread Junio C Hamano
(1) sounds attractive for more than one reason. In addition to avoidance of 
this issue, it would bring bug-to-bug compatibility across platforms.

(4), if we can run grep on streaming data (tweak interface we have for checking 
out a large blob to the working tree), would let us work on dataset larger than 
fit in core. Even though it would be much more work, it might turn out to be a 
better option in the longer run.

Jeff King p...@peff.net wrote:

On Mon, Oct 29, 2012 at 06:47:05PM -0400, Jeff King wrote:

 On Mon, Oct 29, 2012 at 06:35:21PM -0400, Jeff King wrote:
 
  The patch below fixes it, but it's terribly inefficient (it just
detects
  the situation and reallocates). It would be much better to disable
the
  reuse_worktree_file mmap when we populate the filespec, but it is
too
  late to pass an option; we may have already populated from an
earlier
  diffcore stage.
  
  I guess if we teach the whole diff code that -G (and
--pickaxe-regex)
  is brittle, we can disable the optimization from the beginning
based on
  the diff options. I'll take a look.
 
 Hmm. That is problematic for two reasons.
 
   1. The whole diff call chain will have to be modified to pass the
  options around, so they can make it down to the
  diff_populate_filespec level. Alternatively, we could do some
kind
  of global hack, which is ugly but would work OK in practice.
 
   2. Reusing a working tree file is only half of the reason a
filespec
  might be mmap'd. It might also be because we are literally
diffing
  the working tree. -G was meant to be used to limit log
traversal,
  but it also works to reduce the diff output for something like
git
  diff HEAD^.
 
 I really wish there were an alternate regexec interface we could use
 that took a pointer/size pair. Bleh.

Thinking on it more, my patch, hacky thought it seems, may not be the
worst solution. Here are the options that I see:

  1. Use a regex library that does not require NUL termination. If we
 are bound by the regular regexec interface, this is not feasible.
 But the GNU implementation works on arbitrary-length buffers (you
 just have to use a slightly different interface), and we already
carry it in compat. It would mean platforms which provide a working
 but non-GNU regexec would have to start defining NO_REGEX.

  2. Figure out a way to get one extra zero byte via mmap. If the
 requested size does not fall on a page boundary, you get extra
 zero-ed bytes. Unfortunately, requesting an extra byte does not
 do what we want; you get SIGBUS accessing it.

 3. Copy mmap'd data at point-of-use into a NUL-terminated buffer. That
 way we only incur the cost when we need it.

  4. Avoid mmap-ing in the first place when we are using -G or
 --pickaxe-regex (e.g., by doing a big read()). At first glance,
 this sounds more efficient than loading the data one way and then
 making another copy. But mmap+memcpy, aside from the momentary
doubled memory requirement, is probably just as fast or faster than
 calling read() repeatedly.

I am really tempted by (1).

Given that (2) does not work, unless somebody comes up with something
clever there, that would make (3) the next best choice.

-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: crash on git diff-tree -Ganything tree for new files with textconv filter

2012-10-30 Thread Jeff King
On Tue, Oct 30, 2012 at 09:46:01PM +0900, Junio C Hamano wrote:

 (1) sounds attractive for more than one reason. In addition to
 avoidance of this issue, it would bring bug-to-bug compatibility
 across platforms.

Yeah. I mentioned breaking the build for people who would now need to
turn on NO_REGEX. But the only reason to do that is to let people on
glibc systems use the system version of the tools. A much saner approach
would be to just always build with our compat regex, and turn NO_REGEX
into a no-op. We already do the same thing for kwset.

 (4), if we can run grep on streaming data (tweak interface we have for
 checking out a large blob to the working tree), would let us work on
 dataset larger than fit in core. Even though it would be much more
 work, it might turn out to be a better option in the longer run.

Agreed, that would be nice. It's potentially a lot of work, but we could
probably get by with a special streaming version of diff_populate_filespec.

The tricky thing is that we have to run the regex matcher progressively
as we stream data in (since your match might fall in the middle of a
read boundary). Which is certainly going to require switching off of
regular regexec. I don't think glibc regex will handle it either,
though. It looks like pcre can report a partial match at the end of the
string, and you can either continue with the next chunk (if using
pcre_dfa) or append and re-start the pattern match (for regular
pcre_exec).

Which means we'd probably have to make streaming matches an optional
feature, and still do (1) first to fix the correctness issue.

-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: What's cooking in git.git (Oct 2012, #01; Tue, 2)

2012-10-30 Thread Florian Achleitner
Sorry for reacting so late, I didn't read the list carefully in the last weeks 
and my gmail filter somehow didn't trigger on that.

On Tuesday 02 October 2012 16:20:22 Junio C Hamano wrote:
 * fa/remote-svn (2012-09-19) 16 commits
  - Add a test script for remote-svn
  - remote-svn: add marks-file regeneration
  - Add a svnrdump-simulator replaying a dump file for testing
  - remote-svn: add incremental import
  - remote-svn: Activate import/export-marks for fast-import
  - Create a note for every imported commit containing svn metadata
  - vcs-svn: add fast_export_note to create notes
  - Allow reading svn dumps from files via file:// urls
  - remote-svn, vcs-svn: Enable fetching to private refs
  - When debug==1, start fast-import with --stats instead of --quiet
  - Add documentation for the 'bidi-import' capability of remote-helpers
  - Connect fast-import to the remote-helper via pipe, adding 'bidi-import'
 capability - Add argv_array_detach and argv_array_free_detached
  - Add svndump_init_fd to allow reading dumps from arbitrary FDs
  - Add git-remote-testsvn to Makefile
  - Implement a remote helper for svn in C
  (this branch is used by fa/vcs-svn.)
 
  A GSoC project.
  Waiting for comments from mentors and stakeholders.

From my point of view, this is rather complete. It got eight review cycles on 
the list.
Note that the remote helper can only fetch, pushing is not possible at all.

 
 
 * fa/vcs-svn (2012-09-19) 4 commits
  - vcs-svn: remove repo_tree
  - vcs-svn/svndump: rewrite handle_node(), begin|end_revision()
  - vcs-svn/svndump: restructure node_ctx, rev_ctx handling
  - svndump: move struct definitions to .h
  (this branch uses fa/remote-svn.)
 
  A GSoC project.
  Waiting for comments from mentors and stakeholders.

This is the result of what I did when I wanted to start implementing branch 
detection. I found that the existing code is not suitable and restructured it.

The main goal is to seperate svn revision parsing from git commit creation. 
Because for creating commits, you need to know on which branch to create the 
commit.
While for finding out which branch is the right one, you need to read the 
complete svn revision first to see what dirs are changed and how.

It is rather invasive and it doesn't make sense without using it later on.
So I'm not surprised that you may not like it.
Anyways it passes all existing tests (that doesn't mean it's good of course 
;))

Florian
--
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: Why does git-commit --template want the template to be modified ?

2012-10-30 Thread Francis Moreau
Hi,

On Tue, Oct 30, 2012 at 12:09 PM, Tomas Carnecky
tomas.carne...@gmail.com wrote:
 On Tue, 30 Oct 2012 11:53:08 +0100, Francis Moreau francis.m...@gmail.com 
 wrote:
 Hi,

 I'm using git-commit with the --template option. The template I'm
 given is self sufficient for my purpose but as stated in the
 documentation, git-commit wants the template to be edited otherwise it
 aborts the operation.

 Is it possible to change this ?

 It seems you want -F instead of --template.

Yes, but I want git to still parse the log and sanitize it (remove
trailing whitespaces, comments ...)

I actually found that -F --edit is what I needed.

Thanks both for your answer.
-- 
Francis
--
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-users] Git clone fails with bad pack header, how to get remote log

2012-10-30 Thread kevin molcard
I tried to install git 1.8 on the remote server and get exactly the same 
problem :(.


Kevin

On 10/29/12 6:18 PM, Konstantin Khomoutov wrote:

On Mon, 29 Oct 2012 09:52:54 -0700 (PDT)
Kevin Molcard kev2...@gmail.com wrote:


I have a problem with my build system.

I have a remote server with a relatively large repository (around 12
GB, each branch having a size of 3 GB).

I have also 2 build servers (Mac, Windows) that are cloning the repo
from the remote.

Sometimes (very often when several git clone are sent at the same
time), I have the following error:
 
 remote: internal server error

 fatal: protocol error: bad pack header

I know that it happens when the remote is compressing objects (thanks
to `--progress -v` flags) because the last line of the log before the
erro is:
 remote: Compressing objects:  93% (17959/19284)   [K

  * So I have 2 questions, does anybody what is the problem and what
should I do?
  * Is there a way to get a more precise log from the remote to debug
this problem?

This reminds me of a bug fixed in 1.7.12.1 [1]:

* When git push triggered the automatic gc on the receiving end, a
   message from git prune that said it was removing cruft leaked to
   the standard output, breaking the communication protocol.

In any case, bugs should be reported to the main Git list (which is
git at vger.kernel.org), not here.
I'm Cc'ing the main Git list so you'll get any responses from there, if
any.

Kevin, please answer to this message (keeping all the Ccs -- use Reply
to group or Reply to all in your MUA) and describe exactly what Git
versions on which platforms your have.

1. https://raw.github.com/git/git/master/Documentation/RelNotes/1.7.12.1.txt



--
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 v5 00/14] New remote-hg helper

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 11:25 AM, Chris Webb ch...@arachsys.com wrote:
 Hi. I routinely work with projects in both hg and git, so I'm really
 interested in this. Thanks for working on it! I grabbed the latest version
 from

   
 https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg

 and have been trying it out. For the most part, it seems to work very nicely
 for the hg repos I have access to and can test against. I've spotted a couple
 of issues along the way that I thought would be worth reporting.

Great!

 The first is really a symptom of a general difference between hg and git: an 
 hg
 repository can have multiple heads, whereas a git repo has exactly one head. 
 To
 demonstrate:

 Now if I try to convert this:

   $ git clone hg::$PWD/hgtest gittest
   Cloning into 'gittest'...
   WARNING: Branch 'default' has more than one head, consider merging
   Traceback (most recent call last):
 File /home/chris/bin/git-remote-hg, line 773, in module
   sys.exit(main(sys.argv))
 File /home/chris/bin/git-remote-hg, line 759, in main
   do_list(parser)
 File /home/chris/bin/git-remote-hg, line 463, in do_list
   list_branch_head(repo, cur)
 File /home/chris/bin/git-remote-hg, line 425, in list_branch_head
   tip = get_branch_tip(repo, cur)
 File /home/chris/bin/git-remote-hg, line 418, in get_branch_tip
   return repo.branchtip(branch)
   AttributeError: 'mqrepo' object has no attribute 'branchtip'

Yes, it seems this is an API issue; repo.branchtip doesn't exist in
python 2.2. I've added a check for that, and it should work fine now.
We'll be picking a random head (the first one), but the user has been
warned anyway.

 The second thing I spotted is the behaviour of bookmarks on push:

 i.e. the development bookmark hasn't been updated by the push. This might be
 connected to the warning message

This is not an API issue, this is a bug; bookmarks are not updated
(only the first creation works). I've fixed this as well, and added a
test with the example you put above:

http://github.com/felipec/git/commit/d006d54fc84707dffa24d9fad053e574918d

Both issues should be fixed now :)

Cheers.

-- 
Felipe Contreras
--
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 0/3] completion: refactor and zsh wrapper

2012-10-30 Thread Felipe Contreras
Hi,

On Mon, Oct 22, 2012 at 3:45 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Here's a bit of reorganition. I'm introducing a new __gitcompadd helper that 
 is
 useful to wrapp all changes to COMPREPLY, but first, lets get rid of
 unnecessary assignments as SZEDER suggested.

 The zsh wrapper is now very very simple.

Junio, Jeff, just to let you know, this is an updated version of the
new zsh wrapper patch series with the feedback from SZEDER. I see the
old version is in pu, and hasn't been updated.

Cheers.

 Since v5:

  * Get rid of unnecessary COMPREPLY assignments

 Felipe Contreras (3):
   completion: get rid of empty COMPREPLY assignments
   completion: add new __gitcompadd helper
   completion: add new zsh completion

-- 
Felipe Contreras
--
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 push tags

2012-10-30 Thread Chris Rorvick
On Mon, Oct 29, 2012 at 4:35 PM, Jeff King p...@peff.net wrote:
 On Mon, Oct 29, 2012 at 06:23:30PM +0100, Kacper Kornet wrote:

  That patch just blocks non-forced updates to refs/tags/. I think a saner
  start would be to disallow updating non-commit objects without a force.
  We already do so for blobs and trees because they are not (and cannot
  be) fast forwards. The fact that annotated tags are checked for
  fast-forward seems to me to be a case of it happens to work that way
  and not anything planned. Since such a push drops the reference to the
  old version of the tag, it should probably require a force.

 I'm not sure. Looking at 37fde87 (Fix send-pack for non-commitish
 tags.) I have an impression that Junio allowed for fast-forward pushes
 of annotated tags on purpose.

 Hmm. You're right, though I'm not sure I agree with the reasoning of
 that commit. I'd certainly like to get Junio's input on the subject.

  Then on top of that we can talk about what lightweight tags should do.
  I'm not sure. Following the regular fast-forward rules makes some sense
  to me, because you are never losing objects. But there may be
  complications with updating tags in general because of fetch's rules,
  and we would be better off preventing people from accidentally doing so.
  I think a careful review of fetch's tag rules would be in order before
  making any decision there.

 The problem with the current behaviour is, that one can never be 100% sure
 that his push will not overwrite someone else tag.

 Yes, although you do know that you are not throwing away history if you
 do (because it must be a fast forward). Whereas if you have to use -f
 to update a tag, then you have turned off all safety checks. So it is an
 improvement for one case (creating a tag), but a regression for another
 (updating an existing tag). I agree that the latter is probably less
 common, but how much? If virtually nobody is doing it because git-fetch
 makes the fetching side too difficult, then the regression is probably
 not a big deal.

 -Peff

This is probably a bit premature given there are still open questions,
but I was curious and decided to take a stab at this.

The change is to only allow fast-forward when both the old and new are
commits and the reference is not a lightweight tag.  All other
reference updates require --force.  I think this resolves the reported
issue and takes into account feedback on this thread.  This change
only broke one test and it was an expected failure given the change in
behavior (i.e., I needed to add a -f to update a tag in the remote.)

I wasn't sure how to handle provided feedback to the user when there
are multiple refs not pushed for different reasons.  But I think this
adds the plumbing for handling it correctly, whateverever that this.

It needs some work, but thought I'd throw it out for feedback to see
if it's at least in the right direction.

Chris

--- 8 ---
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..fabcea0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,6 +220,10 @@ static const char message_advice_checkout_pull_push[] =
   (e.g. 'git pull') before pushing again.\n
   See the 'Note about fast-forwards' in 'git push --help' for 
details.);

+static const char message_advice_ref_already_exists[] =
+   N_(Updates were rejected because a matching reference already exists 
in\n
+  the remote.  Use git push -f if you really want to make this 
update.);
+
 static void advise_pull_before_push(void)
 {
if (!advice_push_non_ff_current || !advice_push_nonfastforward)
@@ -241,6 +245,11 @@ static void advise_checkout_pull_push(void)
advise(_(message_advice_checkout_pull_push));
 }

+static void advise_ref_already_exists(void)
+{
+   advise(_(message_advice_ref_already_exists));
+}
+
 static int push_with_options(struct transport *transport, int flags)
 {
int err;
@@ -277,6 +286,9 @@ static int push_with_options(struct transport
*transport, int flags)
else
advise_checkout_pull_push();
break;
+   case ALREADY_EXISTS:
+   advise_ref_already_exists();
+   break;
}

return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7d05064..f159ec3 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -202,6 +202,11 @@ static void print_helper_status(struct ref *ref)
msg = non-fast forward;
break;

+   case REF_STATUS_REJECT_ALREADY_EXISTS:
+   res = error;
+   msg = already exists;
+   break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = error;
@@ -288,6 +293,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch 

Re: [PATCH v4 00/13] New remote-hg helper

2012-10-30 Thread Felipe Contreras
On Mon, Oct 29, 2012 at 11:06 PM, Jeff King p...@peff.net wrote:
 On Mon, Oct 29, 2012 at 11:02:31PM +0100, Felipe Contreras wrote:

  If remote-hg is going to live in contrib, it probably makes sense to
  have its tests live there, too, like subtree.

 Probably, I'll check that option.

 But eventually I think it should be installed by default, unless
 somebody can come up for a reason not to. For now contrib might be OK.

 I would one day like to have it as part of the main distribution, too,
 but it would be nice to prove its worth in the field for a while first.
 I especially would like to find out how it compares in practice with the
 work that is in msysgit.

Yeah, I would like to compare it with that work, if only the patches
were readily available somewhere.

-- 
Felipe Contreras
--
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 v4 00/13] New remote-hg helper

2012-10-30 Thread Johannes Schindelin
Hi all,

On Mon, 29 Oct 2012, Jeff King wrote:

 On Mon, Oct 29, 2012 at 10:47:04PM +0100, Felipe Contreras wrote:
 
   Yeah, the test script is not ready for merging, it needs to check
   for python, hg, and hg-git.
  
   Do you have hg-git installed?
  
   No. But it's important that it fail gracefully; I can't even take it
   in pu if I can't run the test suite in a sane way.
  
  The contrib part is fine for 'pu'. The tests aren't even meant to
  exercise stuff in 'contrib', right? There might be some exceptions,
  but either way, there's plenty of stuff in 'contrib' without any
  tests. The tests I'm providing are simply a little sugar.
 
 Yeah, contrib is a bit of a wildcard. Most things do not have tests.

Given that the tests of remote-hg as in git://github.com/msysgit/git's
'devel' branch run just fine without additional dependencies (which
probably triggered the not-quite-constructive and unnecessarily-flaming
bloated comment of Felipe), and given that the code in said branch is
well-tested and exercised by daily use, and given the fact that my major
concern was not understood (and probably not addressed), and also given
the fact that Sverre indicated that he could finalize the work as a 20%
project, I decided that other projects I have to do unfortunately have a
too-high priority to take care of testing and measuring the performance of
the patch series that is discussed in this thread.

Sorry,
Johannes

P.S.: I would still recommend to have a detailed look at the 'devel'
branch, in particular the commits starting with fast-export: do not refer
to non-existing marks and ending with t5801: skip without hg. My
understanding is that it was completely ignored after a brief and maybe
too-cursory look. In the least, it has a couple of lessons we learnt the
hard way, and if git.git is dead set on duplicating the work, making these
mistakes again could be avoided by learning from our lessons.
--
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 v5 00/14] New remote-hg helper

2012-10-30 Thread Johannes Schindelin
Hi Chris,

On Tue, 30 Oct 2012, Chris Webb wrote:

 I routinely work with projects in both hg and git, so I'm really
 interested in this. Thanks for working on it! I grabbed the latest
 version from
 
   
 https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg
 
 and have been trying it out.

You might also want to try out the 'devel' branch of
https://github.com/msysgit/git. It is in production use here.

Ciao,
Johannes
--
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 v5 00/14] New remote-hg helper

2012-10-30 Thread Chris Webb
Felipe Contreras felipe.contre...@gmail.com writes:

 Yes, it seems this is an API issue; repo.branchtip doesn't exist in
 python 2.2.

Hi. Presumably this is a problem with old mercurial not a problem with old
python as mentioned in the commit?

 Both issues should be fixed now :)

They are indeed, and it now works nicely on all the repos I've tested it
with, including http://selenic.com/hg: very impressive!

I wonder whether it's worth ignoring heads with bookmarks pointing to them
when it comes to considering heads of branches, or at least allowing the
hg branch tracking to be easily disabled?

A common idiom when working with hg bookmarks is to completely ignore the
(not very useful) hg branches (i.e. all commits are on the default hg
branch) and have a bookmark for each line of development used exactly as a
git branch would be.

On such a repository, at the moment you will always get a warning about
multiple heads on branches/default, even though you actually don't care
about branches/default (and would prefer it not to exist) and just want the
branches coming from the bookmarks.

I've also seen repositories with no hg branches, but with a single
unbookmarked tip and bookmarks on some other heads to mark non-mainline
development. It would be nice for branches/default to track the unbookmarked
tip in this case, without warning about the other, bookmarked heads.

Finally, on a simple repo with no branches and where there's no clash with a
bookmark called master, I'd love to be able to a get a more idiomatic
origin/master rather than origin/branches/default.

Just some idle thoughts...

Best wishes,

Chris.
--
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] Enable parallelism in git submodule update.

2012-10-30 Thread szager
The --jobs parameter may be used to set the degree of per-submodule
parallel execution.

Signed-off-by: Stefan Zager sza...@google.com
---
 Documentation/git-submodule.txt |8 ++-
 git-submodule.sh|   40 ++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..cb23ba7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,8 @@ SYNOPSIS
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
 'git submodule' [--quiet] init [--] [path...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
- [--reference repository] [--merge] [--recursive] [--] 
[path...]
+ [--reference repository] [--merge] [--recursive]
+ [-j|--jobs [jobs]] [--] [path...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
  [commit] [--] [path...]
 'git submodule' [--quiet] foreach [--recursive] command
@@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want 
to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
 +
+By default, each submodule is treated serially.  You may specify a degree of
+parallel execution with the --jobs flag.  If a parameter is provided, it is
+the maximum number of jobs to run in parallel; without a parameter, all jobs 
are
+run in parallel.
++
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 +
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..60a5f96 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename $0 | sed -e 's/-/ /')
 USAGE=[--quiet] add [-b branch] [-f|--force] [--reference repository] [--] 
repository [path]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
or: $dashless [--quiet] init [--] [path...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
[--rebase] [--reference repository] [--merge] [--recursive] [--] [path...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
[--rebase] [--reference repository] [--merge] [--recursive] [-j|--jobs 
[jobs]] [--] [path...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] 
[commit] [--] [path...]
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--] [path...]
@@ -500,6 +500,7 @@ cmd_update()
 {
# parse $args after submodule ... update.
orig_flags=
+   jobs=1
while test $# -ne 0
do
case $1 in
@@ -518,6 +519,20 @@ cmd_update()
-r|--rebase)
update=rebase
;;
+   -j|--jobs)
+   case $2 in
+   ''|-*)
+   jobs=0
+   ;;
+   *)
+   jobs=$2
+   shift
+   ;;
+   esac
+   # Don't preserve this arg.
+   shift
+   continue
+   ;;
--reference)
case $2 in '') usage ;; esac
reference=--reference=$2
@@ -551,11 +566,34 @@ cmd_update()
shift
done
 
+   # Correctly handle the case where '-q' came before 'update' on the 
command line.
+   if test -n $GIT_QUIET
+   then
+   orig_flags=$orig_flags -q
+   fi
+
if test -n $init
then
cmd_init -- $@ || return
fi
 
+   if test $jobs != 1
+   then
+   if ( echo test | xargs -P $jobs true 2/dev/null )
+   then
+   if ( echo test | xargs --max-lines=1 true 2/dev/null 
); then
+   max_lines=--max-lines=1
+   else
+   max_lines=-L 1
+   fi
+   module_list $@ | awk '{print $4}' |
+   xargs $max_lines -P $jobs git submodule update 
$orig_flags
+   return
+   else
+   echo Warn: parallel execution is not supported on this 
platform.
+   fi
+   fi
+
cloned_modules=
module_list $@ | {
err=
-- 
1.7.7.3

--
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] Enable parallelism in git submodule update.

2012-10-30 Thread szager
The --jobs parameter may be used to set the degree of per-submodule
parallel execution.

Signed-off-by: Stefan Zager sza...@google.com
---
 Documentation/git-submodule.txt |8 ++-
 git-submodule.sh|   40 ++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..cb23ba7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,8 @@ SYNOPSIS
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
 'git submodule' [--quiet] init [--] [path...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
- [--reference repository] [--merge] [--recursive] [--] 
[path...]
+ [--reference repository] [--merge] [--recursive]
+ [-j|--jobs [jobs]] [--] [path...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
  [commit] [--] [path...]
 'git submodule' [--quiet] foreach [--recursive] command
@@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want 
to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
 +
+By default, each submodule is treated serially.  You may specify a degree of
+parallel execution with the --jobs flag.  If a parameter is provided, it is
+the maximum number of jobs to run in parallel; without a parameter, all jobs 
are
+run in parallel.
++
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 +
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..60a5f96 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename $0 | sed -e 's/-/ /')
 USAGE=[--quiet] add [-b branch] [-f|--force] [--reference repository] [--] 
repository [path]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
or: $dashless [--quiet] init [--] [path...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
[--rebase] [--reference repository] [--merge] [--recursive] [--] [path...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
[--rebase] [--reference repository] [--merge] [--recursive] [-j|--jobs 
[jobs]] [--] [path...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] 
[commit] [--] [path...]
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--] [path...]
@@ -500,6 +500,7 @@ cmd_update()
 {
# parse $args after submodule ... update.
orig_flags=
+   jobs=1
while test $# -ne 0
do
case $1 in
@@ -518,6 +519,20 @@ cmd_update()
-r|--rebase)
update=rebase
;;
+   -j|--jobs)
+   case $2 in
+   ''|-*)
+   jobs=0
+   ;;
+   *)
+   jobs=$2
+   shift
+   ;;
+   esac
+   # Don't preserve this arg.
+   shift
+   continue
+   ;;
--reference)
case $2 in '') usage ;; esac
reference=--reference=$2
@@ -551,11 +566,34 @@ cmd_update()
shift
done
 
+   # Correctly handle the case where '-q' came before 'update' on the 
command line.
+   if test -n $GIT_QUIET
+   then
+   orig_flags=$orig_flags -q
+   fi
+
if test -n $init
then
cmd_init -- $@ || return
fi
 
+   if test $jobs != 1
+   then
+   if ( echo test | xargs -P $jobs true 2/dev/null )
+   then
+   if ( echo test | xargs --max-lines=1 true 2/dev/null 
); then
+   max_lines=--max-lines=1
+   else
+   max_lines=-L 1
+   fi
+   module_list $@ | awk '{print $4}' |
+   xargs $max_lines -P $jobs git submodule update 
$orig_flags
+   return
+   else
+   echo Warn: parallel execution is not supported on this 
platform.
+   fi
+   fi
+
cloned_modules=
module_list $@ | {
err=
-- 
1.7.7.3

--
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 v4 00/13] New remote-hg helper

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 6:20 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:

 P.S.: I would still recommend to have a detailed look at the 'devel'
 branch, in particular the commits starting with fast-export: do not refer
 to non-existing marks and ending with t5801: skip without hg. My
 understanding is that it was completely ignored after a brief and maybe
 too-cursory look. In the least, it has a couple of lessons we learnt the
 hard way, and if git.git is dead set on duplicating the work, making these
 mistakes again could be avoided by learning from our lessons.

% g l --grep=t5801: skip without hg devel
1e000d4 t5801: skip without hg
bee410c t5801: skip without hg
5cdc7d0 t5801: skip without hg
05b703f t5801: skip without hg
6bb8d90 t5801: skip without hg
c70b4d0 t5801: skip without hg
2f46371 t5801: skip without hg
39bc40f t5801: skip without hg
d0a618b t5801: skip without hg

% g l --grep=fast-export: do not refer devel
d3ac32c fast-export: do not refer to non-existing marks
bdbb22f fast-export: do not refer to non-existing marks
5d99930 fast-export: do not refer to non-existing marks
381f276 fast-export: do not refer to non-existing marks
b4686c7 fast-export: do not refer to non-existing marks
e3dfe01 fast-export: do not refer to non-existing marks
c00fe59 fast-export: do not refer to non-existing marks
ce357ce fast-export: do not refer to non-existing marks
5c1c7a4 fast-export: do not refer to non-existing marks
9c827d1 fast-export: do not refer to non-existing marks

I'll assume you are referring the latest ones:

% g log --oneline --reverse d3ac32c^..1e000d4
* d3ac32c fast-export: do not refer to non-existing marks

Not needed at all.

* b013fe0 setup_revisions: remember whether a ref was positive or not
* fb89a2c fast-export: do not export negative refs
* 7655869 setup_revisions: remember whether a ref was positive or not

I've fixed this problem already.

The solution proposed in these patches is to convoluted:
1) Requires multiple unrelated changes
2) Proposes change in committish semantics

It's hard to test, because the test to check for this is not in this
patch series, and it's testing for something completely unrelated:

---
cat  expected  EOF
reset refs/heads/master
from $(git rev-parse master)

EOF

test_expect_success 'refs are updated even if no commits need to be exported' '
git fast-export master..master  actual 
test_cmp expected actual
'
---

This is most certainly not what we want.

Notice that in my patch (a single patch) I added the tests at the same
time so it's clear what it's fixing, and I also added a test to the
relevant remote-helper behavior we want:

https://github.com/felipec/git/commit/76e75315bd1bd8d9d8365bb09261a745a10ceae0

* 512cb13 t5800: test pushing a new branch with old content

If this is what the patches above were trying to fix, then yes, my
patch fixes that. Also, it's tainted by changes from another patch.

* a85de2c t5800: point out that deleting branches does not work

Correct, but hardly _necessary_.

* 2412a45 transport-helper: add trailing --

No description what's the problem, or what it's trying to fix, or
tests, so it's not possible to know if this is _needed_ or not. But
probably correct.

* 026d07c remote-helper: check helper status after import/export

Again, no explanation, but the issue was already addressed:

http://article.gmane.org/gmane.comp.version-control.git/208202

The problem is minuscule, not _needed_.

* 5165e26 remote-testgit: factor out RemoteHelper class
* 049f093 git-remote-testgit: make local a function
* f835bb2 git_remote_helpers: add fastimport library
* 088ad33 git-remote-hg: add hgimport, an hg-fast-import equivalent
* 7de6ca0 git-remote-hg: add GitHg, a helper class for converting hg
commits to git
* e3cc5ed git-remote-hg: add hgexport, an hg-fast-export equivalent
* 0edc8e9 git-remote-hg: add GitExporter/GitImporter/NonLocalGit
* 5c73277 remote-hg: adjust to hg 1.9
* 1b47007 git-remote-hg: add the helper
* 4dcc671 git-remote-hg: add tests
* 48b2769 remote-hg: Postel's law dictates we should handle Authorauthor@mail
* 2587cc6 remote-hg: another case of Postel's law
* 9f934c9 remote-hg: handle another funny author line from
http://scelenic.com/hg
* a799904 remote-hg: do not interfer with hg's revs() method

All these are specific to this remote-hg version.

* ac77256 Always auto-gc after calling a fast-import transport

This might be a good idea, but not _needed_.

* 1e000d4 t5801: skip without hg

Specific to this remote-hg.


So, yeah, nothing really needed there. Some patches might be nice, but
that's it.

Now, if this is really the latest and greatest remote-hg patch series,
I can try to port them to git's master and see how it fares.

But you mentioned something about cooperation, and I've yet to see how
is it that you are planning to cooperate. If you say you don't have
time to spend on this, I don't see why I should worry about testing
this series of patches.

Also, you seem to be 

Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Sverre Rabbelier
On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 When an object has already been exported (and thus is in the marks) it
 is flagged as SHOWN, so it will not be exported again, even if this time
 it's exported through a different ref.

 We don't need the object to be exported again, but we want the ref
 updated, which doesn't happen.

 Since we can't know if a ref was exported or not, let's just assume that
 if the commit was marked (flags  SHOWN), the user still wants the ref
 updated.

 So:

  % git branch test master
  % git fast-export $mark_flags master
  % git fast-export $mark_flags test

 Would export 'test' properly.

 Additionally, this fixes issues with remote helpers; now they can push
 refs wich objects have already been exported.

Won't this also export child (or maybe parent) branches that weren't
mentioned? For example:

$ git branch one
$ echo foo  content
$ git commit -m two
$ git fast-export one
$ git fast-export two

I suspect that one of those will export both one and two. If not, this
seems like a great solution to the fast-export problem.

-- 
Cheers,

Sverre Rabbelier
--
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 v5 00/14] New remote-hg helper

2012-10-30 Thread Chris Webb
Chris Webb ch...@arachsys.com writes:

 A common idiom when working with hg bookmarks is to completely ignore the
 (not very useful) hg branches (i.e. all commits are on the default hg
 branch) and have a bookmark for each line of development used exactly as a
 git branch would be.
 
 On such a repository, at the moment you will always get a warning about
 multiple heads on branches/default, even though you actually don't care
 about branches/default (and would prefer it not to exist) and just want the
 branches coming from the bookmarks.

Something which you can do with hg clone is

  hg clone http://my.repo/foo#master

to clone just the history behind the master bookmark from foo. This works
nicely with git-remote-hg too:

  git clone hg::http://my.repo/foo#master

gives you just origin/master and origin/branches/default, not
origin/otherbookmark. This is a case where it would be particularly nice to
be able to kill origin/branches/default and just keep the identical
origin/master.

Cheers,

Chris.
--
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 v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized

2012-10-30 Thread Ramsay Jones
Michael Haggerty wrote:
 Move the responsibility for normalizing prefixes from
 longest_ancestor_length() to its callers. Use slightly different
 normalizations at the two callers:
 
 In setup_git_directory_gently_1(), use the old normalization, which
 ignores paths that are not usable.  In the next commit we will change
 this caller to also resolve symlinks in the paths from
 GIT_CEILING_DIRECTORIES as part of the normalization.
 
 In test-path-utils longest_ancestor_length, use the old
 normalization, but die() if any paths are unusable.  Also change t0060
 to only pass normalized paths to the test program (no empty entries or
 non-absolute paths, strip trailing slashes from the paths, and remove
 tests that thereby become redundant).
 
 The point of this change is to reduce the scope of the ancestor_length
 tests in t0060 from testing normalization+longest_prefix to testing
 only mostly longest_prefix.  This is necessary because when
 setup_git_directory_gently_1() starts resolving symlinks as part of
 its normalization, it will not be reasonable to do the same in the
 test suite, because that would make the test results depend on the
 contents of the root directory of the filesystem on which the test is
 run.  HOWEVER: under Windows, bash mangles arguments that look like
 absolute POSIX paths into DOS paths.

Just to be clear, this is true for the MinGW port to Windows, but *not*
the cygwin port.
:-P

  So we have to retain the level
 of normalization done by normalize_path_copy() to convert the
 bash-mangled DOS paths (which contain backslashes) into paths that use
 forward slashes.
 
 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
  path.c| 26 +++---
  setup.c   | 23 +++
  t/t0060-path-utils.sh | 41 +
  test-path-utils.c | 45 -
  4 files changed, 91 insertions(+), 44 deletions(-)
 

[snip]

 diff --git a/test-path-utils.c b/test-path-utils.c
 index acb0560..0092cbf 100644
 --- a/test-path-utils.c
 +++ b/test-path-utils.c
 @@ -1,6 +1,33 @@
  #include cache.h
  #include string-list.h
  
 +/*
 + * A string_list_each_func_t function that normalizes an entry from
 + * GIT_CEILING_DIRECTORIES.  If the path is unusable for some reason,
 + * die with an explanation.
 + */
 +static int normalize_ceiling_entry(struct string_list_item *item, void 
 *unused)
 +{
 + const char *ceil = item-string;
 + int len = strlen(ceil);
 + char buf[PATH_MAX+1];
 +
 + if (len == 0)
 + die(Empty path is not supported);
 + if (len  PATH_MAX)
 + die(Path \%s\ is too long, ceil);
 + if (!is_absolute_path(ceil))
 + die(Path \%s\ is not absolute, ceil);
 + if (normalize_path_copy(buf, ceil)  0)
 + die(Path \%s\ could not be normalized, ceil);
 + len = strlen(buf);
 + if (len  1  buf[len-1] == '/')
 + die(Normalized path \%s\ ended with slash, buf);
 + free(item-string);
 + item-string = xstrdup(buf);
 + return 1;
 +}
 +
  int main(int argc, char **argv)
  {
   if (argc == 3  !strcmp(argv[1], normalize_path_copy)) {
 @@ -33,10 +60,26 @@ int main(int argc, char **argv)
   if (argc == 4  !strcmp(argv[1], longest_ancestor_length)) {
   int len;
   struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
 + char *path = xstrdup(argv[2]);
  
 + /*
 +  * We have to normalize the arguments because under
 +  * Windows, bash mangles arguments that look like

ditto

 +  * absolute POSIX paths or colon-separate lists of
 +  * absolute POSIX paths into DOS paths (e.g.,
 +  * /foo:/foo/bar might be converted to
 +  * D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar),
 +  * whereas longest_ancestor_length() requires paths
 +  * that use forward slashes.
 +  */
 + if (normalize_path_copy(path, path))
 + die(Path \%s\ could not be normalized, argv[2]);
   string_list_split(ceiling_dirs, argv[3], PATH_SEP, -1);
 - len = longest_ancestor_length(argv[2], ceiling_dirs);
 + filter_string_list(ceiling_dirs, 0,
 +normalize_ceiling_entry, NULL);
 + len = longest_ancestor_length(path, ceiling_dirs);
   string_list_clear(ceiling_dirs, 0);
 + free(path);
   printf(%d\n, len);
   return 0;
   }


HTH

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


Re: [PATCH v5 00/14] New remote-hg helper

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 7:00 PM, Chris Webb ch...@arachsys.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 Yes, it seems this is an API issue; repo.branchtip doesn't exist in
 python 2.2.

 Hi. Presumably this is a problem with old mercurial not a problem with old
 python as mentioned in the commit?

 Both issues should be fixed now :)

 They are indeed, and it now works nicely on all the repos I've tested it
 with, including http://selenic.com/hg: very impressive!

 I wonder whether it's worth ignoring heads with bookmarks pointing to them
 when it comes to considering heads of branches, or at least allowing the
 hg branch tracking to be easily disabled?

 A common idiom when working with hg bookmarks is to completely ignore the
 (not very useful) hg branches (i.e. all commits are on the default hg
 branch) and have a bookmark for each line of development used exactly as a
 git branch would be.

 On such a repository, at the moment you will always get a warning about
 multiple heads on branches/default, even though you actually don't care
 about branches/default (and would prefer it not to exist) and just want the
 branches coming from the bookmarks.

 I've also seen repositories with no hg branches, but with a single
 unbookmarked tip and bookmarks on some other heads to mark non-mainline
 development. It would be nice for branches/default to track the unbookmarked
 tip in this case, without warning about the other, bookmarked heads.

 Finally, on a simple repo with no branches and where there's no clash with a
 bookmark called master, I'd love to be able to a get a more idiomatic
 origin/master rather than origin/branches/default.

Sounds like you might want to try this:
% git config --global remote-hg.hg-git-compat true

This would match the behavior of hg-git, which seems to be what you
are looking for: branches will be ignored. But additionally there will
be other obtrusive changes:

 1) Extra information will be stored in the commit message: branch,
renames, extra info.

Personally I don't like this, which is why it's only enabled with the
hg-git compat mode. Eventually there will be support for this in the
normal mode through git notes, and eventually (hopefully), hg-git will
use notes as well.

 2) Authors will be fixed trying to preserve as much information as possible


The problem with this mode is what happens when there are no
bookmarks, and what I decided to do is create a fake bookmark to the
current branch (e.g. default), so you would get 'origin/default', but
you will be warned if there are multiple heads. We could try to create
a 'master' ref to the 'tip', but that might not be what the user
wants: (s)he might switch to a certain branch, and clone the
repository in git; currently the result working directory would be the
same as in hg, but if we point to 'tip', then it would not.

Maybe we should have a branch pointing to 'tip' either way.

The reason I decided on having 'branches/default' on the normal mode
is that users unfamiliar with mercurial might expect to see all the
branches somewhere. But perhaps that should happen only for other
branches, while 'default' gets a special treatment and gets promoted
to 'origin/master' but only if there's no 'master' bookmark, as you
say. Now the tricky part would be handling the case where there was no
'master' bookmark, and suddenly one gets created, but it might be
overkill to think about that right now.

Either way, try the hg-git-compat mode, should be close :)

Cheers.

-- 
Felipe Contreras
--
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] parse_dirstat_params(): use string_list to split comma-separated string

2012-10-30 Thread Matt Kraai
Michael Haggerty mhagger at alum.mit.edu writes:
...
 -static int parse_dirstat_params(struct diff_options *options, const char ...
 +static int parse_dirstat_params(struct diff_options *options, const char ...
   struct strbuf *errmsg)
  {
 - const char *p = params;
 - int p_len, ret = 0;
 + char *params_copy = xstrdup(params_string);
 + struct string_list params = STRING_LIST_INIT_NODUP;
 + int ret = 0;
 + int i;
 
 - while (*p) {
 - p_len = strchrnul(p, ',') - p;
 - if (!memcmp(p, changes, p_len)) {
 + if (*params_copy)

params_copy is set to the value returned by xstrdup, which cannot be NULL.
This check can be removed and if params_string can be NULL, it should be
checked before being passed to xstrdup.

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


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 7:12 PM, Sverre Rabbelier srabbel...@gmail.com wrote:
 On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 When an object has already been exported (and thus is in the marks) it
 is flagged as SHOWN, so it will not be exported again, even if this time
 it's exported through a different ref.

 We don't need the object to be exported again, but we want the ref
 updated, which doesn't happen.

 Since we can't know if a ref was exported or not, let's just assume that
 if the commit was marked (flags  SHOWN), the user still wants the ref
 updated.

 So:

  % git branch test master
  % git fast-export $mark_flags master
  % git fast-export $mark_flags test

 Would export 'test' properly.

 Additionally, this fixes issues with remote helpers; now they can push
 refs wich objects have already been exported.

 Won't this also export child (or maybe parent) branches that weren't
 mentioned? For example:

 $ git branch one
 $ echo foo  content
 $ git commit -m two
 $ git fast-export one
 $ git fast-export two

 I suspect that one of those will export both one and two. If not, this
 seems like a great solution to the fast-export problem.

Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).

And if you are talking about the ref for the reset at the end, it has
to be both in the list of refs selected by the user (initially in
revs.pending), either marked or the object already referenced by
another ref in the list selected by the user (e.g. fast-export one
two, where one^{commit} == two^{commit}, and not marked as
UNINTERESTING (e.g. ^two).

Cheers.

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


Re: [PATCH v2 1/4] fast-export: trivial cleanup

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 Setting commit to commit is a no-op.

Wrong description.  This should say:

The code uses the idiom of assigning commit to itself to quench a
may be used uninitialized warning.  Luckily at least modern
versions of gcc do not produce that warning here, so we can drop
the self-assignment.

This makes the code clearer to human beings, makes static
analyzers that do not know that idiom happier, and means that
if the code some day evolves to use this variable uninitialized
then we will catch it.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

With that change, for what it's worth,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Patch left unsnipped since it doesn't seem to have hit the list.

 ---
  builtin/fast-export.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/builtin/fast-export.c b/builtin/fast-export.c
 index 12220ad..065f324 100644
 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array 
 *pending,
   for (i = 0; i  pending-nr; i++) {
   struct object_array_entry *e = pending-objects + i;
   unsigned char sha1[20];
 - struct commit *commit = commit;
 + struct commit *commit;
   char *full_name;
  
   if (dwim_ref(e-name, strlen(e-name), sha1, full_name) != 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 v2 2/4] fast-export: fix comparisson in tests

2012-10-30 Thread Jonathan Nieder
(actually cc-ing the git list this time.  Sorry for the noise, all.)
Felipe Contreras wrote:

 [Subject: [PATCH v2 2/4] fast-export: fix comparisson in tests]

 First the expected, then the actual, otherwise the diff would be the
 opposite of what we want.

Spelling: s/comparisson/comparison/.

Semantics: this isn't actually fixing anything --- it's a cosmetic
thing.  It would be clearer to say:

fast-export test: swap arguments to test_cmp

This way if diff output is produced, it describes how the
actual output differs from what was expected rather than the
other way around.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

For what it's worth, with amended message,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Patch left unsnipped because it hadn't hit the list.

 ---
  t/t9350-fast-export.sh | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
 index 3e821f9..49bdb44 100755
 --- a/t/t9350-fast-export.sh
 +++ b/t/t9350-fast-export.sh
 @@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' 
 '
  (
   cd limit-by-paths 
   git fast-export --tag-of-filtered-object=drop mytag -- there  output 
 - test_cmp output expected
 + test_cmp expected output
  )
  '
  
 @@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out 
 object' '
  (
   cd limit-by-paths 
   git fast-export --tag-of-filtered-object=rewrite mytag -- there  
 output 
 - test_cmp output expected
 + test_cmp expected output
  )
  '
  
 @@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
   (
   cd limit-by-paths 
   git fast-export master~2..master~1  output 
 - test_cmp output expected
 + test_cmp expected output
   )
  '
  
 -- 
 1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 They have been marked as UNINTERESTING for a reason, lets respect that.

This patch looks unsafe, and in the examples listed in the patch
description the changed behavior does not look like an improvement.
Worse, the description lists a few examples but gives no convincing
explanation to reassure about the lack of bad behavior for examples
not listed.

Perhaps this patch has a prerequisite and has come out of order.

Hope that helps,
Jonathan

Patch left unsnipped so we can get a copy in the list archive.

 Currently the first ref is handled properly, but not the rest, so:
 
  % git fast-export master ^master
 
 Would currently throw a reset for master (2nd ref), which is not what we
 want.
 
  % git fast-export master ^foo ^bar ^roo
  % git fast-export master salsa..tacos
 
 Even if all these refs point to the same object; foo, bar, roo, salsa,
 and tacos would all get a reset.
 
 This is most certainly not what we want. After this patch, nothing gets
 exported, because nothing was selected (everything is UNINTERESTING).
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  builtin/fast-export.c  | 7 ---
  t/t9350-fast-export.sh | 6 ++
  2 files changed, 10 insertions(+), 3 deletions(-)
 
 diff --git a/builtin/fast-export.c b/builtin/fast-export.c
 index 065f324..7fb6fe1 100644
 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array 
 *pending,
   typename(e-item-type));
   continue;
   }
 - if (commit-util)
 + if (commit-util) {
   /* more than one name for the same object */
 - string_list_append(extra_refs, full_name)-util = 
 commit;
 - else
 + if (!(commit-object.flags  UNINTERESTING))
 + string_list_append(extra_refs, full_name)-util 
 = commit;
 + } else
   commit-util = full_name;
   }
  }
 diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
 index 49bdb44..6ea8f6f 100755
 --- a/t/t9350-fast-export.sh
 +++ b/t/t9350-fast-export.sh
 @@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
   )
  '
  
 +test_expect_success 'proper extra refs handling' '
 + git fast-export master ^master master..master  actual 
 + echo -n  expected 
 + test_cmp expected actual
 +'
 +
  test_done
 -- 
 1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/4] fast-export: general fixes

2012-10-30 Thread Felipe Contreras
Hi,

Note: sorry for the noise, the first try (v2) was silently eaten by the mailing
list handler.

First patches are general cleanups and fixes, the last patch fixes a real issue
that affects remote helpers.

Changes since v2:

 * Actually send it to the ml

Changes since v1:

 * Improved commit messages
 * Use /dev/null in tests
 * Add test for remote helpers

Felipe Contreras (4):
  fast-export: trivial cleanup
  fast-export: fix comparisson in tests
  fast-export: don't handle uninteresting refs
  fast-export: make sure refs are updated properly

 builtin/fast-export.c | 16 +++-
 t/t5800-remote-helpers.sh | 11 +++
 t/t9350-fast-export.sh| 26 +++---
 3 files changed, 45 insertions(+), 8 deletions(-)

-- 
1.8.0

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


[PATCH v3 1/4] fast-export: trivial cleanup

2012-10-30 Thread Felipe Contreras
Setting commit to commit is a no-op. It might have been there to avoid a
compiler warning, but if so, it was the compiler to blame.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 builtin/fast-export.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 12220ad..065f324 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array 
*pending,
for (i = 0; i  pending-nr; i++) {
struct object_array_entry *e = pending-objects + i;
unsigned char sha1[20];
-   struct commit *commit = commit;
+   struct commit *commit;
char *full_name;
 
if (dwim_ref(e-name, strlen(e-name), sha1, full_name) != 1)
-- 
1.8.0

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


[PATCH v3 2/4] fast-export: fix comparisson in tests

2012-10-30 Thread Felipe Contreras
First the expected, then the actual, otherwise the diff would be the
opposite of what we want.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 t/t9350-fast-export.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 3e821f9..49bdb44 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '
 (
cd limit-by-paths 
git fast-export --tag-of-filtered-object=drop mytag -- there  output 
-   test_cmp output expected
+   test_cmp expected output
 )
 '
 
@@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '
 (
cd limit-by-paths 
git fast-export --tag-of-filtered-object=rewrite mytag -- there  
output 
-   test_cmp output expected
+   test_cmp expected output
 )
 '
 
@@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
(
cd limit-by-paths 
git fast-export master~2..master~1  output 
-   test_cmp output expected
+   test_cmp expected output
)
 '
 
-- 
1.8.0

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


[PATCH v3 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Felipe Contreras
When an object has already been exported (and thus is in the marks) it
is flagged as SHOWN, so it will not be exported again, even if this time
it's exported through a different ref.

We don't need the object to be exported again, but we want the ref
updated, which doesn't happen.

Since we can't know if a ref was exported or not, let's just assume that
if the commit was marked (flags  SHOWN), the user still wants the ref
updated.

So:

 % git branch test master
 % git fast-export $mark_flags master
 % git fast-export $mark_flags test

Would export 'test' properly.

Additionally, this fixes issues with remote helpers; now they can push
refs wich objects have already been exported.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 builtin/fast-export.c | 11 ---
 t/t5800-remote-helpers.sh | 11 +++
 t/t9350-fast-export.sh| 14 ++
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7fb6fe1..663a93d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -523,11 +523,16 @@ static void get_tags_and_duplicates(struct object_array 
*pending,
typename(e-item-type));
continue;
}
-   if (commit-util) {
-   /* more than one name for the same object */
+
+   /*
+* This ref will not be updated through a commit, lets make
+* sure it gets properly upddated eventually.
+*/
+   if (commit-util || commit-object.flags  SHOWN) {
if (!(commit-object.flags  UNINTERESTING))
string_list_append(extra_refs, full_name)-util 
= commit;
-   } else
+   }
+   if (!commit-util)
commit-util = full_name;
}
 }
diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index e7dc668..69a145a 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -145,4 +145,15 @@ test_expect_failure 'push new branch with old:new refspec' 
'
compare_refs clone HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'push ref with existing object' '
+   (cd localclone 
+   git branch point-to-master master 
+   git push origin point-to-master
+   ) 
+
+   (cd server 
+   git show-ref refs/heads/point-to-master
+   )
+'
+
 test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 6ea8f6f..a4178e3 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -446,4 +446,18 @@ test_expect_success 'proper extra refs handling' '
test_cmp expected actual
 '
 
+cat  expected  EOF
+reset refs/heads/master
+from :13
+
+EOF
+
+test_expect_success 'refs are updated even if no commits need to be exported' '
+   git fast-export --import-marks=tmp-marks \
+   --export-marks=tmp-marks master  /dev/null 
+   git fast-export --import-marks=tmp-marks \
+   --export-marks=tmp-marks master  actual 
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.0

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


Re: git push tags

2012-10-30 Thread Chris Rorvick
On Tue, Oct 30, 2012 at 1:34 PM, Angelo Borsotti
angelo.borso...@gmail.com wrote:
 Hi Cris,

 I think a key in the config file of the remote repo is better than an
 option on git-push for what concerns security: it allows the owner of
 the remote repo to enforce the policy not to overwrite tags, which
 would not be possible if any user that has push access can --force
 tags.

 -Angelo

Hi Angelo,

Security is orthogonal to what this patch is attempting to resolve.
As Kacper pointed out, you can never be sure you're not going to
clobber an existing tag in the remote repo.  This patch attempts to
give git-push better (i.e., less surprising) semantics for tags.  In
other words, it's should will prevent mistakes, not provide any sort
of security.

So I don't think a config option is better or worse, it's just trying
to solve a different problem.

Thanks,

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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Felipe Contreras
(again to the mailing list)

On Tue, Oct 30, 2012 at 7:59 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 They have been marked as UNINTERESTING for a reason, lets respect that.

That doesn't say anything.

 and in the examples listed in the patch
 description the changed behavior does not look like an improvement.

I disagree.

% git log master ^master

What do you expect? Nothing.

% git fast-export master ^master

What do you expect? Nothing.

 Worse, the description lists a few examples but gives no convincing
 explanation to reassure about the lack of bad behavior for examples
 not listed.

What examples not listed?

-- 
Felipe Contreras
--
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 v4 00/13] New remote-hg helper

2012-10-30 Thread Johannes Schindelin
Hi Felipe,

On Tue, 30 Oct 2012, Felipe Contreras wrote:

 But you mentioned something about cooperation, and I've yet to see how
 is it that you are planning to cooperate. If you say you don't have time
 to spend on this, I don't see why I should worry about testing this
 series of patches.

It has been mentioned before that the communication style including all
these snarky and nasty comments is not helpful. It is hardly the first
time that your mails have been insulting, as can be researched easily from
in the public mailing list archives.

In light of the indignation when advised to keep the tone down a little,
it is probable that the mails were never put through the would I be
insulted or hurt if I was the recipient of this? test, as in do you want
me to throw away my work? when you literally asked us to throw away our
work.

So unlike others, I do not ask you to change your tone, nor your
willingness to work with others. Instead, I prefer to do other things
instead.

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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Johannes Schindelin
Hi Felipe,

On Tue, 30 Oct 2012, Felipe Contreras wrote:

 On Tue, Oct 30, 2012 at 8:01 PM, Jonathan Nieder jrnie...@gmail.com wrote:
  Felipe Contreras wrote:
  On Tue, Oct 30, 2012 at 7:47 PM, Jonathan Nieder jrnie...@gmail.com 
  wrote:
 
  and in the examples listed in the patch
  description the changed behavior does not look like an improvement.
 
  I disagree.
 
  % git log master ^master
 
  What do you expect? Nothing.
 
  Yep.
 
  % git fast-export master ^master
 
  What do you expect? Nothing.
 
  Nope.
 
 That's _your_ opinion. I would like to see what others think.

If you wanted to prove that you can work with others without offending
them, I think that failed.

Ciao,
Johannes
--
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 v4 00/13] New remote-hg helper

2012-10-30 Thread Felipe Contreras
Hi,

On Tue, Oct 30, 2012 at 8:33 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 On Tue, 30 Oct 2012, Felipe Contreras wrote:

 But you mentioned something about cooperation, and I've yet to see how
 is it that you are planning to cooperate. If you say you don't have time
 to spend on this, I don't see why I should worry about testing this
 series of patches.

 It has been mentioned before that the communication style including all
 these snarky and nasty comments is not helpful.

Snarky and nasty comments? How about this?

---
 As to the functionality you seek: git-remote-hg found in
 git://github.com/msysgit/git works. It has the following advantages
 over every other solution, including the one proposed in this thread:

 - it works

 - no really, it works

 - it supports pushes, too

 - it matured over a long time

 - there are tests

 - whenever we fixed bugs, we also added tests for the bug fixes

 - it is rock solid

 - it is in constant use

 Without push support, remote-hg is useless to me. Without regression
 tests proving that it is rock solid, I will not use remote-hg. And I
 will not indulge in efforts to duplicate work.
---

How many times does somebody has to say it works before it becomes a
snarky comment?

Or this?

---
 FTR, the reason that it's crashing is because you're lying. You're
 saying you already have master (by means of ^master), but you don't.
---

Or this?

---
 It seems unlikely to me that this never worked, surely no reviewer
 would accept a patch that doesn't actually implement the feature?
 What's the history here?
---

So what did I say?

 But you mentioned something about cooperation,

That's a fact.

Johannes:
---
  It would be better to work together, but to me the code-styles are way
  too different, the difference between night and day.

 Aha. Well, okay, it was an offer to collaborate.
---

 and I've yet to see how is it that you are planning to cooperate.

This is also a fact. You haven't provided a branch, you haven't reviewed
my implementation, you haven't tried it. You mentioned something about
testing the performance, but then retracted from it.

So, if you were planning to collaborate, now it would be a good time to
mention how.

 If you say you don't have time to spend on this, I don't see why I
 should worry about testing this series of patches.

I'm just clarifying how I'm planning to spend my time, specifically if
you are not going to collaborate.

What is snarky and nasty about any of these comments? I'm simply asking
you if you are going to collaborate and how, because I don't see it,
and what I'm going to do.

You think that's snarkier than the comments above? Well, I disagree.
But I don't blame you when you are snarky, nor do I think I should.

 It is hardly the first time that your mails have been insulting, as
 can be researched easily from in the public mailing list archives.

Those who want to be insulted would get insulted. I asked
a simple question are you going to collaborate?, if you find that
offensive, that's your right.

 In light of the indignation when advised to keep the tone down a little,
 it is probable that the mails were never put through the would I be
 insulted or hurt if I was the recipient of this? test, as in do you want
 me to throw away my work? when you literally asked us to throw away our
 work.

How did I ask you to throw away your work? I have asked multiple times
now for you to provide a branch so that we can take a look and try it.

I don't know of a better way to throw away code than to refuse to
provide it, which is what you have been doing. So, if anybody can be
blamed of trying to throw away code, it shouldn't be me.

I know you will find the previous statement offensive, but it happens to
be true. It is sad that you will concentrate on the statement, rather
than the fact, and instead of providing the branch (which will help
to avoid throwing away the code), and thus nullify the statement, you
choose to be offended and complain about how offended you are.

Well, I'm offended at how much you refuse to collaborate, and at how
much disdain you throw at my code, but I'm not going to complain about
me being offended; people get offended about all sorts of things. Why
is why there's no law against offending people.

Instead, I choose to do something positive about it and improve my code
with your criticism (e.g. lack of tests), even if that criticism is rude
and unwarranted. But that seems to mean nothing to you.

 So unlike others, I do not ask you to change your tone, nor your
 willingness to work with others. Instead, I prefer to do other things
 instead.

I guess that answers the question; you are not going to collaborate. Got
it.

I will not ask you again for a branch with the remote-hg code.

Cheers.

-- 
Felipe Contreras
--
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: Can't understand the behaviour of git-diff --submodule

2012-10-30 Thread Jens Lehmann
Am 28.10.2012 01:02, schrieb Jens Lehmann:
 Am 26.10.2012 22:43, schrieb Francis Moreau:
 On Fri, Oct 26, 2012 at 10:05 PM, Jens Lehmann jens.lehm...@web.de wrote:
 [...]

 That is weird, git diff --submodule should show that too. Is there
 anything unusual about your setup? (The only explanation I can come
 up with after checking the code is that your submodule has neither a
 .git directory nor a gitfile or the objects directory in there doesn't
 contain these commits)

 Oh now you're asking, I think the submodule has been added by using
 the --reference option of git-submodule-add.

   $ cd configs
   $ cat .git
   gitdir: ../.git/modules/configs
 
 Thanks, I suspect the --reference option makes the difference here,
 I'll check that as soon as I find some time.

Since 1.7.11 and 1.7.10.3 git does handle submodules with alternates
(which is what --reference uses) correctly. What version are you
seeing this problem with?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Sverre Rabbelier
On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Why would it? We are not changing the way objects are exported, the
 only difference is what happens at the end
 (handle_tags_and_duplicates()).

Because the marking is per-commit, not per-ref, right? Perhaps you
could add a simple test case to make sure it works as expected?
Something along the lines of the scenario I described in my previous
email?

-- 
Cheers,

Sverre Rabbelier
--
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: change symlink

2012-10-30 Thread Andreas Schwab
shawn wilson ag4ve...@gmail.com writes:

 i'm curious why this is being reported as deleted in status and diff
 and not modified? this was tested on a build of the master branch of
 the current git repo (1.8.0).

 mkdir t cd t; git --init

 touch test
 git add test
 git commit test -m test

 ln -s test t2
 git add t2
 git commit t2 -m symlink

 rm t2
 mkdir -p t2/one
 ln -s test t2/one/test
  git add t2/one/test

 this then shows up as:

  % git status
 # On branch master
 # Changes not staged for commit:
 #   (use git add/rm file... to update what will be committed)
 #   (use git checkout -- file... to discard changes in working directory)
 #
 #   deleted:t2
 #
 no changes added to commit (use git add and/or git commit -a)

I'd expected t2/one/test be reported as untracked.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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: change symlink

2012-10-30 Thread shawn wilson
On Tue, Oct 30, 2012 at 9:19 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 shawn wilson ag4ve...@gmail.com writes:

  % git status
 # On branch master
 # Changes not staged for commit:
 #   (use git add/rm file... to update what will be committed)
 #   (use git checkout -- file... to discard changes in working directory)
 #
 #   deleted:t2
 #
 no changes added to commit (use git add and/or git commit -a)

 I'd expected t2/one/test be reported as untracked.


that's fine - i can do 'git list-files --others' but should t2 be
reported as 'deleted'?
--
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] completion: simplify __gitcomp test helper

2012-10-30 Thread SZEDER Gábor
On Mon, Oct 22, 2012 at 03:39:01AM +0200, Felipe Contreras wrote:
 By using print_comp as suggested by SZEDER Gábor.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  t/t9902-completion.sh | 13 +
  1 file changed, 5 insertions(+), 8 deletions(-)
 
 diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
 index 1c6952a..2e7fc06 100755
 --- a/t/t9902-completion.sh
 +++ b/t/t9902-completion.sh
 @@ -74,15 +74,12 @@ newline=$'\n'

This $newline variable was only used to set IFS to a newline inside SQ
blocks.  AFAICS after this change there are no such places left,
because print_comp() takes care of IFS, so $newline is not necessary
anymore.

  test_gitcomp ()
  {
 + local -a COMPREPLY 
   sed -e 's/Z$//'  expected 
 - (
 - local -a COMPREPLY 
 - cur=$1 
 - shift 
 - __gitcomp $@ 
 - IFS=$newline 
 - echo ${COMPREPLY[*]}  out
 - ) 
 + cur=$1 
 + shift 
 + __gitcomp $@ 
 + print_comp 
   test_cmp expected out
  }
  
 -- 
 1.8.0
 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Teach rm to remove submodules when given with a trailing '/'

2012-10-30 Thread Jens Lehmann
Am 29.10.2012 08:11, schrieb Johannes Sixt:
 Am 10/29/2012 0:28, schrieb Jens Lehmann:
 +/* Remove trailing '/' from directories to find submodules in the index 
 */
 +for (i = 0; i  argc; i++) {
 +size_t pathlen = strlen(argv[i]);
 +if (pathlen  is_directory(argv[i])  (argv[i][pathlen - 1] 
 == '/'))
 +argv[i] = xmemdupz(argv[i], pathlen - 1);
 +}
 +
  pathspec = get_pathspec(prefix, argv);
  refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL);
 
 That's wrong: Either move the check below get_pathspec() (which normalizes
 backslashes to forward-slashes on Windows) or use is_dir_sep().

Thanks for bringing this up.

 But isn't it somewhat dangerous to check pathspec for existance in the
 worktree without interpreting them? Think of magic pathspec syntax (that
 we do not have yet, but which may materialize sometime in the future).

I have to admit I'm not aware of magic pathspec syntax. Do you happen to
have any pointers where I could look at code doing similar things right?
--
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: change symlink

2012-10-30 Thread Andreas Schwab
shawn wilson ag4ve...@gmail.com writes:

 but should t2 be reported as 'deleted'?

Sure, that's what you did.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 % git fast-export $marks_args one
 % git fast-export $marks_args one two

 Then yeah, 'one' will be updated once again in the second command,

That's probably worth a mention in the commit message and tests
(test_expect_failure), to save future readers from some confusion.

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 v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 8:01 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:
 On Tue, Oct 30, 2012 at 7:47 PM, Jonathan Nieder jrnie...@gmail.com wrote:

 and in the examples listed in the patch
 description the changed behavior does not look like an improvement.

 I disagree.

 % git log master ^master

 What do you expect? Nothing.

 Yep.

 % git fast-export master ^master

 What do you expect? Nothing.

So you think what we have now is the correct behavior:

% git fast-export master ^master
reset refs/heads/master
from :0

That of course would crash fast-import. But hey, it's your opinion.

Would be interesting to see if other people think the above is correct.

Cheers.

-- 
Felipe Contreras
--
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] completion: simplify __gitcomp test helper

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 10:27 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Mon, Oct 22, 2012 at 03:39:01AM +0200, Felipe Contreras wrote:
 By using print_comp as suggested by SZEDER Gábor.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  t/t9902-completion.sh | 13 +
  1 file changed, 5 insertions(+), 8 deletions(-)

 diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
 index 1c6952a..2e7fc06 100755
 --- a/t/t9902-completion.sh
 +++ b/t/t9902-completion.sh
 @@ -74,15 +74,12 @@ newline=$'\n'

 This $newline variable was only used to set IFS to a newline inside SQ
 blocks.  AFAICS after this change there are no such places left,
 because print_comp() takes care of IFS, so $newline is not necessary
 anymore.

Right, I thought I did that =/

-- 
Felipe Contreras
--
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: change symlink

2012-10-30 Thread shawn wilson
On Tue, Oct 30, 2012 at 9:35 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 shawn wilson ag4ve...@gmail.com writes:

 but should t2 be reported as 'deleted'?

 Sure, that's what you did.


if i do the same to a file (same repo):

touch test2
git add test2
git commit test2 -m test2

rm test
ln -s test2 test

git status

# On branch master
# Changes not staged for commit:
#   (use git add/rm file... to update what will be committed)
#   (use git checkout -- file... to discard changes in working directory)
#
#   deleted:t2
#   typechange: test
#
no changes added to commit (use git add and/or git commit -a)


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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 So you think what we have now is the correct behavior:

 % git fast-export master ^master
 reset refs/heads/master
 from :0

No, I don't think that, either.

Hope that helps,
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: change symlink

2012-10-30 Thread Andreas Schwab
shawn wilson ag4ve...@gmail.com writes:

 why is this different?

You didn't tell git about t2/one/test.  You need to add it first to make
it known.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Sverre Rabbelier
On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier srabbel...@gmail.com 
 wrote:
 On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 Why would it? We are not changing the way objects are exported, the
 only difference is what happens at the end
 (handle_tags_and_duplicates()).

 Because the marking is per-commit, not per-ref, right?

 Oh, you meant using marks?

No, I meant the 'SHOWN' flag, doesn't it get added per commit, not per
ref? That is, commit-object.flags  SHOWN refers to the object
underlying the ref. So I suspect this scenario doesn't pass the tests:

git init 
echo first  content 
git add content 
git commit -m first 
git branch first 
echo two  content 
git commit -m second 
git branch second 
git fast-export first  actual 
test_cmp actual expected_first 
git fast-export second  actual 
test_cmp actual expected_second

With expected_first being something like:
fast-export stream with the first commit
reset command to set first to the right commit

And expected_second being something like
fast export stream with the first and second command
reset command to set first and second to their respective branches

-- 
Cheers,

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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 10:45 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 So you think what we have now is the correct behavior:

 % git fast-export master ^master
 reset refs/heads/master
 from :0

 No, I don't think that, either.

Well, that's what we have now, and you want to preserve this feature
(aka bug), right?

And I still haven't why this is unsafe, and what are those examples
not listed.

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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 Well, that's what we have now, and you want to preserve this feature
 (aka bug), right?

Nope.  I just don't want regressions, and found a patch description
that did nothing to explain to the reader how it avoids regressions
more than a little disturbing.

I also think the proposed behavior is wrong.

I don't think there's much benefit to be gained from continuing to
discuss this.  Consider it a single data point: I would be deeply
worried if this patch were applied without at least a clearer
description of the change in behavior.  Maybe I'm the only one!

Hope that helps,
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] test-lib: avoid full path to store test results

2012-10-30 Thread Elia Pinto
Thanks. I know that posix support these usages, but exists some
traditional shell that not support it. These are described in the
autoconf manual, last time i have checked. As the construct ; export
var = x should be portable, but it is not. If this is important these
days i don't know.


Best

2012/10/30, Jonathan Nieder jrnie...@gmail.com:
 Elia Pinto wrote:

 The shell word splitting done in base is a bashism, iow not portable.

 No, ${varname##glob} is in POSIX and we already use it here and there.
 See Documentation/CodingGuidelines:

- We use ${parameter#word} and its [#%] siblings, and their
  doubled longest matching form.

 Thanks for looking the patch over.
 Jonathan


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


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 10:59 PM, Sverre Rabbelier srabbel...@gmail.com wrote:
 On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier srabbel...@gmail.com 
 wrote:
 On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 Why would it? We are not changing the way objects are exported, the
 only difference is what happens at the end
 (handle_tags_and_duplicates()).

 Because the marking is per-commit, not per-ref, right?

 Oh, you meant using marks?

 No, I meant the 'SHOWN' flag, doesn't it get added per commit, not per
 ref? That is, commit-object.flags  SHOWN refers to the object
 underlying the ref. So I suspect this scenario doesn't pass the tests:

Without marks you cannot have the SHOWN mark at that point; we haven't
traversed the commits.

 git init 
 echo first  content 
 git add content 
 git commit -m first 
 git branch first 
 echo two  content 
 git commit -m second 
 git branch second 
 git fast-export first  actual 
 test_cmp actual expected_first 
 git fast-export second  actual 
 test_cmp actual expected_second

 With expected_first being something like:
 fast-export stream with the first commit
 reset command to set first to the right commit

Why would a 'reset' command be expected if the 'first' branch is
already pointing to the 'first' commit?

 And expected_second being something like
 fast export stream with the first and second command
 reset command to set first and second to their respective branches

Ditto, plus, why would 'git fast-export second' do anything regarding
'first'? It wasn't specified in the committish; it's not relevant.

Before an after my patch the output is the same:

% git fast-export first:
reset refs/heads/first
commit refs/heads/first

% git fast-export second:
reset refs/heads/second
commit refs/heads/second
commit refs/heads/second

Which is expected and correct; the branch already points to the right
commit, no need for an extra reset.

Cheers.

-- 
Felipe Contreras
--
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: change symlink

2012-10-30 Thread shawn wilson
On Tue, Oct 30, 2012 at 10:09 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 shawn wilson ag4ve...@gmail.com writes:

 and once it's added, status says:
 #   renamed:t2 - t2/one/test

 that's not exactly true, but...

 What's wrong with it?  Both files have the same contents, which is the
 link target for symlinks.


ok, you're right about that (another test where i changed where the
symlink pointed):
#   deleted:test
#   new file:   test/one/t3


however (what got me started wondering about this and a point i forgot
about) - t2/one/test doesn't show up under 'untracked files' in in
status that scenario. shouldn't 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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 11:07 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 Well, that's what we have now, and you want to preserve this feature
 (aka bug), right?

 Nope.  I just don't want regressions, and found a patch description
 that did nothing to explain to the reader how it avoids regressions
 more than a little disturbing.

I see, so you don't have any specific case where this could cause
regressions, you are just saying it _might_ (like all patches).

 I also think the proposed behavior is wrong.

That's a different matter, lets see what others think.

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


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Sverre Rabbelier
On Tue, Oct 30, 2012 at 3:18 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Which is expected and correct; the branch already points to the right
 commit, no need for an extra reset.

I think you're correct. Thanks for confirming.

-- 
Cheers,

Sverre Rabbelier
--
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] completion: refactor __gitcomp related tests

2012-10-30 Thread SZEDER Gábor
On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:
 Lots of duplicated code!
 
 No functional changes.

I'm not sure.
I'm all for removing duplicated application code, but I'm usually more
conservative when it comes to test code.  The more logic, the more
possibility for bugs in tests.  So tests should be dead simple, even
if that means some duplicated test code or the lack of convenience
functions.
While this might be considered just a matter of personal preference, ...

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  t/t9902-completion.sh | 72 
 ---
  1 file changed, 23 insertions(+), 49 deletions(-)
 
 diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
 index cbd0fb6..1c6952a 100755
 --- a/t/t9902-completion.sh
 +++ b/t/t9902-completion.sh
 @@ -72,87 +72,61 @@ test_completion_long ()
  
  newline=$'\n'
  
 -test_expect_success '__gitcomp - trailing space - options' '
 - sed -e s/Z$// expected -\EOF 
 - --reuse-message=Z
 - --reedit-message=Z
 - --reset-author Z
 - EOF
 +test_gitcomp ()
 +{
 + sed -e 's/Z$//'  expected 
   (
   local -a COMPREPLY 
 - cur=--re 
 - __gitcomp --dry-run --reuse-message= --reedit-message=
 - --reset-author 
 + cur=$1 
 + shift 
 + __gitcomp $@ 

... I was really puzzled by how __gitcomp() gets its arguments here,
and had to think for a while to figure out why it's not broken.

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


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 11:35 PM, Sverre Rabbelier srabbel...@gmail.com wrote:
 On Tue, Oct 30, 2012 at 3:18 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 Which is expected and correct; the branch already points to the right
 commit, no need for an extra reset.

 I think you're correct. Thanks for confirming.

Thanks for reviewing. If you are still not convinced, I could pull the
patches from msysgit and simplify them, I'm sure the end result would
be pretty similar, if not exactly the same as this patch (plus other
orthogonal changes). I saw some patches that were not part of the
patch series you sent before, so maybe that's why you expected certain
behavior that wasn't actually there in that particular patch series.

But hopefully that's not needed.

Cheers.

-- 
Felipe Contreras
--
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/3] completion: add new __gitcompadd helper

2012-10-30 Thread SZEDER Gábor
On Mon, Oct 22, 2012 at 03:45:41AM +0200, Felipe Contreras wrote:
 The idea is to never touch the COMPREPLY variable directly.
 
 This allows other completion systems override __gitcompadd, and do
 something different instead.
 
 Also, this allows the simplification of the completion tests (separate
 patch).

This doesn't apply anymore, does it?  The mentioned simplification is
done in the other series.

--
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] completion: refactor __gitcomp related tests

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 11:45 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:
 Lots of duplicated code!

 No functional changes.

 I'm not sure.
 I'm all for removing duplicated application code, but I'm usually more
 conservative when it comes to test code.  The more logic, the more
 possibility for bugs in tests.  So tests should be dead simple, even
 if that means some duplicated test code or the lack of convenience
 functions.
 While this might be considered just a matter of personal preference, ...

Fair enough, but my preference is different: test code should be the
same as normal code; easy to maintain.

 +test_gitcomp ()
 +{
 + sed -e 's/Z$//'  expected 
   (
   local -a COMPREPLY 
 - cur=--re 
 - __gitcomp --dry-run --reuse-message= --reedit-message=
 - --reset-author 
 + cur=$1 
 + shift 
 + __gitcomp $@ 

 ... I was really puzzled by how __gitcomp() gets its arguments here,
 and had to think for a while to figure out why it's not broken.

You mean because $@ is treated in a special way? Without that
behavior I'm not sure many things would be possible :)

Anyway, I don't think we should make our life more difficult while
maintaining test code... that's my opinion.

Cheers.

-- 
Felipe Contreras
--
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/3] completion: add new __gitcompadd helper

2012-10-30 Thread Felipe Contreras
On Tue, Oct 30, 2012 at 11:58 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Mon, Oct 22, 2012 at 03:45:41AM +0200, Felipe Contreras wrote:
 The idea is to never touch the COMPREPLY variable directly.

 This allows other completion systems override __gitcompadd, and do
 something different instead.

 Also, this allows the simplification of the completion tests (separate
 patch).

 This doesn't apply anymore, does it?  The mentioned simplification is
 done in the other series.

Yeah, but you mentioned you didn't like all the COMPREPLY=() changes
and it might be time to get rid of them.

So this series supersedes that one.

-- 
Felipe Contreras
--
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 v5 1/3] completion: add new __gitcompadd helper

2012-10-30 Thread SZEDER Gábor
On Mon, Oct 22, 2012 at 02:41:21AM +0200, Felipe Contreras wrote:
 On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:
 
  diff --git a/contrib/completion/git-completion.bash 
  b/contrib/completion/git-completion.bash
  index d743e56..01325de 100644
  --- a/contrib/completion/git-completion.bash
  +++ b/contrib/completion/git-completion.bash
  @@ -225,6 +225,11 @@ _get_comp_words_by_ref ()
   fi
   fi
 
  +__gitcompadd ()
  +{
  + COMPREPLY=($(compgen -W $1 -P $2 -S $4 -- $3))
  +}
  +
   # Generates completion reply with compgen, appending a space to possible
   # completion words, if necessary.
   # It accepts 1 to 4 arguments:
  @@ -238,13 +243,11 @@ __gitcomp ()
 
case $cur_ in
--*=)
  - COMPREPLY=()
  + __gitcompadd
;;
*)
local IFS=$'\n'
  - COMPREPLY=($(compgen -P ${2-} \
  - -W $(__gitcomp_1 ${1-} ${4-}) \
  - -- $cur_))
  + __gitcompadd $(__gitcomp_1 ${1-} ${4-}) ${2-} 
  $cur_ 
;;
esac
   }
  @@ -261,7 +264,7 @@ __gitcomp ()
   __gitcomp_nl ()
   {
local IFS=$'\n'
  - COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
  + __gitcompadd $1 ${2-} ${3-$cur} ${4- }
   }
 
  I feel hesitant about this change.  One of the ways I'm exploring to
  fix the issues with shell metacharacters and expansion in compgen is
  to actually replace compgen.  We already iterate over all possible
  completion words in __gitcomp_1(), so it doesn't make much of a
  difference to do the filtering for the current word while we are at
  it.  However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen
  ...)), and tha basic idea of never touching COMPREPLY directly make
  this basically impossible.
 
 How is it impossible? You can still replace compgen, all you have to
 do is modify __gitcompadd and replace that code with whatever custom
 code you want. You can change the arguments and everything. The only
 limitation is that it should be the only place where COMPREPLY is
 modified, and all is good. Well, it doesn't have to be only _one_
 place, but the less functions that do this, the better.

That's exactly the problem: there isn't, there can't be one single
whatever custom code I want.

The compgen() in __gitcomp() will be replaced by an enhanced version
of the loop in __gitcomp_1(), while in __gitcomp_nl() it will be
replaced by a little awk scriptlet.  And then there is the oddball
$(git ls-tree |sed magic) in __git_complete_revlist_file(), where
possible completion words are filenames possibly containing newlines,
therefore requiring yet another approach.

--
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: Links broken in ref docs.

2012-10-30 Thread Mike Norman
I just checked and the issue seems to be fixed! Clicked around on a
bunch of previously broken links and they work!

On Tue, Oct 30, 2012 at 3:38 AM, Holger Hellmuth (IKS)
hellm...@ira.uka.de wrote:
 Am 30.10.2012 09:07, schrieb Mike Norman:

 Not seen any recently. I'm guessing the dev is in the path of
 hurricane Sandy? (Not sarcasm, btw.)


 Do you still see failures? I checked out the website just now and it seemed
 to work flawlessly (at least the links I tried, could not find any Sharing
 or Updating section). New design since I last visited, more end-user polish.


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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:
 On Tue, Oct 30, 2012 at 11:07 PM, Jonathan Nieder jrnie...@gmail.com wrote:

 Nope.  I just don't want regressions, and found a patch description
 that did nothing to explain to the reader how it avoids regressions
 more than a little disturbing.

 I see, so you don't have any specific case where this could cause
 regressions, you are just saying it _might_ (like all patches).

Yes, exactly.  The commit log needs a description of the current
behavior, the intent behind the current code, the change the patch
makes, and the motivation behind that change, like all patches.
Despite the nice examples, it doesn't currently have that.

The patch description just raises more questions for the reader.  From
the description, one might imagine that this patch causes

git fast-export mark args master

not to emit anything when another branch that has already been
exported is ahead of master.  If I understand correctly (though
I haven't tested), this patch does cause

git fast-export ^next master

not to emit anything when next is ahead of master.  That doesn't
seem like progress.

I haven't reviewed the later patches in the series; maybe they fix
these things.  But in the long term it is much easier to understand
and maintain a patch series that does not introduce regressions in the
first place, and the context one might use to convincingly explain
that a patch is not introducing a regression turns out to be essential
for many other purposes as well.

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 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -523,11 +523,16 @@ static void get_tags_and_duplicates(struct object_array 
 *pending,
   typename(e-item-type));
   continue;
   }
 - if (commit-util) {
 - /* more than one name for the same object */
 +
 + /*
 +  * This ref will not be updated through a commit, lets make
 +  * sure it gets properly upddated eventually.
 +  */
 + if (commit-util || commit-object.flags  SHOWN) {
   if (!(commit-object.flags  UNINTERESTING))
   string_list_append(extra_refs, full_name)-util 
 = commit;
 - } else
 + }
 + if (!commit-util)
   commit-util = full_name;

Here's an explanation of why the above makes sense to me.

get_tags_and_duplicates() gets called after the marks import and
before the revision walk.  It walks through the revs from the
commandline and for each one:

 - peels it to a refname, and then to a commit
 - stores the refname so fast-export knows what arg to pass to
   the commit command during the revision walk
 - if it already had a refname stored, instead adds the
   (refname, commit) pair to the extra_refs list, so fast-export
   knows to add a reset command later.

If the commit already has the SHOWN flag set because it was pointed to
by a mark, it is not going to come up in the revision walk, so it will
not be mentioned in the output stream unless it is added to
extra_refs.  That's what this patch does.

Incidentally, the change from else to if (!commit-util) is
unnecessary because if a commit is already SHOWN then it will not be
encountered in the revision walk so commit-util does not need to be
set.

If the commit does not have the SHOWN or UNINTERESTING flag set but it
is going to get the UNINTERESTING flag set during the walk because of
a negative commit listed on the command line, this patch won't help.

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 v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Hi again,

Felipe Contreras wrote:

 They have been marked as UNINTERESTING for a reason, lets respect that.

So, the above description conveyed zero information, as you mentioned.

A clearer explanation would be the following:

fast-export: don't emit reset command for negative refs

When git fast-export encounters two refs on the commandline
referring to the same commit, it exports the first during the usual
commit walk and the second using a reset command in a final pass
over extra_refs:

$ git fast-export master next
reset refs/heads/master
commit refs/heads/master
mark :1
author Jonathan Nieder jrnie...@gmail.com 1351644412 -0700
committer Jonathan Nieder jrnie...@gmail.com 1351644412 -0700
data 17
My first commit!

reset refs/heads/next
from :1

Unfortunately the code to do this doesn't distinguish between positive
and negative refs, producing confusing results:

$ git fast-export ^master next
reset refs/heads/next
from :0

$ git fast-export master ^next
reset refs/heads/next
from :0

Use revs-cmdline instead of revs-pending to iterate over the rev-list
arguments, checking the UNINTERESTING flag bit to distinguish between
positive (master, --all, etc) and negative (next.., --not --all, etc)
revs and avoid enqueueing negative revs in extra_revs.

This does not affect revs that were excluded from the revision walk
because pointed to by a mark, since those use the SHOWN bit on the
commit object itself and not UNINTERESTING on the rev_cmdline_entry.

A patch meeting the above description would make perfect sense to me.
Except for the somewhat strange testcase, the patch I am replying to
would also be fine in the short term, as long as it had an analagous
description (i.e., with an appropriate replacement for the
second-to-last paragraph).

Thanks for your patience, and hoping that helps,
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 v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Felipe Contreras
On Wed, Oct 31, 2012 at 12:55 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:
 On Tue, Oct 30, 2012 at 11:07 PM, Jonathan Nieder jrnie...@gmail.com wrote:

 Nope.  I just don't want regressions, and found a patch description
 that did nothing to explain to the reader how it avoids regressions
 more than a little disturbing.

 I see, so you don't have any specific case where this could cause
 regressions, you are just saying it _might_ (like all patches).

 Yes, exactly.  The commit log needs a description of the current
 behavior, the intent behind the current code, the change the patch
 makes, and the motivation behind that change, like all patches.
 Despite the nice examples, it doesn't currently have that.

 The patch description just raises more questions for the reader.  From
 the description, one might imagine that this patch causes

 git fast-export mark args master

 not to emit anything when another branch that has already been
 exported is ahead of master.

This is already the case.

I don't see what part of my patch description would give you the idea
that this would change in any way how the objects are flagged, or how
get_revision() decides how to traverse them.

I clearly stated that this doesn't affect *the first* ref, which is
handled properly already; this patch affects *the rest* of the refs,
of which you have none in that command above.

 If I understand correctly (though
 I haven't tested), this patch does cause

 git fast-export ^next master

 not to emit anything when next is ahead of master.  That doesn't
 seem like progress.

Again, this is already the case RIGHT NOW.

And nothing in my description should give you an idea that anything
would change for this case because the 2nd ref (*the first* doesn't
get affected), is not marked as UNINTERESTING.

Not only you are not reading what is in the description, but I don't
think you understand what the code actually does, and how it behaves.

Let me give you some examples:

% git fast-export ^next next
reset refs/heads/next
from :0

% git fast-export ^next next^{commit}
# nothing
% git fast-export ^next next~0
# nothing
% git fast-export ^next next~1
# nothing
% git fast-export ^next next~2
# nothing
...
# you get the idea

The *only time* when this patch would have any effect is when you
specify more than *one ref*, and they both point to *exactly the same
object*.

Additionally, and this is something I just found out; when the are
pure refs (e.g. 'next'), and not refs to objects (e.g.
'next^{commit}').

In any other case; *there would be no change*.

After my patch:

% git fast-export ^next next
# nothing
% git fast-export ^next next^{commit}
# nothing
% git fast-export ^next next~0
# nothing
% git fast-export ^next next~1
# nothing
% git fast-export ^next next~2
# nothing
...
# you get the idea

 But in the long term it is much easier to understand
 and maintain a patch series that does not introduce regressions in the
 first place

It does not introduce regressions.

I don't think it's my job to explain to you how 'git fast-export'
works. Above you made too many assumptions of what get broken, when in
fact that's the current behavior already... maybe, just maybe, you are
also making wrong assumptions about this patch as well.

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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 I don't think it's my job to explain to you how 'git fast-export'
 works.

Actually, if you are submitting a patch for inclusion, it is your job
to explain to future readers what the patch does.  Yes, the reader
might not be deeply familiar with the part of fast-export you are
modifying.  It might have even been modified since then, by the time
the reader is looking at the change!

Sad but true.

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 v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Felipe Contreras
Hi,

On Wed, Oct 31, 2012 at 1:57 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 They have been marked as UNINTERESTING for a reason, lets respect that.

 So, the above description conveyed zero information, as you mentioned.

I meant, this, of course:
 They have been marked as UNINTERESTING for a reason, lets respect that.

 This patch looks unsafe,

Which you know, because you received that message without the mistake.

 A clearer explanation would be the following:

 fast-export: don't emit reset command for negative refs

What is a negative ref?

 When git fast-export encounters two refs on the commandline

commandline?

Only two refs? How about four?

 referring to the same commit, it exports the first during the usual
 commit walk and the second using a reset command in a final pass
 over extra_refs:

That is not exactly true: (next^{commit}).

 $ git fast-export master next
 reset refs/heads/master
 commit refs/heads/master
 mark :1
 author Jonathan Nieder jrnie...@gmail.com 1351644412 -0700
 committer Jonathan Nieder jrnie...@gmail.com 1351644412 
 -0700
 data 17
 My first commit!

 reset refs/heads/next
 from :1

I don't think this example is good. Where does it say that 'next'
points to master? Using 'points-to-master' or a 'git branch stable
master' and using 'master stable'.

Even simpler would be to use 'git fast-export master master'; it would
show the same behavior.

 Unfortunately the code to do this doesn't distinguish between positive
 and negative refs, producing confusing results:

 $ git fast-export ^master next
 reset refs/heads/next
 from :0

 $ git fast-export master ^next
 reset refs/heads/next
 from :0

 Use revs-cmdline instead of revs-pending to iterate over the 
 rev-list
 arguments, checking the UNINTERESTING flag bit to distinguish between
 positive (master, --all, etc) and negative (next.., --not --all, etc)
 revs and avoid enqueueing negative revs in extra_revs.

Use what? You mean, To solve the problem, lets use.

But this is not correct, cmdline is not being used. Have you even
looked at the patch?

 This does not affect revs that were excluded from the revision walk
 because pointed to by a mark, since those use the SHOWN bit on the
 commit object itself and not UNINTERESTING on the rev_cmdline_entry.

revs? You mean commits?

excluded because point to by a mark? Doesn't sound like proper
grammar. Maybe excluded because they were pointed to by a mark.

And I don't see why this paragraph is needed at all. Why would the
reader think marks have anything to do with this? There's no mention
of marks before.

This might help you, or other people involved in the problem, but not
anybody else. Anything related to marks is completely orthogonal to
this patch, and there's no point in mentioning that.

-- 
Felipe Contreras
--
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] test-lib: avoid full path to store test results

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:
 On Tue, Oct 30, 2012 at 5:46 AM, Jonathan Nieder jrnie...@gmail.com wrote:
  Felipe Contreras wrote:

 No reason to use the full path in case this is used externally.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

 No reason not to is not a reason to do anything.  What symptoms does
 this prevent?  Could you describe the benefit of this patch in a
 paragraph starting Now you can ...?

 ./test-lib.sh: line 394:
 /home/felipec/dev/git/t/test-results//home/felipec/dev/git/contrib/remote-hg/test-21865.counts:
 No such file or directory

Ok, so a description for this patch is

test: use test's basename to name results file

Running a test using its full path produces an error:

$ ~/dev/git/contrib/remote-hg/test-21865.sh
[...]
./test-lib.sh: line 394: /home/felipec/dev/t/\
test-results//home/felipec/dev/git/contrib/remote-hg/\
test-21865.counts: No such file or directory

In --tee and --valgrind modes we already use the basename
to name the .out and .exit files; this patch teaches the test-lib
to name the .counts file the same way.

That is still not enough to tell if it is a good change, though.
Should the test results for contrib/remote-hg/test-* be included with
the results for t/t*.sh when I run make aggregate-results?

Before 60d02ccc, git-svn had its own testsuite under contrib/, with
glue in contrib/git-svn/t/lib-git-svn.sh to use test-lib --- maybe
that code could provide some inspiration for questions like these.

Hope that helps,
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 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Sverre Rabbelier
On Tue, Oct 30, 2012 at 5:37 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -523,11 +523,16 @@ static void get_tags_and_duplicates(struct 
 object_array *pending,
   typename(e-item-type));
   continue;
   }
 - if (commit-util) {
 - /* more than one name for the same object */
 +
 + /*
 +  * This ref will not be updated through a commit, lets make
 +  * sure it gets properly upddated eventually.
 +  */
 + if (commit-util || commit-object.flags  SHOWN) {
   if (!(commit-object.flags  UNINTERESTING))
   string_list_append(extra_refs, 
 full_name)-util = commit;
 - } else
 + }
 + if (!commit-util)
   commit-util = full_name;

 Here's an explanation of why the above makes sense to me.

 get_tags_and_duplicates() gets called after the marks import and
 before the revision walk.  It walks through the revs from the
 commandline and for each one:

  - peels it to a refname, and then to a commit
  - stores the refname so fast-export knows what arg to pass to
the commit command during the revision walk
  - if it already had a refname stored, instead adds the
(refname, commit) pair to the extra_refs list, so fast-export
knows to add a reset command later.

 If the commit already has the SHOWN flag set because it was pointed to
 by a mark, it is not going to come up in the revision walk, so it will
 not be mentioned in the output stream unless it is added to
 extra_refs.  That's what this patch does.

 Incidentally, the change from else to if (!commit-util) is
 unnecessary because if a commit is already SHOWN then it will not be
 encountered in the revision walk so commit-util does not need to be
 set.

 If the commit does not have the SHOWN or UNINTERESTING flag set but it
 is going to get the UNINTERESTING flag set during the walk because of
 a negative commit listed on the command line, this patch won't help.

Thanks for the thorough explanation. Perhaps some of that could make
it's way into the commit message?

-- 
Cheers,

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


Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

It's not my job to
 explain to you that 'git fast-export' doesn't work this way, you have
 a command line to type those commands and see for yourself if they do
 what you think they do with a vanilla version of git. That's exactly
 what I did, to make sure I'm not using assumptions as basis  for
 arguing, it took me a few minutes.

Well no, when I run git blame 10 years down the line and do not
understand what your code is doing, it is not at all reasonable to
expect me to checkout the parent commit, get it to compile with a
modern toolchain, and type those commands for myself.

Instead, the commit message should be self-contained and explain what
the patch does.

That has multiple parts:

 - first, what the current behavior is

 - second, what the intent behind the current behavior is.  This is
   crucial information because presumably we want the change not to
   break that.

 - third, what change the patch makes

 - fourth, what the consequences of that are, in terms of new use
   cases that become possible and old use cases that become less
   convenient

 - fifth, optionally, how the need for this change was discovered
   (real-life usage, code inspection, or something else)

 - sixth, optionally, implementation considerations and alternate
   approaches that were discarded

If you run git log, you'll see many good and bad examples to think
over and compare to this goal.  It's hard work to describe one's work
well in terms that other people can understand, but I think it can be
satisfying, and in any event, it's just as necessary as including
comments near confusing code.

Sincerely,
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] test-lib: avoid full path to store test results

2012-10-30 Thread Felipe Contreras
On Wed, Oct 31, 2012 at 2:27 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:
 On Tue, Oct 30, 2012 at 5:46 AM, Jonathan Nieder jrnie...@gmail.com wrote:
  Felipe Contreras wrote:

 No reason to use the full path in case this is used externally.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

 No reason not to is not a reason to do anything.  What symptoms does
 this prevent?  Could you describe the benefit of this patch in a
 paragraph starting Now you can ...?

 ./test-lib.sh: line 394:
 /home/felipec/dev/git/t/test-results//home/felipec/dev/git/contrib/remote-hg/test-21865.counts:
 No such file or directory

 Ok, so a description for this patch is

 test: use test's basename to name results file

Is this solving an actual problem or is it just something nice to do?
Like in all good novels, one has to read more find out...

 Running a test using its full path produces an error:

I'm not sure what that even means. Do you mean this produces an error?

% make -C t $PWD/t9902-completion.sh

Well, sure it does, but this patch doesn't fix that.

If you want a precise explanation of what kind of usages are enabled
by this patch, that would require some work, and no I haven't done it,
and no, I'm not sure.

 $ ~/dev/git/contrib/remote-hg/test-21865.sh
 [...]
 ./test-lib.sh: line 394: /home/felipec/dev/t/\
 test-results//home/felipec/dev/git/contrib/remote-hg/\
 test-21865.counts: No such file or directory

Except that I didn't do this. So the fact that this happens is an
assumption, and I'm not willing to make that.

Most likely if somebody does that they are doing something wrong; they
didn't define the TESTDIR variable (or something like that).

It's all fun and games to write explanations for things, but it's not
that easy when you want those explanations to be actually true, and
corrent--you have to spend time to make sure of that.

 In --tee and --valgrind modes we already use the basename
 to name the .out and .exit files; this patch teaches the test-lib
 to name the .counts file the same way.

I don't see the point of listing each and every place where this
already happens. As a matter of fact, the base-name is used in other
places as well, and just saying This is already done in other
places, is more than enough. But who says they are not the ones doing
it wrong? Maybe this part of the code is right, and it's the others
that need fixing. I don't see how saying Others are doing it makes
the patch better or worse in any way. There might also be different
reasons for why they do it that doesn't apply here.

 That is still not enough to tell if it is a good change, though.
 Should the test results for contrib/remote-hg/test-* be included with
 the results for t/t*.sh when I run make aggregate-results?

 Before 60d02ccc, git-svn had its own testsuite under contrib/, with
 glue in contrib/git-svn/t/lib-git-svn.sh to use test-lib --- maybe
 that code could provide some inspiration for questions like these.

Or maybe they are the ones that should look for inspiration in
contrib/remote-hg.

The patch is obviously correct; it's generally good not to name files
with slashes in them, and $0 is not guaranteed not to have slashes.
Even if you run all the tests inside the 't' directory, this script is
not only used by git, and others might want sub-directories, and not
thousands of tests on the same directory like git.

Either way, if obvious fixes that are one-liners require an essay for
you, I give up.

Cheers.

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


Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Felipe Contreras
On Wed, Oct 31, 2012 at 1:11 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 (cc-ing the git list)
 Felipe Contreras wrote:

 When an object has already been exported (and thus is in the marks) it
 is flagged as SHOWN, so it will not be exported again, even if this time
 it's exported through a different ref.

 We don't need the object to be exported again, but we want the ref
 updated

 Yes, makes perfect sense.

 For what it's worth,
 Acked-by: Jonathan Nieder jrnie...@gmail.com

Yay!

 [...]
 --- a/t/t5800-remote-helpers.sh
 +++ b/t/t5800-remote-helpers.sh
 @@ -145,4 +145,15 @@ test_expect_failure 'push new branch with old:new 
 refspec' '
   compare_refs clone HEAD server refs/heads/new-refspec
  '

 +test_expect_success 'push ref with existing object' '
 + (cd localclone 
 + git branch point-to-master master 
 + git push origin point-to-master
 + ) 
 +
 + (cd server 
 + git show-ref refs/heads/point-to-master
 + )

 Style: if you indent like this, the test becomes clearer:

And then it would become inconsistent with the rest of the file.

 + git fast-export --import-marks=tmp-marks \
 + --export-marks=tmp-marks master  actual 
 + test_cmp expected actual

 Redirections in git shell scripts are generally spelled as
 do_something actual, without a space between the operator and
 filename.

I generally am OK with adapting to whatever code-style is used
(sometimes under protest), but this is a place where I draw   the
line. Sorry, 'actual' is more annoying to me than a knife screeching
glass. Fortunately, ' actual' is used in many other places in 't/',
so I'm going to use the other people jumping over the bridge argument.

Cheers.

-- 
Felipe Contreras
--
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 4/4] fast-export: make sure refs are updated properly

2012-10-30 Thread Felipe Contreras
On Wed, Oct 31, 2012 at 1:37 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -523,11 +523,16 @@ static void get_tags_and_duplicates(struct 
 object_array *pending,
   typename(e-item-type));
   continue;
   }
 - if (commit-util) {
 - /* more than one name for the same object */
 +
 + /*
 +  * This ref will not be updated through a commit, lets make
 +  * sure it gets properly upddated eventually.
 +  */
 + if (commit-util || commit-object.flags  SHOWN) {
   if (!(commit-object.flags  UNINTERESTING))
   string_list_append(extra_refs, 
 full_name)-util = commit;
 - } else
 + }
 + if (!commit-util)
   commit-util = full_name;

 Here's an explanation of why the above makes sense to me.

 get_tags_and_duplicates() gets called after the marks import and
 before the revision walk.  It walks through the revs from the
 commandline and for each one:

  - peels it to a refname, and then to a commit
  - stores the refname so fast-export knows what arg to pass to
the commit command during the revision walk
  - if it already had a refname stored, instead adds the
(refname, commit) pair to the extra_refs list, so fast-export
knows to add a reset command later.

 If the commit already has the SHOWN flag set because it was pointed to
 by a mark, it is not going to come up in the revision walk, so it will
 not be mentioned in the output stream unless it is added to
 extra_refs.  That's what this patch does.

That is correct.

 Incidentally, the change from else to if (!commit-util) is
 unnecessary because if a commit is already SHOWN then it will not be
 encountered in the revision walk so commit-util does not need to be
 set.

Maybe, but that's yet another change, and with more changes come more
possibilities of regressions. I haven't verified this is the case.

If this makes sense, I would do it in another, separate patch.

 If the commit does not have the SHOWN or UNINTERESTING flag set but it
 is going to get the UNINTERESTING flag set during the walk because of
 a negative commit listed on the command line, this patch won't help.

I don't know what that means in practice.

Cheers.

-- 
Felipe Contreras
--
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] test-lib: avoid full path to store test results

2012-10-30 Thread Jonathan Nieder
Felipe Contreras wrote:

 It's all fun and games to write explanations for things, but it's not
 that easy when you want those explanations to be actually true, and
 corrent--you have to spend time to make sure of that.

That's why it's useful for the patch submitter to write them, asking
for help when necessary.

As a bonus, it helps reviewers understand the effect of the patch.
Bugs averted!

[...]
 Either way, if obvious fixes that are one-liners require an essay for
 you, I give up.

I guess it is fair to call a reasonable subject line plus a couple of
sentences an essay.  Yes, obvious fixes especially require that, since
the bug caused by an obvious fix is one of the worst kinds.
--
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] test-lib: avoid full path to store test results

2012-10-30 Thread Felipe Contreras
On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Felipe Contreras wrote:

 It's all fun and games to write explanations for things, but it's not
 that easy when you want those explanations to be actually true, and
 corrent--you have to spend time to make sure of that.

 That's why it's useful for the patch submitter to write them, asking
 for help when necessary.

 As a bonus, it helps reviewers understand the effect of the patch.
 Bugs averted!

Yeah, that would be nice. Too bad I don't have that information, and
have _zero_ motivation to go and get it for you.

 [...]
 Either way, if obvious fixes that are one-liners require an essay for
 you, I give up.

 I guess it is fair to call a reasonable subject line plus a couple of
 sentences an essay.  Yes, obvious fixes especially require that, since
 the bug caused by an obvious fix is one of the worst kinds.

Yes, I've written essays for one-line fixes, in the Linux kernel,
where details matter, and things are not so obvious.

This is not the case here. This is as obvious and simple as things
get. If there was a problem with it, somebody would have found it by
now.

Let not the perfect be the enemy of the good. Or do. Up to you.

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


Abercrombie has become through this great site

2012-10-30 Thread torriena
   The Abercrombie brand dedication of this kind of a higher level of good
results. abercrombie and fitch http://www.abercrombiefitchonlineschweiz.eu  
clothing webpage previously has everything, your option. You can be in the
position to see the garments of your respective own alternative. The
complete collection in the newest Abercrombie and Fitch apparel sale within
this internet site.

   Usually continue to keep the the  abercrombie and fitch schweiz online
shop http://www.abercrombiefitchonlineschweiz.eu   web page performance
along with the standard goal is in the very best doable strategy to
facilitate buyers. Garments, you will get to order A  F export, the same
stock might be established within the Abercrombie clothing website.

   Abercrombie has become through this great site, it is really possible to
purchase the clothing you like. Will probably be provided once possible to
your clothes, you will be ordered although Abercrombie clothes internet
site. The organization assures that every costs are explicitly tell you. It
has been viewed as loyal buyers are actually shopping through this internet
site, recommend their friends to shopping, Abercrombie garments internet
site, this site is different the clear way of shopping. Garments shopping
has become so easy and fun.

   You do not have to consider break your busy lifestyle, but all you need
to do is sit in the front of one's computer, and your favorite  abercrombie
and fitch schweiz http://www.abercrombiefitchonlineschweiz.eu   clothing
orders. If you want to express your ex girlfriend and affection for your
associates, as well as Abercrombie internet site might help the only supply
of From this point, it is possible to pick a gift, your folks. Abercrombie
apparel, you should more http://www.abercrombiefitchonlineschweiz.eu.



-
abercrombie men , the first big American leisure, today's young adults are 
among the most popular brand, abercrombie online shop schweiz  but also one of 
the most IN brand American college students.

--
View this message in context: 
http://git.661346.n2.nabble.com/Abercrombie-has-become-through-this-great-site-tp7570314.html
Sent from the git mailing list archive at Nabble.com.
--
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-push: update remote tags only with force

2012-10-30 Thread Chris Rorvick
References are allowed to update from one commit-ish to another if the
former is a ancestor of the latter.  This behavior is oriented to
branches which are expected to move with commits.  Tag references are
expected to be static in a repository, though, thus an update to a
tag (lightweight and annotated) should be rejected unless the update is
forced.

To enable this functionality, the following checks have been added to
set_ref_status_for_push() for updating refs (i.e, not new or deletion)
to restrict fast-forwarding in pushes:

  1) The old and new references must be commits.  If this fails,
 it is not a valid update for a branch.

  2) The reference name cannot start with refs/tags/.  This
 catches lightweight tags which (usually) point to commits
 and therefore would not be caught by (1).

If either of these checks fails, then it is flagged (by default) with a
status indicating the update is being rejected due to the reference
already existing in the remote.  This can be overridden by passing
--force to git push.

The new status has the added benefit of being able to provide accurate
feedback as to why the ref update failed and what can be done.
Currently all ref update rejections are assumed to be for branches.

Signed-off-by: Chris Rorvick ch...@rorvick.com
---
 Documentation/git-push.txt | 10 +-
 builtin/push.c | 12 
 builtin/send-pack.c|  6 ++
 cache.h|  3 +++
 remote.c   | 34 +++---
 t/t5516-fetch-push.sh  | 30 +-
 transport-helper.c |  6 ++
 transport.c| 25 -
 transport.h|  3 ++-
 9 files changed, 106 insertions(+), 23 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 22d2580..7107d76 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,11 @@ be named. If `:`dst is omitted, the same ref as src 
will be
 updated.
 +
 The object referenced by src is used to update the dst reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward dst.  By having the optional leading `+`,
-you can tell git to update the dst ref even when the update is not a
-fast-forward.  This does *not* attempt to merge src into dst.  See
-EXAMPLES below for details.
+on the remote side.  By default this is only allowed if the update is
+a branch, and then only if it can fast-forward dst.  By having the
+optional leading `+`, you can tell git to update the dst ref even when
+the update is not a branch or it is not a fast-forward.  This does *not*
+attempt to merge src into dst.  See EXAMPLES below for details.
 +
 `tag tag` means the same as `refs/tags/tag:refs/tags/tag`.
 +
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..fabcea0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,6 +220,10 @@ static const char message_advice_checkout_pull_push[] =
   (e.g. 'git pull') before pushing again.\n
   See the 'Note about fast-forwards' in 'git push --help' for 
details.);
 
+static const char message_advice_ref_already_exists[] =
+   N_(Updates were rejected because a matching reference already exists 
in\n
+  the remote.  Use git push -f if you really want to make this 
update.);
+
 static void advise_pull_before_push(void)
 {
if (!advice_push_non_ff_current || !advice_push_nonfastforward)
@@ -241,6 +245,11 @@ static void advise_checkout_pull_push(void)
advise(_(message_advice_checkout_pull_push));
 }
 
+static void advise_ref_already_exists(void)
+{
+   advise(_(message_advice_ref_already_exists));
+}
+
 static int push_with_options(struct transport *transport, int flags)
 {
int err;
@@ -277,6 +286,9 @@ static int push_with_options(struct transport *transport, 
int flags)
else
advise_checkout_pull_push();
break;
+   case ALREADY_EXISTS:
+   advise_ref_already_exists();
+   break;
}
 
return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7d05064..f159ec3 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -202,6 +202,11 @@ static void print_helper_status(struct ref *ref)
msg = non-fast forward;
break;
 
+   case REF_STATUS_REJECT_ALREADY_EXISTS:
+   res = error;
+   msg = already exists;
+   break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = error;
@@ -288,6 +293,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref-status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+   case 

Re: [PATCH] git-push: update remote tags only with force

2012-10-30 Thread Felipe Contreras
Hi,

(again because the mailing list rejected it) (Gmal switched interface
and HTML is the default)

On Wed, Oct 31, 2012 at 6:37 AM, Chris Rorvick ch...@rorvick.com wrote:

 References are allowed to update from one commit-ish to another if the
 former is a ancestor of the latter.  This behavior is oriented to
 branches which are expected to move with commits.  Tag references are
 expected to be static in a repository, though, thus an update to a
 tag (lightweight and annotated) should be rejected unless the update is
 forced.

 To enable this functionality, the following checks have been added to
 set_ref_status_for_push() for updating refs (i.e, not new or deletion)
 to restrict fast-forwarding in pushes:

   1) The old and new references must be commits.  If this fails,
  it is not a valid update for a branch.

   2) The reference name cannot start with refs/tags/.  This
  catches lightweight tags which (usually) point to commits
  and therefore would not be caught by (1).

 If either of these checks fails, then it is flagged (by default) with a
 status indicating the update is being rejected due to the reference
 already existing in the remote.  This can be overridden by passing
 --force to git push.

 The new status has the added benefit of being able to provide accurate
 feedback as to why the ref update failed and what can be done.
 Currently all ref update rejections are assumed to be for branches.

Makes sense to me. I've believe I've been hit by this a couple of
times when tags were updated, and a colleague did 'git push' and they
went all back, or something like that. To handle that case properly
probably more changes are needed, but this is a change in the right
direction.

 +test_expect_success 'push tag requires --force to update remote tag' '
 +   mk_test heads/master 
 +   mk_child child1 
 +   mk_child child2 
 +   (
 +   cd child1 
 +   git tag lw_tag 
 +   git tag -a -m message 1 ann_tag 
 +   git push ../child2 lw_tag 
 +   git push ../child2 ann_tag 
 +   file1 
 +   git add file1 
 +   git commit -m file1 
 +   git tag -f lw_tag 
 +   git tag -f -a -m message 2 ann_tag 
 +   ! git push ../child2 lw_tag 

You probably should use test_must_fail.

I don't see anything wrong with the patch, but I wonder if it might be
possible to split it to ease the review.

Cheers.

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