Git clonebundles

2017-01-30 Thread Stefan Saasen 
Uniform approach for the supported transports: Our proof-of-concept
only supports HTTP as
  a transport. Ideally the clonebundle capability could be supported by all
  available transports (of which at least ssh would be highly desirable).
* Bundle manifest and bundle download: It is unclear whose responsibility it is
  to generate the bundle manifest with the bundle download URLs. Most likely the
  bundle files will be served using a webserver or CDN, so download
URL generation
  should not be a core git responsibility. For hosting purpose we envision that
  the bundle manifest might contain dynamic download URLs with personalised
  access tokens with expiry.
* Bundle generation: Similar to the above it is unclear how bundle
generation is handled.
  For hosting purposes, the operator would likely want to influence
when and how bundles are generated.



Prior art
~

Our proof-of-concept is built on top of ideas that have been
circulating for a while. We are aware of a number of proposed changes
in this space:


* Jeff King's work on network bundles:
https://github.com/peff/git/commit/17e2409df37edd0c49ef7d35f47a7695f9608900
* Nguyễn Thái Ngọc Duy's work on "[PATCH 0/8] Resumable clone
revisited, proof of concept":
https://www.spinics.net/lists/git/msg267260.html
* Resumable clone work by Kevin Wern:
https://public-inbox.org/git/1473984742-12516-1-git-send-email-kevin.m.w...@gmail.com/


Whilst the above mentioned proposals/proposed changes are in a similar
space, I would be interest to understand whether there is any
consensus on the general idea of supporting static bundle files as a
mechanism to seed a repository?
I would also appreciate any pointers to other discussions in this area.


Best regards,
Stefan Saasen & Erik van Zijst; Atlassian Bitbucket

Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-21 Thread Stefan Saasen 
 Perhaps companies like Atlassian that rely on the stability of the
 open source Git can spare some resources and join forces with like
 minded folks on LTS of older maintenance tracks, if they are truly
 interested in.

We certainly can and would like to. I'm not entirely sure what that
would entail though?

From reading through $gmane/264365 I've identified the following
responsibilities/opportunities to help:

- Monitor git log --first-parent maint-lts..master and find
  the tip of topic branches that need to be down-merged;

- Down-merge such topics to maint-lts; this might involve
  cherry-picking instead of merge, as the bugfix topics may
  originally be done on the codebase newer than maint-lts;

and more importantly testing the maint-lts version to ensure
backported changes don't introduce regressions and the maint-lts
branch is stable.

This suggests specific, spaced LTS versions but in the same thread you
mention maint-2.1or maint-2.2.
So a different model could be maintaining old versions in a sliding
window fashion (e.g. critical issues would be backported to the last 6
months worth of git releases).

Maybe I'm getting ahead of myself here :)

Anyway, long story short. We're interested to help but I'm not
entirely sure what that would look like at the moment. Are there
formed ideas floating around or would you be looking for some form of
proposal instead?

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

Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-21 Thread Stefan Saasen 
 I've noticed Peff's patches on pu which suggest they will be available
 in git 2.5?

 Being on 'pu' (or 'next' for that matter) is not a suggestion for a
 change to appear in any future version at all, even though it often
 means that it would soon be merged to 'master' and will be in the
 upcoming release to be on 'next' in early part of a development
 cycle.  Some larger topics would stay on 'next' for a few cycles.

 Do you Junio, have plans to merge them to maint (2.3.x) and/or next (2.4)?

 The topic will hopefully be merged to 'master' after 2.4 final is
 released end of this month, down to 'maint' early May and will ship
 with 2.4.1, unless there is unforeseen issues discovered in the
 change while people try it out while it is in 'next' (which will
 happen today, hopefully).

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

Re: [PATCH 1/2] sha1_file: freshen pack objects before loose

2015-04-20 Thread Stefan Saasen 
I didn't expect anything else (as the patch is the same as the
previous one) but I verified that applying this patch has the desired
effect (https://bitbucket.org/snippets/ssaasen/9AXg).

Thanks for the fix Jeff.

On 21 April 2015 at 05:54, Jeff King p...@peff.net wrote:
 When writing out an object file, we first check whether it
 already exists and if so optimize out the write. Prior to
 33d4221, we did this by calling has_sha1_file(), which will
 check for packed objects followed by loose. Since that
 commit, we check loose objects first.

 For the common case of a repository whose objects are mostly
 packed, this means we will make a lot of extra access()
 system calls checking for loose objects. We should follow
 the same packed-then-loose order that all of our other
 lookups use.

 Reported-by: Stefan Saasen ssaa...@atlassian.com
 Signed-off-by: Jeff King p...@peff.net
 ---
  sha1_file.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/sha1_file.c b/sha1_file.c
 index 88f06ba..822aaef 100644
 --- a/sha1_file.c
 +++ b/sha1_file.c
 @@ -3014,7 +3014,7 @@ int write_sha1_file(const void *buf, unsigned long len, 
 const char *type, unsign
 write_sha1_file_prepare(buf, len, type, sha1, hdr, hdrlen);
 if (returnsha1)
 hashcpy(returnsha1, sha1);
 -   if (freshen_loose_object(sha1) || freshen_packed_object(sha1))
 +   if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
 return 0;
 return write_loose_object(sha1, hdr, hdrlen, buf, len, 0);
  }
 --
 2.4.0.rc2.384.g7297a4a

--
To unsubscribe from this list: send the line 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] sha1_file: only freshen packs once per run

2015-04-20 Thread Stefan Saasen 
I can confirm that this patch is equivalent to the previous one.

https://bitbucket.org/snippets/ssaasen/9AXg shows both the timing and
the NFS stats showing the effect of applying this patch.

Thanks for the fix Jeff!

Cheers,
Stefan

On 21 April 2015 at 05:55, Jeff King p...@peff.net wrote:
 Since 33d4221 (write_sha1_file: freshen existing objects,
 2014-10-15), we update the mtime of existing objects that we
 would have written out (had they not existed). For the
 common case in which many objects are packed, we may update
 the mtime on a single packfile repeatedly. This can result
 in a noticeable performance problem if calling utime() is
 expensive (e.g., because your storage is on NFS).

 We can fix this by keeping a per-pack flag that lets us
 freshen only once per program invocation.

 An alternative would be to keep the packed_git.mtime flag up
 to date as we freshen, and freshen only once every N
 seconds. In practice, it's not worth the complexity. We are
 racing against prune expiration times here, which inherently
 must be set to accomodate reasonable program running times
 (because they really care about the time between an object
 being written and it becoming referenced, and the latter is
 typically the last step a program takes).

 Signed-off-by: Jeff King p...@peff.net
 ---
 Hopefully I didn't botch the flag logic again. :) I tested with strace
 -c myself this time, so I think it is good.

  cache.h | 1 +
  sha1_file.c | 9 -
  2 files changed, 9 insertions(+), 1 deletion(-)

 diff --git a/cache.h b/cache.h
 index 3d3244b..72c6888 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -1174,6 +1174,7 @@ extern struct packed_git {
 int pack_fd;
 unsigned pack_local:1,
  pack_keep:1,
 +freshened:1,
  do_not_close:1;
 unsigned char sha1[20];
 /* something like .git/objects/pack/x.pack */
 diff --git a/sha1_file.c b/sha1_file.c
 index 822aaef..26b9b2b 100644
 --- a/sha1_file.c
 +++ b/sha1_file.c
 @@ -2999,7 +2999,14 @@ static int freshen_loose_object(const unsigned char 
 *sha1)
  static int freshen_packed_object(const unsigned char *sha1)
  {
 struct pack_entry e;
 -   return find_pack_entry(sha1, e)  freshen_file(e.p-pack_name);
 +   if (!find_pack_entry(sha1, e))
 +   return 0;
 +   if (e.p-freshened)
 +   return 1;
 +   if (!freshen_file(e.p-pack_name))
 +   return 0;
 +   e.p-freshened = 1;
 +   return 1;
  }

  int write_sha1_file(const void *buf, unsigned long len, const char *type, 
 unsigned char *returnsha1)
 --
 2.4.0.rc2.384.g7297a4a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-20 Thread Stefan Saasen 
 If it is critical to some people, they can downmerge to their custom
 old installations of Git they maintain with ease, of course, and
 that with ease part is the reason why I try to apply fixes to tip
 of the original topic branch even though they were merged to the
 mainline eons ago ;-).

 I think it is a bigger deal for folks who do not ship a custom
 installation, but expect to ship a third-party system that interacts
 with whatever version of git their customers happen to have (in which
 case they can only recommend their customers to upgrade).

Yes, this is the situation we are facing. We allow our customers to
use the git version that is supported/available on their OS (within a
certain range of supported versions) so our customers usually don't
compile from source.

 Either way, though, I do not think it is the upstream Git project's
 problem.

That's fair enough, I was mostly enquiring about the official git
versions this will land in so that we can advise customers what git
version to use (or not to use).

I've noticed Peff's patches on pu which suggest they will be available
in git 2.5?
Do you Junio, have plans to merge them to maint (2.3.x) and/or next (2.4)?

While I certainly agree that this is specific to Git on NFS and not a
more widespread git performance problem, I'd love to be able to
message something other than skip all the git version between and
including git 2.2 - 2.4.

I appreciate your consideration and thanks again for the swift response on this.

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

[BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-17 Thread Stefan Saasen 
We became aware of slow merge times with the following setup:

The merge is created in a temporary location that uses alternates. The
temporary repository is on a local disk, the alternate object database
on an NFS mount.

After some investigation we believe that #33d4221 (present in git
2.2.0, absent in 2.1.4) is causing this regression in merge time.

The following are merge times (in seconds) with git@33d4221~
(2.1.2-393-gabcb865) (before the change)

  Elapsed SystemUser
 Min.   :0.3700   Min.   :0.04000   Min.   :0.3000
 1st Qu.:0.3800   1st Qu.:0.05000   1st Qu.:0.3100
 Median :0.4000   Median :0.06000   Median :0.3300
 Mean   :0.4295   Mean   :0.05905   Mean   :0.3519
 3rd Qu.:0.4600   3rd Qu.:0.07000   3rd Qu.:0.3600
 Max.   :0.5900   Max.   :0.09000   Max.   :0.4900


The following are merge times with git@33d4221 (2.1.2-394-g33d4221):

  Elapsed SystemUser
 Min.   : 8.58   Min.   :1.46   Min.   :0.4400
 1st Qu.: 9.63   1st Qu.:1.60   1st Qu.:0.4400
 Median :10.64   Median :1.66   Median :0.4800
 Mean   :10.50   Mean   :1.69   Mean   :0.4986
 3rd Qu.:11.13   3rd Qu.:1.81   3rd Qu.:0.5000
 Max.   :13.96   Max.   :2.05   Max.   :0.6700


As you can see the merge times are an order of magnitude slower after
the change.

The effect of  #33d4221 can be seen when strace'ing the merge:

Running strace on git@33d4221 yields
% time seconds  usecs/call callserrors syscall
-- --- --- - - 
 70.790.714852 178  4018   utime
 14.730.148789   3 50141 50123 lstat
 13.880.140198  17  8074  8067 access
  0.240.002455 614 4   rename
  0.150.001493   3   577   write
  0.060.000618  1065   close
  0.040.000453   3   152   brk
  0.040.000433  2716   mkdir
  0.030.000310   841   fstat


Running strace on git@33d4221~ yields

% time seconds  usecs/call callserrors syscall
-- --- --- - - 
 98.370.138516   3 50141 50123 lstat
  0.920.001292   2   577   write
  0.370.000520  143831 access
  0.180.000252  36 7   getcwd
  0.170.000237   73620 stat
  0.000.00   040   read
  0.000.00   08730 open


My current hypothesis is that the additional `access`, but more
importantly the additional `utime` calls are responsible in the
increased merge times that we see.
NFS stats on the server for the tests seem to confirm this (see
nfsstat-{after,before}-change.txt on
https://bitbucket.org/snippets/ssaasen/oend).
This is certainly due to the fact that this will all happen over NFS
but in 2.1.4 this worked fine and starting with 2.2 this has become
very slow.

Looking at the detailed strace shows that utime will be called
repeatedly in same cases (e.g.
https://bitbucket.org/snippets/ssaasen/oend shows an example where the
same packfile will be updated more than 4000 times in a single merge).

http://www.spinics.net/lists/git/msg240106.html discusses a potential
improvement for this case. Would that be an acceptable avenue to
improve this situation?

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

Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-17 Thread Stefan Saasen 
 If it's not a problem, I'd love to see timings for your case with just
 the first patch, and then with both.

Thanks for the swift response, much appreciated Jeff!

Here are the timings for the two patches:

Patch 1 on top of 33d4221c79

   Elapsed   System  User
 Min.   :6.110   Min.   :0.6700   Min.   :0.3600
 1st Qu.:6.580   1st Qu.:0.6900   1st Qu.:0.3900
 Median :7.260   Median :0.7100   Median :0.4100
 Mean   :7.347   Mean   :0.7248   Mean   :0.4214
 3rd Qu.:8.000   3rd Qu.:0.7400   3rd Qu.:0.4600
 Max.   :8.860   Max.   :0.8700   Max.   :0.5100

I've had to slightly tweak your second patch (`freshened` was never
set) but applying the modified patch yielded even better results
compared to patch 1:

   Elapsed   System  User
 Min.   :0.38   Min.   :0.03000   Min.   :0.2900
 1st Qu.:0.38   1st Qu.:0.04000   1st Qu.:0.3100
 Median :0.39   Median :0.06000   Median :0.3200
 Mean   :0.43   Mean   :0.05667   Mean   :0.3519
 3rd Qu.:0.42   3rd Qu.:0.07000   3rd Qu.:0.3600
 Max.   :0.68   Max.   :0.08000   Max.   :0.5700

This is pretty much back to the before state.
The graph really tells the whole story:
https://bytebucket.org/snippets/ssaasen/GeRE/raw/7367353a58c50ccd7c493af40ffb6ba1533e1490/git-merge-timing-patched.png
(After is the change in #33d4221, Before the parent of #33d4221 and so on)
The graph and the NFS stats can be found here:
https://bitbucket.org/snippets/ssaasen/GeRE

My tweaked version of your second patch is:

diff --git a/cache.h b/cache.h
index 51ee856..8982055 100644
--- a/cache.h
+++ b/cache.h
@@ -1168,6 +1168,7 @@ extern struct packed_git {
int pack_fd;
unsigned pack_local:1,
 pack_keep:1,
+   freshened:1,
 do_not_close:1;
unsigned char sha1[20];
/* something like .git/objects/pack/x.pack */
diff --git a/sha1_file.c b/sha1_file.c
index bc6322e..c0ccd4b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2999,7 +2999,11 @@ static int freshen_loose_object(const unsigned
char *sha1)
 static int freshen_packed_object(const unsigned char *sha1)
 {
struct pack_entry e;
-   return find_pack_entry(sha1, e)  freshen_file(e.p-pack_name);
+   if (!find_pack_entry(sha1, e))
+  return 0;
+   if (e.p-freshened)
+  return 1;
+   return e.p-freshened = freshen_file(e.p-pack_name);
 }

 int write_sha1_file(const void *buf, unsigned long len, const char
*type, unsigned char *returnsha1)



The only change is that I assign the result of `freshen_file` to the
`freshened` flag.

I've only ran this with the test case I was using before but it looks
like this is pretty much fixing the merge time changes we observed.

Thanks again for the swift response. I've got my test setup sitting
here, happy to rerun the tests if that'd be useful.

Is there a chance to backport those changes to the 2.2+ branches?

 You may also be interested in:

   http://thread.gmane.org/gmane.comp.version-control.git/266370

 which addresses another performance problem related to the
 freshen/recent code in v2.2.

Thanks for the pointer, I'll have a look at that as well.

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

Re: [PATCH] gc: support temporarily preserving garbage

2014-11-17 Thread Stefan Saasen 
On 18 November 2014 08:34, Jeff King p...@peff.net wrote:


 I am not sure if this much of code churn is warranted to work around
 issues that only happen on repositories on NFS servers that do not
 keep open-but-deleted files available.  Is it an option to instead
 have a copy of repository locally off NFS?

 I think it is also not sufficient. This patch seems to cover only
 objects. But we assume that we can atomically rename() new versions of
 files into place whenever we like without disrupting existing readers.
 This is the case for ref updates (and packed-refs), as well as the index
 file.  The destination end of the rename is an unlink() in disguise, and
 would be susceptible to the same problems.

I’m going out on a limb here as my NFS knowledge is rather shallow but a
rename should be atomic even on NFS.

The RENAME operation must be atomic to the client.”
(https://www.ietf.org/rfc/rfc1813.txt: 3.3.14)

Does git do something differently here for that not to be the case?
--
To unsubscribe from this list: send the line 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] gc: support temporarily preserving garbage

2014-11-14 Thread Stefan Saasen 
On 15 November 2014 10:01, Junio C Hamano gits...@pobox.com wrote:

  23 files changed, 375 insertions(+), 7 deletions(-)

 I am not sure if this much of code churn is warranted to work around
 issues that only happen on repositories on NFS servers that do not
 keep open-but-deleted files available.  Is it an option to instead
 have a copy of repository locally off NFS?

Unfortunately not providing delete-on-last-close semantics is true for
most (all?) NFS servers that are accessed by multiple clients. NFS v3
is stateless so the server has no way of tracking open files accross
clients and the silly rename work around only works within a single
client. NFS v4 could support delete-on-last-close semantics but I’m
not sure if there are actual implementations providing that.
(This is based on my admittedly limited understanding of NFS, I’d love
to learn that I’ve got this wrong).

We are susceptible to the same problem accessing shared repositories
from multiple clients so we certainly appreciate the use case.
Unfortunately copying repositories to the local nodes is not something
that’d be feasible.

Is there another/better approach solving this problem?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fwd: Add a bugzilla website

2014-01-12 Thread Stefan Saasen 
 In any case, adding value to the existing process is hard (because it
 works quite well!) and probably requires significantly more work to
 even understand what that value might look like. This, I think, is the
 key reason it is hard to truly get started with any bug tracking
 solution; the solution is not obvious, and the current (very
 customised) workflow is not supported directly by any tool.

 Regards,

 Andrew Ardill

 [1] https://git-scm.atlassian.net


I think you summarised the challenges very well and I don’t think there is an
obvious answer to that.

I’d just like to offer any help that I or we (Atlassian) could give you. Given
your experience with JIRA I’m sure that you’ve got everything covered, but if
you need anything, please ping me.

Re-reading the old discussions there was a concern that the issue data was not
available, I just wanted to chime in and mention that if you are using a JIRA
OnDemand (e.g. the https://git-scm.atlassian.net) instance you can get the full
backup of your data (including any attachments, data available as XML) and
there is a JIRA REST API as well.

I know that this is just a very tangential concern given the challenges of
making an issue tracker work with an email based workflow that has proven quite
successful but I at least wanted to address that and offer any help you might
need.


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

Re: [PATCH v2] mergetools/diffmerge: support DiffMerge as a git mergetool

2013-10-12 Thread Stefan Saasen 
Not a problem. I'll change it to:

tools=$tools gvimdiff diffuse diffmerge ecmerge
tools=$tools p4merge araxis bc3 code compare

and send a v3. Thanks for the review David.


On 13 October 2013 06:55, David Aguilar dav...@gmail.com wrote:
 Thanks for the re-roll.  We're very close; see below.

 On Fri, Oct 11, 2013 at 10:01 PM, Stefan Saasen ssaa...@atlassian.com wrote:
 DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and 
 Linux.

 See http://www.sourcegear.com/diffmerge/

 DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch 
 the
 graphical compare tool.

 This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
 option to the mergetool help.

 Signed-off-by: Stefan Saasen ssaa...@atlassian.com
 Acked-by: David Aguilar dav...@gmail.com
 ---
  contrib/completion/git-completion.bash |  2 +-
  git-mergetool--lib.sh  |  3 ++-
  mergetools/diffmerge   | 15 +++
  3 files changed, 18 insertions(+), 2 deletions(-)
  create mode 100644 mergetools/diffmerge

 diff --git a/contrib/completion/git-completion.bash 
 b/contrib/completion/git-completion.bash
 index e1b7313..07b0ba5 100644
 --- a/contrib/completion/git-completion.bash
 +++ b/contrib/completion/git-completion.bash
 @@ -1188,7 +1188,7 @@ _git_diff ()
 __git_complete_revlist_file
  }

 -__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff
 +__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld 
 opendiff
 tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 
 codecompare
  

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index feee6a4..0fcb253 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -250,7 +250,8 @@ list_merge_tool_candidates () {
 else
 tools=opendiff kdiff3 tkdiff xxdiff meld $tools
 fi
 -   tools=$tools gvimdiff diffuse ecmerge p4merge araxis bc3 
 codecompare
 +   tools=$tools gvimdiff diffuse diffmerge ecmerge 
 +   tools+=p4merge araxis bc3 codecompare

 I don't believe += is portable across all POSIX shells.

 I tried this on dash (which is the default /bin/sh on Debian) and it
 was not understood there.

 $ f=1 2 3
 $ f+= 4
 /bin/dash: 2: f+= 4: not found

 I think we should stick to the tools=$tools . style of concatenation.

 Everything else looks good to me.

 Thanks,
 --
 David
--
To unsubscribe from this list: send the line 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] mergetools/diffmerge: support DiffMerge as a git mergetool

2013-10-12 Thread Stefan Saasen 
DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and Linux.

See http://www.sourcegear.com/diffmerge/

DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch the
graphical compare tool.

This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
option to the mergetool help.

Signed-off-by: Stefan Saasen ssaa...@atlassian.com
Acked-by: David Aguilar dav...@gmail.com
---
 contrib/completion/git-completion.bash |  2 +-
 git-mergetool--lib.sh  |  3 ++-
 mergetools/diffmerge   | 15 +++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 mergetools/diffmerge

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e1b7313..07b0ba5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1188,7 +1188,7 @@ _git_diff ()
__git_complete_revlist_file
 }
 
-__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff
+__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 
codecompare
 
 
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index feee6a4..858bc37 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -250,7 +250,8 @@ list_merge_tool_candidates () {
else
tools=opendiff kdiff3 tkdiff xxdiff meld $tools
fi
-   tools=$tools gvimdiff diffuse ecmerge p4merge araxis bc3 
codecompare
+   tools=$tools gvimdiff diffuse diffmerge ecmerge
+   tools=$tools p4merge araxis bc3 codecompare
fi
case ${VISUAL:-$EDITOR} in
*vim*)
diff --git a/mergetools/diffmerge b/mergetools/diffmerge
new file mode 100644
index 000..85ac720
--- /dev/null
+++ b/mergetools/diffmerge
@@ -0,0 +1,15 @@
+diff_cmd () {
+   $merge_tool_path $LOCAL $REMOTE /dev/null 21
+}
+
+merge_cmd () {
+   if $base_present
+   then
+   $merge_tool_path --merge --result=$MERGED \
+   $LOCAL $BASE $REMOTE
+   else
+   $merge_tool_path --merge \
+   --result=$MERGED $LOCAL $REMOTE
+   fi
+   status=$?
+}
-- 
1.8.2.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 v2] mergetools/diffmerge: support DiffMerge as a git mergetool

2013-10-11 Thread Stefan Saasen 
DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and Linux.

See http://www.sourcegear.com/diffmerge/

DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch the
graphical compare tool.

This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
option to the mergetool help.

Signed-off-by: Stefan Saasen ssaa...@atlassian.com
Acked-by: David Aguilar dav...@gmail.com
---
 contrib/completion/git-completion.bash |  2 +-
 git-mergetool--lib.sh  |  3 ++-
 mergetools/diffmerge   | 15 +++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 mergetools/diffmerge

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e1b7313..07b0ba5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1188,7 +1188,7 @@ _git_diff ()
__git_complete_revlist_file
 }
 
-__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff
+__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 
codecompare
 
 
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index feee6a4..0fcb253 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -250,7 +250,8 @@ list_merge_tool_candidates () {
else
tools=opendiff kdiff3 tkdiff xxdiff meld $tools
fi
-   tools=$tools gvimdiff diffuse ecmerge p4merge araxis bc3 
codecompare
+   tools=$tools gvimdiff diffuse diffmerge ecmerge 
+   tools+=p4merge araxis bc3 codecompare
fi
case ${VISUAL:-$EDITOR} in
*vim*)
diff --git a/mergetools/diffmerge b/mergetools/diffmerge
new file mode 100644
index 000..85ac720
--- /dev/null
+++ b/mergetools/diffmerge
@@ -0,0 +1,15 @@
+diff_cmd () {
+   $merge_tool_path $LOCAL $REMOTE /dev/null 21
+}
+
+merge_cmd () {
+   if $base_present
+   then
+   $merge_tool_path --merge --result=$MERGED \
+   $LOCAL $BASE $REMOTE
+   else
+   $merge_tool_path --merge \
+   --result=$MERGED $LOCAL $REMOTE
+   fi
+   status=$?
+}
-- 
1.8.2.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] mergetools/diffmerge: support DiffMerge as a git mergetool

2013-10-08 Thread Stefan Saasen 
Thanks for the review David, much appreciated.

 I think this line was already too long in its current form.  Would you mind
 splitting up this long line?

I've updated the patch and had a look at how to avoid repeating the list of
available merge/difftools.

 ... follow it up with a change that generalizes the list
 of tools thing so that it can be reused here, possibly.  The
 show_tool_help() function, as used by git difftool --tool-help and
 git mergetool --tool-help, might be a good place to look for
 inspiration.

 We were able to eliminate duplication in the docs (see the handling
 for $(mergetools_txt) in Documentation/Makefile) so it'd be nice if we
 could do the same for git-completion.bash, somehow.

I can think of a number of approaches and I would like to get some feedback.

Firstly I think a similar solution to how the duplication is avoided in the
documentation can't be easily applied to the completion script. Looking at the
script itself (and/or usage docs like
http://git-scm.com/book/en/Git-Basics-Tips-and-Tricks) the recommended way of
using it is by copying the script as-is. That means there won't be a build step
we could rely on unless I've overlooked something?

That leaves a different approach (run- vs. build time) where I can think of two
possible solutions.
The first would be similar to what is being done at the moment by looking at
the MERGE_TOOLS_DIR and in addition considering any custom merge tools
configured. I'm working with the premise that it is a reasonable assumption
that users of the git completion script have a git installation available even
though they may have gotten the script by other means.
For users to still be able to install the script by simply copying it
to any location
on the filesystem the list generation function(s) would either have to
be sourced
from the git installation or duplicated. I suppose the former would need to
take into account that the completion script doesn't necessarily matches the
installed version of git with some potential brittleness around
relying on external
files and directories. The latter doesn't buy us anything as it duplicates even
more code than the current list of available mergetools.

The second approach would be to do something similar to resolving the merge
strategies (in __git_list_merge_strategies) by parsing the output of the `git
merge tool --tools-help` option with a very similar disadvantage that it relies
on the textual output of the help command and doesn't work outside of a git
repository.


I'm currently leaning towards the last approach as it seems less reliant on
implementation details but it doesn't look ideal either and I may be missing
another approach that would be better suited.

 It might be worth leaving the git-completion.bash bits alone in this
 first patch and follow it up with a change that generalizes the list
 of tools thing so that it can be reused here, possibly.

To decouple this and adding the diffmerge merge tool option, I'd rather keep the
git-completion change part of the patch. That way the patch is self contained
and covers the change including the completion using the current approach and
doesn't rely on the duplication change. Any concerns around that, otherwise I'll
resend the patch with only the long line fixed?

Cheers,
Stefan


On 6 October 2013 14:21, David Aguilar dav...@gmail.com wrote:

 On Sat, Oct 5, 2013 at 1:29 AM, Stefan Saasen ssaa...@atlassian.com wrote:
  DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and 
  Linux.
 
  See http://www.sourcegear.com/diffmerge/
 
  DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch 
  the
  graphical compare tool.
 
  This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
  option to the mergetool help.
 
  Signed-off-by: Stefan Saasen ssaa...@atlassian.com
  ---
   contrib/completion/git-completion.bash |  2 +-
   git-mergetool--lib.sh  |  2 +-
   mergetools/diffmerge   | 15 +++
   3 files changed, 17 insertions(+), 2 deletions(-)
   create mode 100644 mergetools/diffmerge
 
  diff --git a/contrib/completion/git-completion.bash 
  b/contrib/completion/git-completion.bash
  index e1b7313..07b0ba5 100644
  --- a/contrib/completion/git-completion.bash
  +++ b/contrib/completion/git-completion.bash
  @@ -1188,7 +1188,7 @@ _git_diff ()
  __git_complete_revlist_file
   }
 
  -__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff
  +__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld 
  opendiff
  tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 
  codecompare
   

 It's a little unfortunate that we have to keep repeating ourselves
 here.  mergetool--lib has a list_merge_tool_candidate() that populates
 $tools and help us avoid having to maintain these lists in separate
 files.

 It might be worth leaving the git-completion.bash bits alone in this
 first patch and follow

[PATCH] mergetools/diffmerge: support DiffMerge as a git mergetool

2013-10-05 Thread Stefan Saasen 
DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and Linux.

See http://www.sourcegear.com/diffmerge/

DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch the
graphical compare tool.

This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
option to the mergetool help.

Signed-off-by: Stefan Saasen ssaa...@atlassian.com
---
 contrib/completion/git-completion.bash |  2 +-
 git-mergetool--lib.sh  |  2 +-
 mergetools/diffmerge   | 15 +++
 3 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 mergetools/diffmerge

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e1b7313..07b0ba5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1188,7 +1188,7 @@ _git_diff ()
__git_complete_revlist_file
 }
 
-__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff
+__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 
codecompare
 
 
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index feee6a4..6d0fa3b 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -250,7 +250,7 @@ list_merge_tool_candidates () {
else
tools=opendiff kdiff3 tkdiff xxdiff meld $tools
fi
-   tools=$tools gvimdiff diffuse ecmerge p4merge araxis bc3 
codecompare
+   tools=$tools gvimdiff diffuse diffmerge ecmerge p4merge araxis 
bc3 codecompare
fi
case ${VISUAL:-$EDITOR} in
*vim*)
diff --git a/mergetools/diffmerge b/mergetools/diffmerge
new file mode 100644
index 000..85ac720
--- /dev/null
+++ b/mergetools/diffmerge
@@ -0,0 +1,15 @@
+diff_cmd () {
+   $merge_tool_path $LOCAL $REMOTE /dev/null 21
+}
+
+merge_cmd () {
+   if $base_present
+   then
+   $merge_tool_path --merge --result=$MERGED \
+   $LOCAL $BASE $REMOTE
+   else
+   $merge_tool_path --merge \
+   --result=$MERGED $LOCAL $REMOTE
+   fi
+   status=$?
+}
-- 
1.8.4.475.g3a5bb13

--
To unsubscribe from this list: send the line 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] mergetool--lib: Fix typo in the merge/difftool help

2013-10-04 Thread Stefan Saasen 
The help text for the `tool` flag should mention:

--tool=tool

instead of:

--tool-tool

Signed-off-by: Stefan Saasen ssaa...@atlassian.com
---
 git-mergetool--lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index feee6a4..e1c7eb1 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -263,7 +263,7 @@ list_merge_tool_candidates () {
 }
 
 show_tool_help () {
-   tool_opt='git ${TOOL_MODE}tool --tool-tool'
+   tool_opt='git ${TOOL_MODE}tool --tool=tool'
 
tab='   '
LF='
-- 
1.8.4.474.g128a96c.dirty

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