* Re: subdirectory-filter does not delete files before the directory came into existence?
From: Thomas Rast @ 2010-12-19 2:23 UTC (permalink / raw)
To: Jan Wielemaker; +Cc: git
In-Reply-To: <1292415733.11607.51.camel@hppc323.few.vu.nl>
Jan Wielemaker wrote:
> The reported problems also apply to the next module. What appears to
> work is this:
>
> * Walk through the history, finding the commit where the directory
> is created.
> * use git tag -l --contains <commit that created dir> to get the
> tags we want to keep.
> * get all tags, use comm and delete the tags not in the `contained'
> set above.
>
> Not very friendly and I'm (with Thomas) about the status of these
> findings. I like to thank Thomas for giving me the right clue.
Now I finally remember where I knew this problem from:
http://article.gmane.org/gmane.comp.version-control.git/91708
(My memory really sucks.)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH v2 3/4] userdiff: fix typo in ruby and python word regexes
From: Thomas Rast @ 2010-12-19 2:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Johnson, Michael J Gruber, Matthijs Kooijman, git
In-Reply-To: <7vr5deg5fp.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
> > Both had an unclosed ] that ruined the safeguard against not matching
> > a non-space char.
>
> Thanks.
>
> Couldn't we have found this without your "sanity check" patch? Are we
> ignoring error returns from regcomp() in some codepath, or is it just that
> we are catching them but our test suite lacks ruby and python test
> vectors?
We lacked test vectors, but we still couldn't have caught it. We do
check for errors in regcomp():
if (o->word_regex) {
ecbdata.diff_words->word_regex = (regex_t *)
xmalloc(sizeof(regex_t));
if (regcomp(ecbdata.diff_words->word_regex,
o->word_regex,
REG_EXTENDED | REG_NEWLINE))
die ("Invalid regular expression: %s",
o->word_regex);
}
(Now that I'm seeing this and comparing with regcomp(3), we should
actually report regerror() as part of the error message.)
The problem is that the pattern is still valid. Consider that it was
a final two arms to the regex:
- "|[^[:space:]|[\x80-\xff]+"),
+ "|[^[:space:]]|[\x80-\xff]+"),
In the preimage, it parses like so:
| [^
[:space:]|[\x80-\xff
]+
That is, the third [ is part of the (negated) character class. So the
only problem is with | or [ characters in the input. Any other
non-space character is part of the class.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH v2 2/4] diff.c: implement a sanity check for word regexes
From: Thomas Rast @ 2010-12-19 1:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Johnson, Michael J Gruber, Matthijs Kooijman, git
In-Reply-To: <7vvd2qg5jj.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
> > * The word regex matches anything that is !isspace().
> >
> > * The word regex does not match '\n'. (This case is not very harmful,
> > but we used to silently cut off at the '\n' which may go against
> > user expectations.)
>
> How expensive to run this check twice, every time word_regex finds a
> match?
It runs the first bullet point for every non-match, and the second
bullet point for every match. So it looks at every input character
exactly once.
> As this is about making sure that we got a sane regex from the user (or a
> builtin pattern), I wonder if we can make it not depend on the payload we
> are matching the regex against. Then before using a word_regex that we
> have not checked, we check if that regex is sane, mark it checked, and do
> not have to do the check over and over again.
Algorithmically it should be easy once you have the finite state
automaton corresponding to the regex: just verify that for every
possible non-terminal state, there is a transition for every
!isspace() character to a state other than "fail to match" or "match
the empty string".
In the implementation, it might be doable if we switch to compat/regex
on all platforms, since we then have ready access to all internal
structures regcomp() creates, including the DFA.
I'll think about at least using compat/regex for a static check of all
*builtin* patterns, which would be superior to the brute force
approach in 4/4.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] builtin/rm.c: Use ALLOC_GROW instead of alloc_nr and xrealloc.
From: Thiago Farina @ 2010-12-18 23:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder
In-Reply-To: <ffd7b63f3ef5e409775a0d730691efdaf4e41592.1292291262.git.tfransosi@gmail.com>
On Mon, Dec 13, 2010 at 11:48 PM, Thiago Farina <tfransosi@gmail.com> wrote:
> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
> ---
> builtin/rm.c | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index c7b7bb3..faeedfc 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -22,10 +22,7 @@ static struct {
>
> static void add_list(const char *name)
> {
> - if (list.nr >= list.alloc) {
> - list.alloc = alloc_nr(list.alloc);
> - list.name = xrealloc(list.name, list.alloc * sizeof(const char *));
> - }
> + ALLOC_GROW(list.name, list.nr + 1, list.alloc);
> list.name[list.nr++] = name;
> }
>
+Jonathan, he might Ack this.
^ permalink raw reply
* smudge/clean filter needs filename
From: Pete Wyckoff @ 2010-12-18 22:38 UTC (permalink / raw)
To: git
I'm using git-p4 to import and work with upstream p4
repositories. Some of the files are ktext, meaning they expect
expansion of $Id$ and similar identifiers.
Using the filter driver for this file, I can do the "clean" part
easily, but to calculate the "smudge" correctly, I need to know
the filename inside the filter driver.
E.g., inside file foo/Makefile, the clean line:
# $File$
should be smudged into:
# $File: //depot/project/foo/Makefile $
I know the //depot/project location from context in the commit
log message that git-p4 produces. But I don't know the pathname
in the git repo that my smudge script is working on.
Would it make sense to pass that on the command line? E.g.
[filter "p4"]
clean = git-p4smudge --clean %s
smudge = git-p4smudge --smudge %s
Or maybe put the path in an environment variable?
-- Pete
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #05; Thu, 16)
From: Ramsay Jones @ 2010-12-18 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4j8kfwy.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
>
> --------------------------------------------------
> [New Topics]
>
> * rj/maint-difftool-cygwin-workaround (2010-12-14) 1 commit
> - difftool: Fix failure on Cygwin
>
> * rj/maint-test-fixes (2010-12-14) 5 commits
> - t9501-*.sh: Fix a test failure on Cygwin
> - lib-git-svn.sh: Add check for mis-configured web server variables
This lib-git-svn.sh patch (above) has a quite serious fault :(
I'm very annoyed with myself, because I was trying so hard to be
conservative and safe! :-P
The original branch containing these lib-git-svn.sh patches (#3-5 in the
original series) had another commit with the highly informative commit
message: WIP. I could not remember what this commit was about, or if it
was complete (but I thought not), so I didn't send it along with the
others ... Well, this evening I remembered what it was about, along
with the fact that it fixed a problem with the above and that I had
intended to squash all of these patches into one.
The problem with the above is this: because lib-git-svn.sh is sourced
by 57 test files, only four of which even remotely care about using
apache, if SVN_HTTPD_PORT is set and your apache install cannot be
found, then *all* 57 tests will be skipped.
Sorry about that ...
I've added a diff of the additional commit below (well, I changed a
couple of "test ! -e" to "test ! -f", given your comment on patch
04/14) so you can see that the fix will involve moving the code used
to check your apache installation into a separate function which will
only be called from start_httpd; ie only tests that want to use the
web-server will execute this code.
I'll send a proper patch soon. Again, I must apologize for messing up!
ATB,
Ramsay Jones
--- >8 ---
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 5acc0ec..d48fe6b 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -68,8 +68,7 @@ svn_cmd () {
svn "$orig_svncmd" --config-dir "$svnconf" "$@"
}
-if test -n "$SVN_HTTPD_PORT"
-then
+prepare_httpd () {
for d in \
"$SVN_HTTPD_PATH" \
/usr/sbin/apache2 \
@@ -83,8 +82,8 @@ then
done
if test -z "$SVN_HTTPD_PATH"
then
- skip_all='skipping git svn tests, Apache not found'
- test_done
+ echo >&2 'Apache not found'
+ return 1
fi
for d in \
"$SVN_HTTPD_MODULE_PATH" \
@@ -99,10 +98,15 @@ then
done
if test -z "$SVN_HTTPD_MODULE_PATH"
then
- skip_all='skipping git svn tests, Apache module dir not found'
- test_done
+ echo >&2 'Apache module dir not found'
+ return 1
fi
-fi
+ if test ! -f "$SVN_HTTPD_MODULE_PATH/mod_dav_svn.so"
+ then
+ echo >&2 'Apache module "mod_dav_svn.so" not found'
+ return 1
+ fi
+}
start_httpd () {
repo_base_path="$1"
@@ -111,11 +115,9 @@ start_httpd () {
echo >&2 'SVN_HTTPD_PORT is not defined!'
return
fi
- if test ! -e "$SVN_HTTPD_MODULE_PATH/mod_dav_svn.so"
- then
- echo >&2 'Apache module "mod_dav_svn.so" not found'
- return 1
- fi
+
+ prepare_httpd || return 1
+
if test -z "$repo_base_path"
then
repo_base_path=svn
@@ -143,7 +145,7 @@ EOF
stop_httpd () {
test -z "$SVN_HTTPD_PORT" && return
- test ! -e "$GIT_DIR/httpd.conf" && return
+ test ! -f "$GIT_DIR/httpd.conf" && return
"$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k stop
}
--- 8< ---
^ permalink raw reply related
* Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
From: Ramsay Jones @ 2010-12-18 20:41 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, jrnieder, vmiklos
In-Reply-To: <201012172246.45038.j6t@kdbg.org>
Johannes Sixt wrote:
> On Donnerstag, 16. Dezember 2010, Ramsay Jones wrote:
>> Johannes Sixt wrote:
>>> On Dienstag, 14. Dezember 2010, Ramsay Jones wrote:
>>>> @@ -126,7 +126,10 @@ static int is_executable(const char *name)
>>>> !S_ISREG(st.st_mode))
>>>> return 0;
>>>>
>>>> -#ifdef WIN32
>>>> +#if defined(WIN32) || defined(__CYGWIN__)
>>>> +#if defined(__CYGWIN__)
>>>> +if ((st.st_mode & S_IXUSR) == 0)
>>>> +#endif
>>>> { /* cannot trust the executable bit, peek into the file instead */
>>>> char buf[3] = { 0 };
>>>> int n;
>>> Do you gain a lot by this extra condition? Wouldn't
>>>
>>> -#ifdef WIN32
>>> +#if defined(WIN32) || defined(__CYGWIN__)
>>>
>>> be sufficient?
>> Yes, that would be sufficient. No, I probably don't gain a great deal
>> (but I have *not* timed it), since the number of files that are tested
>> by is_executable() is fairly low anyway since they are already filtered
>> by a filename prefix (eg. git-merge-).
>>
>> However, if the executable bit is set, then executing the WIN32 code
>> block is wasted effort (we already know the answer), so why bother?
>
> It would have made to code a bit easier to read with one less #if defined(),
> but it's not a big deal.
Yep, I did consider this:
-#idef WIN32
+#if defined(WIN32) || defined(__CYGWIN__)
+if ((st.st_mode & S_IXUSR) == 0)
but chickened out! ;-) (No, I didn't even test it)
I decided to be conservative here ...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 07/14] t7800-difftool.sh: Fix a test failure on Cygwin
From: Ramsay Jones @ 2010-12-18 20:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Aguilar, GIT Mailing-list
In-Reply-To: <7vwrn8i31n.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>>> diff --git a/git-difftool.perl b/git-difftool.perl
>>> index e95e4ad..ced1615 100755
>>> --- a/git-difftool.perl
>>> +++ b/git-difftool.perl
>>> @@ -52,6 +52,7 @@ sub generate_command
>>> my @command = (exe('git'), 'diff');
>>> my $skip_next = 0;
>>> my $idx = -1;
>>> + my $prompt = '';
>> Would it be simpler to set $prompt = 1 and then
>> flip it to 0 when -y | or --no-prompt is supplied?
>> ...
>>
>> This would become:
>>
>> if ($prompt) {
>> ...
>> }
>> else {
>> ...
>> }
>
> But that is not what the patch does, is it?
>
> Isn't it more like initializing $prompt to undef and then the above
> condition becomes:
>
> if (!defined $prompt) {
> ; # nothing
> elsif ($prompt) {
> setenv PROMPT true
> } else {
> setenv NO_PROMPT true
> }
>
> no?
Yes. :-P
At least that is what I was aiming for.
In particular, if none of -y, --no-prompt or --prompt is passed on
the command-line, I don't want to set either key in the hash.
(Or, to put it another way, I don't want to change the behaviour
in this case, where neither variable is set in the environment
of the helper script)
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 11/14] t3032-*.sh: Pass the -b (--binary) option to sed on cygwin
From: Ramsay Jones @ 2010-12-18 20:16 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Eric Sunshine, Junio C Hamano, GIT Mailing-list, Pat Thoyts
In-Reply-To: <4D0B126A.8080702@viscovery.net>
Johannes Sixt wrote:
>> [I thought I was unusual in having such an old sed version, but
>> apparently not... ;-) ]
>
> As far as I'm concerned, I'm not married to this old version, and I'll
> update to a recent msysgit/MinGW environment RSN. So, in the long run,
> your setup might turn out to be unusal ;-)
Yeah, I've been meaning to upgrade too. (However, I've been saying that
for quite some time now!)
[I had to install the Perl CGI module by hand to get one test to run.
(There is something funny going on with the Config, ExtUtils and Test
modules as well!)]
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 0/14] misc test-suite patches
From: Ramsay Jones @ 2010-12-18 20:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
In-Reply-To: <7v4oagt9zu.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
>> [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
>> [PATCH 09/14] t1301-*.sh: Fix the 'forced modes' test on cygwin
>
> I sense that there is something fundamentally wrong with the tricky l/stat
> emulation set-up code there, but I lack an expertise to judge if these two
> patches is going in the right direction. Help from people savvier on
> Windows issues than I is very much appreciated.
Just as a matter of interest, since I put some extra effort into writing
them, did you find the commit messages to be insufficient/unhelpful/confusing?
What could I have done better?
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] gitweb: Include links to feeds in HTML header only for '200 OK' response
From: J.H. @ 2010-12-18 21:53 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Junio C Hamano
In-Reply-To: <201012182248.40731.jnareb@gmail.com>
On 12/18/2010 01:48 PM, Jakub Narebski wrote:
> J.H. wrote:
>
>> I've no objection, you can add a sign-off from me.
>
> Errr... sign-off or ack? Signed-off-by is about provenance of code...
Considering this has already been running on kernel.org - sign-off.
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=commit;h=41d9c63ad27a8d3c95bf8bb1ec5876483d39fbd6
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [PATCH] gitweb: Include links to feeds in HTML header only for '200 OK' response
From: Jakub Narebski @ 2010-12-18 21:48 UTC (permalink / raw)
To: J.H.; +Cc: git, Junio C Hamano
In-Reply-To: <4D0D1529.6030806@eaglescrag.net>
J.H. wrote:
> I've no objection, you can add a sign-off from me.
Errr... sign-off or ack? Signed-off-by is about provenance of code...
> On 12/18/2010 12:02 PM, Jakub Narebski wrote:
> > To do that, generating "<link />"s to feeds were refactored into
> > print_feed_meta() subroutine, to keep nesting (indent) level in
> > git_header_html() low. This has also the advantage of making code
> > more clear.
> >
> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> > ---
> > gitweb/gitweb.perl | 89 +++++++++++++++++++++++++++-------------------------
> > 1 files changed, 47 insertions(+), 42 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index d521c93..d965cda 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -3479,6 +3479,51 @@ sub get_page_title {
> > return $title;
> > }
> >
> > +sub print_feed_meta {
> > + if (defined $project) {
> > + my %href_params = get_feed_info();
> > + if (!exists $href_params{'-title'}) {
> > + $href_params{'-title'} = 'log';
> > + }
> > +
> > + foreach my $format qw(RSS Atom) {
> > + my $type = lc($format);
> > + my %link_attr = (
> > + '-rel' => 'alternate',
> > + '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
> > + '-type' => "application/$type+xml"
> > + );
> > +
> > + $href_params{'action'} = $type;
> > + $link_attr{'-href'} = href(%href_params);
> > + print "<link ".
> > + "rel=\"$link_attr{'-rel'}\" ".
> > + "title=\"$link_attr{'-title'}\" ".
> > + "href=\"$link_attr{'-href'}\" ".
> > + "type=\"$link_attr{'-type'}\" ".
> > + "/>\n";
> > +
> > + $href_params{'extra_options'} = '--no-merges';
> > + $link_attr{'-href'} = href(%href_params);
> > + $link_attr{'-title'} .= ' (no merges)';
> > + print "<link ".
> > + "rel=\"$link_attr{'-rel'}\" ".
> > + "title=\"$link_attr{'-title'}\" ".
> > + "href=\"$link_attr{'-href'}\" ".
> > + "type=\"$link_attr{'-type'}\" ".
> > + "/>\n";
> > + }
> > +
> > + } else {
> > + printf('<link rel="alternate" title="%s projects list" '.
> > + 'href="%s" type="text/plain; charset=utf-8" />'."\n",
> > + esc_attr($site_name), href(project=>undef, action=>"project_index"));
> > + printf('<link rel="alternate" title="%s projects feeds" '.
> > + 'href="%s" type="text/x-opml" />'."\n",
> > + esc_attr($site_name), href(project=>undef, action=>"opml"));
> > + }
> > +}
> > +
> > sub git_header_html {
> > my $status = shift || "200 OK";
> > my $expires = shift;
> > @@ -3528,48 +3573,8 @@ EOF
> > print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
> > }
> > }
> > - if (defined $project) {
> > - my %href_params = get_feed_info();
> > - if (!exists $href_params{'-title'}) {
> > - $href_params{'-title'} = 'log';
> > - }
> > -
> > - foreach my $format qw(RSS Atom) {
> > - my $type = lc($format);
> > - my %link_attr = (
> > - '-rel' => 'alternate',
> > - '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
> > - '-type' => "application/$type+xml"
> > - );
> > -
> > - $href_params{'action'} = $type;
> > - $link_attr{'-href'} = href(%href_params);
> > - print "<link ".
> > - "rel=\"$link_attr{'-rel'}\" ".
> > - "title=\"$link_attr{'-title'}\" ".
> > - "href=\"$link_attr{'-href'}\" ".
> > - "type=\"$link_attr{'-type'}\" ".
> > - "/>\n";
> > -
> > - $href_params{'extra_options'} = '--no-merges';
> > - $link_attr{'-href'} = href(%href_params);
> > - $link_attr{'-title'} .= ' (no merges)';
> > - print "<link ".
> > - "rel=\"$link_attr{'-rel'}\" ".
> > - "title=\"$link_attr{'-title'}\" ".
> > - "href=\"$link_attr{'-href'}\" ".
> > - "type=\"$link_attr{'-type'}\" ".
> > - "/>\n";
> > - }
> > -
> > - } else {
> > - printf('<link rel="alternate" title="%s projects list" '.
> > - 'href="%s" type="text/plain; charset=utf-8" />'."\n",
> > - esc_attr($site_name), href(project=>undef, action=>"project_index"));
> > - printf('<link rel="alternate" title="%s projects feeds" '.
> > - 'href="%s" type="text/x-opml" />'."\n",
> > - esc_attr($site_name), href(project=>undef, action=>"opml"));
> > - }
> > + print_feed_meta()
> > + if ($status eq '200 OK');
> > if (defined $favicon) {
> > print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
> > }
>
>
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Arnout Engelen @ 2010-12-18 21:28 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20101218200516.GA10031@burratino>
Improve error messages when creating a temporary file fails.
Before, when creating a temporary file failed, a generic 'Unable to create
temporary file' message was printed. In some cases this could lead to
confusion as to which directory should be checked for correct permissions etc.
This patch adds the template for the temporary filename to the error message,
converting it to an absolute path if needed. A test verifies that the template
is indeed printed when pointing to a nonexistent or unwritable directory.
(a copy of the original template is made in case mkstemp clears the template)
Signed-off-by: Arnout Engelen <arnouten@bzzt.net>
---
The use of a fixed-length buffer to temporarily store the template
for the error path is now sort of moot since make_nonrelative_path doesn't
allow filenames larger than PATH_MAX anyway.
Some more tweaks based on Jonathan's feedback: xstrdup, #include of
git-compat-util.h in the unittest and for whitespace around the '*' in
pointer declarations.
Makefile | 1 +
t/t0070-fundamental.sh | 13 +++++++++++++
test-mktemp.c | 14 ++++++++++++++
wrapper.c | 32 ++++++++++++++++++++++++++++----
4 files changed, 56 insertions(+), 4 deletions(-)
create mode 100644 test-mktemp.c
diff --git a/Makefile b/Makefile
index 57d9c65..03a51cb 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,7 @@ TEST_PROGRAMS_NEED_X += test-string-pool
TEST_PROGRAMS_NEED_X += test-svn-fe
TEST_PROGRAMS_NEED_X += test-treap
TEST_PROGRAMS_NEED_X += test-index-version
+TEST_PROGRAMS_NEED_X += test-mktemp
TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 680d7d6..9bee8bf 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -12,4 +12,17 @@ test_expect_success 'character classes (isspace, isalpha etc.)' '
test-ctype
'
+test_expect_success 'mktemp to nonexistent directory prints filename' '
+ test_must_fail test-mktemp doesnotexist/testXXXXXX 2>err &&
+ grep "doesnotexist/test" err
+'
+
+test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '
+ mkdir cannotwrite &&
+ chmod -w cannotwrite &&
+ test_when_finished "chmod +w cannotwrite" &&
+ test_must_fail test-mktemp cannotwrite/testXXXXXX 2>err &&
+ grep "cannotwrite/test" err
+'
+
test_done
diff --git a/test-mktemp.c b/test-mktemp.c
new file mode 100644
index 0000000..c8c5421
--- /dev/null
+++ b/test-mktemp.c
@@ -0,0 +1,14 @@
+/*
+ * test-mktemp.c: code to exercise the creation of temporary files
+ */
+#include "git-compat-util.h"
+
+int main(int argc, char *argv[])
+{
+ if (argc != 2)
+ usage("Expected 1 parameter defining the temporary file template");
+
+ xmkstemp(xstrdup(argv[1]));
+
+ return 0;
+}
diff --git a/wrapper.c b/wrapper.c
index 4c1639f..0c208c2 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -196,10 +196,22 @@ FILE *xfdopen(int fd, const char *mode)
int xmkstemp(char *template)
{
int fd;
+ char origtemplate[PATH_MAX];
+ strlcpy(origtemplate, template, sizeof(origtemplate));
fd = mkstemp(template);
- if (fd < 0)
- die_errno("Unable to create temporary file");
+ if (fd < 0) {
+ int saved_errno = errno;
+ const char *nonrelative_template;
+
+ if (!template[0])
+ template = origtemplate;
+
+ nonrelative_template = make_nonrelative_path(template);
+ errno = saved_errno;
+ die_errno("Unable to create temporary file '%s'",
+ nonrelative_template);
+ }
return fd;
}
@@ -319,10 +331,22 @@ int gitmkstemps(char *pattern, int suffix_len)
int xmkstemp_mode(char *template, int mode)
{
int fd;
+ char origtemplate[PATH_MAX];
+ strlcpy(origtemplate, template, sizeof(origtemplate));
fd = git_mkstemp_mode(template, mode);
- if (fd < 0)
- die_errno("Unable to create temporary file");
+ if (fd < 0) {
+ int saved_errno = errno;
+ const char *nonrelative_template;
+
+ if (!template[0])
+ template = origtemplate;
+
+ nonrelative_template = make_nonrelative_path(template);
+ errno = saved_errno;
+ die_errno("Unable to create temporary file '%s'",
+ nonrelative_template);
+ }
return fd;
}
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH v2 3/4] userdiff: fix typo in ruby and python word regexes
From: Junio C Hamano @ 2010-12-18 21:02 UTC (permalink / raw)
To: Thomas Rast
Cc: Junio C Hamano, Scott Johnson, Michael J Gruber,
Matthijs Kooijman, git
In-Reply-To: <a358c476187b1220bf6e2368c0723cefe60a5763.1292688058.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Both had an unclosed ] that ruined the safeguard against not matching
> a non-space char.
Thanks.
Couldn't we have found this without your "sanity check" patch? Are we
ignoring error returns from regcomp() in some codepath, or is it just that
we are catching them but our test suite lacks ruby and python test
vectors?
^ permalink raw reply
* Re: [PATCH v2 2/4] diff.c: implement a sanity check for word regexes
From: Junio C Hamano @ 2010-12-18 21:00 UTC (permalink / raw)
To: Thomas Rast; +Cc: Scott Johnson, Michael J Gruber, Matthijs Kooijman, git
In-Reply-To: <ee3026bd997fc6d8508b8e5617e572f99c8bf3d6.1292688058.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Word regexes are a bit of a dangerous beast, since it is easily
> possible to not match a non-space part, which is subsequently ignored
> for the purposes of emitting the word diff. This was clearly stated
> in the docs, but users still tripped over it.
>
> Implement a safeguard that verifies two basic sanity assumptions:
>
> * The word regex matches anything that is !isspace().
>
> * The word regex does not match '\n'. (This case is not very harmful,
> but we used to silently cut off at the '\n' which may go against
> user expectations.)
>
> This is configurable via 'diff.wordRegexCheck', and defaults to
> 'warn'.
How expensive to run this check twice, every time word_regex finds a
match?
As this is about making sure that we got a sane regex from the user (or a
builtin pattern), I wonder if we can make it not depend on the payload we
are matching the regex against. Then before using a word_regex that we
have not checked, we check if that regex is sane, mark it checked, and do
not have to do the check over and over again.
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Kirill Smelkov @ 2010-12-18 20:55 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, git, Axel Bonnet, Cl??ment Poulain,
Diane Gasselin
In-Reply-To: <20101218161337.GB18643@sigill.intra.peff.net>
On Sat, Dec 18, 2010 at 11:13:37AM -0500, Jeff King wrote:
> On Sat, Dec 18, 2010 at 05:54:12PM +0300, Kirill Smelkov wrote:
>
> > It turned out, under blame there are requests to fill_textconv() with
> > sha1=0000000000000000000000000000000000000000 and sha1_valid=0.
> >
> > As the code did not analyzed sha1 validity, we ended up putting 000000
> > into textconv cache which was fooling later blames to discover lots of
> > lines in 'Not Yet Committed' state.
> > [...]
> > - if (driver->textconv_cache) {
> > + if (driver->textconv_cache && df->sha1_valid) {
> > *outbuf = notes_cache_get(driver->textconv_cache, df->sha1,
> > &size);
>
> In short:
>
> Acked-by: Jeff King <peff@peff.net>
>
> But it took some thinking to convince myself, so the long answer is
> below if anyone cares.
>
> I was dubious at first that this could be the right solution. We still
> end up putting the filespec through run_textconv, which didn't seem
> right if it is not valid.
>
> But reading into it more, there are two levels of invalidity:
>
> 1. !DIFF_FILE_VALID(df) - we are not a valid file at all. I.e., we are
> /dev/null.
>
> 2. !df->sha1_valid - we are pointing to a working tree file whose sha1
> we don't know
>
> I think level (2) never happens at all in the regular diff code, which
> is why this case was completely unhandled. But it is OK in that case
> (required, even) to put the contents through run_textconv.
>
> In theory we could actually calculate the sha1 in case (2) and cache
> under that, but I don't know how much it would buy us in practice. It
> saves us running the textconv filter at the expense of computing the
> sha1. Which is probably a win for most filters, but on the other hand,
> it is the wrong place to compute such a sha1. If it is a working tree
> file, we should ideally update our stat info in the index so that the
> info can be reused.
Jeff,
Thanks for your ACK and for the explanation.
My last patches to git were blame related so semi-intuitively I knew
that invalid sha1 are coming from files in worktree. Your description
makes things much more clear and I'd put it into patch log as well.
What is the best practice for this? For me to re-roll, or for Junio to
merge texts?
Also experimenting shows that, as you say, regular diff does not have this
problem, and also that diff calculates sha1 for files in worktree and so
their textconv results are cached. So clearly, there are some behaviour
differences between diff and blame.
Thanks again,
Kirill
^ permalink raw reply
* Re: [PATCH v2 0/4] --word-regex sanity checking and such
From: Junio C Hamano @ 2010-12-18 20:48 UTC (permalink / raw)
To: Thomas Rast; +Cc: Scott Johnson, Michael J Gruber, Matthijs Kooijman, git
In-Reply-To: <201012181724.29916.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> I wrote:
>> diff.c: pass struct diff_words into find_word_boundaries
>> diff.c: implement a sanity check for word regexes
>> userdiff: fix typo in ruby and python word regexes
>> t4034: bulk verify builtin word regex sanity
>
> BTW, Junio, you could move the third patch to the front and merge it
> to maint. I think it's an obvious fix to a real bug, and it does not
> depend on the other patches except that the test in 4/4 will fail
> without the fix.
Makes sense; thanks.
^ permalink raw reply
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Junio C Hamano @ 2010-12-18 20:47 UTC (permalink / raw)
To: Arnout Engelen; +Cc: Jonathan Nieder, git
In-Reply-To: <20101218200516.GA10031@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Hi again,
>
> Arnout Engelen wrote:
>
>> Updated version of the patch
>
> Almost there. Some remaining nits from my pov:
>
>> , taking into account the feedback from this
>> thread.
> [...]
>
> This text above the "---" becomes part of the log message when a patch
> is committed to git.git, so it is best to make it self-contained. The
> usual advice is "describe the current behavior, why the proposed
> behavior is better, and then how the proposed behavior is achieved."
Thanks, Jonathan.
The important part of self-containedness is that people who are reading
the resulting commit and the history that contains it should not have to
unnecessarily refer to outside resources, especially the previous rounds
that weren't satisfactory. The comparison with previous rounds however is
a very valuable resource for reviewers on the mailing list, so don't
remove what you wrote there, but move it below "---" lines.
^ permalink raw reply
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Junio C Hamano @ 2010-12-18 20:47 UTC (permalink / raw)
To: Arnout Engelen; +Cc: Jonathan Nieder, git
In-Reply-To: <20101218200516.GA10031@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Hi again,
>
> Arnout Engelen wrote:
>
>> Updated version of the patch
>
> Almost there. Some remaining nits from my pov:
>
>> , taking into account the feedback from this
>> thread.
> [...]
>
> This text above the "---" becomes part of the log message when a patch
> is committed to git.git, so it is best to make it self-contained. The
> usual advice is "describe the current behavior, why the proposed
> behavior is better, and then how the proposed behavior is achieved."
Thanks, Jonathan.
The important part of self-containedness is that people who are reading
the resulting commit and the history that contains it should not have to
unnecessarily refer to outside resources, especially the previous rounds
that weren't satisfactory. The comparison with previous rounds however is
a very valuable resource for reviewers on the mailing list, so don't
remove what you wrote there, but move it below "---" lines.
^ permalink raw reply
* Re: [PATCHv2 0/8] docs: use metavariables consistently
From: Junio C Hamano @ 2010-12-18 20:27 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Mark Lodato, git
In-Reply-To: <20101218084908.GE6187@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Ah, missed this before. It is not clear to me why this difference ---
> is it just to be conservative about changing behavior?
Yes. It is clear from the context you mean its tree if you feed a commit
to commit-tree, but the original intent of not dereferncing was to avoid
mistakes by scripts and people, so you need to make a convincing argument
that not derefencing does not help avoiding mistakes, and prove that
nobody's script is relying on the current behaviour for type-checking
purposes.
^ permalink raw reply
* Re: [PATCH] gitweb: Include links to feeds in HTML header only for '200 OK' response
From: J.H. @ 2010-12-18 20:10 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Junio C Hamano
In-Reply-To: <20101218195848.16201.67691.stgit@localhost.localdomain>
I've no objection, you can add a sign-off from me.
- John
On 12/18/2010 12:02 PM, Jakub Narebski wrote:
> To do that, generating "<link />"s to feeds were refactored into
> print_feed_meta() subroutine, to keep nesting (indent) level in
> git_header_html() low. This has also the advantage of making code
> more clear.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 89 +++++++++++++++++++++++++++-------------------------
> 1 files changed, 47 insertions(+), 42 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index d521c93..d965cda 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3479,6 +3479,51 @@ sub get_page_title {
> return $title;
> }
>
> +sub print_feed_meta {
> + if (defined $project) {
> + my %href_params = get_feed_info();
> + if (!exists $href_params{'-title'}) {
> + $href_params{'-title'} = 'log';
> + }
> +
> + foreach my $format qw(RSS Atom) {
> + my $type = lc($format);
> + my %link_attr = (
> + '-rel' => 'alternate',
> + '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
> + '-type' => "application/$type+xml"
> + );
> +
> + $href_params{'action'} = $type;
> + $link_attr{'-href'} = href(%href_params);
> + print "<link ".
> + "rel=\"$link_attr{'-rel'}\" ".
> + "title=\"$link_attr{'-title'}\" ".
> + "href=\"$link_attr{'-href'}\" ".
> + "type=\"$link_attr{'-type'}\" ".
> + "/>\n";
> +
> + $href_params{'extra_options'} = '--no-merges';
> + $link_attr{'-href'} = href(%href_params);
> + $link_attr{'-title'} .= ' (no merges)';
> + print "<link ".
> + "rel=\"$link_attr{'-rel'}\" ".
> + "title=\"$link_attr{'-title'}\" ".
> + "href=\"$link_attr{'-href'}\" ".
> + "type=\"$link_attr{'-type'}\" ".
> + "/>\n";
> + }
> +
> + } else {
> + printf('<link rel="alternate" title="%s projects list" '.
> + 'href="%s" type="text/plain; charset=utf-8" />'."\n",
> + esc_attr($site_name), href(project=>undef, action=>"project_index"));
> + printf('<link rel="alternate" title="%s projects feeds" '.
> + 'href="%s" type="text/x-opml" />'."\n",
> + esc_attr($site_name), href(project=>undef, action=>"opml"));
> + }
> +}
> +
> sub git_header_html {
> my $status = shift || "200 OK";
> my $expires = shift;
> @@ -3528,48 +3573,8 @@ EOF
> print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
> }
> }
> - if (defined $project) {
> - my %href_params = get_feed_info();
> - if (!exists $href_params{'-title'}) {
> - $href_params{'-title'} = 'log';
> - }
> -
> - foreach my $format qw(RSS Atom) {
> - my $type = lc($format);
> - my %link_attr = (
> - '-rel' => 'alternate',
> - '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
> - '-type' => "application/$type+xml"
> - );
> -
> - $href_params{'action'} = $type;
> - $link_attr{'-href'} = href(%href_params);
> - print "<link ".
> - "rel=\"$link_attr{'-rel'}\" ".
> - "title=\"$link_attr{'-title'}\" ".
> - "href=\"$link_attr{'-href'}\" ".
> - "type=\"$link_attr{'-type'}\" ".
> - "/>\n";
> -
> - $href_params{'extra_options'} = '--no-merges';
> - $link_attr{'-href'} = href(%href_params);
> - $link_attr{'-title'} .= ' (no merges)';
> - print "<link ".
> - "rel=\"$link_attr{'-rel'}\" ".
> - "title=\"$link_attr{'-title'}\" ".
> - "href=\"$link_attr{'-href'}\" ".
> - "type=\"$link_attr{'-type'}\" ".
> - "/>\n";
> - }
> -
> - } else {
> - printf('<link rel="alternate" title="%s projects list" '.
> - 'href="%s" type="text/plain; charset=utf-8" />'."\n",
> - esc_attr($site_name), href(project=>undef, action=>"project_index"));
> - printf('<link rel="alternate" title="%s projects feeds" '.
> - 'href="%s" type="text/x-opml" />'."\n",
> - esc_attr($site_name), href(project=>undef, action=>"opml"));
> - }
> + print_feed_meta()
> + if ($status eq '200 OK');
> if (defined $favicon) {
> print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
> }
^ permalink raw reply
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Jonathan Nieder @ 2010-12-18 20:05 UTC (permalink / raw)
To: Arnout Engelen; +Cc: git
In-Reply-To: <20101218165514.GT25767@bzzt.net>
Hi again,
Arnout Engelen wrote:
> Updated version of the patch
Almost there. Some remaining nits from my pov:
> , taking into account the feedback from this
> thread.
[...]
This text above the "---" becomes part of the log message when a patch
is committed to git.git, so it is best to make it self-contained. The
usual advice is "describe the current behavior, why the proposed
behavior is better, and then how the proposed behavior is achieved."
> --- /dev/null
> +++ b/test-mktemp.c
> @@ -0,0 +1,15 @@
> +/*
> + * test-mktemp.c: code to exercise the creation of temporary files
> + */
> +#include <string.h>
> +#include "git-compat-util.h"
git-compat-util.h takes care of portably including system headers in
the right order. (For example, #include-ing <string.h> before
setting _POSIX_SOURCE will cause some symbols not to be defined in
_other_ headers on some operating systems, iirc.) I'd suggest
removing the redundant #include <string.h>.
See Documentation/CodingGuidelines.
> +int main(int argc, char *argv[])
> +{
> + if (argc != 2)
> + usage("Expected 1 parameter defining the temporary file template");
> +
> + xmkstemp(strdup(argv[1]));
Why not xstrdup(), which diagnoses allocation failures?
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -196,10 +196,22 @@ FILE *xfdopen(int fd, const char *mode)
> int xmkstemp(char *template)
> {
> int fd;
> + char origtemplate[PATH_MAX];
> + strlcpy(origtemplate, template, sizeof(origtemplate));
>
> fd = mkstemp(template);
> - if (fd < 0)
> - die_errno("Unable to create temporary file");
> + if (fd < 0) {
> + int saved_errno = errno;
> + const char * nonrelative_template;
It would be more usual not to include a space between the '*' and
'nonrelative_template'.
Thanks for your perseverance.
^ permalink raw reply
* [PATCH] gitweb: Include links to feeds in HTML header only for '200 OK' response
From: Jakub Narebski @ 2010-12-18 20:02 UTC (permalink / raw)
To: git; +Cc: J.H., Junio C Hamano
To do that, generating "<link />"s to feeds were refactored into
print_feed_meta() subroutine, to keep nesting (indent) level in
git_header_html() low. This has also the advantage of making code
more clear.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 89 +++++++++++++++++++++++++++-------------------------
1 files changed, 47 insertions(+), 42 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d521c93..d965cda 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3479,6 +3479,51 @@ sub get_page_title {
return $title;
}
+sub print_feed_meta {
+ if (defined $project) {
+ my %href_params = get_feed_info();
+ if (!exists $href_params{'-title'}) {
+ $href_params{'-title'} = 'log';
+ }
+
+ foreach my $format qw(RSS Atom) {
+ my $type = lc($format);
+ my %link_attr = (
+ '-rel' => 'alternate',
+ '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
+ '-type' => "application/$type+xml"
+ );
+
+ $href_params{'action'} = $type;
+ $link_attr{'-href'} = href(%href_params);
+ print "<link ".
+ "rel=\"$link_attr{'-rel'}\" ".
+ "title=\"$link_attr{'-title'}\" ".
+ "href=\"$link_attr{'-href'}\" ".
+ "type=\"$link_attr{'-type'}\" ".
+ "/>\n";
+
+ $href_params{'extra_options'} = '--no-merges';
+ $link_attr{'-href'} = href(%href_params);
+ $link_attr{'-title'} .= ' (no merges)';
+ print "<link ".
+ "rel=\"$link_attr{'-rel'}\" ".
+ "title=\"$link_attr{'-title'}\" ".
+ "href=\"$link_attr{'-href'}\" ".
+ "type=\"$link_attr{'-type'}\" ".
+ "/>\n";
+ }
+
+ } else {
+ printf('<link rel="alternate" title="%s projects list" '.
+ 'href="%s" type="text/plain; charset=utf-8" />'."\n",
+ esc_attr($site_name), href(project=>undef, action=>"project_index"));
+ printf('<link rel="alternate" title="%s projects feeds" '.
+ 'href="%s" type="text/x-opml" />'."\n",
+ esc_attr($site_name), href(project=>undef, action=>"opml"));
+ }
+}
+
sub git_header_html {
my $status = shift || "200 OK";
my $expires = shift;
@@ -3528,48 +3573,8 @@ EOF
print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
}
}
- if (defined $project) {
- my %href_params = get_feed_info();
- if (!exists $href_params{'-title'}) {
- $href_params{'-title'} = 'log';
- }
-
- foreach my $format qw(RSS Atom) {
- my $type = lc($format);
- my %link_attr = (
- '-rel' => 'alternate',
- '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
- '-type' => "application/$type+xml"
- );
-
- $href_params{'action'} = $type;
- $link_attr{'-href'} = href(%href_params);
- print "<link ".
- "rel=\"$link_attr{'-rel'}\" ".
- "title=\"$link_attr{'-title'}\" ".
- "href=\"$link_attr{'-href'}\" ".
- "type=\"$link_attr{'-type'}\" ".
- "/>\n";
-
- $href_params{'extra_options'} = '--no-merges';
- $link_attr{'-href'} = href(%href_params);
- $link_attr{'-title'} .= ' (no merges)';
- print "<link ".
- "rel=\"$link_attr{'-rel'}\" ".
- "title=\"$link_attr{'-title'}\" ".
- "href=\"$link_attr{'-href'}\" ".
- "type=\"$link_attr{'-type'}\" ".
- "/>\n";
- }
-
- } else {
- printf('<link rel="alternate" title="%s projects list" '.
- 'href="%s" type="text/plain; charset=utf-8" />'."\n",
- esc_attr($site_name), href(project=>undef, action=>"project_index"));
- printf('<link rel="alternate" title="%s projects feeds" '.
- 'href="%s" type="text/x-opml" />'."\n",
- esc_attr($site_name), href(project=>undef, action=>"opml"));
- }
+ print_feed_meta()
+ if ($status eq '200 OK');
if (defined $favicon) {
print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
}
^ permalink raw reply related
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Arnout Engelen @ 2010-12-18 16:55 UTC (permalink / raw)
To: git
In-Reply-To: <20101207181633.GF25767@bzzt.net>
Updated version of the patch, taking into account the feedback from this
thread. (the use of a fixed-length buffer to temporarily store the template
for the error path is now sort of moot since make_nonrelative_path doesn't
allow filenames larger than PATH_MAX anyway)
Signed-off-by: Arnout Engelen <arnouten@bzzt.net>
---
Makefile | 1 +
t/t0070-fundamental.sh | 13 +++++++++++++
test-mktemp.c | 15 +++++++++++++++
wrapper.c | 32 ++++++++++++++++++++++++++++----
4 files changed, 57 insertions(+), 4 deletions(-)
create mode 100644 test-mktemp.c
diff --git a/Makefile b/Makefile
index 57d9c65..03a51cb 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,7 @@ TEST_PROGRAMS_NEED_X += test-string-pool
TEST_PROGRAMS_NEED_X += test-svn-fe
TEST_PROGRAMS_NEED_X += test-treap
TEST_PROGRAMS_NEED_X += test-index-version
+TEST_PROGRAMS_NEED_X += test-mktemp
TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 680d7d6..9bee8bf 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -12,4 +12,17 @@ test_expect_success 'character classes (isspace, isalpha etc.)' '
test-ctype
'
+test_expect_success 'mktemp to nonexistent directory prints filename' '
+ test_must_fail test-mktemp doesnotexist/testXXXXXX 2>err &&
+ grep "doesnotexist/test" err
+'
+
+test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '
+ mkdir cannotwrite &&
+ chmod -w cannotwrite &&
+ test_when_finished "chmod +w cannotwrite" &&
+ test_must_fail test-mktemp cannotwrite/testXXXXXX 2>err &&
+ grep "cannotwrite/test" err
+'
+
test_done
diff --git a/test-mktemp.c b/test-mktemp.c
new file mode 100644
index 0000000..00fdd78
--- /dev/null
+++ b/test-mktemp.c
@@ -0,0 +1,15 @@
+/*
+ * test-mktemp.c: code to exercise the creation of temporary files
+ */
+#include <string.h>
+#include "git-compat-util.h"
+
+int main(int argc, char *argv[])
+{
+ if (argc != 2)
+ usage("Expected 1 parameter defining the temporary file template");
+
+ xmkstemp(strdup(argv[1]));
+
+ return 0;
+}
diff --git a/wrapper.c b/wrapper.c
index 4c1639f..9af40ee 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -196,10 +196,22 @@ FILE *xfdopen(int fd, const char *mode)
int xmkstemp(char *template)
{
int fd;
+ char origtemplate[PATH_MAX];
+ strlcpy(origtemplate, template, sizeof(origtemplate));
fd = mkstemp(template);
- if (fd < 0)
- die_errno("Unable to create temporary file");
+ if (fd < 0) {
+ int saved_errno = errno;
+ const char * nonrelative_template;
+
+ if (!template[0])
+ template = origtemplate;
+
+ nonrelative_template = make_nonrelative_path(template);
+ errno = saved_errno;
+ die_errno("Unable to create temporary file '%s'",
+ nonrelative_template);
+ }
return fd;
}
@@ -319,10 +331,22 @@ int gitmkstemps(char *pattern, int suffix_len)
int xmkstemp_mode(char *template, int mode)
{
int fd;
+ char origtemplate[PATH_MAX];
+ strlcpy(origtemplate, template, sizeof(origtemplate));
fd = git_mkstemp_mode(template, mode);
- if (fd < 0)
- die_errno("Unable to create temporary file");
+ if (fd < 0) {
+ int saved_errno = errno;
+ const char * nonrelative_template;
+
+ if (!template[0])
+ template = origtemplate;
+
+ nonrelative_template = make_nonrelative_path(template);
+ errno = saved_errno;
+ die_errno("Unable to create temporary file '%s'",
+ nonrelative_template);
+ }
return fd;
}
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH 3/8] docs: use <sha1> to mean unabbreviated ID
From: Junio C Hamano @ 2010-12-18 18:50 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Mark Lodato, git
In-Reply-To: <20101218074718.GA6187@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Mark Lodato wrote:
>
>> There are some places that literally require a full, 40-character SHA-1
>> ID, rather than a generic revision specifier.
>
> One name I have seen for these is "object IDs", as in git get-tar-commit-id
> (to avoid putting too much emphasis on precisely how the identifiers are
> chosen).
Yes, I think I also saw "a full object name" (cf. diff --full-index) and
evan "object name" (cf. git-pack-objects(1)). These are all deliberate
efforts to stay away from spelling SHA-1 explicitly, and I think it makes
sense to do so here. "40-digit object ID" is a mouthful, though.
As to what this series tries to do, I think it is generally a good idea to
start allowing documents to say <tree> when a reader can plug either a
commit or a tree, rather than <tree-ish>, only because all Porcelains
automatically dereference object names to needed types these days.
But such a change needs to mark places that _must_ take the name of an
object of the specific type somehow. It is Ok to say ls-tree takes a
tree, but then we must say you should feed commit-tree nothing but a tree.
It is a separate issue to allow plumbings to automatically dereference
when both of the following two conditions hold: (1) the intent is clear
from the context; (2) nobody sane would have been relying on the side
effect of the current typechecking.
Letting commit-tree to unwrap a tree-ish to a tree would be a good example
that clearly satisfies (1)---I am not sure about the other one.
^ 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