Re: [PATCH 2/3] setup_pager: set MORE=R
On Jan 20, 2014, at 21:30, Jeff King wrote: Ugh. Having just read the LESS discussion, it makes me wonder if the strchr(getenv(LESS), 'R') check I add elsewhere in the series is sufficient. I suspect in practice it is good enough, but I would not be surprised if there is a way to fool it with a sufficiently complex LESS variable. Maybe we should just assume it is enough, and people with crazy LESS settings can set color.pager as appropriate? This attempts to show colors but doesn't work properly: LESS=-+R git -c color.ui=auto -c color.pager=true log --oneline -- graph but this does LESS=-R git -c color.ui=auto -c color.pager=true log --oneline -- graph so yeah, just checking for 'R' in $LESS is only approximate. :) Also -r is good enough to show colors too... -- 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] gitk i18n: Added Bulgarian translation (304t)
On Wed, Jan 15, 2014 at 01:27:29PM +0200, al_sho...@yahoo.com wrote: From: Alexander Shopov a...@kambanaria.org Signed-off-by: Alexander Shopov a...@kambanaria.org Thanks, applied. Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: Comply with XDG base directory specification
On Fri, Dec 13, 2013 at 07:46:36PM +, Astril Hayato wrote:
Write the gitk config data to $XDG_CONFIG_HOME/git/gitk
($HOME/.config/git/gitk
by default) in line with the XDG specification. This makes it consistent with
git which also follows the spec.
If $HOME/.gitk already exists use that for backward compatibility, so only new
installations are affected.
Sounds reasonable...
@@ -12058,7 +12061,27 @@ namespace import ::msgcat::mc
## And eventually load the actual message catalog
::msgcat::mcload $gitk_msgsdir
-catch {source ~/.gitk}
+catch {
+# follow the XDG base directory specification by default. See
+# http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
+if {[info exists env(XDG_CONFIG_HOME)] $env(XDG_CONFIG_HOME) ne } {
+ # XDG_CONFIG_HOME environment variable is set
+ set config_file [file join $env(XDG_CONFIG_HOME) git gitk]
+ set config_file_tmp [file join $env(XDG_CONFIG_HOME) git gitk-tmp]
+} else {
+ # default XDG_CONFIG_HOME
+ set config_file ~/.config/git/gitk
+ set config_file_tmp ~/.config/git/gitk-tmp
+}
+if {![file exists $config_file]} {
+ if {![file exists [file dirname $config_file]]} {
+ file mkdir [file dirname $config_file]
+ }
+ # for backward compatability use the old config file if it exists
+ if {[file exists ~/.gitk]} {set config_file ~/.gitk}
Don't we need to set config_file_tmp here too?
And, do we really want to create the ~/.config/git directory if we are
using the old-style ~/.gitk as the config file?
Paul.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: fix mistype
On Sat, Jan 18, 2014 at 02:58:51PM +0200, Max Kirillov wrote: Signed-off-by: Max Kirillov m...@max630.net Thanks, applied. Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: chmod +x po2msg
On Mon, Dec 02, 2013 at 11:12:43AM -0800, Junio C Hamano wrote: From: Jonathan Nieder jrnie...@gmail.com Date: Mon, 25 Nov 2013 13:00:10 -0800 The Makefile only runs it using tclsh, but because the fallback po2msg script has the usual tcl preamble starting with #!/bin/sh it can also be run directly. Signed-off-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com Thanks, applied. Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] gitk: Replace next and prev buttons with down and up arrows.
On 13-12-18 11:04 AM, Marc Branchaud wrote:
Users often find that next and prev do the opposite of what they
expect. For example, next moves to the next match down the list, but
that is almost always backwards in time. Replacing the text with arrows
makes it clear where the buttons will take the user.
Any opinions on this, either way?
I've grown fond of the down/up arrows. I find them much clearer than the
current next/prev buttons.
My only niggle about this patch is that the buttons are much smaller,
requiring a bit more precision clicking. But the smaller buttons allow more
room for other widgets.
M.
Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
Finally got around to drawing some up and down arrows.
M.
gitk | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/gitk b/gitk
index 33c3a6c..abd2ef3 100755
--- a/gitk
+++ b/gitk
@@ -2263,9 +2263,35 @@ proc makewindow {} {
# build up the bottom bar of upper window
${NS}::label .tf.lbar.flabel -text [mc Find]
-${NS}::button .tf.lbar.fnext -text [mc next] -command {dofind 1 1}
-${NS}::button .tf.lbar.fprev -text [mc prev] -command {dofind -1 1}
+
+set bm_down_data {
+ #define down_width 16
+ #define down_height 16
+ static unsigned char down_bits[] = {
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01,
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01,
+ 0x87, 0xe1, 0x8e, 0x71, 0x9c, 0x39, 0xb8, 0x1d,
+ 0xf0, 0x0f, 0xe0, 0x07, 0xc0, 0x03, 0x80, 0x01};
+}
+image create bitmap bm-down -data $bm_down_data -foreground $uifgcolor
+${NS}::button .tf.lbar.fnext -width 26 -command {dofind 1 1}
+.tf.lbar.fnext configure -image bm-down
+
+set bm_up_data {
+ #define up_width 16
+ #define up_height 16
+ static unsigned char up_bits[] = {
+ 0x80, 0x01, 0xc0, 0x03, 0xe0, 0x07, 0xf0, 0x0f,
+ 0xb8, 0x1d, 0x9c, 0x39, 0x8e, 0x71, 0x87, 0xe1,
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01,
+ 0x80, 0x01, 0x80, 0x01, 0x80, 0x01, 0x80, 0x01};
+}
+image create bitmap bm-up -data $bm_up_data -foreground $uifgcolor
+${NS}::button .tf.lbar.fprev -width 26 -command {dofind -1 1}
+.tf.lbar.fprev configure -image bm-up
+
${NS}::label .tf.lbar.flab2 -text [mc commit]
+
pack .tf.lbar.flabel .tf.lbar.fnext .tf.lbar.fprev .tf.lbar.flab2 \
-side left -fill y
set gdttype [mc containing:]
--
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 0/2] Two janitorial patches for builtin/blame.c
David Kastrup d...@gnu.org writes: This is more a warmup than anything else: I'm actually doing a quite more involved rewrite of git-blame right now. But it's been a long time since I sent patches for Git, so I'm starting out with something reasonably uncontroversial. Ping? Now I might have sent at an unopportune time: blame.c is mostly attributed to Junio who seems to have been a few days absent now. I also have seen quite a few mails and patch submissions on the list go basically unanswered in the last few days. So it might just be that this is business as usual. However, since I have not been on this list for quite a while, I would want to avoid causing large delays by some oversight. I have not so far signed off on the patches: it would appear that this is required. The submission guidelines in Documentation/SubmittingPatches state for signing off: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or [...] (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. Now the file involved (builtin/blame.c) itself does not state _any_ license. Instead it states /* * Blame * * Copyright (c) 2006, Junio C Hamano */ I do not intend my contribution to constitute a copyright assignment (and it hardly could be one). The top file COPYING in Git states Note that the only valid version of the GPL as far as this project is concerned is _this_ particular version of the license (ie v2, not v2.2 or v3.x or whatever), unless explicitly otherwise stated. HOWEVER, in order to allow a migration to GPLv3 if that seems like a good idea, I also ask that people involved with the project make their preferences known. In particular, if you trust me to make that decision, you might note so in your copyright message, ie something like This file is licensed under the GPL v2, or a later version at the discretion of Linus. might avoid issues. But we can also just decide to synchronize and contact all copyright holders on record if/when the occasion arises. As far as I am concerned, I am willing to license my work under the GPLv2 or any later version at the discretion of whoever wants to work with it. I think that should be compatible with the project goals. Now the above passage states you might note so in your copyright message, but my patches do not even contain a copyright message and it is not clear to me that they should, or that there is a sensible place to place such copyright messages. So any guidance about that? -- David Kastrup -- 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 0/2] Two janitorial patches for builtin/blame.c
David Kastrup wrote: Now I might have sent at an unopportune time: blame.c is mostly attributed to Junio who seems to have been a few days absent now. I also have seen quite a few mails and patch submissions on the list go basically unanswered in the last few days. In the U.S., yesterday was a federal holiday (Martin Luther King, Jr. day) and the two days before were the weekend. [...] maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. Now the file involved (builtin/blame.c) itself does not state _any_ license. Most of git is GPLv2-only. (As an aside, if there's interest then I'd be happy to see most of it change to GPLv2-or-later since that makes it possible to link to code under the Apache License. But I'm also happy with the status quo.) [...] As far as I am concerned, I am willing to license my work under the GPLv2 or any later version at the discretion of whoever wants to work with it. I think that should be compatible with the project goals. Now the above passage states you might note so in your copyright message, but my patches do not even contain a copyright message and it is not clear to me that they should, or that there is a sensible place to place such copyright messages. Yeah, since these patches aren't adding a large new chunk of code there's no need for a new copyright notice and so no place to put that kind of thing unless Junio wants to relicense blame (which would be orthogonal to these patches). Thanks and 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: [msysGit] Consecutive git gc fails on Windows network share
Hi kusma, On Tue, 21 Jan 2014, Erik Faye-Lund wrote: On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On Mon, 20 Jan 2014, Torsten Bögershausen wrote: b) add +++ at the places where you added the stat() and chmod(), c) and to send the question is this a good implementation ? to upstream git. I think your implementation makes sense. As I said in my other reply, I think that the problem would be addressed more generally in compat/mingw.c. It is to be doubted highly that upstream wants to handle cases such as rename() cannot overwrite read-only files on Windows everywhere they call rename() because the platforms upstream cares about do not have that problem. I'm not so sure. A quick test shows me that this is not the case for NTFS. Since this is over a network-share, the problem is probably server-side, and can affect other systems as well. So a work-around might be appropriate for all systems, no? I do not think that the problem occurs if you run the same commands on Linux, on a mounted Samba share. So I guess that upstream Git can enjoy their luxury of not having to care. In any case, if we would need this also for Linux, doing it for only one user of rename() would probably not be good enough, either... so something similar to mingw_rename() would be needed (interfering with mingw_rename itself, of course). Ciao, Dscho
Re: [msysGit] Consecutive git gc fails on Windows network share
On Tue, Jan 21, 2014 at 5:57 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
Hi kusma,
On Tue, 21 Jan 2014, Erik Faye-Lund wrote:
On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
On Mon, 20 Jan 2014, Torsten Bögershausen wrote:
b) add +++ at the places where you added the stat() and chmod(),
c) and to send the question is this a good implementation ? to upstream
git.
I think your implementation makes sense.
As I said in my other reply, I think that the problem would be
addressed more generally in compat/mingw.c. It is to be doubted highly
that upstream wants to handle cases such as rename() cannot overwrite
read-only files on Windows everywhere they call rename() because the
platforms upstream cares about do not have that problem.
I'm not so sure. A quick test shows me that this is not the case for
NTFS. Since this is over a network-share, the problem is probably
server-side, and can affect other systems as well.
So a work-around might be appropriate for all systems, no?
I do not think that the problem occurs if you run the same commands on
Linux, on a mounted Samba share. So I guess that upstream Git can enjoy
their luxury of not having to care.
In any case, if we would need this also for Linux, doing it for only one
user of rename() would probably not be good enough, either... so something
similar to mingw_rename() would be needed (interfering with mingw_rename
itself, of course).
Indeed. I was thinking of something along the lines of adding a
xrename in wrapper.c.
But you're probably right that this doesn't happen under Samba; surely
Samba would have added a work-around for such a filesystem by now. So
yeah, you've convinced me. mingw_rename is probably the place to do
that.
The work-around would probably look something like this:
diff --git a/compat/mingw.c b/compat/mingw.c
index a37bbf3..580b820 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1697,6 +1697,14 @@ int mingw_rename(const char *pold, const char *pnew)
*/
if (!_wrename(wpold, wpnew))
return 0;
+
+if (errno == EPERM) {
+/* read-only files cannot be moved over on network shares */
+_wchmod(wpnew, 0666);
+if (!_wrename(wpold, wpnew))
+return 0;
+}
+
if (errno != EEXIST)
return -1;
repeat:
--
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 0/2] Two janitorial patches for builtin/blame.c
David Kastrup wrote: So my understanding is that when we are talking about _significant_ additions to builtin/blame.c (the current patches don't qualify as such really) that a) builtin/blame.c is licensed under GPLv2 b) significant contributions to it will not be relicensed under different licenses without the respective contributors' explicit consent. Yep, that's how it works. [...] The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ -- 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] gitk: Comply with XDG base directory specification
On Tue, Jan 21, 2014 at 11:10 AM, Paul Mackerras pau...@samba.org wrote:
+if {![file exists $config_file]} {
+ if {![file exists [file dirname $config_file]]} {
+ file mkdir [file dirname $config_file]
+ }
+ # for backward compatability use the old config file if it exists
+ if {[file exists ~/.gitk]} {set config_file ~/.gitk}
Don't we need to set config_file_tmp here too?
Yeah it's probably best to keep the two files together.
And, do we really want to create the ~/.config/git directory if we are
using the old-style ~/.gitk as the config file?
Probably not. I'll prepare a new patch restoring the old behaviour.
Regards,
Astril.
--
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 0/2] Two janitorial patches for builtin/blame.c
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: Now I might have sent at an unopportune time: blame.c is mostly attributed to Junio who seems to have been a few days absent now. I also have seen quite a few mails and patch submissions on the list go basically unanswered in the last few days. In the U.S., yesterday was a federal holiday (Martin Luther King, Jr. day) and the two days before were the weekend. I see. Now the file involved (builtin/blame.c) itself does not state _any_ license. Most of git is GPLv2-only. Does that include builtin/blame.c? Yeah, since these patches aren't adding a large new chunk of code Well, _significant_ chunks of code are in the works already (and my question was really more about them). there's no need for a new copyright notice and so no place to put that kind of thing unless Junio wants to relicense blame (which would be orthogonal to these patches). So my understanding is that when we are talking about _significant_ additions to builtin/blame.c (the current patches don't qualify as such really) that a) builtin/blame.c is licensed under GPLv2 b) significant contributions to it will not be relicensed under different licenses without the respective contributors' explicit consent. This question is not academical to me. I don't have any source of income apart from people paying me to write free software (mainly LilyPond users), so if I'm writing significant pieces of code, I don't want to see them distributed as proprietary software (for example, by traveling through the very unrestrictively licensed libgit2) without being in the situation of negotiating recompensation for that. The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. -- David Kastrup -- 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 0/2] Two janitorial patches for builtin/blame.c
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: So my understanding is that when we are talking about _significant_ additions to builtin/blame.c (the current patches don't qualify as such really) that a) builtin/blame.c is licensed under GPLv2 b) significant contributions to it will not be relicensed under different licenses without the respective contributors' explicit consent. Yep, that's how it works. [...] The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ Probably somewhat more verbose like This file may be distributed under the conditions of the GPLv2. See the file COPYING for details. I think there are boilerplate texts for that. Whatever the exact wording, that would be the cleanest way I think. The respective Documentation/SubmittingPatches text looks like it is quoted from somewhere else, so adapting it to the realities of files without clear copyright statement seems less straightforward. -- David Kastrup -- 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] gitk: Comply with XDG base directory specification
Write the gitk config data to $XDG_CONFIG_HOME/git/gitk ($HOME/.config/git/gitk
by default) in line with the XDG specification. This makes it consistent with
git which also follows the spec.
If $HOME/.gitk already exists use that for backward compatibility, so only new
installations are affected.
Signed-off-by: Astril Hayato astrilhay...@gmail.com
---
gitk | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/gitk b/gitk
index 33c3a6c..d592d7e 100755
--- a/gitk
+++ b/gitk
@@ -2761,14 +2761,17 @@ proc savestuff {w} {
global linkfgcolor circleoutlinecolor
global autoselect autosellen extdifftool perfile_attrs markbgcolor use_ttk
global hideremotes want_ttk maxrefs
+global config_file config_file_tmp
if {$stuffsaved} return
if {![winfo viewable .]} return
catch {
- if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
- set f [open ~/.gitk-new w]
+ if {[file exists $config_file_tmp]} {
+ file delete -force $config_file_tmp
+ }
+ set f [open $config_file_tmp w]
if {$::tcl_platform(platform) eq {windows}} {
- file attributes ~/.gitk-new -hidden true
+ file attributes $config_file_tmp -hidden true
}
puts $f [list set mainfont $mainfont]
puts $f [list set textfont $textfont]
@@ -2845,7 +2848,7 @@ proc savestuff {w} {
}
puts $f }
close $f
- file rename -force ~/.gitk-new ~/.gitk
+ file rename -force $config_file_tmp $config_file
}
set stuffsaved 1
}
@@ -12058,7 +12061,29 @@ namespace import ::msgcat::mc
## And eventually load the actual message catalog
::msgcat::mcload $gitk_msgsdir
-catch {source ~/.gitk}
+catch {
+# follow the XDG base directory specification by default. See
+# http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
+if {[info exists env(XDG_CONFIG_HOME)] $env(XDG_CONFIG_HOME) ne } {
+ # XDG_CONFIG_HOME environment variable is set
+ set config_file [file join $env(XDG_CONFIG_HOME) git gitk]
+ set config_file_tmp [file join $env(XDG_CONFIG_HOME) git gitk-tmp]
+} else {
+ # default XDG_CONFIG_HOME
+ set config_file ~/.config/git/gitk
+ set config_file_tmp ~/.config/git/gitk-tmp
+}
+if {![file exists $config_file]} {
+ # for backward compatibility use the old config file if it exists
+ if {[file exists ~/.gitk]} {
+ set config_file ~/.gitk
+ set config_file_tmp ~/.gitk-tmp
+ } elseif {![file exists [file dirname $config_file]]} {
+ file mkdir [file dirname $config_file]
+ }
+}
+source $config_file
+}
parsefont mainfont $mainfont
eval font create mainfont [fontflags mainfont]
--
1.8.5.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 0/2] Two janitorial patches for builtin/blame.c
David Kastrup wrote: Jonathan Nieder jrnie...@gmail.com writes: Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ Probably somewhat more verbose like This file may be distributed under the conditions of the GPLv2. See the file COPYING for details. I think there are boilerplate texts for that. All else being equal, longer is worse. Whatever the exact wording, that would be the cleanest way I think. The respective Documentation/SubmittingPatches text looks like it is quoted from somewhere else, so adapting it to the realities of files without clear copyright statement seems less straightforward. Hm, the wording comes from the Linux kernel project, where it's also pretty normal not to have a license notice in every file (and where the default license is also GPLv2). Is the problem the phrase indicated in the file, or is the problem e.g. the lack of a pointer to https://github.com/libgit2/libgit2/blob/development/git.git-authors? 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] safe_create_leading_directories(): on Windows, \ can separate path components
Sebastian Schuberth sschube...@gmail.com writes: On Sun, Jan 19, 2014 at 12:40 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This patch applies on top of v3 of mh/safe-create-leading-directories. The only logical change from Sebastian's patch is that this version restores the original slash character rather than always restoring it to '/' (as suggested by Junio). Thanks Michael. This is very similar to Junio's current merge conflict resultion in pu between your original series and my patch (c2fec83). I like this patch of yours slightly better, however, as it uses a while instead of a for loop and the more conclusive slash_character than the was_slash variable name. Yeah, I agree this patch that uses more descriptive names is a much more desirable end-result than the tentative merge in 'pu'. -- 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/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes: ... does complicate the point of my series, which was to add more intimate logic about how we handle LESS. ... return !x || strchr(x, 'R'); } [...] } but we are still hard-coding a lot of intelligence about less here. I am not sure if it is even a good idea for us to have so intimate logic for various pagers in the first place. I'd seriously wonder if it is better to take this position: A platform packager who sets the default pager and/or the default environment for the pager at the build time, or an individual user who tells your Git what pager you want to use and/or with what setting that pager should be run under with environment variables. These people ought to know far better than Git what their specific choices do. Do not try to second-guess them. The potential breakage caused by setting MORE=R unconditionally is a good example. A careless intimate logic may think that any pager that is called 'more' would behave like traditional 'more', breaking half the 'more' user population while catering to the other half. I wonder if the abstraction provided by the Makefile variable is really worthwhile. Anybody adding a new line to it would also want to tweak pager_can_handle_color to add similar logic. And that is why I am not enthused by the idea of adding such logic in the first place. I view the Makefile customization as a way for the packager to offer a sensible default for their platform without touching the code, which is slightly different from your 1. below. Taking a step back for a moment, we are getting two things out of such a Makefile variable: 1. An easy way for packager to add intelligence about common pagers on their system. 2. DRY between git-sh-setup and the C code. I do like (1), but I do not know if we want to try to encode the can handle color logic into a Makefile variable. What would it look like? For (2), an alternate method would be to simply provide git pager as C code, which spawns the appropriate pager as the C code would. Then git-sh-setup can easily build around that. And as to 2., if the answer to the other issue do we want our code to be intimately aware of pager-specific quirks, or do we just want to give packagers a knob to express their choice of the default? resolves to the former, I would think that git pager would be not just a workable alternative, but would be the only viable one. -- 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: Verifiable git archives?
Michael Haggerty mhag...@alum.mit.edu writes: On 01/09/2014 09:11 PM, Junio C Hamano wrote: Andy Lutomirski l...@amacapital.net writes: It's possible, in principle, to shove enough metadata into the output of 'git archive' to allow anyone to verify (without cloning the repo) to verify that the archive is a correct copy of a given commit. Would this be considered a useful feature? Presumably there would be a 'git untar' command that would report failure if it fails to verify the archive contents. This could be as simple as including copies of the commit object and all relevant tree objects and checking all of the hashes when untarring. You only need the object name of the top-level tree. After untar the archive into an empty directory, make it a new repository and git add . git write-tree---the result should match the top-level tree the archive was supposed to contain. [...] This wouldn't work if any files were excluded from the archive using gitattribute export-ignore (or export-subst, which you already mentioned in a follow-up email). Correct. By and such below, I meant any and all futzing that makes the resulting working tree different from the tree object being archived ;-) That includes the line-ending configuration and other things as well. Also, if you used keyword substitution and such when creating an archive, then the filesystem entities resulting from expanding it would not match the original. -- 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/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes:
Perhaps instead of just dumping it into a macro, we could actually parse
it at compile time into C code, and store the result as a header file.
Then that header file becomes the proper dependency, and this run-time
error:
+while (*pager_env) {
+struct strbuf buf = STRBUF_INIT;
+const char *cp = strchrnul(pager_env, '=');
+
+if (!*cp)
+die(malformed build-time PAGER_ENV);
would become a compile-time error. We could do the same for the shell
code, too.
I'm thinking something like:
Heh, great minds think alike. I did start writing a toy-patch with
a new file called pager-env.h, before discarding it and sending a
simpler alternative which you are responding to.
I do not mind the approach at all; the primary reason I stopped
doing that was because the amount of code to get quotes right turned
out to be sufficiently big to obscure the real point of the how
about doing it this way illustration patch.
diff --git a/Makefile b/Makefile
index b4af1e2..3a8d15e 100644
--- a/Makefile
+++ b/Makefile
@@ -2182,6 +2182,16 @@ GIT-LDFLAGS: FORCE
echo $$FLAGS GIT-LDFLAGS; \
fi
+GIT-PAGER-ENV:
+ @PAGER_ENV='$(PAGER_ENV)'; \
+ if test x$$PAGER_ENV != x`cat GIT-PAGER-ENV 2/dev/null`; then \
+ echo $$PAGER_ENV GIT-PAGER-ENV; \
+ fi
+
+pager-env.h: GIT-PAGER-ENV mkcarray
+ $(SHELL_PATH) mkcarray pager_env $ $@+
+ mv $@+ $@
+
# We need to apply sq twice, once to protect from the shell
# that runs GIT-BUILD-OPTIONS, and then again to protect it
# and the first level quoting from the shell that runs echo.
diff --git a/mkcarray b/mkcarray
index e69de29..5962440 100644
--- a/mkcarray
+++ b/mkcarray
@@ -0,0 +1,21 @@
+name=$1; shift
+guard=$(echo $name | tr a-z A-Z)
+
+cat -EOF
+#ifndef ${guard}_H
+#define ${guard}_H
+
+static const char *${name} = {
+EOF
+
+for i in $(cat); do
+ # XXX c-quote the values?
+ printf '\t%s,\n' $i
+done
+
+cat EOF
+ NULL
+};
+
+#endif /* ${guard}_H */
+EOF
--
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 0/2] Two janitorial patches for builtin/blame.c
David Kastrup wrote: The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. BTW, thanks for bringing this up. It last came up at [1]. Perhaps we can do better by adding a note to README or some similar file explaining that git is under the GPLv2 and files use that license when not otherwise specified. [1] http://thread.gmane.org/gmane.comp.version-control.git/234705/focus=234709 -- 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 0/2] Two janitorial patches for builtin/blame.c
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: Jonathan Nieder jrnie...@gmail.com writes: Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ Probably somewhat more verbose like This file may be distributed under the conditions of the GPLv2. See the file COPYING for details. I think there are boilerplate texts for that. All else being equal, longer is worse. I am not sure that all else is equal. Whatever the exact wording, that would be the cleanest way I think. The respective Documentation/SubmittingPatches text looks like it is quoted from somewhere else, so adapting it to the realities of files without clear copyright statement seems less straightforward. Hm, the wording comes from the Linux kernel project, where it's also pretty normal not to have a license notice in every file (and where the default license is also GPLv2). Is the problem the phrase indicated in the file, At least that's what I perceive as a problem in combination with the complete absence of any such notice in the file I am contributing to. git grep -i license actually shows a dearth of licensing information outside of subprojects and contrib. The README file states Git is an Open Source project covered by the GNU General Public License version 2 (some parts of it are under different licenses, compatible with the GPLv2). It was originally written by Linus Torvalds with help of a group of hackers around the net. without mentioning _which_ parts are under different licenses. The license file COPYING itself does not specify which files are covered, and there is _also_ LGPL-2.1 which has a statement While most of this project is under the GPL (see COPYING), the xdiff/ library and some libc code from compat/ are licensed under the GNU LGPL, version 2.1 or (at your option) any later version and some other files are under other licenses. Check the individual files to be sure. Well, and when checking the individual files, there is really nothing to be found for being sure. The net result is that when signing off on a patch according to the rules in Documentation/SubmittingPatches, for most files you don't really have a definite statement just _what_ license you are agreeing your work to be distributed under. or is the problem e.g. the lack of a pointer to https://github.com/libgit2/libgit2/blob/development/git.git-authors? No, not at all. libgit2 is not in any way special among projects that might want to have access to Git code under different licenses. It would be possible to state something like Unless indicated otherwise, consent will be assumed for contributions to Git as being redistributable in the libgit2 project under its respective licenses or something, but I think that would be seriously surprising, and not noticing such a clause could not be construed as implying consent. -- David Kastrup -- 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 0/2] Two janitorial patches for builtin/blame.c
David Kastrup wrote: and contrib. The README file states Git is an Open Source project covered by the GNU General Public License version 2 (some parts of it are under different licenses, compatible with the GPLv2). It was originally written by Linus Torvalds with help of a group of hackers around the net. without mentioning _which_ parts are under different licenses. Okay, how about this patch? diff --git i/README w/README index 15a8e23..6745db5 100644 --- i/README +++ w/README @@ -21,8 +21,9 @@ and full access to internals. Git is an Open Source project covered by the GNU General Public License version 2 (some parts of it are under different licenses, -compatible with the GPLv2). It was originally written by Linus -Torvalds with help of a group of hackers around the net. +compatible with the GPLv2, and have notices to that effect). It was +originally written by Linus Torvalds with help of a group of hackers +around the net. Please read the file INSTALL for installation instructions. -- 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] Fix safe_create_leading_directories() for Windows
Sebastian Schuberth sschube...@gmail.com writes: On Thu, Jan 2, 2014 at 10:08 PM, Junio C Hamano gits...@pobox.com wrote: Seems like the path to clone to is taken as-is from argv in cmd_clone(). So maybe another solution would be to replace all backslashes with forward slashes already there? That sounds like a workable alternative, and it might even be a preferable solution but if and only if clone's use of the function to create paths that lead to a new working tree location is the only (ab)use of the function. That was what I meant when I said it may be that it is a bug in the specific caller. AFAIK, the function I think Dscho made valid points in his other mail that the better solution still is to make safe_create_leading_directories() actually safe, also regarding its arguments. Sorry, but I think I misremebered the reason why we have that function. There are two reasons we may need to do a rough equivalent of system(mkdir -p $(dirname a/b/c)) given an argument 'a/b/c' in our codebase. 1. We would want to be able to create 'c' such that we can later refer to a/b/c to retrieve what we wrote there, so we need to make sure that a/b/ refers to a writable directory. 2. We were told by the index (or a caller that will then later update the index in a consistent way) that 'a/b/c' must exist in the working tree, so we need to make sure that a/ and a/b/ are both directories (not a symlink to a directory elsewhere). Seeing hits git grep safe_create_leading_directories from apply and merge-recursive led me to incorrectly assume that this function was meant to do the latter [*1*]. But the original purpose of safe_create_leading_directories() in sha1_file.c was to mkdir -p inside .git/ directories when writing to refs e.g. refs/heads/foo/bar, and for this kind of use, we must honor existing symlinks. .git/refs may be a symbolic link in a working tree managed by new-workdir to the real repository, and we do not want to unlink mkdir it. We even have an offset-1st-component call in the function, which is a clear sign that we would want this as usable for the full path in the filesystem, which is an indication that this should not be used to create paths in the working tree. So I think it is the right thing to do to allow the function accept platform specific path here, and it is OK for clone to call it to create the path leading to the location of the new repository. A related tangent is that somebody needs to audit the callers to make sure this function is _not_ used to create leading paths in the working tree. If a merge tells us to create foo/bar/baz.test, we should not do an equivalent of mkdir -p foo/bar cat baz.test; we must make sure foo/ and foo/bar are real directories without any symbolic link in the leading paths components (the same thing can be said for apply). I have a suspicion that the two hits from apply and merge-recursive are showing an existing bug that is not platform specific. Thanks. [Footnote] *1* In such a context, getting a/b\c/d and failing to make sure a/ is a directory that has b\c/ as its immediate subdirectory is a bug---especially, splitting at the backslash between 'b' and 'c' and creating 'a/b/c/' is not acceptable. The platform code should instead say sorry, we cannot do that if it cannot create a directory entry b\c in the directory a/ (which would make sure such a non-portable path in projects will be detected). -- 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 0/2] Two janitorial patches for builtin/blame.c
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: So my understanding is that when we are talking about _significant_ additions to builtin/blame.c (the current patches don't qualify as such really) that a) builtin/blame.c is licensed under GPLv2 b) significant contributions to it will not be relicensed under different licenses without the respective contributors' explicit consent. Yep, that's how it works. [...] The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ I vaguely recall that jgit folks at one point wanted to lift this implementation and were interested in seeing it to be dual licensed to BSD but that was a long time ago. http://git.661346.n2.nabble.com/JGIT-Blame-functionality-for-jgit-td2142726.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: GIT_WORK_TREE and GIT_DIR with git clone
Cosmin Apreutesei cosmin.apreute...@gmail.com writes: I would like to be able to tell my users that they can simply do: git clone --git-dir=_/git/module1/.git module1-url git clone --git-dir=_/git/module2/.git module2-url which will overlay the files from both modules into the current directory, which from git's perspective is the work-tree for both modules. Would there be a sensible semantics in the resulting working tree? Which repository would a new file in such a combined working tree belong to? -- 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/RFC] Makefile: Fix compilation of windows resource file
Ramsay Jones ram...@ramsay1.demon.co.uk writes: If the git version number consists of less than three period separated numbers, then the windows resource file compilation issues a syntax error: $ touch git.rc $ make V=1 git.res GIT_VERSION = 1.9.rc0 windres -O coff \ -DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \ -DGIT_VERSION=\\\1.9.rc0\\\ git.rc -o git.res C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error make: *** [git.res] Error 1 $ [Note that -DPATCH=rc0] Thanks for a report. I've been wondering how many distros and packagers would have an issue like this when we go to 2-digit release naming. Of course we knew everybody can grok 3-or-4 ;-) In order to fix the syntax error, we replace any rcX with zero and include some additional 'zero' padding to the version number list. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Hi Junio, This patch is marked RFC because, as I was just about to send this email, I realized it wouldn't always work: Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is not even working for maintenance releases. Does it differenciate between 1.8.5.1 and 1.8.5.2, for example?. Or does windres always assume that a package version is always 3-dewey-decimal (not 2, not 4)? -- 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: REQUEST-PULL: Please pull from git-gui.
Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Two janitorial patches for builtin/blame.c
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: and contrib. The README file states Git is an Open Source project covered by the GNU General Public License version 2 (some parts of it are under different licenses, compatible with the GPLv2). It was originally written by Linus Torvalds with help of a group of hackers around the net. without mentioning _which_ parts are under different licenses. Okay, how about this patch? diff --git i/README w/README index 15a8e23..6745db5 100644 --- i/README +++ w/README @@ -21,8 +21,9 @@ and full access to internals. Git is an Open Source project covered by the GNU General Public License version 2 (some parts of it are under different licenses, -compatible with the GPLv2). It was originally written by Linus -Torvalds with help of a group of hackers around the net. +compatible with the GPLv2, and have notices to that effect). It was +originally written by Linus Torvalds with help of a group of hackers +around the net. Clearer. I think it would be most accurate to state: Git is an Open Source project covered by the GNU General Public License version 2. Those parts of it which may be also be distributed under other licenses contain notices to that effect. The point is that as a whole, the software is distributed under GPLv2 (that's what compatible licensing actually means since the GPL demands distribution of the software as a whole under the GPL) but parts of it may optionally be distributed under other licenses. -- David Kastrup -- 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/RFC] Makefile: Fix compilation of windows resource file
Junio C Hamano gits...@pobox.com writes:
Ramsay Jones ram...@ramsay1.demon.co.uk writes:
If the git version number consists of less than three period
separated numbers, then the windows resource file compilation
issues a syntax error:
$ touch git.rc
$ make V=1 git.res
GIT_VERSION = 1.9.rc0
windres -O coff \
-DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \
-DGIT_VERSION=\\\1.9.rc0\\\ git.rc -o git.res
C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
make: *** [git.res] Error 1
$
[Note that -DPATCH=rc0]
Thanks for a report. I've been wondering how many distros and
packagers would have an issue like this when we go to 2-digit
release naming. Of course we knew everybody can grok 3-or-4 ;-)
In order to fix the syntax error, we replace any rcX with zero and
include some additional 'zero' padding to the version number list.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Junio,
This patch is marked RFC because, as I was just about to send this
email, I realized it wouldn't always work:
Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
not even working for maintenance releases. Does it differenciate
between 1.8.5.1 and 1.8.5.2, for example?. Or does windres always
assume that a package version is always 3-dewey-decimal (not 2, not
4)?
Perhaps like this? Just grab digit-only segments that are separated
with either dot or dash (and stop when we see a non-digit like
'dirty' or 'rcX'), and make them separated with comma.
Note that I am merely guessing that short-digit version numbers
are acceptable by now after seeing
https://sourceware.org/ml/binutils/2012-07/msg00199.html
without knowing the current state of affairs. If that is not the
case you may have to count the iteration of the loop and append or
chop the resulting string as necessary.
Makefile | 2 +-
gen-version-string.sh | 13 +
git.rc| 4 ++--
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index b4af1e2..329f942 100644
--- a/Makefile
+++ b/Makefile
@@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
git.res: git.rc GIT-VERSION-FILE
$(QUIET_RC)$(RC) \
- $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst
., ,$(GIT_VERSION) \
+ -DVERSIONSTRING=$$(./gen-version-string.sh $(GIT_VERSION)) \
-DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@
ifndef NO_PERL
diff --git a/gen-version-string.sh b/gen-version-string.sh
new file mode 100755
index 000..00af718
--- /dev/null
+++ b/gen-version-string.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+IFS=.- result=
+for v in $1
+do
+ if expr $v : '[0-9][0-9]*$' /dev/null
+ then
+ result=$result${result:+,}$v
+ else
+ break
+ fi
+done
+echo $result
diff --git a/git.rc b/git.rc
index bce6db9..6f2a8d2 100644
--- a/git.rc
+++ b/git.rc
@@ -1,6 +1,6 @@
1 VERSIONINFO
-FILEVERSION MAJOR,MINOR,PATCH,0
-PRODUCTVERSION MAJOR,MINOR,PATCH,0
+FILEVERSION VERSIONSTRING,0
+PRODUCTVERSION VERSIONSTRING,0
BEGIN
BLOCK StringFileInfo
BEGIN
--
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_WORK_TREE and GIT_DIR with git clone
Hi, thanks for answering. I would like to be able to tell my users that they can simply do: git clone --git-dir=_/git/module1/.git module1-url git clone --git-dir=_/git/module2/.git module2-url which will overlay the files from both modules into the current directory, which from git's perspective is the work-tree for both modules. Would there be a sensible semantics in the resulting working tree? Which repository would a new file in such a combined working tree belong to? The developer would have to decide by way of `git add`. Ignoring other repos' files would be done by way of local config option `excludesfile` (.gitignore is out). To make it easier to work with git this way, a script that creates a subshell in the context of a repo can be done with something like `PS1=[$1 $PS1] GIT_DIR=_git/$1/.git bash -i -- git would then work as usual in that subshell for that specific repo, never leaving the working tree. I successfully employed the above scheme with luapower[1] packages, which are all different and mostly unrelated libraries, but which need to be overlaid over a common directory structure. And that's just an example. I can think of many projects that are modularized and yet the modules need to place many files in many places to make a working system (web frameworks, the linux filesystem, etc.) Currently, to clone a repo one has to do: export GIT_DIR=_git/submodule/.git git init git config --local core.worktree ../../.. git remote add origin ssh://g...@github.com/luapower/submodule.git git fetch git branch --track master origin/master git checkout That's 6 commands for what could be: git clone --git-dir=_git/submodule/.git ssh://g...@github.com/luapower/submodule.git Or even better: git clone --git-dirs=_git ssh://g...@github.com/luapower/submodule.git [1] http://luapower.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: [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
David Kastrup d...@gnu.org writes:
---
Thanks. At some point during its development I must have thought
that having it as a dual-linked list may make it easier when we have
to split a block into pieces, but it seems that split_overlap() does
not need to look at this information.
Needs sign-off.
builtin/blame.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2195595 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
* scoreboard structure, sorted by the target line number.
*/
struct blame_entry {
- struct blame_entry *prev;
struct blame_entry *next;
/* the first line of this group in the final image;
@@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
ent-s_lno + ent-num_lines == next-s_lno) {
ent-num_lines += next-num_lines;
ent-next = next-next;
- if (ent-next)
- ent-next-prev = ent;
origin_decref(next-suspect);
free(next);
ent-score = 0;
@@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
prev = ent;
/* prev, if not NULL, is the last one that is below e */
- e-prev = prev;
+
if (prev) {
e-next = prev-next;
prev-next = e;
@@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
e-next = sb-ent;
sb-ent = e;
}
- if (e-next)
- e-next-prev = e;
}
/*
@@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb,
struct blame_entry *e)
*/
static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
{
- struct blame_entry *p, *n;
+ struct blame_entry *n;
- p = dst-prev;
n = dst-next;
origin_incref(src-suspect);
origin_decref(dst-suspect);
memcpy(dst, src, sizeof(*src));
- dst-prev = p;
dst-next = n;
dst-score = 0;
}
@@ -2502,8 +2495,6 @@ parse_done:
ent-suspect = o;
ent-s_lno = bottom;
ent-next = next;
- if (next)
- next-prev = ent;
origin_incref(o);
}
origin_decref(o);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.9-rc0
An early preview release Git v1.9-rc0 is now available for testing at the usual places. I tagged this late last week, but forgot to send out the announcement before I left for the weekend, and then I wasn't well yesterday, but anyway. There still may be a few minor topics not in this preview but should be in the final release, but otherwise, this should be pretty close to it. It has been reported that turning git.rc into git.res does not like the new 2-dewey-decimal release numbering scheme; packagers of various distro might find similar issues in their build procedures, in which case they have about 3 weeks to update them until the final release. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 495108620f8547ec8e979549857dae96bfabb0f7 git-1.9.rc0.tar.gz daf964f46acd9ba72ebcdfbd1ef0c668206a0d92 git-htmldocs-1.9.rc0.tar.gz a3b83356f738d1452bcee1d43ac5267a08986292 git-manpages-1.9.rc0.tar.gz The following public repositories all have a copy of the v1.9-rc0 tag and the v1.9-rc0 branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.9 Release Notes (draft) == Backward compatibility notes git submodule foreach $cmd $args used to treat $cmd $args the same way ssh did, concatenating them into a single string and letting the shell unquote. Careless users who forget to sufficiently quote $args gets their argument split at $IFS whitespaces by the shell, and got unexpected results due to this. Starting from this release, the command line is passed directly to the shell, if it has an argument. Read-only support for experimental loose-object format, in which users could optionally choose to write in their loose objects for a short while between v1.4.3 to v1.5.3 era, has been dropped. The meanings of --tags option to git fetch has changed; the command fetches tags _in addition to_ what are fetched by the same command line without the option. The way git push $there $what interprets $what part given on the command line, when it does not have a colon that explicitly tells us what ref at the $there repository is to be updated, has been enhanced. A handful of ancient commands that have long been deprecated are finally gone (repo-config, tar-tree, lost-found, and peek-remote). Backward compatibility notes (for Git 2.0) -- When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics, which pushes: - only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch, if you are pushing to the same remote as you fetch from; or - only the current branch to the branch with the same name, if you are pushing to a remote that is not where you usually fetch from. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. The default prefix for git svn will change in Git 2.0. For a long time, git svn created its remote-tracking branches directly under refs/remotes, but it will place them under refs/remotes/origin/ unless it is told otherwise with its --prefix option. Updates since v1.8.5 Foreign
Re: [PATCH 0/6] Make 'git help everyday' work - relnotes
From: Stefan Näwe stefan.na...@atlas-elektronik.com Am 16.01.2014 22:14, schrieb Philip Oakley: From: Stefan Näwe stefan.na...@atlas-elektronik.com [...] I'd really like to see 'git help relnotes' working as well... Stefan Stefan, Were you thinking that all the release notes would be quoted verbatim in the one long man page? Or that it would be a set of links to each of the individual text files (see the ifdef::stalenotes[] in git/Documentation/git.txt)? The latter allows individual release notes to be checked, but still leaves folks with a difficult search problem if they want to find when some command was 'tweaked'. Obviously, any method would need to be easy to maintain. And the RelNotes symlink would need handling. 'git help relnotes' should show the current release note with a link to the previous. OK, that seems very sensible, as the concatenated release notes run to 15k lines! Determining which is the current release note is possibly more problematic, which should be when making the documentation. And 'git help git' should link to the current release note. In some sense that 'current' should be the same as the 'git --version', but through an assumption of a common distribution of git and the documentation, rather than any run time determination. At the moment the Documenation/git.txt 'stalenotes' section could be separated into its own file to act as the basis for the links, but as yet I don't have a good view as to how the current release notes (with / without maint notes?) would be embedded without a maintenance burden for Junio. Philip -- 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 04/16] trailer: process command line trailer arguments
Christian Couder chrisc...@tuxfamily.org writes:
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 77
+++
1 file changed, 77 insertions(+)
diff --git a/trailer.c b/trailer.c
index e7d8244..bb1fcfb 100644
--- a/trailer.c
+++ b/trailer.c
@@ -363,3 +363,80 @@ static int git_trailer_config(const char *conf_key,
const char *value, void *cb)
}
return 0;
}
+
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ char *end = strchr(trailer, '=');
+ if (!end)
+ end = strchr(trailer, ':');
+ if (end) {
+ strbuf_add(tok, trailer, end - trailer);
+ strbuf_trim(tok);
+ strbuf_addstr(val, end + 1);
+ strbuf_trim(val);
+ } else {
+ strbuf_addstr(tok, trailer);
+ strbuf_trim(tok);
+ }
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ struct trailer_item *new;
+
+ parse_trailer(tok, val, string);
+
+ int tok_alnum_len = alnum_len(tok.buf, tok.len);
decl-after-stmt.
+
+ /* Lookup if the token matches something in the config */
+ struct trailer_item *item;
+ for (item = first_conf_item; item; item = item-next)
+ {
+ if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
+ !strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
+ new = xcalloc(sizeof(struct trailer_item), 1);
+ new-conf = item-conf;
+ new-token = xstrdup(item-conf-key);
+ new-value = strbuf_detach(val, NULL);
+ strbuf_release(tok);
+ return new;
+ }
+ }
+
+ new = xcalloc(sizeof(struct trailer_item), 1);
+ new-conf = xcalloc(sizeof(struct conf_info), 1);
+ new-token = strbuf_detach(tok, NULL);
+ new-value = strbuf_detach(val, NULL);
+
+ return new;
+}
+
+static void add_trailer_item(struct trailer_item **first,
+ struct trailer_item **last,
+ struct trailer_item *new)
+{
+ if (!*last) {
+ *first = new;
+ *last = new;
+ } else {
+ (*last)-next = new;
+ new-previous = *last;
+ *last = new;
+ }
+}
+
+static struct trailer_item *process_command_line_args(int argc, const char
**argv)
+{
+ int i;
+ struct trailer_item *arg_tok_first = NULL;
+ struct trailer_item *arg_tok_last = NULL;
+
+ for (i = 0; i argc; i++) {
+ struct trailer_item *new = create_trailer_item(argv[i]);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+
+ return arg_tok_first;
+}
--
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 repack --max-pack-size broken in git-next
With git-next, the --max-pack-size option to git repack doesn't work. With git at b139ac2, `git repack --max-pack-size=1g` says error: option `max-pack-size' expects a numerical value while `git repack --max-pack-size=1073741824` says error: unknown option `max_pack_size=1073741824' I bisected this down to a1bbc6c, which rewrote git repack in C. -- 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/2] list-objects: only look at cmdline trees with edge_hint
Jeff King p...@peff.net writes:
This tradeoff probably makes sense in the context of
pack-objects, where we have set revs-edge_hint to have the
traversal feed us the set of boundary objects. For a
regular rev-list, though, it is probably not a good
tradeoff. It is true that it makes our list slightly closer
to a true set difference, but it is a rare case where this
is important. And because we do not have revs-edge_hint
set, we do nothing useful with the larger set of boundary
objects.
This patch therefore ties the extra tree examination to the
revs-edge_hint flag; it is the presence of that flag that
makes the tradeoff worthwhile.
Makes sense. Thanks. Will queue.
Here is output from the p0001-rev-list showing the
improvement in performance:
Test HEAD^ HEAD
-
0001.1: rev-list --all 0.69(0.65+0.02)
0.69(0.66+0.02) +0.0%
0001.2: rev-list --all --objects 3.22(3.19+0.03)
3.23(3.20+0.03) +0.3%
0001.4: rev-list $commit --not --all 0.04(0.04+0.00)
0.04(0.04+0.00) +0.0%
0001.5: rev-list --objects $commit --not --all 0.27(0.26+0.01)
0.04(0.04+0.00) -85.2%
Signed-off-by: Jeff King p...@peff.net
---
list-objects.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index 6cbedf0..206816f 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -162,15 +162,17 @@ void mark_edges_uninteresting(struct rev_info *revs,
show_edge_fn show_edge)
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
- for (i = 0; i revs-cmdline.nr; i++) {
- struct object *obj = revs-cmdline.rev[i].item;
- struct commit *commit = (struct commit *)obj;
- if (obj-type != OBJ_COMMIT || !(obj-flags UNINTERESTING))
- continue;
- mark_tree_uninteresting(commit-tree);
- if (revs-edge_hint !(obj-flags SHOWN)) {
- obj-flags |= SHOWN;
- show_edge(commit);
+ if (revs-edge_hint) {
+ for (i = 0; i revs-cmdline.nr; i++) {
+ struct object *obj = revs-cmdline.rev[i].item;
+ struct commit *commit = (struct commit *)obj;
+ if (obj-type != OBJ_COMMIT || !(obj-flags
UNINTERESTING))
+ continue;
+ mark_tree_uninteresting(commit-tree);
+ if (!(obj-flags SHOWN)) {
+ obj-flags |= SHOWN;
+ show_edge(commit);
+ }
}
}
}
--
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 04/16] trailer: process command line trailer arguments
Junio C Hamano gits...@pobox.com writes:
+static struct trailer_item *create_trailer_item(const char *string)
+{
+struct strbuf tok = STRBUF_INIT;
+struct strbuf val = STRBUF_INIT;
+struct trailer_item *new;
+
+parse_trailer(tok, val, string);
+
+int tok_alnum_len = alnum_len(tok.buf, tok.len);
decl-after-stmt.
+
+/* Lookup if the token matches something in the config */
+struct trailer_item *item;
ditto.
+for (item = first_conf_item; item; item = item-next)
+{
Style.
I wonder if Cc list is being a bit too wide for this series, by the
way.
--
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/RFC] Makefile: Fix compilation of windows resource file
On 21/01/14 21:36, Junio C Hamano wrote:
Junio C Hamano gits...@pobox.com writes:
Ramsay Jones ram...@ramsay1.demon.co.uk writes:
If the git version number consists of less than three period
separated numbers, then the windows resource file compilation
issues a syntax error:
$ touch git.rc
$ make V=1 git.res
GIT_VERSION = 1.9.rc0
windres -O coff \
-DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \
-DGIT_VERSION=\\\1.9.rc0\\\ git.rc -o git.res
C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
make: *** [git.res] Error 1
$
[Note that -DPATCH=rc0]
Thanks for a report. I've been wondering how many distros and
packagers would have an issue like this when we go to 2-digit
release naming. Of course we knew everybody can grok 3-or-4 ;-)
In order to fix the syntax error, we replace any rcX with zero and
include some additional 'zero' padding to the version number list.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Junio,
This patch is marked RFC because, as I was just about to send this
email, I realized it wouldn't always work:
Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
not even working for maintenance releases. Does it differenciate
between 1.8.5.1 and 1.8.5.2, for example?. Or does windres always
assume that a package version is always 3-dewey-decimal (not 2, not
4)?
I'm no expert on '.rc' file syntax, but the code certainly does not
(currently) support four digit versions.
Perhaps like this? Just grab digit-only segments that are separated
with either dot or dash (and stop when we see a non-digit like
'dirty' or 'rcX'), and make them separated with comma.
Oh, this is *much* better than my new (unsent) attempt to fix this! ;-)
Note that I am merely guessing that short-digit version numbers
are acceptable by now after seeing
https://sourceware.org/ml/binutils/2012-07/msg00199.html
Ah, nice find!
I will test your patch (below) and let you know soon, but it looks
good to me. (I can't test it tonight, unfortunately.)
ATB,
Ramsay Jones
without knowing the current state of affairs. If that is not the
case you may have to count the iteration of the loop and append or
chop the resulting string as necessary.
Makefile | 2 +-
gen-version-string.sh | 13 +
git.rc| 4 ++--
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index b4af1e2..329f942 100644
--- a/Makefile
+++ b/Makefile
@@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
git.res: git.rc GIT-VERSION-FILE
$(QUIET_RC)$(RC) \
- $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst
., ,$(GIT_VERSION) \
+ -DVERSIONSTRING=$$(./gen-version-string.sh $(GIT_VERSION)) \
-DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@
ifndef NO_PERL
diff --git a/gen-version-string.sh b/gen-version-string.sh
new file mode 100755
index 000..00af718
--- /dev/null
+++ b/gen-version-string.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+IFS=.- result=
+for v in $1
+do
+ if expr $v : '[0-9][0-9]*$' /dev/null
+ then
+ result=$result${result:+,}$v
+ else
+ break
+ fi
+done
+echo $result
diff --git a/git.rc b/git.rc
index bce6db9..6f2a8d2 100644
--- a/git.rc
+++ b/git.rc
@@ -1,6 +1,6 @@
1 VERSIONINFO
-FILEVERSION MAJOR,MINOR,PATCH,0
-PRODUCTVERSION MAJOR,MINOR,PATCH,0
+FILEVERSION VERSIONSTRING,0
+PRODUCTVERSION VERSIONSTRING,0
BEGIN
BLOCK StringFileInfo
BEGIN
.
--
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 0/2] Two janitorial patches for builtin/blame.c
Junio C Hamano gits...@pobox.com writes: Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: So my understanding is that when we are talking about _significant_ additions to builtin/blame.c (the current patches don't qualify as such really) that a) builtin/blame.c is licensed under GPLv2 b) significant contributions to it will not be relicensed under different licenses without the respective contributors' explicit consent. Yep, that's how it works. [...] The combination of the SubmittingPatches text with the file notices in builtin/blame.c is not really painting a full picture of the situation. Any idea how this could be made more clear? E.g., maybe we should bite the bullet and add a line to all source files that don't already state a license: /* * License: GPLv2. See COPYING for details. */ I vaguely recall that jgit folks at one point wanted to lift this implementation and were interested in seeing it to be dual licensed to BSD but that was a long time ago. http://git.661346.n2.nabble.com/JGIT-Blame-functionality-for-jgit-td2142726.html Ok, let me state quite clearly before we waste time, energy and goodwill: a) I am reworking the core logic of blame.c to make it produce the same results while being orders of magnitude faster. Git's current implementation is a roadblock for serious use. Keeping its current core algorithms and data flow, it would have been reasonably easy to speed the current code up by a factor of 2 or more by doing local optimizations. But I've chosen _not_ to keep the current logic and data flow. That means quite a bit more work, and it means completely understanding the existing code before being able to replace it. The core part of blame.c spends literally billions of iterations in real-life situations leafing through one large linear list for tiny bits of information. One could use a better searchable data structure and speed up the access in that manner, but better than a fast search is no search at all. I am separating the data so that at any given time I am only accessing actually relevant data. O(n) beats O(n lg n), and the code remains almost as readable as the current O(n^2). b) This will require thoroughly reworking the core parts of the algorithm which will then be about 50/50 old and new code that cannot sensibly be separated since significant parts of the previous code will be gone completely as the data flow is fundamentally different. c) The fine points of blame.c, in particular all the various command line options and the implementation of their exact meaning would stay the same. I hope I can avoid touching more than 50% of the code. d) I am fine with distributing my work under the GPLv2 or later, but no other license will be implied. While this does not affect the core Git distribution itself: for distribution under more permissive licenses for the purpose of making inclusion in proprietary software possible, I'd probably attach a big price tag that reflects the amount of work and quality of code going in and the fact that I have no other source of income. e) No idea whether this would affect JGIT: it depends on how much JGIT would be a literal translation of blame.c into Java (?) or a functionally equivalent rewrite employing different and/or native data structures to achieve the same effect. To me it's irritating that something like the fine but boring points of option parsing might be more susceptible to copyright protection than doing a careful algorithmic design, but that's the way the world is wired. At any rate: JGIT or not, I'll be contributing work with the understanding that it will be licensed under the _current_ licensing scheme of Git. And I think that's a reasonable expectation. -- David Kastrup -- 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 repack --max-pack-size broken in git-next
Siddharth Agarwal s...@fb.com writes: With git-next, the --max-pack-size option to git repack doesn't work. With git at b139ac2, `git repack --max-pack-size=1g` says error: option `max-pack-size' expects a numerical value Thanks, Siddharth. It seems that we have a hand-crafted parser outside parse-options framework in pack-objects, and the scripted git-repack used to pass this option without interpreting itself. We probably should lift the OPT_ULONG() implementation out of builtin/pack-objects.c and move it to parse-options.[ch] and use it in the reimplementation of repack. And probably use it in other places where these integers in human-friendly units may make sense, but that is a separate topic. -- 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] builtin/blame.c: struct blame_entry does not need a prev link
Junio C Hamano gits...@pobox.com writes: David Kastrup d...@gnu.org writes: --- Thanks. At some point during its development I must have thought that having it as a dual-linked list may make it easier when we have to split a block into pieces, but it seems that split_overlap() does not need to look at this information. Needs sign-off. Well, as I said: it's quite possible that the double-linking might be useful for some particular hypothetical rewrite of the code. It isn't for the current code, and it's not useful for my own rewrite. Will be posting a signed-off version presently. -- David Kastrup -- 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 01/11] git p4 test: wildcards are supported
Since 9d57c4a (git p4: implement view spec wildcards with p4 where, 2013-08-30), all the wildcard types should be supported. Change must-fail tests to mark that they now pass. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9809-git-p4-client-view.sh | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh index 77f6349..23a827f 100755 --- a/t/t9809-git-p4-client-view.sh +++ b/t/t9809-git-p4-client-view.sh @@ -76,28 +76,28 @@ test_expect_success 'init depot' ' ' # double % for printf -test_expect_success 'unsupported view wildcard %%n' ' +test_expect_success 'view wildcard %%n' ' client_view //depot/1/sub/... //client/sub/1/... test_when_finished cleanup_git - test_must_fail git p4 clone --use-client-spec --dest=$git //depot + git p4 clone --use-client-spec --dest=$git //depot ' -test_expect_success 'unsupported view wildcard *' ' +test_expect_success 'view wildcard *' ' client_view //depot/*/bar/... //client/*/bar/... test_when_finished cleanup_git - test_must_fail git p4 clone --use-client-spec --dest=$git //depot + git p4 clone --use-client-spec --dest=$git //depot ' -test_expect_success 'wildcard ... only supported at end of spec 1' ' +test_expect_success 'wildcard ... in the middle' ' client_view //depot/.../file11 //client/.../file11 test_when_finished cleanup_git - test_must_fail git p4 clone --use-client-spec --dest=$git //depot + git p4 clone --use-client-spec --dest=$git //depot ' -test_expect_success 'wildcard ... only supported at end of spec 2' ' +test_expect_success 'wildcard ... in the middle and at the end' ' client_view //depot/.../a/... //client/.../a/... test_when_finished cleanup_git - test_must_fail git p4 clone --use-client-spec --dest=$git //depot + git p4 clone --use-client-spec --dest=$git //depot ' test_expect_success 'basic map' ' -- 1.8.5.2.320.g99957e5 -- 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 00/11] git p4 tests and a few bug fixes
Most of this is work on tests for git p4. Patch 03 is a regression fix, found and narrowed down thanks to much work by Damien Gérard. But it is obscure enough that I'm not proposing it for a maintenance release. There are a couple other behavior fixes, but again, these are quite minor and can wait for the next release. Pete Wyckoff (11): git p4 test: wildcards are supported git p4 test: ensure p4 symlink parsing works git p4: work around p4 bug that causes empty symlinks git p4 test: explicitly check p4 wildcard delete git p4 test: is_cli_file_writeable succeeds git p4 test: run as user author git p4 test: do not pollute /tmp git p4: handle files with wildcards when doing RCS scrubbing git p4: fix an error message when p4 where fails git p4 test: examine behavior with locked (+l) files git p4 doc: use two-line style for options with multiple spellings Documentation/git-p4.txt | 6 +- git-p4.py | 17 +++-- t/lib-git-p4.sh| 23 +- t/t9802-git-p4-filetype.sh | 83 + t/t9805-git-p4-skip-submit-edit.sh | 6 +- t/t9807-git-p4-submit.sh | 2 +- t/t9809-git-p4-client-view.sh | 16 ++-- t/t9812-git-p4-wildcards.sh| 50 + t/t9813-git-p4-preserve-users.sh | 38 -- t/t9816-git-p4-locked.sh | 145 + 10 files changed, 342 insertions(+), 44 deletions(-) create mode 100755 t/t9816-git-p4-locked.sh -- 1.8.5.2.320.g99957e5 -- 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 02/11] git p4 test: ensure p4 symlink parsing works
While this happens to work, there was no test to make sure that the basic importing of a symlink from p4 to git functioned. Add a simple test to create a symlink in p4 and import it into git, then verify that the symlink exists and has the correct target. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9802-git-p4-filetype.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh index a82744b..94d7be9 100755 --- a/t/t9802-git-p4-filetype.sh +++ b/t/t9802-git-p4-filetype.sh @@ -250,6 +250,23 @@ test_expect_success 'ignore apple' ' ) ' +test_expect_success SYMLINKS 'create p4 symlink' ' + cd $cli + ln -s symlink-target symlink + p4 add symlink + p4 submit -d add symlink +' + +test_expect_success SYMLINKS 'ensure p4 symlink parsed correctly' ' + test_when_finished cleanup_git + git p4 clone --dest=$git //depot@all + ( + cd $git + test -L symlink + test $(readlink symlink) = symlink-target + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.8.5.2.320.g99957e5 -- 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 04/11] git p4 test: explicitly check p4 wildcard delete
There was no test where p4 deleted a file with a wildcard character. Make sure git p4 applies the wildcard decoding properly when importing a delete that includes a wildcard. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9812-git-p4-wildcards.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh index 6763325..f2ddbc5 100755 --- a/t/t9812-git-p4-wildcards.sh +++ b/t/t9812-git-p4-wildcards.sh @@ -161,6 +161,33 @@ test_expect_success 'wildcard files submit back to p4, delete' ' ) ' +test_expect_success 'p4 deleted a wildcard file' ' + ( + cd $cli + echo wild delete test wild@delete + p4 add -f wild@delete + p4 submit -d add wild@delete + ) + test_when_finished cleanup_git + git p4 clone --dest=$git //depot + ( + cd $git + test_path_is_file wild@delete + ) + ( + cd $cli + # must use its encoded name + p4 delete wild%40delete + p4 submit -d delete wild@delete + ) + ( + cd $git + git p4 sync + git merge --ff-only p4/master + test_path_is_missing wild@delete + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.8.5.2.320.g99957e5 -- 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 03/11] git p4: work around p4 bug that causes empty symlinks
Damien Gérard highlights an interesting problem. Some p4
repositories end up with symlinks that have an empty target. It
is not possible to create this with current p4, but they do
indeed exist.
The effect in git p4 is that p4 print on the symlink returns an
empty string, confusing the curret symlink-handling code.
Such broken repositories cause problems in p4 as well, even with
no git involved. In p4, syncing to a change that includes a
bogus symlink causes errors:
//depot/empty-symlink - updating /home/me/p4/empty-symlink
rename: /home/me/p4/empty-symlink: No such file or directory
and leaves no symlink.
In git, replicate the p4 behavior by ignoring these bad symlinks.
If, in a later p4 revision, the symlink happens to point to
something non-null, the symlink will be replaced properly.
Add a big test for all this too.
This happens to be a regression introduced by 1292df1 (git-p4:
Fix occasional truncation of symlink contents., 2013-08-08) and
appeared first in 1.8.5. But it only shows up only in p4
repositories of dubious character, so can wait for a proper
release.
Tested-by: Damien Gérard dam...@iwi.me
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 9 ++-
t/t9802-git-p4-filetype.sh | 66 ++
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 5ea8bb8..e798ecf 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2075,7 +2075,14 @@ class P4Sync(Command, P4UserMap):
# p4 print on a symlink sometimes contains target\n;
# if it does, remove the newline
data = ''.join(contents)
-if data[-1] == '\n':
+if not data:
+# Some version of p4 allowed creating a symlink that pointed
+# to nothing. This causes p4 errors when checking out such
+# a change, and errors here too. Work around it by ignoring
+# the bad symlink; hopefully a future change fixes it.
+print \nIgnoring empty symlink in %s % file['depotFile']
+return
+elif data[-1] == '\n':
contents = [data[:-1]]
else:
contents = [data]
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 94d7be9..66d3fc9 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -267,6 +267,72 @@ test_expect_success SYMLINKS 'ensure p4 symlink parsed
correctly' '
)
'
+test_expect_success SYMLINKS 'empty symlink target' '
+ (
+ # first create the file as a file
+ cd $cli
+ empty-symlink
+ p4 add empty-symlink
+ p4 submit -d add empty-symlink as a file
+ )
+ (
+ # now change it to be a symlink to target1
+ cd $cli
+ p4 edit empty-symlink
+ p4 reopen -t symlink empty-symlink
+ rm empty-symlink
+ ln -s target1 empty-symlink
+ p4 add empty-symlink
+ p4 submit -d make empty-symlink point to target1
+ )
+ (
+ # Hack the p4 depot to make the symlink point to nothing;
+ # this should not happen in reality, but shows up
+ # in p4 repos in the wild.
+ #
+ # The sed expression changes this:
+ # @@
+ # text
+ # @target1
+ # @
+ # to this:
+ # @@
+ # text
+ # @@
+ #
+ cd $db/depot
+ sed /@target1/{; s/target1/@/; n; d; } \
+ empty-symlink,v empty-symlink,v.tmp
+ mv empty-symlink,v.tmp empty-symlink,v
+ )
+ (
+ # Make sure symlink really is empty. Asking
+ # p4 to sync here will make it generate errors.
+ cd $cli
+ p4 print -q //depot/empty-symlink#2 out
+ test ! -s out
+ )
+ test_when_finished cleanup_git
+
+ # make sure git p4 handles it without error
+ git p4 clone --dest=$git //depot@all
+
+ # fix the symlink, make it point to target2
+ (
+ cd $cli
+ p4 open empty-symlink
+ rm empty-symlink
+ ln -s target2 empty-symlink
+ p4 submit -d make empty-symlink point to target2
+ )
+ cleanup_git
+ git p4 clone --dest=$git //depot@all
+ (
+ cd $git
+ test $(readlink empty-symlink) = target2
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
1.8.5.2.320.g99957e5
--
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
[PATCH 06/11] git p4 test: run as user author
The tests use aut...@example.com as the canonical submitter,
but he does not have an entry in the p4 users database.
This causes the generated change description to complain
that the git and p4 users disagree. The complaint message
is still valid, just isn't useful in tests. It was was
introduced in 848de9c (git-p4: warn if git authorship won't
be retained, 2011-05-13).
Fix t9813 to use @example.com instead of @localhost due to
change in p4_add_user(). Move the function into the git p4
test library so author can be added at initialization time.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 15 ++-
t/t9813-git-p4-preserve-users.sh | 38 ++
2 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index ccd918e..4ff2bb1 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -47,9 +47,10 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
P4PORT=localhost:$P4DPORT
P4CLIENT=client
+P4USER=author
P4EDITOR=:
unset P4CHARSET
-export P4PORT P4CLIENT P4EDITOR P4CHARSET
+export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
db=$TRASH_DIRECTORY/db
cli=$TRASH_DIRECTORY/cli
@@ -96,12 +97,24 @@ start_p4d() {
return 1
fi
+ # build a p4 user so aut...@example.com has an entry
+ p4_add_user author
+
# build a client
client_view //depot/... //client/...
return 0
}
+p4_add_user() {
+ name=$1
+ p4 user -f -i -EOF
+ User: $name
+ Email: $n...@example.com
+ FullName: Dr. $name
+ EOF
+}
+
kill_p4d() {
pid=$(cat $pidfile)
# it had better exist for the first kill
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index f2e85e5..166b840 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -19,16 +19,6 @@ test_expect_success 'create files' '
)
'
-p4_add_user() {
- name=$1 fullname=$2
- p4 user -f -i -EOF
- User: $name
- Email: $name@localhost
- FullName: $fullname
- EOF
- p4 passwd -P secret $name
-}
-
p4_grant_admin() {
name=$1
{
@@ -51,8 +41,8 @@ make_change_by_user() {
# Test username support, submitting as user 'alice'
test_expect_success 'preserve users' '
- p4_add_user alice Alice
- p4_add_user bob Bob
+ p4_add_user alice
+ p4_add_user bob
p4_grant_admin alice
git p4 clone --dest=$git //depot
test_when_finished cleanup_git
@@ -60,8 +50,8 @@ test_expect_success 'preserve users' '
cd $git
echo username: a change by alice file1
echo username: a change by bob file2
- git commit --author Alice alice@localhost -m a change by
alice file1
- git commit --author Bob bob@localhost -m a change by bob
file2
+ git commit --author Alice al...@example.com -m a change by
alice file1
+ git commit --author Bob b...@example.com -m a change by
bob file2
git config git-p4.skipSubmitEditCheck true
P4EDITOR=touch P4USER=alice P4PASSWD=secret git p4 commit
--preserve-user
p4_check_commit_author file1 alice
@@ -78,7 +68,7 @@ test_expect_success 'refuse to preserve users without perms' '
cd $git
git config git-p4.skipSubmitEditCheck true
echo username-noperms: a change by alice file1
- git commit --author Alice alice@localhost -m perms: a
change by alice file1
+ git commit --author Alice al...@example.com -m perms: a
change by alice file1
P4EDITOR=touch P4USER=bob P4PASSWD=secret
export P4EDITOR P4USER P4PASSWD
test_must_fail git p4 commit --preserve-user
@@ -94,9 +84,9 @@ test_expect_success 'preserve user where author is unknown to
p4' '
cd $git
git config git-p4.skipSubmitEditCheck true
echo username-bob: a change by bob file1
- git commit --author Bob bob@localhost -m preserve: a
change by bob file1
+ git commit --author Bob b...@example.com -m preserve: a
change by bob file1
echo username-unknown: a change by charlie file1
- git commit --author Charlie charlie@localhost -m preserve:
a change by charlie file1
+ git commit --author Charlie char...@example.com -m
preserve: a change by charlie file1
P4EDITOR=touch P4USER=alice P4PASSWD=secret
export P4EDITOR P4USER P4PASSWD
test_must_fail git p4 commit --preserve-user
@@ -121,24 +111,24 @@ test_expect_success 'not preserving user with mixed
authorship' '
(
cd $git
git config git-p4.skipSubmitEditCheck true
- p4_add_user
[PATCH 05/11] git p4 test: is_cli_file_writeable succeeds
Commit e9df0f9 (git p4: cygwin p4 client does not mark read-only, 2013-01-26) fixed a problem with test -w on cygwin, but mistakenly marked the new test as failing. Fix this. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9807-git-p4-submit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh index 1fb7bc7..4caf36e 100755 --- a/t/t9807-git-p4-submit.sh +++ b/t/t9807-git-p4-submit.sh @@ -17,7 +17,7 @@ test_expect_success 'init depot' ' ) ' -test_expect_failure 'is_cli_file_writeable function' ' +test_expect_success 'is_cli_file_writeable function' ' ( cd $cli echo a a -- 1.8.5.2.320.g99957e5 -- 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 07/11] git p4 test: do not pollute /tmp
Generating the submit template for p4 uses tempfile.mkstemp(),
which by default puts files in /tmp. For a test that fails,
possibly on purpose, this is not cleaned up. Run with TMPDIR
pointing into the trash directory so the temp files go away
with the test results.
To do this required some other minor changes. First, the editor
is launched using system(editor + + template_file), using
shell expansion to build the command string. This doesn't work
if editor has a space in it. And is generally unwise as it's
easy to fool the shell into doing extra work. Exec the args
directly, without shell expansion.
Second, without shell expansion, the trick of P4EDITOR=: used
in the tests doesn't work. Use a real command, true, as the
non-interactive editor for testing.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 2 +-
t/lib-git-p4.sh| 8 +++-
t/t9805-git-p4-skip-submit-edit.sh | 6 --
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index e798ecf..a4414b5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1220,7 +1220,7 @@ class P4Submit(Command, P4UserMap):
editor = os.environ.get(P4EDITOR)
else:
editor = read_pipe(git var GIT_EDITOR).strip()
-system(editor + + template_file)
+system([editor, template_file])
# If the file was not saved, prompt to see if this patch should
# be skipped. But skip this verification step if configured so.
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 4ff2bb1..5aa8adc 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -48,7 +48,7 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
P4PORT=localhost:$P4DPORT
P4CLIENT=client
P4USER=author
-P4EDITOR=:
+P4EDITOR=true
unset P4CHARSET
export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
@@ -57,6 +57,12 @@ cli=$TRASH_DIRECTORY/cli
git=$TRASH_DIRECTORY/git
pidfile=$TRASH_DIRECTORY/p4d.pid
+# git p4 submit generates a temp file, which will
+# not get cleaned up if the submission fails. Don't
+# clutter up /tmp on the test machine.
+TMPDIR=$TRASH_DIRECTORY
+export TMPDIR
+
start_p4d() {
mkdir -p $db $cli $git
rm -f $pidfile
diff --git a/t/t9805-git-p4-skip-submit-edit.sh
b/t/t9805-git-p4-skip-submit-edit.sh
index ff2cc79..8931188 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -17,7 +17,7 @@ test_expect_success 'init depot' '
)
'
-# this works because EDITOR is set to :
+# this works because P4EDITOR is set to true
test_expect_success 'no config, unedited, say yes' '
git p4 clone --dest=$git //depot
test_when_finished cleanup_git
@@ -90,7 +90,9 @@ test_expect_success 'no config, edited' '
cd $git
echo line file1
git commit -a -m change 5
- P4EDITOR= EDITOR=\$TRASH_DIRECTORY/ed.sh\ git p4 submit
+ P4EDITOR=$TRASH_DIRECTORY/ed.sh
+ export P4EDITOR
+ git p4 submit
p4 changes //depot/... wc
test_line_count = 5 wc
)
--
1.8.5.2.320.g99957e5
--
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 08/11] git p4: handle files with wildcards when doing RCS scrubbing
Commit 9d7d446 (git p4: submit files with wildcards, 2012-04-29) fixed problems with handling files that had p4 wildcard characters, like @ and *. But it missed one case, that of RCS keyword scrubbing, which uses p4 fstat to extract type information. Fix it by calling wildcard_encode() on the raw filename. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 4 ++-- t/t9812-git-p4-wildcards.sh | 23 +++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/git-p4.py b/git-p4.py index a4414b5..26b874f 100755 --- a/git-p4.py +++ b/git-p4.py @@ -310,8 +310,8 @@ def split_p4_type(p4type): # # return the raw p4 type of a file (text, text+ko, etc) # -def p4_type(file): -results = p4CmdList([fstat, -T, headType, file]) +def p4_type(f): +results = p4CmdList([fstat, -T, headType, wildcard_encode(f)]) return results[0]['headType'] # diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh index f2ddbc5..c7472cb 100755 --- a/t/t9812-git-p4-wildcards.sh +++ b/t/t9812-git-p4-wildcards.sh @@ -188,6 +188,29 @@ test_expect_success 'p4 deleted a wildcard file' ' ) ' +test_expect_success 'wildcard files requiring keyword scrub' ' + ( + cd $cli + cat -\EOF scrub@wild + $Id$ + line2 + EOF + p4 add -t text+k -f scrub@wild + p4 submit -d scrub at wild + ) + test_when_finished cleanup_git + git p4 clone --dest=$git //depot + ( + cd $git + git config git-p4.skipSubmitEdit true + git config git-p4.attemptRCSCleanup true + sed s/^line2/line2 edit/ scrub@wild sc...@wild.tmp + mv -f sc...@wild.tmp scrub@wild + git commit -m scrub at wild line2 edit scrub@wild + git p4 submit + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.8.5.2.320.g99957e5 -- 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 09/11] git p4: fix an error message when p4 where fails
When p4 where fails, for whatever reason, the error message tries to show an undefined variable. This minor bug applies only when using a client spec, and was introduced recently in 9d57c4a (git p4: implement view spec wildcards with p4 where, 2013-08-30). Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 26b874f..cdfa2df 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1871,7 +1871,7 @@ class View(object): # assume error is ... file(s) not in client view continue if clientFile not in res: -die(No clientFile from 'p4 where %s' % depot_path) +die(No clientFile in 'p4 where' output) if unmap in res: # it will list all of them, but only one not unmap-ped continue -- 1.8.5.2.320.g99957e5 -- 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 10/11] git p4 test: examine behavior with locked (+l) files
The p4 server can enforce file locking, so that only one user
can edit a file at a time. Git p4 is unable to submit changes
to locked files. Currently it exits poorly. Ideally it would
notice the locked condition and clean up nicely.
Add a bunch of tests that describe the problem, hoping that
fixes appear in the future.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/t9816-git-p4-locked.sh | 145 +++
1 file changed, 145 insertions(+)
create mode 100755 t/t9816-git-p4-locked.sh
diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
new file mode 100755
index 000..e71e543
--- /dev/null
+++ b/t/t9816-git-p4-locked.sh
@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='git p4 locked file behavior'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+# See
+#
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563
+# for suggestions on how to configure sitewide pessimistic locking
+# where only one person can have a file open for edit at a time.
+test_expect_success 'init depot' '
+ (
+ cd $cli
+ echo TypeMap: +l //depot/... | p4 typemap -i
+ echo file1 file1
+ p4 add file1
+ p4 submit -d add file1
+ )
+'
+
+test_expect_success 'edit with lock not taken' '
+ test_when_finished cleanup_git
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ echo line2 file1
+ git add file1
+ git commit -m line2 in file1
+ git config git-p4.skipSubmitEdit true
+ git p4 submit
+ )
+'
+
+test_expect_failure 'add with lock not taken' '
+ test_when_finished cleanup_git
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ echo line1 add-lock-not-taken
+ git add file2
+ git commit -m add add-lock-not-taken
+ git config git-p4.skipSubmitEdit true
+ git p4 submit --verbose
+ )
+'
+
+lock_in_another_client() {
+ # build a different client
+ cli2=$TRASH_DIRECTORY/cli2
+ mkdir -p $cli2
+ test_when_finished p4 client -f -d client2 rm -rf \$cli2\
+ (
+ cd $cli2
+ P4CLIENT=client2
+ cli=$cli2
+ client_view //depot/... //client2/...
+ p4 sync
+ p4 open file1
+ )
+}
+
+test_expect_failure 'edit with lock taken' '
+ lock_in_another_client
+ test_when_finished cleanup_git
+ test_when_finished cd \$cli\ p4 sync -f file1
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ echo line3 file1
+ git add file1
+ git commit -m line3 in file1
+ git config git-p4.skipSubmitEdit true
+ git p4 submit --verbose
+ )
+'
+
+test_expect_failure 'delete with lock taken' '
+ lock_in_another_client
+ test_when_finished cleanup_git
+ test_when_finished cd \$cli\ p4 sync -f file1
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ git rm file1
+ git commit -m delete file1
+ git config git-p4.skipSubmitEdit true
+ git p4 submit --verbose
+ )
+'
+
+test_expect_failure 'chmod with lock taken' '
+ lock_in_another_client
+ test_when_finished cleanup_git
+ test_when_finished cd \$cli\ p4 sync -f file1
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ chmod +x file1
+ git add file1
+ git commit -m chmod +x file1
+ git config git-p4.skipSubmitEdit true
+ git p4 submit --verbose
+ )
+'
+
+test_expect_failure 'copy with lock taken' '
+ lock_in_another_client
+ test_when_finished cleanup_git
+ test_when_finished cd \$cli\ p4 revert file2 rm -f file2
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ cp file1 file2
+ git add file2
+ git commit -m cp file1 to file2
+ git config git-p4.skipSubmitEdit true
+ git config git-p4.detectCopies true
+ git p4 submit --verbose
+ )
+'
+
+test_expect_failure 'move with lock taken' '
+ lock_in_another_client
+ test_when_finished cleanup_git
+ test_when_finished cd \$cli\ p4 sync file1 rm -f file2
+ git p4 clone --dest=$git //depot
+ (
+ cd $git
+ git mv file1 file2
+ git commit -m mv file1 to file2
+ git config git-p4.skipSubmitEdit true
+ git config git-p4.detectRenames true
+ git p4 submit --verbose
+ )
+'
+
+test_expect_success 'kill p4d' '
+
[PATCH 11/11] git p4 doc: use two-line style for options with multiple spellings
Thomas Rast noticed the docs have a mix of styles when it comes to options with multiple spellings. Standardize the couple in git-p4.txt that are odd. Instead of: -n, --dry-run:: Do this: -n:: --dry-run:: See http://thread.gmane.org/gmane.comp.version-control.git/219936/focus=219945 Signed-off-by: Pete Wyckoff p...@padd.com --- Documentation/git-p4.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 8cba16d..6ab5f94 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -168,7 +168,8 @@ All commands except clone accept these options. --git-dir dir:: Set the 'GIT_DIR' environment variable. See linkgit:git[1]. ---verbose, -v:: +-v:: +--verbose:: Provide more progress information. Sync options @@ -279,7 +280,8 @@ These options can be used to modify 'git p4 submit' behavior. Export tags from Git as p4 labels. Tags found in Git are applied to the perforce working directory. ---dry-run, -n:: +-n:: +--dry-run:: Show just what commits would be submitted to p4; do not change state in Git or p4. -- 1.8.5.2.320.g99957e5 -- 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 repack --max-pack-size broken in git-next
Junio C Hamano gits...@pobox.com writes:
Siddharth Agarwal s...@fb.com writes:
With git-next, the --max-pack-size option to git repack doesn't work.
With git at b139ac2, `git repack --max-pack-size=1g` says
error: option `max-pack-size' expects a numerical value
Thanks, Siddharth.
It seems that we have a hand-crafted parser outside parse-options
framework in pack-objects, and the scripted git-repack used to pass
this option without interpreting itself.
We probably should lift the OPT_ULONG() implementation out of
builtin/pack-objects.c and move it to parse-options.[ch] and use it
in the reimplementation of repack.
And probably use it in other places where these integers in
human-friendly units may make sense, but that is a separate topic.
The first step may look something like this (totally untested)..
builtin/pack-objects.c | 22 +++---
parse-options.c| 17 +
parse-options.h| 3 +++
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 541667f..b264f1f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2416,23 +2416,6 @@ static int option_parse_unpack_unreachable(const struct
option *opt,
return 0;
}
-static int option_parse_ulong(const struct option *opt,
- const char *arg, int unset)
-{
- if (unset)
- die(_(option %s does not accept negative form),
- opt-long_name);
-
- if (!git_parse_ulong(arg, opt-value))
- die(_(unable to parse value '%s' for option %s),
- arg, opt-long_name);
- return 0;
-}
-
-#define OPT_ULONG(s, l, v, h) \
- { OPTION_CALLBACK, (s), (l), (v), n, (h), \
- PARSE_OPT_NONEG, option_parse_ulong }
-
int cmd_pack_objects(int argc, const char **argv, const char *prefix)
{
int use_internal_rev_list = 0;
@@ -2462,8 +2445,9 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_(ignore packed objects)),
OPT_INTEGER(0, window, window,
N_(limit pack window by objects)),
- OPT_ULONG(0, window-memory, window_memory_limit,
- N_(limit pack window by memory in addition to object
limit)),
+ { OPTION_ULONG, 0, window-memory, window_memory_limit,
N_(n),
+ N_(limit pack window by memory in addition to object limit),
+ PARSE_OPT_HUMAN_NUMBER },
OPT_INTEGER(0, depth, depth,
N_(maximum length of delta chain allowed in the
resulting pack)),
OPT_BOOL(0, reuse-delta, reuse_delta,
diff --git a/parse-options.c b/parse-options.c
index 7b8d3fa..3a47538 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -180,6 +180,23 @@ static int get_value(struct parse_opt_ctx_t *p,
return opterror(opt, expects a numerical value,
flags);
return 0;
+ case OPTION_ULONG:
+ if (unset) {
+ *(unsigned long *)opt-value = 0;
+ } else if (opt-flags PARSE_OPT_OPTARG !p-opt) {
+ *(unsigned long *)opt-value = opt-defval;
+ } else if (get_arg(p, opt, flags, arg)) {
+ return -1;
+ } else if (opt-flags PARSE_OPT_HUMAN_NUMBER) {
+ if (!git_parse_ulong(arg, (unsigned long *)opt-value))
+ return opterror(opt, expects a numerical
value, flags);
+ } else {
+ *(unsigned long *)opt-value = strtoul(arg, (char
**)s, 10);
+ if (*s)
+ return opterror(opt, expects a numerical
value, flags);
+ }
+ return 0;
+
default:
die(should not happen, someone must be hit on the forehead);
}
diff --git a/parse-options.h b/parse-options.h
index d670cb9..6a3cce0 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -17,6 +17,7 @@ enum parse_opt_type {
/* options with arguments (usually) */
OPTION_STRING,
OPTION_INTEGER,
+ OPTION_ULONG,
OPTION_CALLBACK,
OPTION_LOWLEVEL_CALLBACK,
OPTION_FILENAME
@@ -38,6 +39,7 @@ enum parse_opt_option_flags {
PARSE_OPT_LASTARG_DEFAULT = 16,
PARSE_OPT_NODASH = 32,
PARSE_OPT_LITERAL_ARGHELP = 64,
+ PARSE_OPT_HUMAN_NUMBER = 128,
PARSE_OPT_SHELL_EVAL = 256
};
@@ -133,6 +135,7 @@ struct option {
#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(n),
(h) }
+#define OPT_ULONG(s, l, v, h) { OPTION_ULONG, (s), (l), (v), N_(n),
(h) }
#define OPT_STRING(s,
Re: [PATCH 0/6] Make 'git help everyday' work - relnotes
Philip Oakley philipoak...@iee.org writes: Determining which is the current release note is possibly more problematic, which should be when making the documentation. Hmmm Why? You are already aware of the stale-notes section, no? Isn't the top one the latest? -- 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/RFC] Makefile: Fix compilation of windows resource file
Ramsay Jones ram...@ramsay1.demon.co.uk writes:
Note that I am merely guessing that short-digit version numbers
are acceptable by now after seeing
https://sourceware.org/ml/binutils/2012-07/msg00199.html
Ah, nice find!
I will test your patch (below) and let you know soon, but it looks
good to me. (I can't test it tonight, unfortunately.)
One thing to note is that I don't know why the existing code dropped
the fourth digit from the maintenance series. The updated one will
give you 1,8,5,3,0 (because I just have a hardcoded ,0 at the
end for no good reason there), and if the missing fourth digit in
the original was a deliberate workaround for this file having an
upper limit of the number of digits (like four), this change will
break it, so if that is the case, you may have to count and stop the
loop early, perhaps like...
diff --git a/gen-version-string.sh b/gen-version-string.sh
new file mode 100755
index 000..00af718
--- /dev/null
+++ b/gen-version-string.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+IFS=.- result=
Add
num_digits=0
here, and...
+for v in $1
+do
+if expr $v : '[0-9][0-9]*$' /dev/null
+then
+result=$result${result:+,}$v
... insert these here.
num_digits=$(( $num_digits + 1 ))
if test $num_digits = 4
then
break
fi
+else
+break
+fi
+done
+echo $result
diff --git a/git.rc b/git.rc
index bce6db9..6f2a8d2 100644
--- a/git.rc
+++ b/git.rc
@@ -1,6 +1,6 @@
1 VERSIONINFO
-FILEVERSION MAJOR,MINOR,PATCH,0
-PRODUCTVERSION MAJOR,MINOR,PATCH,0
+FILEVERSION VERSIONSTRING,0
+PRODUCTVERSION VERSIONSTRING,0
BEGIN
BLOCK StringFileInfo
BEGIN
.
--
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 00/11] git p4 tests and a few bug fixes
Pete Wyckoff p...@padd.com writes: Most of this is work on tests for git p4. Patch 03 is a regression fix, found and narrowed down thanks to much work by Damien Gérard. But it is obscure enough that I'm not proposing it for a maintenance release. There are a couple other behavior fixes, but again, these are quite minor and can wait for the next release. Thanks. I am inclined to say that we should queue this on a fork from 'maint, merge the result to 'master' before 1.9-rc1 and ship the result as part of the upcoming release, and then possibly merging the topic to 1.8.5.x maintenance release after that. This is primarily because I personally do not have p4 expertise to test or properly judge this (iow, you are the area maintainer, the authority), and I somehow have this feeling that parking in 'next' for extended period of time would not give meaningfully larger exposure to the code. What do you think? If you feel uneasy about such a fast-track, I wouldn't push it, though. Pete Wyckoff (11): git p4 test: wildcards are supported git p4 test: ensure p4 symlink parsing works git p4: work around p4 bug that causes empty symlinks git p4 test: explicitly check p4 wildcard delete git p4 test: is_cli_file_writeable succeeds git p4 test: run as user author git p4 test: do not pollute /tmp git p4: handle files with wildcards when doing RCS scrubbing git p4: fix an error message when p4 where fails git p4 test: examine behavior with locked (+l) files git p4 doc: use two-line style for options with multiple spellings Documentation/git-p4.txt | 6 +- git-p4.py | 17 +++-- t/lib-git-p4.sh| 23 +- t/t9802-git-p4-filetype.sh | 83 + t/t9805-git-p4-skip-submit-edit.sh | 6 +- t/t9807-git-p4-submit.sh | 2 +- t/t9809-git-p4-client-view.sh | 16 ++-- t/t9812-git-p4-wildcards.sh| 50 + t/t9813-git-p4-preserve-users.sh | 38 -- t/t9816-git-p4-locked.sh | 145 + 10 files changed, 342 insertions(+), 44 deletions(-) create mode 100755 t/t9816-git-p4-locked.sh -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
Signed-off-by: David Kastrup d...@gnu.org
---
builtin/blame.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2195595 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
* scoreboard structure, sorted by the target line number.
*/
struct blame_entry {
- struct blame_entry *prev;
struct blame_entry *next;
/* the first line of this group in the final image;
@@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
ent-s_lno + ent-num_lines == next-s_lno) {
ent-num_lines += next-num_lines;
ent-next = next-next;
- if (ent-next)
- ent-next-prev = ent;
origin_decref(next-suspect);
free(next);
ent-score = 0;
@@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
prev = ent;
/* prev, if not NULL, is the last one that is below e */
- e-prev = prev;
+
if (prev) {
e-next = prev-next;
prev-next = e;
@@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
e-next = sb-ent;
sb-ent = e;
}
- if (e-next)
- e-next-prev = e;
}
/*
@@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
*/
static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
{
- struct blame_entry *p, *n;
+ struct blame_entry *n;
- p = dst-prev;
n = dst-next;
origin_incref(src-suspect);
origin_decref(dst-suspect);
memcpy(dst, src, sizeof(*src));
- dst-prev = p;
dst-next = n;
dst-score = 0;
}
@@ -2502,8 +2495,6 @@ parse_done:
ent-suspect = o;
ent-s_lno = bottom;
ent-next = next;
- if (next)
- next-prev = ent;
origin_incref(o);
}
origin_decref(o);
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Eliminate same_suspect function in builtin/blame.c
Since the origin pointers are interned and reference-counted, comparing
the pointers rather than the content is enough. The only uninterned
origins are cached values kept in commit-util, but same_suspect is not
called on them.
Signed-off-by: David Kastrup d...@gnu.org
---
builtin/blame.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 2195595..ead6148 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -255,15 +255,6 @@ struct scoreboard {
int *lineno;
};
-static inline int same_suspect(struct origin *a, struct origin *b)
-{
- if (a == b)
- return 1;
- if (a-commit != b-commit)
- return 0;
- return !strcmp(a-path, b-path);
-}
-
static void sanity_check_refcnt(struct scoreboard *);
/*
@@ -276,7 +267,7 @@ static void coalesce(struct scoreboard *sb)
struct blame_entry *ent, *next;
for (ent = sb-ent; ent (next = ent-next); ent = next) {
- if (same_suspect(ent-suspect, next-suspect)
+ if (ent-suspect == next-suspect
ent-guilty == next-guilty
ent-s_lno + ent-num_lines == next-s_lno) {
ent-num_lines += next-num_lines;
@@ -735,7 +726,7 @@ static int find_last_in_target(struct scoreboard *sb,
struct origin *target)
int last_in_target = -1;
for (e = sb-ent; e; e = e-next) {
- if (e-guilty || !same_suspect(e-suspect, target))
+ if (e-guilty || e-suspect != target)
continue;
if (last_in_target e-s_lno + e-num_lines)
last_in_target = e-s_lno + e-num_lines;
@@ -755,7 +746,7 @@ static void blame_chunk(struct scoreboard *sb,
struct blame_entry *e;
for (e = sb-ent; e; e = e-next) {
- if (e-guilty || !same_suspect(e-suspect, target))
+ if (e-guilty || e-suspect != target)
continue;
if (same = e-s_lno)
continue;
@@ -985,7 +976,7 @@ static int find_move_in_parent(struct scoreboard *sb,
while (made_progress) {
made_progress = 0;
for (e = sb-ent; e; e = e-next) {
- if (e-guilty || !same_suspect(e-suspect, target) ||
+ if (e-guilty || e-suspect != target ||
ent_score(sb, e) blame_move_score)
continue;
find_copy_in_blob(sb, e, parent, split, file_p);
@@ -1020,14 +1011,14 @@ static struct blame_list *setup_blame_list(struct
scoreboard *sb,
for (e = sb-ent, num_ents = 0; e; e = e-next)
if (!e-scanned !e-guilty
- same_suspect(e-suspect, target)
+ e-suspect == target
min_score ent_score(sb, e))
num_ents++;
if (num_ents) {
blame_list = xcalloc(num_ents, sizeof(struct blame_list));
for (e = sb-ent, i = 0; e; e = e-next)
if (!e-scanned !e-guilty
- same_suspect(e-suspect, target)
+ e-suspect == target
min_score ent_score(sb, e))
blame_list[i++].ent = e;
}
@@ -1171,7 +1162,7 @@ static void pass_whole_blame(struct scoreboard *sb,
origin-file.ptr = NULL;
}
for (e = sb-ent; e; e = e-next) {
- if (!same_suspect(e-suspect, origin))
+ if (e-suspect != origin)
continue;
origin_incref(porigin);
origin_decref(e-suspect);
@@ -1560,7 +1551,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
/* Take responsibility for the remaining entries */
for (ent = sb-ent; ent; ent = ent-next)
- if (same_suspect(ent-suspect, suspect))
+ if (ent-suspect == suspect)
found_guilty_entry(ent);
origin_decref(suspect);
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Two mostly janitorial patches for builtin/blame.c
Same series as sent previously, just signed off this time. David Kastrup (2): builtin/blame.c: struct blame_entry does not need a prev link Eliminate same_suspect function in builtin/blame.c builtin/blame.c | 38 ++ 1 file changed, 10 insertions(+), 28 deletions(-) -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Make 'git help everyday' work - relnotes
From: Junio C Hamano gits...@pobox.com Philip Oakley philipoak...@iee.org writes: Determining which is the current release note is possibly more problematic, which should be when making the documentation. Hmmm Why? You are already aware of the stale-notes section, no? Isn't the top one the latest? It's that the 'git help release-notes' would _include_ the latest release notes, not just link to them (which is what the stalenotes currently does). Or at least that was the idea. Trying to determine the latest version, and then include those release notes, and the subsequent maint notes, into the putative release-notes(7) man page, without causing you any maintenance hassle, was the conceptual problem. I already have a local patch that creates a stalenote.txt file, and includes that in a release-notes(7) man page, but it still leaves the actual release notes in a separate plain text file, linked from the man page, rather than being right at hand, which is what I think readers would expect. My other question would be to ask how you normally manage the up-issue of the stalenotes, and when you would normally create that section in git(1) as I didn't see any ifdef::stalenotes[] being defined anywhere else. Philip -- 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 0/6] Make 'git help everyday' work - relnotes
Philip Oakley philipoak...@iee.org writes: I already have a local patch that creates a stalenote.txt file, and includes that in a release-notes(7) man page, but it still leaves the actual release notes in a separate plain text file, linked from the man page, rather than being right at hand, which is what I think readers would expect. Sorry, but I still do not get it. If you have a script that reads git.txt and extracts its stale-notes section to generate the source to be processed into release-notes(7), why can't that script also include the contents of the latest release notes inline into its output? My release notes are _not_ written to be compatible with/processable by AsciiDoc (they are meant to be mere plain text)---perhaps you are wondering if that would make it harder to maintain your script that produces release-notes.txt? Confused... My other question would be to ask how you normally manage the up-issue of the stalenotes, and when you would normally create that section in git(1) as I didn't see any ifdef::stalenotes[] being defined anywhere else. I'm not sure if I am understanding the question right (up-issue?), but it used to be that the preformatted and web-reachable manual pages at k.org were processed with stalenotes defined (which by the way was disabled with adaa3caf Meta/dodoc.sh: adjust to the new layout, 2011-11-15 on the todo branch), and 26cfcfbf Add release notes to the distribution., 2007-02-13 used that facility to prepare something like this: docs/git.html /git-cat-file.html ... docs/vX.Y.Z/git.html docs/vX.Y.Z/git-cat-file.html ... where the latest one lived immediately underneath docs/*, while older ones were in versioned subdirectories. -- 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 03/11] git p4: work around p4 bug that causes empty symlinks
On Tue, Jan 21, 2014 at 6:16 PM, Pete Wyckoff p...@padd.com wrote: Damien Gérard highlights an interesting problem. Some p4 repositories end up with symlinks that have an empty target. It is not possible to create this with current p4, but they do indeed exist. The effect in git p4 is that p4 print on the symlink returns an empty string, confusing the curret symlink-handling code. Such broken repositories cause problems in p4 as well, even with no git involved. In p4, syncing to a change that includes a bogus symlink causes errors: //depot/empty-symlink - updating /home/me/p4/empty-symlink rename: /home/me/p4/empty-symlink: No such file or directory and leaves no symlink. In git, replicate the p4 behavior by ignoring these bad symlinks. If, in a later p4 revision, the symlink happens to point to something non-null, the symlink will be replaced properly. Add a big test for all this too. This happens to be a regression introduced by 1292df1 (git-p4: Fix occasional truncation of symlink contents., 2013-08-08) and appeared first in 1.8.5. But it only shows up only in p4 Redundant only. repositories of dubious character, so can wait for a proper release. Tested-by: Damien Gérard dam...@iwi.me Signed-off-by: Pete Wyckoff p...@padd.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: [PATCH 06/11] git p4 test: run as user author
On Tue, Jan 21, 2014 at 6:16 PM, Pete Wyckoff p...@padd.com wrote: The tests use aut...@example.com as the canonical submitter, but he does not have an entry in the p4 users database. This causes the generated change description to complain that the git and p4 users disagree. The complaint message is still valid, just isn't useful in tests. It was was s/was was/was/ introduced in 848de9c (git-p4: warn if git authorship won't be retained, 2011-05-13). Fix t9813 to use @example.com instead of @localhost due to change in p4_add_user(). Move the function into the git p4 test library so author can be added at initialization time. Signed-off-by: Pete Wyckoff p...@padd.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: [PATCH] improve git svn performance
manjian2...@gmail.com wrote:
From: linzj li...@ucweb.com
I am trying to improve git svn's performance according to some
profiling data.As the data showed,_rev_list subroutine and rebuild
subroutine are consuming a large proportion of time.So I improve
_rev_list's performance by memoize its results,and avoid subprocess
invocation by memoize rebuild subroutine's key data.Here's my patch:
Hi, I'm interested in this. How much did performance improve by
(and how many revisions is the repository)
However, I cannot accept the patch in the current state.
The commit message is inadequate; and you need a Signed-off-by and follow
existing coding convention/style. See Documentation/SubmittingPatches
and Documentation/CodingGuidelines for more info.
Some comments inline...
tie_for_persistent_memoization(\%lookup_svn_merge_cache,
- $cache_path/lookup_svn_merge);
+ $cache_path/lookup_svn_merge);
memoize 'lookup_svn_merge',
- SCALAR_CACHE = 'FAULT',
- LIST_CACHE = ['HASH' = \%lookup_svn_merge_cache],
- ;
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' =
\%lookup_svn_merge_cache],
+ ;
Please no extraneous whitespace changes.
+#define a global associate map to record rebuild status
+my %rebuildStatus;
We prefer snake_case variables, not camelCase.
sub rebuild {
my ($self) = @_;
my $map_path = $self-map_path;
my $partial = (-e $map_path ! -z $map_path);
- return unless ::verify_ref($self-refname.'^0');
+my $verifyKey = $self-refname.'^0';
+if (! exists $rebuildVerifyStatus{$verifyKey} || ! defined
$rebuildVerifyStatus{$verifyKey} ) {
+my $verifyResult = ::verify_ref($verifyKey);
+if ($verifyResult) {
+$rebuildVerifyStatus{$verifyKey} = 1;
+}
+}
+if (! exists $rebuildVerifyStatus{$verifyKey}) {
+return;
+}
Please use tabs for indentation to match surrounding code
(most of git is tabs).
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: BREAKING NEWS
From: Cindye T. Richburg Sent: Wednesday, January 22, 2014 12:39 AM To: Cindye T. Richburg Subject: RE: BREAKING NEWS New Year Donation To You, Contact Mr Paul White (paul-wh...@zbavitu.netmailto:paul-wh...@zbavitu.net) for more info -- 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/RFC] Makefile: Fix compilation of windows resource file
[Cc Pat, who added git.rc] Am 1/22/2014 0:48, schrieb Junio C Hamano: Ramsay Jones ram...@ramsay1.demon.co.uk writes: Note that I am merely guessing that short-digit version numbers are acceptable by now after seeing https://sourceware.org/ml/binutils/2012-07/msg00199.html Ah, nice find! I will test your patch (below) and let you know soon, but it looks good to me. (I can't test it tonight, unfortunately.) One thing to note is that I don't know why the existing code dropped the fourth digit from the maintenance series. I don't know either. But it does not really matter. When there are 4 digits in the FILEVERSION and PRODUCTVERSION statements, then the user does not see them as-are, but, for example, 1.8.1283 for FILEVERSION 1,8,5,3 (1283 = 5*256+3). Therefore, I think that there is no point in providing 4 numbers, and the patch below should be sufficient. diff --git a/Makefile b/Makefile index b4af1e2..99b2b89 100644 --- a/Makefile +++ b/Makefile @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES git.res: git.rc GIT-VERSION-FILE $(QUIET_RC)$(RC) \ - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ -DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@ ifndef NO_PERL diff --git a/git.rc b/git.rc index bce6db9..33aafb7 100644 --- a/git.rc +++ b/git.rc @@ -1,6 +1,6 @@ 1 VERSIONINFO -FILEVERSION MAJOR,MINOR,PATCH,0 -PRODUCTVERSION MAJOR,MINOR,PATCH,0 +FILEVERSION MAJOR,MINOR,0,0 +PRODUCTVERSION MAJOR,MINOR,0,0 BEGIN BLOCK StringFileInfo BEGIN -- 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
