* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Marc Branchaud @ 2011-12-13 15:35 UTC (permalink / raw)
To: Phil Hord; +Cc: Leif Gruenwoldt, Andreas T.Auer, Junio C Hamano, git
In-Reply-To: <CABURp0rFOGQ9kAbAn65W3UAHTWbk5prH7spjJnFvL5fqzbFp1w@mail.gmail.com>
On 11-12-12 05:56 PM, Phil Hord wrote:
> On Mon, Dec 12, 2011 at 2:13 PM, Leif Gruenwoldt <leifer@gmail.com> wrote:
>> On Mon, Dec 12, 2011 at 1:42 PM, Andreas T.Auer
>> <andreas.t.auer_gtml_37453@ursus.ath.cx> wrote:
>>
>>> The next question is: Wouldn't you like to have the new stable branch only
>>> pulled in, when the projectX (as the superproject) is currently on that new
>>> development branch (maybe master)?
>>>
>>> But if you checkout that fixed released version 1.2.9.8, wouldn't it be
>>> better that in that case the gitlinked version of the submodule is checked
>>> out instead of some unrelated new version? I mean, when the gitlinks are
>>> tracked with the projectX commits, this should work well.
>>>
>>> And what about a maintenance branch, which is not a fixed version but a
>>> quite stable branch which should only have bugfixes. Shouldn't the auto-pull
>>> be disabled in that case, too?
>>>
>>> I think the "auto-pull" behavior should depend on the currently checked out
>>> branch. So the configuration options should allow the definition of one or
>>> more mappings.
>>
>> Yes. I think you nailed it. The floating behaviour would best be
>> configured per branch.
>
> Yes, I think you nailed it too. I've been thinking the same thing for
> a while now, but I didn't know how to express it completely. Some of
> the discussion on here last week gelled the last bits in my mind.
>
> To wit, I think I would want something like this in my project:
>
> Use gitlinks when the superproject HEAD is one of these:
> refs/heads/maint/*
> refs/heads/svn/* (historic branches)
> refs/tags/*
> <SHA1> (detached)
>
> Float on the rest, using the branch given in .gitmodules (which may be
> * to mean "use the same branch as the superproject".)
>
> But maybe it is foolish of me to keep branches where I really want
> lightweight tags. If so, I could get away with this:
>
> Float if .git/HEAD begins with "refs/heads"
> Else, use the SHA1.
Wouldn't this break creating a bugfix topic branch based on an earlier
revision of the repo? I wouldn't want such a branch to automatically give me
the latest submodules.
I'd prefer to have floating be explicitly configured on a per-branch (or
per-branch-glob) basis. So in addition to what Jens described yesterday [1]
to configure an individual submodule's floating branch, I suggest there also
be a new section in the .gitmodules file for configuring the super-repo's
floating branches, e.g.
[super]
floaters = refs/heads/master refs/heads/dev*
[submodule "Sub1"]
path = foo/bar
branch = maint
url = ...
[submodule "Sub2"]
path = other/place
url = ...
This would mean that whenever the super-repo checks out either the "master"
branch or a branch whose name starts with "dev" (assuming recursive checkouts
are on):
* The Sub1 submodule automatically checks out the tip of its
"maint" branch.
* The Sub2 submodule (lacking a "branch" variable) would not float
and would check out the commit recorded in the super-repo.
A super-repo recursive-checkout that doesn't match a floaters pattern would
work in the regular, non-floating way.
M.
[1] http://article.gmane.org/gmane.comp.version-control.git/186969
^ permalink raw reply
* [PATCH] gitk: use a tabbed dialog to edit preferences
From: Pat Thoyts @ 2011-12-13 14:56 UTC (permalink / raw)
To: Git; +Cc: Paul Mackerras
This commit converts the user preferences dialog into a tabbed property
sheet grouping general properties, colours and font selections onto
separate pages. The previous implementation was exceeding the screen
height on some systems and this avoids such problems and permits extension
using new pages in the future.
If themed Tk is unavailable or undesired a reasonable facsimile of the
tabbed notebook widget is used instead.
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
I have done this patch against the git-core version of gitk as gitk's
own repository on kernel.org has not re-appeared. Is this likely to get
resurrected or should I perhaps make a github fork for any future work
on gitk?
Cheers,
Pat.
gitk-git/gitk | 264 +++++++++++++++++++++++++++++++++++----------------------
1 files changed, 164 insertions(+), 100 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 2a92e20..8d95bfc 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -10770,6 +10770,139 @@ proc chg_fontparam {v sub op} {
font config sample -$sub $fontparam($sub)
}
+# Create a property sheet tab page
+proc create_prefs_page {w} {
+ global NS
+ set parent [join [lrange [split $w .] 0 end-1] .]
+ if {[winfo class $parent] eq "TNotebook"} {
+ ${NS}::frame $w
+ } else {
+ ${NS}::labelframe $w
+ }
+}
+
+proc prefspage_general {notebook} {
+ global NS maxwidth maxgraphpct showneartags showlocalchanges
+ global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
+ global hideremotes want_ttk have_ttk
+
+ set page [create_prefs_page $notebook.general]
+
+ ${NS}::label $page.ldisp -text [mc "Commit list display options"]
+ grid $page.ldisp - -sticky w -pady 10
+ ${NS}::label $page.spacer -text " "
+ ${NS}::label $page.maxwidthl -text [mc "Maximum graph width (lines)"]
+ spinbox $page.maxwidth -from 0 -to 100 -width 4 -textvariable maxwidth
+ grid $page.spacer $page.maxwidthl $page.maxwidth -sticky w
+ ${NS}::label $page.maxpctl -text [mc "Maximum graph width (% of pane)"]
+ spinbox $page.maxpct -from 1 -to 100 -width 4 -textvariable maxgraphpct
+ grid x $page.maxpctl $page.maxpct -sticky w
+ ${NS}::checkbutton $page.showlocal -text [mc "Show local changes"] \
+ -variable showlocalchanges
+ grid x $page.showlocal -sticky w
+ ${NS}::checkbutton $page.autoselect -text [mc "Auto-select SHA1 (length)"] \
+ -variable autoselect
+ spinbox $page.autosellen -from 1 -to 40 -width 4 -textvariable autosellen
+ grid x $page.autoselect $page.autosellen -sticky w
+ ${NS}::checkbutton $page.hideremotes -text [mc "Hide remote refs"] \
+ -variable hideremotes
+ grid x $page.hideremotes -sticky w
+
+ ${NS}::label $page.ddisp -text [mc "Diff display options"]
+ grid $page.ddisp - -sticky w -pady 10
+ ${NS}::label $page.tabstopl -text [mc "Tab spacing"]
+ spinbox $page.tabstop -from 1 -to 20 -width 4 -textvariable tabstop
+ grid x $page.tabstopl $page.tabstop -sticky w
+ ${NS}::checkbutton $page.ntag -text [mc "Display nearby tags"] \
+ -variable showneartags
+ grid x $page.ntag -sticky w
+ ${NS}::checkbutton $page.ldiff -text [mc "Limit diffs to listed paths"] \
+ -variable limitdiffs
+ grid x $page.ldiff -sticky w
+ ${NS}::checkbutton $page.lattr -text [mc "Support per-file encodings"] \
+ -variable perfile_attrs
+ grid x $page.lattr -sticky w
+
+ ${NS}::entry $page.extdifft -textvariable extdifftool
+ ${NS}::frame $page.extdifff
+ ${NS}::label $page.extdifff.l -text [mc "External diff tool" ]
+ ${NS}::button $page.extdifff.b -text [mc "Choose..."] -command choose_extdiff
+ pack $page.extdifff.l $page.extdifff.b -side left
+ pack configure $page.extdifff.l -padx 10
+ grid x $page.extdifff $page.extdifft -sticky ew
+
+ ${NS}::label $page.lgen -text [mc "General options"]
+ grid $page.lgen - -sticky w -pady 10
+ ${NS}::checkbutton $page.want_ttk -variable want_ttk \
+ -text [mc "Use themed widgets"]
+ if {$have_ttk} {
+ ${NS}::label $page.ttk_note -text [mc "(change requires restart)"]
+ } else {
+ ${NS}::label $page.ttk_note -text [mc "(currently unavailable)"]
+ }
+ grid x $page.want_ttk $page.ttk_note -sticky w
+ return $page
+}
+
+proc prefspage_colors {notebook} {
+ global NS uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
+
+ set page [create_prefs_page $notebook.colors]
+
+ ${NS}::label $page.cdisp -text [mc "Colors: press to choose"]
+ grid $page.cdisp - -sticky w -pady 10
+ label $page.ui -padx 40 -relief sunk -background $uicolor
+ ${NS}::button $page.uibut -text [mc "Interface"] \
+ -command [list choosecolor uicolor {} $page.ui [mc "interface"] setui]
+ grid x $page.uibut $page.ui -sticky w
+ label $page.bg -padx 40 -relief sunk -background $bgcolor
+ ${NS}::button $page.bgbut -text [mc "Background"] \
+ -command [list choosecolor bgcolor {} $page.bg [mc "background"] setbg]
+ grid x $page.bgbut $page.bg -sticky w
+ label $page.fg -padx 40 -relief sunk -background $fgcolor
+ ${NS}::button $page.fgbut -text [mc "Foreground"] \
+ -command [list choosecolor fgcolor {} $page.fg [mc "foreground"] setfg]
+ grid x $page.fgbut $page.fg -sticky w
+ label $page.diffold -padx 40 -relief sunk -background [lindex $diffcolors 0]
+ ${NS}::button $page.diffoldbut -text [mc "Diff: old lines"] \
+ -command [list choosecolor diffcolors 0 $page.diffold [mc "diff old lines"] \
+ [list $ctext tag conf d0 -foreground]]
+ grid x $page.diffoldbut $page.diffold -sticky w
+ label $page.diffnew -padx 40 -relief sunk -background [lindex $diffcolors 1]
+ ${NS}::button $page.diffnewbut -text [mc "Diff: new lines"] \
+ -command [list choosecolor diffcolors 1 $page.diffnew [mc "diff new lines"] \
+ [list $ctext tag conf dresult -foreground]]
+ grid x $page.diffnewbut $page.diffnew -sticky w
+ label $page.hunksep -padx 40 -relief sunk -background [lindex $diffcolors 2]
+ ${NS}::button $page.hunksepbut -text [mc "Diff: hunk header"] \
+ -command [list choosecolor diffcolors 2 $page.hunksep \
+ [mc "diff hunk header"] \
+ [list $ctext tag conf hunksep -foreground]]
+ grid x $page.hunksepbut $page.hunksep -sticky w
+ label $page.markbgsep -padx 40 -relief sunk -background $markbgcolor
+ ${NS}::button $page.markbgbut -text [mc "Marked line bg"] \
+ -command [list choosecolor markbgcolor {} $page.markbgsep \
+ [mc "marked line background"] \
+ [list $ctext tag conf omark -background]]
+ grid x $page.markbgbut $page.markbgsep -sticky w
+ label $page.selbgsep -padx 40 -relief sunk -background $selectbgcolor
+ ${NS}::button $page.selbgbut -text [mc "Select bg"] \
+ -command [list choosecolor selectbgcolor {} $page.selbgsep [mc "background"] setselbg]
+ grid x $page.selbgbut $page.selbgsep -sticky w
+ return $page
+}
+
+proc prefspage_fonts {notebook} {
+ global NS
+ set page [create_prefs_page $notebook.fonts]
+ ${NS}::label $page.cfont -text [mc "Fonts: press to choose"]
+ grid $page.cfont - -sticky w -pady 10
+ mkfontdisp mainfont $page [mc "Main font"]
+ mkfontdisp textfont $page [mc "Diff display font"]
+ mkfontdisp uifont $page [mc "User interface font"]
+ return $page
+}
+
proc doprefs {} {
global maxwidth maxgraphpct use_ttk NS
global oldprefs prefstop showneartags showlocalchanges
@@ -10790,106 +10923,37 @@ proc doprefs {} {
ttk_toplevel $top
wm title $top [mc "Gitk preferences"]
make_transient $top .
- ${NS}::label $top.ldisp -text [mc "Commit list display options"]
- grid $top.ldisp - -sticky w -pady 10
- ${NS}::label $top.spacer -text " "
- ${NS}::label $top.maxwidthl -text [mc "Maximum graph width (lines)"]
- spinbox $top.maxwidth -from 0 -to 100 -width 4 -textvariable maxwidth
- grid $top.spacer $top.maxwidthl $top.maxwidth -sticky w
- ${NS}::label $top.maxpctl -text [mc "Maximum graph width (% of pane)"]
- spinbox $top.maxpct -from 1 -to 100 -width 4 -textvariable maxgraphpct
- grid x $top.maxpctl $top.maxpct -sticky w
- ${NS}::checkbutton $top.showlocal -text [mc "Show local changes"] \
- -variable showlocalchanges
- grid x $top.showlocal -sticky w
- ${NS}::checkbutton $top.autoselect -text [mc "Auto-select SHA1 (length)"] \
- -variable autoselect
- spinbox $top.autosellen -from 1 -to 40 -width 4 -textvariable autosellen
- grid x $top.autoselect $top.autosellen -sticky w
- ${NS}::checkbutton $top.hideremotes -text [mc "Hide remote refs"] \
- -variable hideremotes
- grid x $top.hideremotes -sticky w
-
- ${NS}::label $top.ddisp -text [mc "Diff display options"]
- grid $top.ddisp - -sticky w -pady 10
- ${NS}::label $top.tabstopl -text [mc "Tab spacing"]
- spinbox $top.tabstop -from 1 -to 20 -width 4 -textvariable tabstop
- grid x $top.tabstopl $top.tabstop -sticky w
- ${NS}::checkbutton $top.ntag -text [mc "Display nearby tags"] \
- -variable showneartags
- grid x $top.ntag -sticky w
- ${NS}::checkbutton $top.ldiff -text [mc "Limit diffs to listed paths"] \
- -variable limitdiffs
- grid x $top.ldiff -sticky w
- ${NS}::checkbutton $top.lattr -text [mc "Support per-file encodings"] \
- -variable perfile_attrs
- grid x $top.lattr -sticky w
-
- ${NS}::entry $top.extdifft -textvariable extdifftool
- ${NS}::frame $top.extdifff
- ${NS}::label $top.extdifff.l -text [mc "External diff tool" ]
- ${NS}::button $top.extdifff.b -text [mc "Choose..."] -command choose_extdiff
- pack $top.extdifff.l $top.extdifff.b -side left
- pack configure $top.extdifff.l -padx 10
- grid x $top.extdifff $top.extdifft -sticky ew
-
- ${NS}::label $top.lgen -text [mc "General options"]
- grid $top.lgen - -sticky w -pady 10
- ${NS}::checkbutton $top.want_ttk -variable want_ttk \
- -text [mc "Use themed widgets"]
- if {$have_ttk} {
- ${NS}::label $top.ttk_note -text [mc "(change requires restart)"]
+
+ if {[set use_notebook [expr {$use_ttk && [info command ::ttk::notebook] ne ""}]]} {
+ set notebook [ttk::notebook $top.notebook]
} else {
- ${NS}::label $top.ttk_note -text [mc "(currently unavailable)"]
- }
- grid x $top.want_ttk $top.ttk_note -sticky w
-
- ${NS}::label $top.cdisp -text [mc "Colors: press to choose"]
- grid $top.cdisp - -sticky w -pady 10
- label $top.ui -padx 40 -relief sunk -background $uicolor
- ${NS}::button $top.uibut -text [mc "Interface"] \
- -command [list choosecolor uicolor {} $top.ui [mc "interface"] setui]
- grid x $top.uibut $top.ui -sticky w
- label $top.bg -padx 40 -relief sunk -background $bgcolor
- ${NS}::button $top.bgbut -text [mc "Background"] \
- -command [list choosecolor bgcolor {} $top.bg [mc "background"] setbg]
- grid x $top.bgbut $top.bg -sticky w
- label $top.fg -padx 40 -relief sunk -background $fgcolor
- ${NS}::button $top.fgbut -text [mc "Foreground"] \
- -command [list choosecolor fgcolor {} $top.fg [mc "foreground"] setfg]
- grid x $top.fgbut $top.fg -sticky w
- label $top.diffold -padx 40 -relief sunk -background [lindex $diffcolors 0]
- ${NS}::button $top.diffoldbut -text [mc "Diff: old lines"] \
- -command [list choosecolor diffcolors 0 $top.diffold [mc "diff old lines"] \
- [list $ctext tag conf d0 -foreground]]
- grid x $top.diffoldbut $top.diffold -sticky w
- label $top.diffnew -padx 40 -relief sunk -background [lindex $diffcolors 1]
- ${NS}::button $top.diffnewbut -text [mc "Diff: new lines"] \
- -command [list choosecolor diffcolors 1 $top.diffnew [mc "diff new lines"] \
- [list $ctext tag conf dresult -foreground]]
- grid x $top.diffnewbut $top.diffnew -sticky w
- label $top.hunksep -padx 40 -relief sunk -background [lindex $diffcolors 2]
- ${NS}::button $top.hunksepbut -text [mc "Diff: hunk header"] \
- -command [list choosecolor diffcolors 2 $top.hunksep \
- [mc "diff hunk header"] \
- [list $ctext tag conf hunksep -foreground]]
- grid x $top.hunksepbut $top.hunksep -sticky w
- label $top.markbgsep -padx 40 -relief sunk -background $markbgcolor
- ${NS}::button $top.markbgbut -text [mc "Marked line bg"] \
- -command [list choosecolor markbgcolor {} $top.markbgsep \
- [mc "marked line background"] \
- [list $ctext tag conf omark -background]]
- grid x $top.markbgbut $top.markbgsep -sticky w
- label $top.selbgsep -padx 40 -relief sunk -background $selectbgcolor
- ${NS}::button $top.selbgbut -text [mc "Select bg"] \
- -command [list choosecolor selectbgcolor {} $top.selbgsep [mc "background"] setselbg]
- grid x $top.selbgbut $top.selbgsep -sticky w
-
- ${NS}::label $top.cfont -text [mc "Fonts: press to choose"]
- grid $top.cfont - -sticky w -pady 10
- mkfontdisp mainfont $top [mc "Main font"]
- mkfontdisp textfont $top [mc "Diff display font"]
- mkfontdisp uifont $top [mc "User interface font"]
+ set notebook [${NS}::frame $top.notebook -borderwidth 0 -relief flat]
+ }
+
+ lappend pages [prefspage_general $notebook] [mc "General"]
+ lappend pages [prefspage_colors $notebook] [mc "Colors"]
+ lappend pages [prefspage_fonts $notebook] [mc "Fonts"]
+ foreach {page title} $pages {
+ if {$use_notebook} {
+ $notebook add $page -text $title
+ } else {
+ set btn [${NS}::button $notebook.b_[string map {. X} $page] \
+ -text $title -command [list raise $page]]
+ $page configure -text $title
+ grid $btn -row 0 -column [incr col] -sticky w
+ grid $page -row 1 -column 0 -sticky news -columnspan 100
+ }
+ }
+
+ if {!$use_notebook} {
+ grid columnconfigure $notebook 0 -weight 1
+ grid rowconfigure $notebook 1 -weight 1
+ raise [lindex $pages 0]
+ }
+
+ grid $notebook -sticky news -padx 2 -pady 2
+ grid rowconfigure $top 0 -weight 1
+ grid columnconfigure $top 0 -weight 1
${NS}::frame $top.buts
${NS}::button $top.buts.ok -text [mc "OK"] -command prefsok -default active
@@ -10901,7 +10965,7 @@ proc doprefs {} {
grid columnconfigure $top.buts 1 -weight 1 -uniform a
grid $top.buts - - -pady 10 -sticky ew
grid columnconfigure $top 2 -weight 1
- bind $top <Visibility> "focus $top.buts.ok"
+ bind $top <Visibility> [list focus $top.buts.ok]
}
proc choose_extdiff {} {
--
1.7.8.msysgit.0
^ permalink raw reply related
* Re: migrating from svn: How to clean up history?
From: Jakub Narebski @ 2011-12-13 15:12 UTC (permalink / raw)
To: Hartmut Goebel; +Cc: git
In-Reply-To: <4EE766D5.5040702@goebel-consult.de>
Hartmut Goebel <h.goebel@goebel-consult.de> writes:
> I'm one of the developers of www.pyinstaller.org, a tool for
> bundling/packagin Python scripts and al required modules into a single
> executable for easy distribution. We are in the process of migrating
> from Subversion to git.
>
> Our SVN-Repo contains some stuff we do not want or do not need in the
> git repo. How can we clean this up?
>
> 1) Useless commits e.g. tagging -> I want to remove these
> 2) copy or move mistakes -> I want to "correct" the copy
[...]
> Any hints how to to this clean-up?
You can try "gitsvnparse" command from 'reposurgeon' tool to
automatically correct some conversion mistakes, and use this tool to
do further fixes
http://www.catb.org/esr/dvcs-migration-guide.html
http://www.catb.org/~esr/reposurgeon/
HTH
--
Jakub Narebski
^ permalink raw reply
* [PATCH 2/2] run-command: Add interpreter permissions check
From: Frans Klaver @ 2011-12-13 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Frans Klaver
In-Reply-To: <1323788917-4141-1-git-send-email-fransklaver@gmail.com>
If a script is started and the interpreter of that script given in the
shebang cannot be started due to permissions, we can get a rather
obscure situation. All permission checks pass for the script itself,
but we still get EACCES from execvp.
Try to find out if the above is the case and warn the user about it.
Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
run-command.c | 59 ++++++++++++++++++++++++++++++++++++++++++++---
t/t0061-run-command.sh | 22 ++++++++++++++++++
2 files changed, 77 insertions(+), 4 deletions(-)
diff --git a/run-command.c b/run-command.c
index 3f136f4..9ddd409 100644
--- a/run-command.c
+++ b/run-command.c
@@ -149,6 +149,55 @@ static int have_read_execute_permissions(const char *path)
return 0;
}
+static void check_interpreter(const char *cmd)
+{
+ FILE *f;
+ struct strbuf sb = STRBUF_INIT;
+ /*
+ * bash reads an 80 character line when determining the interpreter.
+ * BSD apparently only allows 32 characters, as it is the size of
+ * your average binary executable header.
+ */
+ char firstline[80];
+ size_t s, start, end;
+
+ f = fopen(cmd, "r");
+ if (!f) {
+ error("cannot open file '%s': %s\n", cmd, strerror(errno));
+ return;
+ }
+
+ s = fread(firstline, 1, sizeof(firstline), f);
+ if (s < 2) {
+ trace_printf("cannot determine file type");
+ fclose(f);
+ return;
+ }
+
+ if (firstline[0] != '#' || firstline[1] != '!') {
+ trace_printf("file '%s' is not a script or"
+ " is a script without '#!'", cmd);
+ fclose(f);
+ return;
+ }
+
+ /* see if the given path has the executable bit set */
+ start = strspn(&firstline[2], " \t") + 2;
+ end = strcspn(&firstline[start], " \t\r\n") + start;
+ if (start >= end) {
+ error("could not determine interpreter\n");
+ return;
+ }
+
+ strbuf_add(&sb, &firstline[start], end - start);
+
+ if (!have_read_execute_permissions(sb.buf))
+ error("bad interpreter: no read/execute permissions on '%s'\n",
+ sb.buf);
+
+ strbuf_release(&sb);
+}
+
static void diagnose_execvp_eacces(const char *cmd, const char **argv)
{
/*
@@ -167,6 +216,8 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
if (strchr(cmd, '/')) {
if (!have_read_execute_permissions(cmd))
error("no read/execute permissions on '%s'\n", cmd);
+ else
+ check_interpreter(cmd);
return;
}
@@ -197,11 +248,11 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
if (!have_read_execute_permissions(sb.buf))
error("no read/execute permissions on '%s'\n",
sb.buf);
+ else if (access(sb.buf, R_OK) == 0)
+ check_interpreter(sb.buf);
else
- warning("file '%s' exists and permissions "
- "seem OK.\nIf this is a script, see if you "
- "have sufficient privileges to run the "
- "interpreter", sb.buf);
+ trace_printf("cannot determine interpreter "
+ "on '%s'\n", sb.buf);
}
strbuf_release(&sb);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index b39bd16..39bfaef 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -13,6 +13,18 @@ cat >hello-script <<-EOF
EOF
>empty
+cat >someinterpreter <<-EOF
+ #!$SHELL_PATH
+ cat hello-script
+EOF
+>empty
+
+cat >incorrect-interpreter-script <<-EOF
+ #!someinterpreter
+ cat hello-script
+EOF
+>empty
+
test_expect_success 'start_command reports ENOENT' '
test-run-command start-command-ENOENT ./does-not-exist
'
@@ -48,4 +60,14 @@ test_expect_success POSIXPERM 'run_command reports EACCES, search path permision
grep "no read/execute permissions on" err
'
+test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
+ cat incorrect-interpreter-script >hello.sh &&
+ chmod +x hello.sh &&
+ chmod -x someinterpreter &&
+ test_must_fail test-run-command run-command ./hello.sh 2>err &&
+
+ grep "fatal: cannot exec.*hello.sh" err &&
+ grep "bad interpreter" err
+'
+
test_done
--
1.7.8
^ permalink raw reply related
* [PATCH 1/2] run-command: Add checks after execvp fails with EACCES
From: Frans Klaver @ 2011-12-13 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Frans Klaver
In-Reply-To: <1323788917-4141-1-git-send-email-fransklaver@gmail.com>
execvp returns ENOENT if a command was not found after searching PATH.
If path contains a directory that current user has insufficient
privileges to, EACCES is returned. This may still mean the program
wasn't found and may cause confusion to the user, especially when the
file mentioned doesn't exist -- that is, the user would expect NOENT to
be returned -- and the user was actually hoping for an alias to be executed.
To help users track down the core issue more easily, perform some checks
on the path and file permissions involved. Output errors when paths or
files don't have enough permissions.
Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
run-command.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
t/t0061-run-command.sh | 16 +++++++++-
2 files changed, 94 insertions(+), 1 deletions(-)
diff --git a/run-command.c b/run-command.c
index 1c51043..3f136f4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,7 @@
#include "run-command.h"
#include "exec_cmd.h"
#include "argv-array.h"
+#include "dir.h"
static inline void close_pair(int fd[2])
{
@@ -134,6 +135,80 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
return code;
}
+#ifndef WIN32
+static int have_read_execute_permissions(const char *path)
+{
+ if (access(path, R_OK|X_OK) == 0)
+ return 1;
+
+ if (errno == EACCES)
+ return 0;
+
+ trace_printf("could not determine permissions for '%s': %s\n", path,
+ strerror(errno));
+ return 0;
+}
+
+static void diagnose_execvp_eacces(const char *cmd, const char **argv)
+{
+ /*
+ * man 2 execve states that EACCES is returned for:
+ * - Search permission is denied on a component of the path prefix
+ * of cmd or the name of a script interpreter
+ * - The file or script interpreter is not a regular file
+ * - Execute permission is denied for the file, script or ELF
+ * interpreter
+ * - The file system is mounted noexec
+ */
+ struct strbuf sb = STRBUF_INIT;
+ char *path;
+ char *next;
+
+ if (strchr(cmd, '/')) {
+ if (!have_read_execute_permissions(cmd))
+ error("no read/execute permissions on '%s'\n", cmd);
+ return;
+ }
+
+ path = getenv("PATH");
+ while (path) {
+ next = strchrnul(path, ':');
+ if (path < next)
+ strbuf_add(&sb, path, next - path);
+ else
+ strbuf_addch(&sb, '.');
+
+ if (!*next)
+ path = NULL;
+ else
+ path = next + 1;
+
+ if (!have_read_execute_permissions(sb.buf)) {
+ error("no read/execute permissions on '%s'\n", sb.buf);
+ strbuf_release(&sb);
+ continue;
+ }
+
+ if (sb.len && sb.buf[sb.len - 1] != '/')
+ strbuf_addch(&sb, '/');
+ strbuf_addstr(&sb, cmd);
+
+ if (file_exists(sb.buf)) {
+ if (!have_read_execute_permissions(sb.buf))
+ error("no read/execute permissions on '%s'\n",
+ sb.buf);
+ else
+ warning("file '%s' exists and permissions "
+ "seem OK.\nIf this is a script, see if you "
+ "have sufficient privileges to run the "
+ "interpreter", sb.buf);
+ }
+
+ strbuf_release(&sb);
+ }
+}
+#endif
+
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@@ -285,6 +360,10 @@ fail_pipe:
error("cannot run %s: %s", cmd->argv[0],
strerror(ENOENT));
exit(127);
+ } else if (errno == EACCES) {
+ diagnose_execvp_eacces(cmd->argv[0], cmd->argv);
+ die("cannot exec '%s': %s", cmd->argv[0],
+ strerror(EACCES));
} else {
die_errno("cannot exec '%s'", cmd->argv[0]);
}
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 8d4938f..b39bd16 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -26,7 +26,7 @@ test_expect_success 'run_command can run a command' '
test_cmp empty err
'
-test_expect_success POSIXPERM 'run_command reports EACCES' '
+test_expect_success POSIXPERM 'run_command reports EACCES, file permissions' '
cat hello-script >hello.sh &&
chmod -x hello.sh &&
test_must_fail test-run-command run-command ./hello.sh 2>err &&
@@ -34,4 +34,18 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
grep "fatal: cannot exec.*hello.sh" err
'
+test_expect_success POSIXPERM 'run_command reports EACCES, search path permisions' '
+ mkdir -p inaccessible &&
+ PATH=$(pwd)/inaccessible:$PATH &&
+ export PATH &&
+
+ cat hello-script >inaccessible/hello.sh &&
+ chmod 400 inaccessible &&
+ test_must_fail test-run-command run-command hello.sh 2>err &&
+ chmod 755 inaccessible &&
+
+ grep "fatal: cannot exec.*hello.sh" err &&
+ grep "no read/execute permissions on" err
+'
+
test_done
--
1.7.8
^ permalink raw reply related
* [PATCH 0/2 v2] run-command: Add eacces diagnostics
From: Frans Klaver @ 2011-12-13 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <op.v5e8mgbc0aolir@keputer>
This replaces $gmane/186388
I had a lot of short stints incorporating the review remarks, so I might just
have missed something.
[PATCH 1/2] run-command: Add checks after execvp fails with EACCES
[PATCH 2/2] run-command: Add interpreter permissions check
run-command.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++
t/t0061-run-command.sh | 38 ++++++++++++++-
2 files changed, 167 insertions(+), 1 deletions(-)
^ permalink raw reply
* Re: [PATCH/RFC] Makefile: add 'help' target for target summary
From: Jakub Narebski @ 2011-12-13 15:00 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1323782164-11759-1-git-send-email-pclouds@gmail.com>
乧畹ễ渠周á椠乧ọ挠䑵礠‼灣汯畤獀杭慩氮捯派⁷物瑥猺ഊഊ㸠䤠晩湤⁴桩猠≭慫攠桥汰∠癥特敬灦畬
慴⁷潲欬渠愠摩晦敲敮琍ਾ⁰牯橥捴⤮⁗楴栠瑨楳⁉潮❴慶攠瑯牡睬⁴桲潵杨⁍慫敦楬攠睨敮⁉敥損ਾ潭整桩湧畴慮湯琠牥浥浢敲⁷桡琧猠瑨攠瑡牧整慭攮⁉琠獨潵汤汳漍ਾ敬瀠摩獣潶敲敷⁴慲来瑳⸍ਾਾ⁗攠浡礠慬獯慶攠≭慫攠癡牳∠⡯爠獯浥瑨楮朠汩步⁴桡琩⁴桡琠獨潷猠汩獴ഊ㸠潦⁵獥爭捯湦楧畲慢汥⁶慲楡扬敳Ⱐ扡獩捡汬礠愠捯湶敲獩潮映瑨攠扩服ਾ潭浥湴汯捫敡爠瑨攠浡步晩汥❳⁴潰湴漠愠灲楮瑡扬攠瑡牧整⸍ਾਾ⁉潮❴⁷潲欠睩瑨⁴桩猠䵡步晩汥畣栬漠瑨楳猠橵獴渠楤敡⸠䅮祯湥ഊ㸠異⁴漠瑵牮琠瑯潭整桩湧捴畡汬礠畳敦畬㼍桯畬搠瑨楳攠楮潭浩琠浥獳慧攬爠楮潭浥湴猠⡢敬潷•ⴭⵜ渢⤿ഊഊ乢⸠瑨敲攠睡猠慴敡獴湥瑴敭灴⁴漠慤搠≭慫攠桥汰∠畳楮服੩渭䵡步晩汥 慮湯瑡瑩潮獟Ⱐ獯⁴桡琠≭慫攠桥汰∠睯畬摮❴整慤汹畴ഊ潦祮挮⸮ഊഊ㸠卩杮敤ⵯ晦ⵢ示⁎杵秾＞앮⁔棾⁎柾＞쵣⁄畹‼灣汯畤獀杭慩氮捯派ഊ㸠ⴭⴍਾ†䵡步晩汥⁼†‱㐠⬫⬫⬫⬫⬫⬫⬫ഊ㸠‱楬敳桡湧敤Ⱐㄴ湳敲瑩潮猨⬩Ⱐ〠摥汥瑩潮猨ഊ㸠ഊ㸠摩晦ⵧ楴⽍慫敦楬攠戯䵡步晩汥ഊ㸠楮摥砠敤㠲㌲〮扦㙣昹‱〰㘴㐍ਾⴭ⽍慫敦楬攍ਾ⬫⽍慫敦楬攍ਾ⁀䀠ⴲ㘰㌬㌠⬲㘰㌬ㄷ⁀䀠灲潦楬攭慬氺⁰牯晩汥ⵣ汥慮ഊ㸠 䵁䭅⤠䍆䱁䝓㴢偒但䥌䕟䝅也䍆䱁䝓⤢汬ഊ㸠 䵁䭅⤠䍆䱁䝓㴢偒但䥌䕟䝅也䍆䱁䝓⤢樱⁴敳琍ਾ†त⡍䅋䔩⁃䙌䅇匽∤⡐剏䙉䱅录卅彃䙌䅇匩∠慬氍ਾഊ㸠⬮偈低夺敬瀍ਾഊ㸠⭨敬瀺ഊ㸠⬉䁥捨漠≴敳琉॒畮⁴桥⁴敳琠獵楴攢ഊ㸠⬉䁥捨漠≣潶敲慧攉䉵楬搠杩琠睩瑨潶敲慧攠獵灰潲琢ഊ㸠⬉䁥捨漠≣潶敲彤戉䝥湥牡瑥潶敲慧攠摡瑡扡獥牯洠⨮杣潶∍ਾी散桯•捯癥牟摢彨瑭氉䝥湥牡瑥潶敲慧攠牥灯牴∍ਾी散桯•灲潦楬攭慬氉䉵楬搠杩琠睩瑨⁰牯晩汩湧異灯牴∍ਾी散桯•捬敡渉ृ汥慮湴敲浥摩慴攠晩汥猢ഊ㸠⬉䁥捨漠≤楳瑣汥慮ृ汥慮癥渠浯牥潲楳琠灡捫慧楮朢ഊ㸠⬉䁥捨漠≳灡牳攉॒畮楴⁷楴栠獰慲獥∍ਾी散桯•捳捯灥उ䝥湥牡瑥獣潰攠獹浢潬慴慢慳攢ഊ㸠⬉䁥捨漠≣桥捫ⵤ潣猉䍨散欠摯捵浥湴慴楯渢ഊ㸠ⴭਾ‱⸷⸸⸳㘮朶㥥攲ഊ㸠ഊഊⴭ慫畢⁎慲ę扳歩ഊ
^ permalink raw reply
* migrating from svn: How to clean up history?
From: Hartmut Goebel @ 2011-12-13 14:53 UTC (permalink / raw)
To: git
Hello,
I'm one of the developers of www.pyinstaller.org, a tool for
bundling/packagin Python scripts and al required modules into a single
executable for easy distribution. We are in the process of migrating
from Subversion to git.
Our SVN-Repo contains some stuff we do not want or do not need in the
git repo. How can we clean this up?
1) Useless commits e.g. tagging -> I want to remove these
2) copy or move mistakes -> I want to "correct" the copy
Example for 1): This occurs e.g. if this is done
svn cp trunk tags/v0.1 -m 'release v0.1' <<<--- this on in not needed
svn rm tags/v0.1 -m 'release v0.1'
svn cp trunk tags/v0.1 -m 'release v0.1'
In this case I want to completely eliminate the first commit.
Example for 2): This occurs e.g. if this is done
svn cp trunk tags/v0.1 -m 'release v0.1'
svn cp trunk tags/v0.1 -m 'release v0.1' <<-- actually copies to
tags/v0.1/trunk
In this case I want to change eliminate the first commit (this is he
same as case 1) and rewrite commit 2 to rewrite the path.
I already did some experiments with `git filter-branch` without success.
(I manage renaming the tags, though.)
Any hints how to to this clean-up?
--
Schönen Gruß - Regards
Hartmut Goebel
Dipl.-Informatiker (univ.), CISSP, CSSLP
Goebel Consult
Spezialist für IT-Sicherheit in komplexen Umgebungen
http://www.goebel-consult.de
Monatliche Kolumne: http://www.cissp-gefluester.de/
Goebel Consult ist Mitglied bei http://www.7-it.de
^ permalink raw reply
* [PATCH] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Nguyễn Thái Ngọc Duy @ 2011-12-13 14:17 UTC (permalink / raw)
To: git, Michael Haggerty
Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <4EE75C88.5060700@alum.mit.edu>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
2011/12/13 Michael Haggerty <mhagger@alum.mit.edu>:
>> This uses "one 'const char *' pointer that is used for reading data from
>> and an extra 'char *' pointer that is used only for freeing" approach,
>> which has two advantages and one disadvantage:
>> [...]
>> When naming a "for-freeing" pointer variable, the kind of data the area of
>> memory happens to contain is of secondary importance. Being deliberately
>> vague about what the area of memory may contain is a good thing, because
>> it actively discourages the program from looking at the area via the
>> pointer if the variable is named "to_free" or something that does not
>> specify what it contains.
>
> The to_free variable could even be declared void* to make it even less
> (ab)usable.
Good thinking. Here's the version with "char *foo_to_free" converted
to "void *foo_to_free".
builtin/branch.c | 12 +++++-------
builtin/checkout.c | 15 +++++++--------
builtin/fmt-merge-msg.c | 7 ++++---
builtin/for-each-ref.c | 7 ++-----
builtin/merge.c | 12 +++++-------
builtin/notes.c | 7 ++++---
builtin/receive-pack.c | 7 +++----
builtin/show-branch.c | 4 +---
cache.h | 1 +
reflog-walk.c | 15 ++++++++-------
refs.c | 6 ++++++
wt-status.c | 4 +---
12 files changed, 47 insertions(+), 50 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index e1e486e..c459f0b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -104,6 +104,7 @@ static int branch_merged(int kind, const char *name,
*/
struct commit *reference_rev = NULL;
const char *reference_name = NULL;
+ void *reference_name_to_free = NULL;
int merged;
if (kind == REF_LOCAL_BRANCH) {
@@ -114,11 +115,9 @@ static int branch_merged(int kind, const char *name,
branch->merge &&
branch->merge[0] &&
branch->merge[0]->dst &&
- (reference_name =
- resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) {
- reference_name = xstrdup(reference_name);
+ (reference_name = reference_name_to_free =
+ resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
reference_rev = lookup_commit_reference(sha1);
- }
}
if (!reference_rev)
reference_rev = head_rev;
@@ -143,7 +142,7 @@ static int branch_merged(int kind, const char *name,
" '%s', even though it is merged to HEAD."),
name, reference_name);
}
- free((char *)reference_name);
+ free(reference_name_to_free);
return merged;
}
@@ -731,10 +730,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
track = git_branch_track;
- head = resolve_ref("HEAD", head_sha1, 0, NULL);
+ head = resolve_refdup("HEAD", head_sha1, 0, NULL);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
- head = xstrdup(head);
if (!strcmp(head, "HEAD")) {
detached = 1;
} else {
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b7c6302..00740bd 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -696,17 +696,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
{
int ret = 0;
struct branch_info old;
+ void *path_to_free;
unsigned char rev[20];
int flag;
memset(&old, 0, sizeof(old));
- old.path = resolve_ref("HEAD", rev, 0, &flag);
- if (old.path)
- old.path = xstrdup(old.path);
+ old.path = path_to_free = resolve_refdup("HEAD", rev, 0, &flag);
old.commit = lookup_commit_reference_gently(rev, 1);
- if (!(flag & REF_ISSYMREF)) {
- free((char *)old.path);
+ if (!(flag & REF_ISSYMREF))
old.path = NULL;
- }
if (old.path && !prefixcmp(old.path, "refs/heads/"))
old.name = old.path + strlen("refs/heads/");
@@ -720,8 +717,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
}
ret = merge_working_tree(opts, &old, new);
- if (ret)
+ if (ret) {
+ free(path_to_free);
return ret;
+ }
if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
orphaned_commit_warning(old.commit);
@@ -729,7 +728,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
update_refs_for_switch(opts, &old, new);
ret = post_checkout_hook(old.commit, new->commit, 1);
- free((char *)old.path);
+ free(path_to_free);
return ret || opts->writeout_error;
}
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index bdfa0ea..c81a7fe 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -372,14 +372,15 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
int i = 0, pos = 0;
unsigned char head_sha1[20];
const char *current_branch;
+ void *current_branch_to_free;
/* get current branch */
- current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
+ current_branch = current_branch_to_free =
+ resolve_refdup("HEAD", head_sha1, 1, NULL);
if (!current_branch)
die("No current branch");
if (!prefixcmp(current_branch, "refs/heads/"))
current_branch += 11;
- current_branch = xstrdup(current_branch);
/* get a line */
while (pos < in->len) {
@@ -421,7 +422,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
}
strbuf_complete_line(out);
- free((char *)current_branch);
+ free(current_branch_to_free);
return 0;
}
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index d90e5d2..b01d76a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -628,11 +628,8 @@ static void populate_value(struct refinfo *ref)
if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
unsigned char unused1[20];
- const char *symref;
- symref = resolve_ref(ref->refname, unused1, 1, NULL);
- if (symref)
- ref->symref = xstrdup(symref);
- else
+ ref->symref = resolve_refdup(ref->refname, unused1, 1, NULL);
+ if (!ref->symref)
ref->symref = "";
}
diff --git a/builtin/merge.c b/builtin/merge.c
index a1c8534..a3cd5e8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1096,6 +1096,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit_list *common = NULL;
const char *best_strategy = NULL, *wt_strategy = NULL;
struct commit_list **remotes = &remoteheads;
+ void *branch_to_free;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_merge_usage, builtin_merge_options);
@@ -1104,12 +1105,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* Check if we are _not_ on a detached HEAD, i.e. if there is a
* current branch.
*/
- branch = resolve_ref("HEAD", head_sha1, 0, &flag);
- if (branch) {
- if (!prefixcmp(branch, "refs/heads/"))
- branch += 11;
- branch = xstrdup(branch);
- }
+ branch = branch_to_free = resolve_refdup("HEAD", head_sha1, 0, &flag);
+ if (branch && !prefixcmp(branch, "refs/heads/"))
+ branch += 11;
if (!branch || is_null_sha1(head_sha1))
head_commit = NULL;
else
@@ -1520,6 +1518,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
ret = suggest_conflicts(option_renormalize);
done:
- free((char *)branch);
+ free(branch_to_free);
return ret;
}
diff --git a/builtin/notes.c b/builtin/notes.c
index 10b8bc7..667e20a 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o)
struct notes_tree *t;
struct commit *partial;
struct pretty_print_context pretty_ctx;
+ void *local_ref_to_free;
int ret;
/*
@@ -826,10 +827,10 @@ static int merge_commit(struct notes_merge_options *o)
t = xcalloc(1, sizeof(struct notes_tree));
init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
- o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
+ o->local_ref = local_ref_to_free =
+ resolve_refdup("NOTES_MERGE_REF", sha1, 0, NULL);
if (!o->local_ref)
die("Failed to resolve NOTES_MERGE_REF");
- o->local_ref = xstrdup(o->local_ref);
if (notes_merge_commit(o, t, partial, sha1))
die("Failed to finalize notes merge");
@@ -846,7 +847,7 @@ static int merge_commit(struct notes_merge_options *o)
free_notes(t);
strbuf_release(&msg);
ret = merge_abort(o);
- free((char *)o->local_ref);
+ free(local_ref_to_free);
return ret;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b6d957c..62afac3 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -37,6 +37,7 @@ static int prefer_ofs_delta = 1;
static int auto_update_server_info;
static int auto_gc = 1;
static const char *head_name;
+static void *head_name_to_free;
static int sent_capabilities;
static enum deny_action parse_deny_action(const char *var, const char *value)
@@ -695,10 +696,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
check_aliased_updates(commands);
- free((char *)head_name);
- head_name = resolve_ref("HEAD", sha1, 0, NULL);
- if (head_name)
- head_name = xstrdup(head_name);
+ free(head_name_to_free);
+ head_name = head_name_to_free = resolve_refdup("HEAD", sha1, 0, NULL);
for (cmd = commands; cmd; cmd = cmd->next)
if (!cmd->skip_update)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 4b480d7..a1f148e 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -726,10 +726,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
if (ac == 0) {
static const char *fake_av[2];
- const char *refname;
- refname = resolve_ref("HEAD", sha1, 1, NULL);
- fake_av[0] = xstrdup(refname);
+ fake_av[0] = resolve_refdup("HEAD", sha1, 1, NULL);
fake_av[1] = NULL;
av = fake_av;
ac = 1;
diff --git a/cache.h b/cache.h
index 8c98d05..4887a3e 100644
--- a/cache.h
+++ b/cache.h
@@ -866,6 +866,7 @@ extern int read_ref(const char *filename, unsigned char *sha1);
* errno is sometimes set on errors, but not always.
*/
extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/reflog-walk.c b/reflog-walk.c
index da71a85..64c677f 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -50,11 +50,12 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
for_each_reflog_ent(ref, read_one_reflog, reflogs);
if (reflogs->nr == 0) {
unsigned char sha1[20];
- const char *name = resolve_ref(ref, sha1, 1, NULL);
+ const char *name;
+ void *name_to_free;
+ name = name_to_free = resolve_refdup(ref, sha1, 1, NULL);
if (name) {
- name = xstrdup(name);
for_each_reflog_ent(name, read_one_reflog, reflogs);
- free((char *)name);
+ free(name_to_free);
}
}
if (reflogs->nr == 0) {
@@ -171,11 +172,11 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
else {
if (*branch == '\0') {
unsigned char sha1[20];
- const char *head = resolve_ref("HEAD", sha1, 0, NULL);
- if (!head)
- die ("No current branch");
free(branch);
- branch = xstrdup(head);
+ branch = resolve_refdup("HEAD", sha1, 0, NULL);
+ if (!branch)
+ die ("No current branch");
+
}
reflogs = read_complete_reflog(branch);
if (!reflogs || reflogs->nr == 0) {
diff --git a/refs.c b/refs.c
index f5cb297..8ffb32f 100644
--- a/refs.c
+++ b/refs.c
@@ -605,6 +605,12 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return ref;
}
+char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag)
+{
+ const char *ret = resolve_ref(ref, sha1, reading, flag);
+ return ret ? xstrdup(ret) : NULL;
+}
+
/* The argument to filter_refs */
struct ref_filter {
const char *pattern;
diff --git a/wt-status.c b/wt-status.c
index 70fdb76..9ffc535 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -111,7 +111,6 @@ void status_printf_more(struct wt_status *s, const char *color,
void wt_status_prepare(struct wt_status *s)
{
unsigned char sha1[20];
- const char *head;
memset(s, 0, sizeof(*s));
memcpy(s->color_palette, default_wt_status_colors,
@@ -119,8 +118,7 @@ void wt_status_prepare(struct wt_status *s)
s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
s->use_color = -1;
s->relative_paths = 1;
- head = resolve_ref("HEAD", sha1, 0, NULL);
- s->branch = head ? xstrdup(head) : NULL;
+ s->branch = resolve_refdup("HEAD", sha1, 0, NULL);
s->reference = "HEAD";
s->fp = stdout;
s->index_file = get_index_file();
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Git blame only current branch
From: Frans Klaver @ 2011-12-13 14:18 UTC (permalink / raw)
To: Junio C Hamano, Vijay Lakshminarayanan
Cc: Jeff King, Stephen Bash, git discussion list
In-Reply-To: <87y5ugsguj.fsf@gmail.com>
On Tue, 13 Dec 2011 15:09:56 +0100, Vijay Lakshminarayanan
<laksvij@gmail.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Vijay Lakshminarayanan <laksvij@gmail.com> writes:
>>
>>> The code reads fine when there's no numeral 1 around but now it doesn't
>>> read well. I think refactoring
>>>
>>> struct commit_list *l
>>>
>>> to
>>>
>>> struct commit_list *lst
>>>
>>> is justified. Thoughts?
>>
>> Not justified at all.
>>
>> What is "lst" and why is it not spelled "list"? It is a disease to drop
>> vowels when you do not have to.
>
> lst is better than l in this particular context. I think fried_chicken
> is better than l in this particular context ;-)
I tend to agree. If you casually look over the code it may look odd, and
with several monospace fonts there really isn't a very big difference
between 1?1:0 and l?1:0. You shouldn't have to squint to properly see the
intention of the code.
If there's going to be a rename, there is no reason to leave out the i
though.
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Phil Hord @ 2011-12-13 14:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Leif Gruenwoldt, git
In-Reply-To: <7vaa6x4m5l.fsf@alter.siamese.dyndns.org>
On Mon, Dec 12, 2011 at 2:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
[...]
> Distro package dependency tracking is a poor analogy for many reasons, but
> I'll only touch a few.
[...]
> Naively, one might think that two branches, branch-1.0 and branch-2.0, can
> be defined in the repository of L, tell somebody (like "superproject that
> covers all the packages in the distro") that A wants branch-1.0 and B
> wants branch-2.0 of L respectively, to emulate this, but if one thinks
> further, one would realize that it is insufficient. For one thing, it is
> unclear what should happen when both A and B are asked to be checked out,
> but more importantly, in dependency requirements on real distro packaging,
> the application C could say "I want v1.0 API but v1.4 is broken and not
> compatible with me", which won't fit on the two-branches model. A
> workaround to add more branches to L could be devised but any workaround
> cannot be a good solution that allows a random application C among 47
> others to dictate how the branch structure of L project should look like.
>
> Fortunately, the dependency management is a solved problem by distro
> package management and build systems, and they do so without using
> anything from submodules. There is no point reinventing these logic in git
> submodules and emulating poorly.
>
> The only remotely plausible analogy around distro packaging would be a
> superproject full of all the packages in a distro as its submodules, and
> records exact versions of each and every package that goes on a release
> CD (or DVD). In that case, you do want to have a central registry that
> records what exact version of each package is used to cut the CD and the
> mother of all modules superproject could be one way to implement it. But
> that is not an example of floating, but is a direct opposite.
>
> This exchange convinced me further that anybody who wishes to use
> "floating" is better off either by doing one or both of the following:
>
> - using "exact" but not updating religiously, as the interdepency
> requirement in their project is not strict; or
>
> - not using submodules at all, but merely keeping these unrelated A, B, C
> and L as standalone repositories next to each other in the directory
> structure.
My interdependency requirements are not so cut-and-dry. We use
submodules to isolate controlled regions of code. We may need to
share our project with a contractor who is allowed to see code
pertaining to "vendorA" but not that for "vendorB" or "VendorN". But
our in-house developers want to have all the vendor code in one place
for convenient integration. Submodules do this nicely for us. We can
give the contractor just the main modules and the VendorA modules and
he'll be fine. In-house devs get all the submodules (using the
vendor-ALL superproject).
But this necessarily means there is too much coupling for comfort
between our submodules. For example, when an API changes in the main
submodule, each of the vendor submodules is affected because they each
implement that API in a custom method. Some of those vendor modules
belong to different people. Submodule synchronization becomes a real
chore.
Floating would help, I think. Instead I do this:
git pull origin topic_foo && git submodule foreach 'git pull origin topic_foo'
git submodule foreach 'git push origin topic_foo' && git push origin topic_foo
But not all my developers are git-gurus yet, and they sometimes mess
up their ad hoc scripts or miss important changes they forgot to push
in one submodule or another. Or worse, their pull or push fails and
they can't see the problem for all the noise. So they email it to me.
On my git server, I have a hook that automatically propagates each
push to "master" from the submodules into the superproject. But this
is tedious and limited. And it relies on a centralized server.
You may say this itch is all in my head, but it sure seems real to me.
Phil
^ permalink raw reply
* Re: Git blame only current branch
From: Vijay Lakshminarayanan @ 2011-12-13 14:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Stephen Bash, git discussion list
In-Reply-To: <7vobvdvx9c.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Vijay Lakshminarayanan <laksvij@gmail.com> writes:
>
>> The code reads fine when there's no numeral 1 around but now it doesn't
>> read well. I think refactoring
>>
>> struct commit_list *l
>>
>> to
>>
>> struct commit_list *lst
>>
>> is justified. Thoughts?
>
> Not justified at all.
>
> What is "lst" and why is it not spelled "list"? It is a disease to drop
> vowels when you do not have to.
lst is better than l in this particular context. I think fried_chicken
is better than l in this particular context ;-)
> If I were to name a new variable that points at one element of a linked
> list and is used to walk the list (surprise!) "element" or perhaps "elem"
> for short, but in the context of that short function I honestly do not see
> much need for such a naming. The variable is extremely short-lived and
> there is no room for confusion.
Before the introduction of the numeral 1, I am in complete agreement
with you for the exact reasons you've mentioned above. Post
introduction of "l ? 1 : 0" it warrants a refactoring. It's possible
you're using a different font so you never encounter the issue, but this
definitely isn't a problem I alone face. For instance, it is a
sufficiently common problem that it's one of the "Java Puzzlers" in Josh
Bloch's book of the same name. (Yes, elem is better than lst.)
My $0.02.
--
Cheers
~vijay
Gnus should be more complicated.
^ permalink raw reply
* Re: [PATCH 2/4] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Michael Haggerty @ 2011-12-13 14:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, git, Jonathan Nieder
In-Reply-To: <7vpqft4qap.fsf@alter.siamese.dyndns.org>
On 12/12/2011 07:07 PM, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> Thanks.
>
> The patch looks correct but I have a slight maintainability concern and a
> suggestion for possible improvement.
>
>> ...
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index b7c6302..a66d3eb 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -696,17 +696,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
>> {
>> int ret = 0;
>> struct branch_info old;
>> + char *path;
>> unsigned char rev[20];
>> int flag;
>> memset(&old, 0, sizeof(old));
>> - old.path = resolve_ref("HEAD", rev, 0, &flag);
>> - if (old.path)
>> - old.path = xstrdup(old.path);
>> + old.path = path = resolve_refdup("HEAD", rev, 0, &flag);
>
> This uses "one 'const char *' pointer that is used for reading data from
> and an extra 'char *' pointer that is used only for freeing" approach,
> which has two advantages and one disadvantage:
> [...]
> When naming a "for-freeing" pointer variable, the kind of data the area of
> memory happens to contain is of secondary importance. Being deliberately
> vague about what the area of memory may contain is a good thing, because
> it actively discourages the program from looking at the area via the
> pointer if the variable is named "to_free" or something that does not
> specify what it contains.
The to_free variable could even be declared void* to make it even less
(ab)usable.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* [PATCH/RFC] Makefile: add 'help' target for target summary
From: Nguyễn Thái Ngọc Duy @ 2011-12-13 13:16 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
I find this "make help" very helpful (at work, on a different
project). With this I don't have to crawl through Makefile when I need
something but cannot remember what's the target name. It should also
help discover new targets.
We may also have "make vars" (or something like that) that shows list
of user-configurable variables, basically a conversion of the big
comment block near the makefile's top into a printable target.
I don't work with this Makefile much, so this is just an idea. Anyone
up to turn it to something actually useful?
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Makefile | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index ed82320..abf6cf9 100644
--- a/Makefile
+++ b/Makefile
@@ -2603,3 +2603,17 @@ profile-all: profile-clean
$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" all
$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test
$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all
+
+.PHONY: help
+
+help:
+ @echo "test Run the test suite"
+ @echo "coverage Build git with coverage support"
+ @echo "cover_db Generate coverage database from *.gcov"
+ @echo "cover_db_html Generate coverage report"
+ @echo "profile-all Build git with profiling support"
+ @echo "clean Clean intermediate files"
+ @echo "distclean Clean even more for dist packaging"
+ @echo "sparse Run git with sparse"
+ @echo "cscope Generate cscope symbol database"
+ @echo "check-docs Check documentation"
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Question about commit message wrapping
From: Holger Hellmuth @ 2011-12-13 13:14 UTC (permalink / raw)
To: Michael Haggerty
Cc: Frans Klaver, Andrew Ardill, Jakub Narebski,
Sidney San Martín, git
In-Reply-To: <4EE6C31C.60909@alum.mit.edu>
On 13.12.2011 04:14, Michael Haggerty wrote:
> On 12/12/2011 11:16 PM, Frans Klaver wrote:
>> Wrapped code as in auto-wrapped? Or as in manually wrapped? Python
>> programmers have significant white space, but you can still hard wrap
>> stuff, as long as the next statement is properly indented.
I meant as in auto-wrapped and also not as a permanent change but
something done to a long line on output to the screen.
> FWIW I think automatic wrapping of commit messages is a bad idea. I
> wrap my commit messages deliberately to make them look the way I want
Which you still can do (since hard line endings would not be ignored).
On displays wider than your line limit you will still see it exactly
like intended. Only on narrow displays your commit message would look
bad, admittedly even worse than cut-off lines.
> them to look. The assumption of an 80-character display has historical
> reasons, but it is also a relatively comfortable line-width to read
> (even on wider displays). And given that commit messages sometimes
> contain "flowable" paragraph text, sometimes code snippets, sometimes
> ASCII art, etc, no automatic wrapping will work correctly unless
> everybody agrees that commit messages must be written in some specific
> form of markup (or lightweight markup). And I can't imagine such a
> thing ever happening.
With that assumption everyone could be happy with automatic wrapping of
lines on screen output. You can hard wrap and it will look exactly as
intended. In the same commit message you could also just write a
paragraph without hitting the return-key at all and have a commit
message that looks good in web browsers and too narrow gitk windows.
^ permalink raw reply
* Please confirm from Libyian Embassy
From: Jimmy Zakia @ 2011-12-13 21:34 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 177 bytes --]
In a brief introduction, I am an embassy attachй with the Libyan Government in the Hague, Netherlands.
Please kindly see the attachment for more details and respond immediately
[-- Attachment #2: attachment.rtf --]
[-- Type: application/octet-stream, Size: 3042 bytes --]
^ permalink raw reply
* [PATCH] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Nguyễn Thái Ngọc Duy @ 2011-12-13 12:31 UTC (permalink / raw)
To: git, Junio C Hamano
Cc: Jonathan Nieder, Nguyễn Thái Ngọc Duy
In-Reply-To: <7vpqft4qap.fsf@alter.siamese.dyndns.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
2011/12/13 Junio C Hamano <gitster@pobox.com>:
> A handful of places in the existing codebase use this "two pointers"
> approach primarily for the second advantage. The way they avoid the
> disadvantage is by naming the other pointer "$something_to_free" (and the
> "$something_" part is optional if there is only one such variable in the
> context) to make it clear that the variable is not meant to be looked at
> in the code and its primary purpose is for cleaning up at the end.
>
> Perhaps renaming this "path" to "to_free" (or "path_to_free" if you want
> to hint the "path-ness" to the readers, but see below) would make it less
> likely that future tweakers mistakenly look at, modify the string thru, or
> worse yet move the pointer itself.
Thanks. The patch looks better with "_to_free" naming convention. I
learn something new today.
builtin/branch.c | 12 +++++-------
builtin/checkout.c | 15 +++++++--------
builtin/fmt-merge-msg.c | 7 ++++---
builtin/for-each-ref.c | 7 ++-----
builtin/merge.c | 12 +++++-------
builtin/notes.c | 7 ++++---
builtin/receive-pack.c | 7 +++----
builtin/show-branch.c | 4 +---
cache.h | 1 +
reflog-walk.c | 15 ++++++++-------
refs.c | 6 ++++++
wt-status.c | 4 +---
12 files changed, 47 insertions(+), 50 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index e1e486e..59efe2c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -104,6 +104,7 @@ static int branch_merged(int kind, const char *name,
*/
struct commit *reference_rev = NULL;
const char *reference_name = NULL;
+ char *reference_name_to_free = NULL;
int merged;
if (kind == REF_LOCAL_BRANCH) {
@@ -114,11 +115,9 @@ static int branch_merged(int kind, const char *name,
branch->merge &&
branch->merge[0] &&
branch->merge[0]->dst &&
- (reference_name =
- resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) {
- reference_name = xstrdup(reference_name);
+ (reference_name = reference_name_to_free =
+ resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
reference_rev = lookup_commit_reference(sha1);
- }
}
if (!reference_rev)
reference_rev = head_rev;
@@ -143,7 +142,7 @@ static int branch_merged(int kind, const char *name,
" '%s', even though it is merged to HEAD."),
name, reference_name);
}
- free((char *)reference_name);
+ free(reference_name_to_free);
return merged;
}
@@ -731,10 +730,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
track = git_branch_track;
- head = resolve_ref("HEAD", head_sha1, 0, NULL);
+ head = resolve_refdup("HEAD", head_sha1, 0, NULL);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
- head = xstrdup(head);
if (!strcmp(head, "HEAD")) {
detached = 1;
} else {
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b7c6302..dde44d7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -696,17 +696,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
{
int ret = 0;
struct branch_info old;
+ char *path_to_free;
unsigned char rev[20];
int flag;
memset(&old, 0, sizeof(old));
- old.path = resolve_ref("HEAD", rev, 0, &flag);
- if (old.path)
- old.path = xstrdup(old.path);
+ old.path = path_to_free = resolve_refdup("HEAD", rev, 0, &flag);
old.commit = lookup_commit_reference_gently(rev, 1);
- if (!(flag & REF_ISSYMREF)) {
- free((char *)old.path);
+ if (!(flag & REF_ISSYMREF))
old.path = NULL;
- }
if (old.path && !prefixcmp(old.path, "refs/heads/"))
old.name = old.path + strlen("refs/heads/");
@@ -720,8 +717,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
}
ret = merge_working_tree(opts, &old, new);
- if (ret)
+ if (ret) {
+ free(path_to_free);
return ret;
+ }
if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
orphaned_commit_warning(old.commit);
@@ -729,7 +728,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
update_refs_for_switch(opts, &old, new);
ret = post_checkout_hook(old.commit, new->commit, 1);
- free((char *)old.path);
+ free(path_to_free);
return ret || opts->writeout_error;
}
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index bdfa0ea..e55e994 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -372,14 +372,15 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
int i = 0, pos = 0;
unsigned char head_sha1[20];
const char *current_branch;
+ char *current_branch_to_free;
/* get current branch */
- current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
+ current_branch = current_branch_to_free =
+ resolve_refdup("HEAD", head_sha1, 1, NULL);
if (!current_branch)
die("No current branch");
if (!prefixcmp(current_branch, "refs/heads/"))
current_branch += 11;
- current_branch = xstrdup(current_branch);
/* get a line */
while (pos < in->len) {
@@ -421,7 +422,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
}
strbuf_complete_line(out);
- free((char *)current_branch);
+ free(current_branch_to_free);
return 0;
}
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index d90e5d2..b01d76a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -628,11 +628,8 @@ static void populate_value(struct refinfo *ref)
if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
unsigned char unused1[20];
- const char *symref;
- symref = resolve_ref(ref->refname, unused1, 1, NULL);
- if (symref)
- ref->symref = xstrdup(symref);
- else
+ ref->symref = resolve_refdup(ref->refname, unused1, 1, NULL);
+ if (!ref->symref)
ref->symref = "";
}
diff --git a/builtin/merge.c b/builtin/merge.c
index a1c8534..d4079e6 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1096,6 +1096,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit_list *common = NULL;
const char *best_strategy = NULL, *wt_strategy = NULL;
struct commit_list **remotes = &remoteheads;
+ char *branch_to_free;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_merge_usage, builtin_merge_options);
@@ -1104,12 +1105,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* Check if we are _not_ on a detached HEAD, i.e. if there is a
* current branch.
*/
- branch = resolve_ref("HEAD", head_sha1, 0, &flag);
- if (branch) {
- if (!prefixcmp(branch, "refs/heads/"))
- branch += 11;
- branch = xstrdup(branch);
- }
+ branch = branch_to_free = resolve_refdup("HEAD", head_sha1, 0, &flag);
+ if (branch && !prefixcmp(branch, "refs/heads/"))
+ branch += 11;
if (!branch || is_null_sha1(head_sha1))
head_commit = NULL;
else
@@ -1520,6 +1518,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
ret = suggest_conflicts(option_renormalize);
done:
- free((char *)branch);
+ free(branch_to_free);
return ret;
}
diff --git a/builtin/notes.c b/builtin/notes.c
index 10b8bc7..11a24d7 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o)
struct notes_tree *t;
struct commit *partial;
struct pretty_print_context pretty_ctx;
+ char *local_ref_to_free;
int ret;
/*
@@ -826,10 +827,10 @@ static int merge_commit(struct notes_merge_options *o)
t = xcalloc(1, sizeof(struct notes_tree));
init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
- o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
+ o->local_ref = local_ref_to_free =
+ resolve_refdup("NOTES_MERGE_REF", sha1, 0, NULL);
if (!o->local_ref)
die("Failed to resolve NOTES_MERGE_REF");
- o->local_ref = xstrdup(o->local_ref);
if (notes_merge_commit(o, t, partial, sha1))
die("Failed to finalize notes merge");
@@ -846,7 +847,7 @@ static int merge_commit(struct notes_merge_options *o)
free_notes(t);
strbuf_release(&msg);
ret = merge_abort(o);
- free((char *)o->local_ref);
+ free(local_ref_to_free);
return ret;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b6d957c..fa251ec 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -37,6 +37,7 @@ static int prefer_ofs_delta = 1;
static int auto_update_server_info;
static int auto_gc = 1;
static const char *head_name;
+static char *head_name_to_free;
static int sent_capabilities;
static enum deny_action parse_deny_action(const char *var, const char *value)
@@ -695,10 +696,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
check_aliased_updates(commands);
- free((char *)head_name);
- head_name = resolve_ref("HEAD", sha1, 0, NULL);
- if (head_name)
- head_name = xstrdup(head_name);
+ free(head_name_to_free);
+ head_name = head_name_to_free = resolve_refdup("HEAD", sha1, 0, NULL);
for (cmd = commands; cmd; cmd = cmd->next)
if (!cmd->skip_update)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 4b480d7..a1f148e 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -726,10 +726,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
if (ac == 0) {
static const char *fake_av[2];
- const char *refname;
- refname = resolve_ref("HEAD", sha1, 1, NULL);
- fake_av[0] = xstrdup(refname);
+ fake_av[0] = resolve_refdup("HEAD", sha1, 1, NULL);
fake_av[1] = NULL;
av = fake_av;
ac = 1;
diff --git a/cache.h b/cache.h
index 8c98d05..4887a3e 100644
--- a/cache.h
+++ b/cache.h
@@ -866,6 +866,7 @@ extern int read_ref(const char *filename, unsigned char *sha1);
* errno is sometimes set on errors, but not always.
*/
extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/reflog-walk.c b/reflog-walk.c
index da71a85..52eddb7 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -50,11 +50,12 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
for_each_reflog_ent(ref, read_one_reflog, reflogs);
if (reflogs->nr == 0) {
unsigned char sha1[20];
- const char *name = resolve_ref(ref, sha1, 1, NULL);
+ const char *name;
+ char *name_to_free;
+ name = name_to_free = resolve_refdup(ref, sha1, 1, NULL);
if (name) {
- name = xstrdup(name);
for_each_reflog_ent(name, read_one_reflog, reflogs);
- free((char *)name);
+ free(name_to_free);
}
}
if (reflogs->nr == 0) {
@@ -171,11 +172,11 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
else {
if (*branch == '\0') {
unsigned char sha1[20];
- const char *head = resolve_ref("HEAD", sha1, 0, NULL);
- if (!head)
- die ("No current branch");
free(branch);
- branch = xstrdup(head);
+ branch = resolve_refdup("HEAD", sha1, 0, NULL);
+ if (!branch)
+ die ("No current branch");
+
}
reflogs = read_complete_reflog(branch);
if (!reflogs || reflogs->nr == 0) {
diff --git a/refs.c b/refs.c
index f5cb297..8ffb32f 100644
--- a/refs.c
+++ b/refs.c
@@ -605,6 +605,12 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return ref;
}
+char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag)
+{
+ const char *ret = resolve_ref(ref, sha1, reading, flag);
+ return ret ? xstrdup(ret) : NULL;
+}
+
/* The argument to filter_refs */
struct ref_filter {
const char *pattern;
diff --git a/wt-status.c b/wt-status.c
index 70fdb76..9ffc535 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -111,7 +111,6 @@ void status_printf_more(struct wt_status *s, const char *color,
void wt_status_prepare(struct wt_status *s)
{
unsigned char sha1[20];
- const char *head;
memset(s, 0, sizeof(*s));
memcpy(s->color_palette, default_wt_status_colors,
@@ -119,8 +118,7 @@ void wt_status_prepare(struct wt_status *s)
s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
s->use_color = -1;
s->relative_paths = 1;
- head = resolve_ref("HEAD", sha1, 0, NULL);
- s->branch = head ? xstrdup(head) : NULL;
+ s->branch = resolve_refdup("HEAD", sha1, 0, NULL);
s->reference = "HEAD";
s->fp = stdout;
s->index_file = get_index_file();
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH/RFC 2/2] change all unchecked calls to setenv to xsetenv
From: Erik Faye-Lund @ 2011-12-13 12:10 UTC (permalink / raw)
To: git
In-Reply-To: <1323778227-1664-1-git-send-email-kusmabite@gmail.com>
This avoids us from accidentally dropping state, possibly leading
to unexpected behaviour.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
While reviewing some patches for Git for Windows, I realized that
we almost never check the return-value from setenv. This can lead
to quite surprising errors in unusual sitations. Mostly, an error
would probably be preferred. So here we go.
However, I'm not at all convinced myself that all of these make
sense; in particular settings like GIT_EDITOR and GIT_PAGER could
perhaps benefit from having a warning printed rather than a hard
error.
Thoughts?
builtin/clone.c | 2 +-
builtin/commit.c | 6 +++---
builtin/help.c | 4 ++--
builtin/init-db.c | 2 +-
builtin/merge.c | 4 ++--
builtin/notes.c | 2 +-
builtin/remote-ext.c | 4 ++--
builtin/revert.c | 2 +-
config.c | 2 +-
exec_cmd.c | 4 ++--
git.c | 18 +++++++++---------
| 2 +-
setup.c | 6 +++---
13 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index efe8b6c..8d81c29 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -566,7 +566,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
atexit(remove_junk);
sigchain_push_common(remove_junk_on_signal);
- setenv(CONFIG_ENVIRONMENT, mkpath("%s/config", git_dir), 1);
+ xsetenv(CONFIG_ENVIRONMENT, mkpath("%s/config", git_dir), 1);
if (safe_create_leading_directories_const(git_dir) < 0)
die(_("could not create leading directories of '%s'"), git_dir);
diff --git a/builtin/commit.c b/builtin/commit.c
index e36e9ad..2b87da9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,13 +361,13 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
die(_("unable to create temporary index"));
old_index_env = getenv(INDEX_ENVIRONMENT);
- setenv(INDEX_ENVIRONMENT, index_lock.filename, 1);
+ xsetenv(INDEX_ENVIRONMENT, index_lock.filename, 1);
if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
die(_("interactive add failed"));
if (old_index_env && *old_index_env)
- setenv(INDEX_ENVIRONMENT, old_index_env, 1);
+ xsetenv(INDEX_ENVIRONMENT, old_index_env, 1);
else
unsetenv(INDEX_ENVIRONMENT);
@@ -1023,7 +1023,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (edit_flag)
use_editor = 1;
if (!use_editor)
- setenv("GIT_EDITOR", ":", 1);
+ xsetenv("GIT_EDITOR", ":", 1);
/* Sanity check options */
if (amend && !current_head)
diff --git a/builtin/help.c b/builtin/help.c
index 61ff798..e7dc15b 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -330,7 +330,7 @@ static void setup_man_path(void)
if (old_path)
strbuf_addstr(&new_path, old_path);
- setenv("MANPATH", new_path.buf, 1);
+ xsetenv("MANPATH", new_path.buf, 1);
strbuf_release(&new_path);
}
@@ -371,7 +371,7 @@ static void show_man_page(const char *git_cmd)
static void show_info_page(const char *git_cmd)
{
const char *page = cmd_to_page(git_cmd);
- setenv("INFOPATH", system_path(GIT_INFO_PATH), 1);
+ xsetenv("INFOPATH", system_path(GIT_INFO_PATH), 1);
execlp("info", "info", "gitman", page, (char *)NULL);
die("no info viewer handled the request");
}
diff --git a/builtin/init-db.c b/builtin/init-db.c
index d07554c..21ff09e 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -537,7 +537,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
if (is_bare_repository_cfg == 1) {
static char git_dir[PATH_MAX+1];
- setenv(GIT_DIR_ENVIRONMENT,
+ xsetenv(GIT_DIR_ENVIRONMENT,
getcwd(git_dir, sizeof(git_dir)), argc > 0);
}
diff --git a/builtin/merge.c b/builtin/merge.c
index a1c8534..a4ae473 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1257,7 +1257,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
strbuf_addstr(&buf, "merge");
for (i = 0; i < argc; i++)
strbuf_addf(&buf, " %s", argv[i]);
- setenv("GIT_REFLOG_ACTION", buf.buf, 0);
+ xsetenv("GIT_REFLOG_ACTION", buf.buf, 0);
strbuf_reset(&buf);
for (i = 0; i < argc; i++) {
@@ -1267,7 +1267,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
remotes = &commit_list_insert(commit, remotes)->next;
strbuf_addf(&buf, "GITHEAD_%s",
sha1_to_hex(commit->object.sha1));
- setenv(buf.buf, argv[i], 1);
+ xsetenv(buf.buf, argv[i], 1);
strbuf_reset(&buf);
if (merge_remote_util(commit) &&
merge_remote_util(commit)->obj &&
diff --git a/builtin/notes.c b/builtin/notes.c
index 10b8bc7..7b53c69 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1076,7 +1076,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
struct strbuf sb = STRBUF_INIT;
strbuf_addstr(&sb, override_notes_ref);
expand_notes_ref(&sb);
- setenv("GIT_NOTES_REF", sb.buf, 1);
+ xsetenv("GIT_NOTES_REF", sb.buf, 1);
strbuf_release(&sb);
}
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 692c834..0b2169a 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -38,8 +38,8 @@ static char *strip_escapes(const char *str, const char *service,
psoff = 4;
/* Pass the service to command. */
- setenv("GIT_EXT_SERVICE", service, 1);
- setenv("GIT_EXT_SERVICE_NOPREFIX", service + psoff, 1);
+ xsetenv("GIT_EXT_SERVICE", service, 1);
+ xsetenv("GIT_EXT_SERVICE_NOPREFIX", service + psoff, 1);
/* Scan the length of argument. */
while (str[rpos] && (escape || str[rpos] != ' ')) {
diff --git a/builtin/revert.c b/builtin/revert.c
index 1ea525c..955a99f 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1007,7 +1007,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
struct commit_list *cur;
int res;
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ xsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || opts->edit));
diff --git a/config.c b/config.c
index 5ea101f..f461a62 100644
--- a/config.c
+++ b/config.c
@@ -43,7 +43,7 @@ void git_config_push_parameter(const char *text)
strbuf_addch(&env, ' ');
}
sq_quote_buf(&env, text);
- setenv(CONFIG_DATA_ENVIRONMENT, env.buf, 1);
+ xsetenv(CONFIG_DATA_ENVIRONMENT, env.buf, 1);
strbuf_release(&env);
}
diff --git a/exec_cmd.c b/exec_cmd.c
index 171e841..a5a92dd 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -63,7 +63,7 @@ void git_set_argv_exec_path(const char *exec_path)
/*
* Propagate this setting to external programs.
*/
- setenv(EXEC_PATH_ENVIRONMENT, exec_path, 1);
+ xsetenv(EXEC_PATH_ENVIRONMENT, exec_path, 1);
}
@@ -108,7 +108,7 @@ void setup_path(void)
else
strbuf_addstr(&new_path, _PATH_DEFPATH);
- setenv("PATH", new_path.buf, 1);
+ xsetenv("PATH", new_path.buf, 1);
strbuf_release(&new_path);
}
diff --git a/git.c b/git.c
index 8e34903..cb80de2 100644
--- a/git.c
+++ b/git.c
@@ -54,7 +54,7 @@ int check_pager_config(const char *cmd)
static void commit_pager_choice(void) {
switch (use_pager) {
case 0:
- setenv("GIT_PAGER", "cat", 1);
+ xsetenv("GIT_PAGER", "cat", 1);
break;
case 1:
setup_pager();
@@ -109,7 +109,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--no-replace-objects")) {
read_replace_refs = 0;
- setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "1", 1);
+ xsetenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "1", 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--git-dir")) {
@@ -117,13 +117,13 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
fprintf(stderr, "No directory given for --git-dir.\n" );
usage(git_usage_string);
}
- setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
+ xsetenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
if (envchanged)
*envchanged = 1;
(*argv)++;
(*argc)--;
} else if (!prefixcmp(cmd, "--git-dir=")) {
- setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
+ xsetenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--namespace")) {
@@ -131,13 +131,13 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
fprintf(stderr, "No namespace given for --namespace.\n" );
usage(git_usage_string);
}
- setenv(GIT_NAMESPACE_ENVIRONMENT, (*argv)[1], 1);
+ xsetenv(GIT_NAMESPACE_ENVIRONMENT, (*argv)[1], 1);
if (envchanged)
*envchanged = 1;
(*argv)++;
(*argc)--;
} else if (!prefixcmp(cmd, "--namespace=")) {
- setenv(GIT_NAMESPACE_ENVIRONMENT, cmd + 12, 1);
+ xsetenv(GIT_NAMESPACE_ENVIRONMENT, cmd + 12, 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--work-tree")) {
@@ -145,19 +145,19 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
fprintf(stderr, "No directory given for --work-tree.\n" );
usage(git_usage_string);
}
- setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1);
+ xsetenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1);
if (envchanged)
*envchanged = 1;
(*argv)++;
(*argc)--;
} else if (!prefixcmp(cmd, "--work-tree=")) {
- setenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
+ xsetenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--bare")) {
static char git_dir[PATH_MAX+1];
is_bare_repository_cfg = 1;
- setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
+ xsetenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "-c")) {
--git a/pager.c b/pager.c
index 975955b..d3a1299 100644
--- a/pager.c
+++ b/pager.c
@@ -76,7 +76,7 @@ void setup_pager(void)
if (!pager)
return;
- setenv("GIT_PAGER_IN_USE", "true", 1);
+ xsetenv("GIT_PAGER_IN_USE", "true", 1);
/* spawn the pager */
pager_argv[0] = pager;
diff --git a/setup.c b/setup.c
index 61c22e6..06f38d0 100644
--- a/setup.c
+++ b/setup.c
@@ -309,7 +309,7 @@ void setup_work_tree(void)
* if $GIT_WORK_TREE is set relative
*/
if (getenv(GIT_WORK_TREE_ENVIRONMENT))
- setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+ xsetenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
set_git_dir(relative_path(git_dir, work_tree));
initialized = 1;
@@ -683,9 +683,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
prefix = setup_git_directory_gently_1(nongit_ok);
if (prefix)
- setenv("GIT_PREFIX", prefix, 1);
+ xsetenv("GIT_PREFIX", prefix, 1);
else
- setenv("GIT_PREFIX", "", 1);
+ xsetenv("GIT_PREFIX", "", 1);
if (startup_info) {
startup_info->have_repository = !nongit_ok || !*nongit_ok;
--
1.7.7.1.msysgit.0.272.g9e47e
^ permalink raw reply related
* [PATCH/RFC 1/2] wrapper: supply xsetenv
From: Erik Faye-Lund @ 2011-12-13 12:10 UTC (permalink / raw)
To: git
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
git-compat-util.h | 1 +
wrapper.c | 6 ++++++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 77062ed..551a0be 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -439,6 +439,7 @@ extern int xdup(int fd);
extern FILE *xfdopen(int fd, const char *mode);
extern int xmkstemp(char *template);
extern int xmkstemp_mode(char *template, int mode);
+extern int xsetenv(const char *name, const char *val, int override);
extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
extern int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1);
diff --git a/wrapper.c b/wrapper.c
index 85f09df..442800b 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -381,3 +381,9 @@ int remove_or_warn(unsigned int mode, const char *file)
{
return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);
}
+
+int xsetenv(const char *name, const char *val, int overwrite)
+{
+ if (setenv(name, val, overwrite))
+ die_errno("setenv failed");
+}
--
1.7.7.1.msysgit.0.272.g9e47e
^ permalink raw reply related
* [PATCH resend] Do not create commits whose message contains NUL
From: Nguyễn Thái Ngọc Duy @ 2011-12-13 11:56 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
We assume that the commit log messages are uninterpreted sequences of
non-NUL bytes (see Documentation/i18n.txt). However the assumption
does not really stand out and it's quite easy to set an editor to save
in a NUL-included encoding. Currently we silently cut at the first NUL
we see.
Make it more obvious that NUL is not welcome by refusing to create
such commits. Those who deliberately want to create them can still do
with hash-object.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
This is from UTF-16 in commit message discussion [1] a few months
ago. I don't want to resurrect the discussion again. However I think
it's a good idea to stop users from shooting themselves in this case,
especially at porcelain level.
There were no comments on this patch previously. So, any comments
this time ? Should I drop it?
[1] http://thread.gmane.org/gmane.comp.version-control.git/184123/focus=184335
commit.c | 3 +++
t/t3900-i18n-commit.sh | 6 ++++++
t/t3900/UTF-16.txt | Bin 0 -> 32 bytes
3 files changed, 9 insertions(+), 0 deletions(-)
create mode 100644 t/t3900/UTF-16.txt
diff --git a/commit.c b/commit.c
index d67b8c7..0775eec 100644
--- a/commit.c
+++ b/commit.c
@@ -855,6 +855,9 @@ int commit_tree(const char *msg, size_t msg_len, unsigned char *tree,
assert_sha1_type(tree, OBJ_TREE);
+ if (strlen(msg) < msg_len)
+ die(_("cannot commit with NUL in commit message"));
+
/* Not having i18n.commitencoding is the same as having utf-8 */
encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index 1f62c15..d48a7c0 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -34,6 +34,12 @@ test_expect_success 'no encoding header for base case' '
test z = "z$E"
'
+test_expect_failure 'UTF-16 refused because of NULs' '
+ echo UTF-16 >F &&
+ git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
+'
+
+
for H in ISO8859-1 eucJP ISO-2022-JP
do
test_expect_success "$H setup" '
diff --git a/t/t3900/UTF-16.txt b/t/t3900/UTF-16.txt
new file mode 100644
index 0000000000000000000000000000000000000000..53296be684253f40964c0604be7fa7ff12e200cb
GIT binary patch
literal 32
mcmezOpWz6@X@-jo=NYasZ~@^#h9rjP3@HpR7}6Nh8Mpw;r3yp<
literal 0
HcmV?d00001
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Auto update submodules after merge and reset
From: Andreas T.Auer @ 2011-12-13 9:45 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Phil Hord, git
In-Reply-To: <4EE682A3.8070704@web.de>
On 12.12.2011 23:39 Jens Lehmann wrote:
> Am 11.12.2011 22:15, schrieb Andreas T.Auer:
> >
> > In my use case the branches on the submodules follow the
> > superproject and (mostly) versions that are committed there, it
> > just adds the possibility to keep on working without checking out
> > a branch after an update and without colliding with existing
> > branchnames in the submodule.
>
> Using superproject branch names in the submodules make no sense for a
> lot of use cases.
The most useful workflows for some use cases make no sense for a lot of
other use cases. That is why configuration options are useful, right?
There is no one-size-fits-all. It surely won't make sense for
independent submodules.
> > The other use case wants to follow the commits of that other
> > submodule without checking the corresponding gitlinks into the
> > superproject. But wouldn't it also make sense here to define
> > actually a mapping in the .gitmodule that says "if the branch
> > 'develop' is checkedout in the supermodule then with every
> > submodule update automatically pull the newest 'unstable' commit
> > from the submodule"? Or for "master" follow "stable" or for the
> > "maint" branch follow updates in the "bugfixes" branch.
> >
> > For example
> >
> > [submodule "commonlib"] update = heads develop:unstable
> > master:stable maint:bugfixes
>
> Having that configured with "branch=unstable", "branch=stable" etc.
> in .gitmodules on the superprojects branches would be simpler and
> achieve the same functionality.
Yes, this has been my first thought also, but there is also a good point
to keep the .gitmodules stable, or you always have to change the file
when branches change their names. So when the maint branch of version
1.3 become an archive branch and the new maint is on 1.4, which was the
master before then you have to change the .gitmodules on these branches.
I.e. .gitmodules of 1.4 have "stable" and must have "bugfixes" now and
.gitmodules of 1.3 has "bugfixes" and must remove the floating
completely. I'm not sure that this can always be solved with "easy"
merging and therefore it is probably not really simpler, when you have
to do this for every new release. Or what do you think?
> > So whenever a defined branch is checked out in the superproject
> > the mapped branch will be checked out in the submodule ("new"
> > commit), but if a (e.g. tagged) commit is checked out ("old"
> > commit) then the gitlink in the superproject is used to check out
> > the referenced commit in the submodule.
>
> I think checkout should only use the submodule commit recorded in the
> superproject and a subsequent "git submodule update" should be needed
> to update the submodule to tip. Otherwise you record SHA-1 but still
> won't be able to bisect ...
bisect would leave the branch and therefore uses the recorded SHA1 for
the submodule checkout instead of the tip. "follow-the-tip" should only
work if the superproject follows the tip.
If you configure auto-update on checkout you would not expect that a
separate git submodule update has a different behavior.
> > In http://thread.gmane.org/gmane.comp.version-control.git/183837
> > was discussed whether the gitlink in the superproject should be
> > set to all-zero if updates follow the tip or maybe use the SHA1 of
> > the commit when the submodule was added. I think the gitlink should
> > be updated everytime when a new commit in the superproject is
> > created.
>
> Nope, only when "git submodule update" is run. Otherwise you'll
> spray the history with submodule updates totally unrelated to the
> commits in the superproject, which is rather confusing.
Of course, committing a new version to the superproject should not
trigger pulling in a new version for the submodule or an automatic jump
to the tip of the submodule. I just meant a normal manual "commit -a"
behavior. Putting a 0{40} hash in the gitlink or only the hash of the
submodule, when it first was added would be a special treatment that is
neither needed nor wanted.
^ permalink raw reply
* Re: [PATCH v4 2/2] push: teach --recurse-submodules the on-demand option
From: Jens Lehmann @ 2011-12-13 8:48 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, Heiko Voigt, Fredrik Gustafsson, git
In-Reply-To: <CABURp0qkKXCW-U=78OpnejdtdpphhJtOoDubz77m7Gt3o5sC=Q@mail.gmail.com>
Am 13.12.2011 00:50, schrieb Phil Hord:
> On Mon, Dec 12, 2011 at 5:29 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 12.12.2011 22:16, schrieb Phil Hord:
>>> I thought-experimented with this a bit last year and came to the
>>> conclusion that I should be able to 'float' to tips (developer
>>> convenience) and also to store the SHA-1 of each gitlink through
>>> history (automated maybe; as-needed).
>>
>> Which means that after "git submodule update" floated a submodule branch
>> further, you would have to commit that in the superproject.
>
> Sadly, yes. Currently I have my CI-server do this for me after it
> verifies each new submodule commit is able to build successfully.
Which I think is a good thing to do, as you have a good chance of
catching breakage introduced by the submodule updates. "float-only"
submodules won't always be a pleasant experience, as they can (and
sometimes will) get you into trouble when advancing them introduces
bugs (and then you can't even bisect that breakage).
>>> The problem with "float + gitlinks", of course, is that it looks like
>>> "not floating" to the developers (git-status is dirty unless
>>> overridden, etc.)
>>
>> Yeah. But what if each "git submodule update" would update the tip of
>> the submodule branch and add that to the superproject? You could follow
>> a tip but still produce reproducible trees.
>
> Yes, and that's what I want.
>
> Not what it sounded like was being suggested before, which (to my
> eyes) implied that the submodule gitlinks were useless noise.
It was suggested in other threads in the past. For a start, you could
write a script doing that and play around with it. And if that works
well for you, we can discuss if implementing that functionality into
"git submodule update" makes sense.
^ permalink raw reply
* Re: [PATCH v3 0/3] grep attributes and multithreading
From: Thomas Rast @ 2011-12-13 8:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, René Scharfe, Jeff King, Eric Herman
In-Reply-To: <7vsjkpz763.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Thanks; sign-off?
Ooops, not again!
> [PATCH v3 1/3] grep: load funcname patterns for -W
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: Auto update submodules after merge and reset
From: Jens Lehmann @ 2011-12-13 7:52 UTC (permalink / raw)
To: Phil Hord; +Cc: Andreas T.Auer, git
In-Reply-To: <CABURp0r37+VHBVVKepHPC4jwa-wJ0b+qwLrhhFR8KXnMQYTT3w@mail.gmail.com>
Am 13.12.2011 00:43, schrieb Phil Hord:
> On Mon, Dec 12, 2011 at 5:39 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 11.12.2011 22:15, schrieb Andreas T.Auer:
>>> In http://thread.gmane.org/gmane.comp.version-control.git/183837 was discussed whether the gitlink in the superproject should be set to all-zero if updates follow the tip or maybe use the SHA1 of the commit when the submodule was added. I think the gitlink should be updated everytime when a new commit in the superproject is created.
>>
>> Nope, only when "git submodule update" is run. Otherwise you'll spray the
>> history with submodule updates totally unrelated to the commits in the
>> superproject, which is rather confusing.
>
> And this is why my superproject is a makefile, a .gitmodules file and
> a bunch of gitlinks. We only use it to track the advancement of
> submodule activity.
Which is fine. Did you think about having a branch where only the
submodules are updated (and built and tested) and committed to by a
buildbot when everything is fine? You could then merge that branch
whenever you want up-to-date submodules and have the reproducibility
of the exact model while being able to "float" along the updates of
that branch?
I think it always boils down to this: Either commit new gitlinks, so
the submodules get updated in a reproducible manner, or don't use
gitlinks at all so the submodules can float wherever they want.
> So yes, I want my superproject's gitlinks to update automatically. I
> just don't know a smart way to make that happen.
Yup, that has been the endpoint of all discussions about that topic
so far. And until someone comes up with a smart way to make that
happen, I would rather not put something half-baked into git.
(But please tell us when you found a smart way to do that! ;-)
^ permalink raw reply
* Re: [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir
From: Junio C Hamano @ 2011-12-13 6:37 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <4EE6E61F.8080405@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Apropos testing, it is unsettling that our test suite doesn't show any
> failures after my changes. The dir entries in extra_refs are now always
> sorted and de-duped when do_for_each_ref() is called. Could it be that
> duplicate ".have" references never come up in the test suite? It sounds
> like an important code path is not being tested. In particular, I won't
> be able to test whether my fix works :-/
I doubt anybody thought of using more than one --reference while cloning
or having more than one entry in $GIT_DIR/objects/info/alternates in a
repository that is being pushed into, even though we might have tests for
simpler single --reference and single alternate cases.
As to the order of the changes, my gut feeling is that it would make it
harder to debug your series to delay the removal of "extra_ref" hackery,
as it would be a corner case that your "hierarchical" structure never has
to worry about in the end result.
Another possibility is to keep the extra_ref interface in refs.c, without
any of your changes (i.e. keep it just as a flat list, with the original
interface to append to it without any dedup and other fancy features) and
also keep the special casing of it in for_each_ref(). AFAIK, that is the
only iterator that should care about the extra refs. Thinking about it a
bit more, removal of add_extra_ref() API may be unnecessarily complex with
no real gain. For example, extra refs added in builtin/clone.c is used by
builtin/fetch-pack.c, meaning that the codepath that discovers and
remembers these extra history anchor points and the codepath that uses
them while walking the history are not localized and we would need some
sort of "extra ref API" anyway. I am leaning towards this option.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox