* Re: Fwd: git status options feature suggestion
From: Jeff King @ 2008-10-12 6:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Johannes Schindelin, Caleb Cushing, git
In-Reply-To: <7vwsgegvsh.fsf@gitster.siamese.dyndns.org>
On Sat, Oct 11, 2008 at 11:41:18PM -0700, Junio C Hamano wrote:
> And just make it mimic whatever folks accustomed to "svn st" would expect,
> modulo we would need two status letters to signal difference between
> (HEAD, index), and (index, worktree). Perhaps three if you want to show
> difference between (HEAD, worktree) while at it.
I remember a long time ago you started on a parallel diff walker that
could diff the working tree, the index, and a tree at once. Do you
remember the issues with it?
I think that would be the right tool here to show each file only once,
but with multiple status flags. Something like:
A M foo
to show that "foo" has been added since the last commit, but there are
modifications in the working tree that have not yet been staged.
-Peff
^ permalink raw reply
* Re: Fwd: git status options feature suggestion
From: Junio C Hamano @ 2008-10-12 6:41 UTC (permalink / raw)
To: Jeff King; +Cc: Michael J Gruber, Johannes Schindelin, Caleb Cushing, git
In-Reply-To: <20081012044900.GA27845@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I think it is probably reasonable to think about a new command (which
> would not be called status) that shows this information.
I was going to suggest the same. "git st" for people who come from "svn st"
so that "git status" can be kept as traditional "preview of 'git commit'".
And just make it mimic whatever folks accustomed to "svn st" would expect,
modulo we would need two status letters to signal difference between
(HEAD, index), and (index, worktree). Perhaps three if you want to show
difference between (HEAD, worktree) while at it.
And no, I have not seen any argument good enough to change ls-files nor
diff-$lowlevel output and break people's existing scripts.
^ permalink raw reply
* Re: git apply --directory broken for new files
From: Junio C Hamano @ 2008-10-12 6:36 UTC (permalink / raw)
To: Jeff King
Cc: H. Peter Anvin, Johannes Schindelin, Shawn O. Pearce,
Git Mailing List
In-Reply-To: <20081012040611.GA11199@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> It would be easier to do this --directory prefixing in the sole caller of
>> git_header_name(), though.
>
> I went this route, and it is much more readable if slightly less
> efficient (one extra malloc/free).
I do not think efficiency is an issue in this codepath --- especially
because it is only triggered when --directory is in use.
Acked-by: Junio C Hamano <gitster@pobox.com>
>
> -- >8 --
> git apply --directory broken for new files
>
> We carefully verify that the input to git-apply is sane,
> including cross-checking that the filenames we see in "+++"
> headers match what was provided on the command line of "diff
> --git". When --directory is used, however, we ended up
> comparing the unadorned name to one with the prepended root,
> causing us to complain about a mismatch.
>
> We simply need to prepend the root directory, if any, when
> pulling the name out of the git header.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> builtin-apply.c | 7 ++++++
> t/t4128-apply-root.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-apply.c b/builtin-apply.c
> index bf80610..be7e1bd 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -810,6 +810,13 @@ static int parse_git_header(char *line, int len, unsigned int size, struct patch
> * the default name from the header.
> */
> patch->def_name = git_header_name(line, len);
> + if (patch->def_name && root) {
> + char *s = xmalloc(root_len + strlen(patch->def_name) + 1);
> + strcpy(s, root);
> + strcpy(s + root_len, patch->def_name);
> + free(patch->def_name);
> + patch->def_name = s;
> + }
>
> line += len;
> size -= len;
> diff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh
> index 2dd0c75..bc7a8a8 100755
> --- a/t/t4128-apply-root.sh
> +++ b/t/t4128-apply-root.sh
> @@ -40,4 +40,56 @@ test_expect_success 'apply --directory -p (2) ' '
>
> '
>
> +cat > patch << EOF
> +diff --git a/newfile b/newfile
> +new file mode 100644
> +index 0000000..d95f3ad
> +--- /dev/null
> ++++ b/newfile
> +@@ -0,0 +1 @@
> ++content
> +EOF
> +
> +test_expect_success 'apply --directory (new file)' '
> + git reset --hard initial &&
> + git apply --directory=some/sub/dir/ --index patch &&
> + test content = $(git show :some/sub/dir/newfile) &&
> + test content = $(cat some/sub/dir/newfile)
> +'
> +
> +cat > patch << EOF
> +diff --git a/delfile b/delfile
> +deleted file mode 100644
> +index d95f3ad..0000000
> +--- a/delfile
> ++++ /dev/null
> +@@ -1 +0,0 @@
> +-content
> +EOF
> +
> +test_expect_success 'apply --directory (delete file)' '
> + git reset --hard initial &&
> + echo content >some/sub/dir/delfile &&
> + git add some/sub/dir/delfile &&
> + git apply --directory=some/sub/dir/ --index patch &&
> + ! git ls-files | grep delfile
> +'
> +
> +cat > patch << 'EOF'
> +diff --git "a/qu\157tefile" "b/qu\157tefile"
> +new file mode 100644
> +index 0000000..d95f3ad
> +--- /dev/null
> ++++ "b/qu\157tefile"
> +@@ -0,0 +1 @@
> ++content
> +EOF
> +
> +test_expect_success 'apply --directory (quoted filename)' '
> + git reset --hard initial &&
> + git apply --directory=some/sub/dir/ --index patch &&
> + test content = $(git show :some/sub/dir/quotefile) &&
> + test content = $(cat some/sub/dir/quotefile)
> +'
> +
> test_done
> --
> 1.6.0.2.519.g6cb82.dirty
^ permalink raw reply
* Re: [PATCH] git-svn: call 'fatal' correctly in set-tree
From: Junio C Hamano @ 2008-10-12 5:34 UTC (permalink / raw)
To: Luc Heinrich; +Cc: git, spearce
In-Reply-To: <1222696698-97356-1-git-send-email-luc@honk-honk.com>
Luc Heinrich <luc@honk-honk.com> writes:
> When doing a set-tree and there is no revision to commit to, the
> following unrelated error message is displayed: "Undefined subroutine
> &Git::SVN::fatal called at /opt/local/libexec/git-core/git-svn line
> 2575." The following patch fixes the problem and allows the real error
> message to be shown.
>
> Signed-off-by: Luc Heinrich <luc@honk-honk.com>
> ---
> git-svn.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 80a5728..7609a83 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2591,7 +2591,7 @@ sub set_tree {
> my ($self, $tree) = (shift, shift);
> my $log_entry = ::get_commit_entry($tree);
> unless ($self->{last_rev}) {
> - fatal("Must have an existing revision to commit");
> + ::fatal("Must have an existing revision to commit");
> }
> my %ed_opts = ( r => $self->{last_rev},
> log => $log_entry->{log},
Shouldn't the same change be done for the fatal call from apply_diff in
SVN::Git::Editor package?
^ permalink raw reply
* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Junio C Hamano @ 2008-10-12 5:24 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081007153543.GA26531@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Let's say I have a subdirectory of files, some of which are binary. But
> for those that _aren't_ binary, I want to use a particular funcname
> pattern. So I do this:
>
> echo '* diff=foo' >subdir/.gitattributes
> git config diff.foo.funcname some-regex
> ...
> In practice, this doesn't happen much, because funcname tends to follow
> the file extension, as does binary-ness.
I find this a highly contrived example. Is this ever be useful in
practice?
>> The reason why I'd like to understand the issue is because I like the
>> diff.foo.textconv that you introduce in patch 4/4, but I dislike that I
>> have to set diff.foo.binary to false in order to use the textconv. So, why
>> is this .binary needed?
>
> I think this .binary is something we can and should get rid of; as it
> stands now, you always end up having to set it along with .textconv. I
> have considered two ways:
The logic behind the original behaviour was that the file ought to be
"diff-able" if you are setting up funcname pattern because the funcname
pattern only makes sense if you are doing the textual diff. In other
words, "should we do textual diff?" and "what funcname pattern should we
use?" are _not_ orthogonal, as wanting to configure the latter does imply
that you do want to see the change in the textual diff format.
For the same rationale, if you have .textconv, I think it is natural for
us to say that you do want to see the change in the textual diff format.
So I'd agree that you can get rid of this .binary business by saying that
having .textconv marks it diffable (IOW, I think your first alternative
makes more sense).
^ permalink raw reply
* Re: Fwd: git status options feature suggestion
From: Jeff King @ 2008-10-12 4:49 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Johannes Schindelin, Caleb Cushing, git, Junio C Hamano
In-Reply-To: <48EE1F58.2060707@drmicha.warpmail.net>
On Thu, Oct 09, 2008 at 05:12:24PM +0200, Michael J Gruber wrote:
> - people are used to "svn status [-v]" like output which can include
> untracked as well as tracked unmodified files; there are other valid
> reasons why you would want that info
>
> - porc can't do it: git status can't show ignored files, doesn't use
> status letters, can't show files with specific status; git diff
> --name-status can't show ignored nor untracked files
> [In fact, the description of "git diff" says "files which you could
> add", which should include untracked files, but doesn't.]
>
> - plumb uses conflicting letters: git ls-files output conflicts with git
> diff --name-status output
>
> So I guess it's time for a usability effort in this area. A few
> questions before going about that:
A week or two ago I came across yet another git-status annoyance: it
needs write access to the repository to run (I was helping somebody with
a task on a shared box, and I wanted to run status in their repository
using my account).
I considered submitting a patch to fix this, but I think it is really
more fundamental. I use status to get an overview of what's going on in
a repo, but it is intimately related to a potential commit.
And this bleeds into other areas, too. Why should the "what's going on
in this repo" command prefix all lines with "#"? We would have more
freedom to change the format if it weren't required to be a comment
line in a commit message.
So I think it is probably reasonable to think about a new command (which
would not be called status) that shows this information. What do people
want to see? And in what format? Some things I would want or have seen
requested are:
- staged and unstaged changes in --name-status format
- files without changes (with a -v flag).
- untracked files
- current branch / detached HEAD (with relationship to tracked branch,
if any)
And maybe after hashing it out, it turns out it's not that different
from "git status" and we should just stick with that. But I would be
curious to hear proposals.
> - I think change of existing behaviour is unavoidable (make ls-files and
> diff --name-status consistent). Is that something to do now or rather
> before 1.7? Is porc (diff) supposed to be changed or plumb (ls-files)?
I don't think you would want to just change the default; you would
probably add a new option to ls-files to use the --name-status letters,
and then use that in your new porcelain.
> - How strong should the tie between git status and git commit be?
> Current git status is basically git commit -n, with the usual meaning of
> "-n" (such as for prune etc."), not with the current meaning of git
> commit -n, sigh...
I think the theoretical tool I mentioned would benefit from breaking
this connection. But I don't know whether it is prudent to take the
"status" name in doing so. Even if we decided to do so, it would
probably happen something like:
1. Introduce git-wtf, a new status-like tool. Deprecate git-status in
its current form.
2. Wait a really long time.
3. Rename git-wtf to git-status.
So either way, the first step is an alternative replacement command.
-Peff
^ permalink raw reply
* Re: git apply --directory broken for new files
From: Jeff King @ 2008-10-12 4:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: H. Peter Anvin, Johannes Schindelin, Shawn O. Pearce,
Git Mailing List
In-Reply-To: <7vk5ceijqo.fsf@gitster.siamese.dyndns.org>
On Sat, Oct 11, 2008 at 08:18:39PM -0700, Junio C Hamano wrote:
> I suspect this is only half of the story, because the code to parse:
>
> diff --git "a/f\244o/bar.c" "b/f\244o/bar.c"
>
> in the same function before the part you patched needs similar
> treatment. There are two return of strbuf_detach(&first, NULL)
> in the if(){} block, and the return value needs to be prefixed with the
> value of --directory when given.
Ah, indeed. I remember looking for other returns, but somehow I managed
to miss these completely obvious ones. Here is an updated patch, with a
third test that fails on the old patch but works here.
> It would be easier to do this --directory prefixing in the sole caller of
> git_header_name(), though.
I went this route, and it is much more readable if slightly less
efficient (one extra malloc/free).
-- >8 --
git apply --directory broken for new files
We carefully verify that the input to git-apply is sane,
including cross-checking that the filenames we see in "+++"
headers match what was provided on the command line of "diff
--git". When --directory is used, however, we ended up
comparing the unadorned name to one with the prepended root,
causing us to complain about a mismatch.
We simply need to prepend the root directory, if any, when
pulling the name out of the git header.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-apply.c | 7 ++++++
t/t4128-apply-root.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index bf80610..be7e1bd 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -810,6 +810,13 @@ static int parse_git_header(char *line, int len, unsigned int size, struct patch
* the default name from the header.
*/
patch->def_name = git_header_name(line, len);
+ if (patch->def_name && root) {
+ char *s = xmalloc(root_len + strlen(patch->def_name) + 1);
+ strcpy(s, root);
+ strcpy(s + root_len, patch->def_name);
+ free(patch->def_name);
+ patch->def_name = s;
+ }
line += len;
size -= len;
diff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh
index 2dd0c75..bc7a8a8 100755
--- a/t/t4128-apply-root.sh
+++ b/t/t4128-apply-root.sh
@@ -40,4 +40,56 @@ test_expect_success 'apply --directory -p (2) ' '
'
+cat > patch << EOF
+diff --git a/newfile b/newfile
+new file mode 100644
+index 0000000..d95f3ad
+--- /dev/null
++++ b/newfile
+@@ -0,0 +1 @@
++content
+EOF
+
+test_expect_success 'apply --directory (new file)' '
+ git reset --hard initial &&
+ git apply --directory=some/sub/dir/ --index patch &&
+ test content = $(git show :some/sub/dir/newfile) &&
+ test content = $(cat some/sub/dir/newfile)
+'
+
+cat > patch << EOF
+diff --git a/delfile b/delfile
+deleted file mode 100644
+index d95f3ad..0000000
+--- a/delfile
++++ /dev/null
+@@ -1 +0,0 @@
+-content
+EOF
+
+test_expect_success 'apply --directory (delete file)' '
+ git reset --hard initial &&
+ echo content >some/sub/dir/delfile &&
+ git add some/sub/dir/delfile &&
+ git apply --directory=some/sub/dir/ --index patch &&
+ ! git ls-files | grep delfile
+'
+
+cat > patch << 'EOF'
+diff --git "a/qu\157tefile" "b/qu\157tefile"
+new file mode 100644
+index 0000000..d95f3ad
+--- /dev/null
++++ "b/qu\157tefile"
+@@ -0,0 +1 @@
++content
+EOF
+
+test_expect_success 'apply --directory (quoted filename)' '
+ git reset --hard initial &&
+ git apply --directory=some/sub/dir/ --index patch &&
+ test content = $(git show :some/sub/dir/quotefile) &&
+ test content = $(cat some/sub/dir/quotefile)
+'
+
test_done
--
1.6.0.2.519.g6cb82.dirty
^ permalink raw reply related
* Re: git apply --directory broken for new files
From: Junio C Hamano @ 2008-10-12 3:18 UTC (permalink / raw)
To: Jeff King
Cc: H. Peter Anvin, Johannes Schindelin, Shawn O. Pearce,
Git Mailing List
In-Reply-To: <20080927015422.GA31783@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> apply --directory: handle creation and deletion patches
>
> We carefully verify that the input to git-apply is sane, including
> cross-checking that the filenames we see in "+++" headers match what was
> provided on the command line of "diff --git". When --directory is used,
> however, we ended up comparing the unadorned name to one with the
> prepended root, causing us to complain about a mismatch.
>
> We simply need to prepend the root directory, if any, when pulling the
> name out of the git header.
Thanks.
c4730f3 (Teach "git apply" to prepend a prefix with "--root=<root>",
2008-07-01) did a half-baked job to teach find_name() which is used to
parse traditional diff and also is used to set patch->old_name and
patch->new_name by gitdiff_verify_name() when parsing "copy from", "copy
to", "rename from", and "rename to". The caller of git_header_name() uses
the return value to set patch->def_name that is used when "deleted file"
and "new file" are parsed, which should have been taught this trick by the
same commit.
However,...
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 2ab4aba..f9070d5 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -787,6 +787,13 @@ static char *git_header_name(char *line, int llen)
> break;
> }
> if (second[len] == '\n' && !memcmp(name, second, len)) {
> + if (root) {
> + char *ret = xmalloc(root_len + len + 1);
> + strcpy(ret, root);
> + memcpy(ret + root_len, name, len);
> + ret[root_len + len] = '\0';
> + return ret;
> + }
> return xmemdupz(name, len);
> }
> }
I suspect this is only half of the story, because the code to parse:
diff --git "a/f\244o/bar.c" "b/f\244o/bar.c"
in the same function before the part you patched needs similar
treatment. There are two return of strbuf_detach(&first, NULL)
in the if(){} block, and the return value needs to be prefixed with the
value of --directory when given.
It would be easier to do this --directory prefixing in the sole caller of
git_header_name(), though.
^ permalink raw reply
* Re: [PATCH v2] correct verify_path for Windows
From: Alex Riesen @ 2008-10-11 22:58 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Johannes Sixt, Joshua Juran, Giovanni Funchal, git,
Shawn O. Pearce
In-Reply-To: <20081011163310.GZ21650@dpotapov.dyndns.org>
2008/10/11 Dmitry Potapov <dpotapov@gmail.com>:
>> > + /* On Windows, file names are case-insensitive */
>> > + case 'G':
>> > + if ((rest[1]|0x20) != 'i')
>> > + break;
>> > + if ((rest[2]|0x20) != 't')
>> > + break;
>>
>> We have tolower().
>
> I am aware of that, but I am not sure what we gain by using it. It seems
> it makes only code bigger and slow.
It does? Care to look into git-compat-util.h?
> ... As to readability, I don't see much
> improvement... Isn't obvious what this code does, especially with the
> above comment?
You want to seriously argue that "a | 0x20" is as readable as "tolower(a)"?
For the years to come? With a person who does not even know what ASCII is?
Ok, I'm exaggerating. But the point is: it is not us who will be
reading the code.
And even if they do this just to remove Windows quirks it is well worth to
use a bit more of english language so that they don't need a second look.
As to comment: it is just additional info. It can't be checked by compiler
if you make and accidental typo in your code (like, for example, accidentally
putting an extra pipe in that expression, should happen to that emacs users
from time to time).
BTW, is it such a critical path? Can't the code be unified and do
without #ifdef?
^ permalink raw reply
* [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true
From: Mark Levedahl @ 2008-10-11 22:56 UTC (permalink / raw)
To: gitster; +Cc: spearce, dpotapov, git, Mark Levedahl
In-Reply-To: <7v1vymke85.fsf@gitster.siamese.dyndns.org>
Cygwin's POSIX emulation allows use of core.filemode true, unlike native
Window's implementation of stat / lstat, and Cygwin/git users who have
configured core.filemode true in various repositories will be very
unpleasantly surprised to find that git is no longer honoring that option.
So, this patch forces use of Cygwin's stat functions if core.filemode is
set true, regardless of any other considerations.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Documentation/config.txt | 4 +++-
compat/cygwin.c | 18 +++++++++++++++---
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7161597..a3a9495 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::
one hierarchy using Cygwin mount. If true, Git uses native Win32 API
whenever it is possible and falls back to Cygwin functions only to
handle symbol links. The native mode is more than twice faster than
- normal Cygwin l/stat() functions. True by default.
+ normal Cygwin l/stat() functions. True by default, unless core.filemode
+ is true, in which case ignoreCygwinFSTricks is ignored as Cygwin's
+ POSIX emulation is required to support core.filemode.
core.trustctime::
If false, the ctime differences between the index and the
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 423ff20..1fed265 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -91,13 +91,20 @@ static int cygwin_stat(const char *path, struct stat *buf)
* functions should be used. The choice is determined by core.ignorecygwinfstricks.
* Reading this option is not always possible immediately as git_dir may be
* not be set yet. So until it is set, use cygwin lstat/stat functions.
+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as
+ * the Windows stat fuctions do not determine posix filemode.
*/
static int native_stat = 1;
+static int core_filemode = 0;
static int git_cygwin_config(const char *var, const char *value, void *cb)
{
- if (!strcmp(var, "core.ignorecygwinfstricks"))
+ if (!strcmp(var, "core.filemode"))
+ core_filemode = git_config_bool(var, value);
+
+ else if (!strcmp(var, "core.ignorecygwinfstricks"))
native_stat = git_config_bool(var, value);
+
return 0;
}
@@ -105,8 +112,13 @@ static int init_stat(void)
{
if (have_git_dir()) {
git_config(git_cygwin_config, NULL);
- cygwin_stat_fn = native_stat ? cygwin_stat : stat;
- cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
+ if (!core_filemode && native_stat) {
+ cygwin_stat_fn = cygwin_stat;
+ cygwin_lstat_fn = cygwin_lstat;
+ } else {
+ cygwin_stat_fn = stat;
+ cygwin_lstat_fn = lstat;
+ }
return 0;
}
return 1;
--
1.6.0.2.536.ga36e
^ permalink raw reply related
* Re: [RFC PATCH] describe: Make --tags and --all match lightweight tags more often
From: Andreas Ericsson @ 2008-10-11 22:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Junio C Hamano, Pierre Habouzit, Erez Zilber
In-Reply-To: <20081010165952.GI8203@spearce.org>
Shawn O. Pearce wrote:
> If the caller supplies --tags they want the lightweight, unannotated
> tags to be searched for a match. If a lightweight tag is closer
> in the history, it should be matched, even if an annotated tag is
> reachable further back in the commit chain.
>
> The same applies with --all when matching any other type of ref.
>
In 99% of the cases, "--all" will then give back the currently
checked out branch unless a revision is specified, right?
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH] gitweb: Better processing format string in custom links in navbar
From: Jakub Narebski @ 2008-10-11 22:02 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Shawn O. Pearce, Petr Baudis
Make processing format string in custom links in action bar ('actions'
feature) more robust. Now there would be no problems if one of
expanded values (for example project name, of project filename)
contains '%'; additionally format string supports '%' escaping by
doubling, i.e. '%%' expands to '%'.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Neither of parameters we expand to ($projec, $git_dir, $treehash,
$treebase) is ensured to have no '%' in the value. Therefore
sequential expansion, i.e. expanding one parameter after another could
lead to error.
Also there was no way to put '%' on the link; now it is possible
thanks to '%%' -> '%' expansion.
I have tred to be not too generic; you can check unquote() subroutine
to see how such more generic function (something like tsprintf_simple)
should look like
Pasky, could you check if it works correctly?
gitweb/gitweb.perl | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1116800..cc6edbe 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -290,10 +290,10 @@ our %feature = (
# The 'default' value consists of a list of triplets in the form
# (label, link, position) where position is the label after which
- # to inster the link and link is a format string where %n expands
+ # to insert the link and link is a format string where %n expands
# to the project name, %f to the project path within the filesystem,
# %h to the current hash (h gitweb parameter) and %b to the current
- # hash base (hb gitweb parameter).
+ # hash base (hb gitweb parameter); %% expands to %.
# To enable system wide have in $GITWEB_CONFIG e.g.
# $feature{'actions'}{'default'} = [('graphiclog',
@@ -2866,14 +2866,19 @@ sub git_print_page_nav {
$arg{'tree'}{'hash_base'} = $treebase if defined $treebase;
my @actions = gitweb_check_feature('actions');
+ my %repl = (
+ '%' => '%',
+ 'n' => $project, # project name
+ 'f' => $git_dir, # project path within filesystem
+ 'h' => $treehead || '', # current hash ('h' parameter)
+ 'b' => $treebase || '', # hash base ('hb' parameter)
+ );
while (@actions) {
- my ($label, $link, $pos) = (shift(@actions), shift(@actions), shift(@actions));
+ my ($label, $link, $pos) = splice(@actions,0,3);
+ # insert
@navs = map { $_ eq $pos ? ($_, $label) : $_ } @navs;
# munch munch
- $link =~ s#%n#$project#g;
- $link =~ s#%f#$git_dir#g;
- $treehead ? $link =~ s#%h#$treehead#g : $link =~ s#%h##g;
- $treebase ? $link =~ s#%b#$treebase#g : $link =~ s#%b##g;
+ $link =~ s/%([%nfhb])/$repl{$1}/g;
$arg{$label}{'_href'} = $link;
}
--
Stacked GIT 0.14.3
git version 1.6.0.2
^ permalink raw reply related
* Re: [PATCH] fetch: refuse to fetch into the current branch in a non-bare repository
From: Junio C Hamano @ 2008-10-11 21:44 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster, spearce
In-Reply-To: <alpine.DEB.1.00.0810111336350.22125@pacific.mpi-cbg.de.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Some confusing tutorials suggest that it would be a good idea to call
> something like this:
>
> git pull origin master:master
>
> While it might make sense to store what you want to merge, it typically
> is plain wrong.
I am somewhat confused.
This "confusion" has been there for very long time and (at least the
scripted version of) git-pull/git-fetch pair has supported a workaround in
the form of --update-head-ok option.
Have we broken the workaround ll.127..147 in git-pull.sh recently, or are
you trying to address something else?
^ permalink raw reply
* Re: [PATCH] "git diff <tree>{3,}": do not reverse order of arguments
From: Junio C Hamano @ 2008-10-11 21:36 UTC (permalink / raw)
To: Mikael Magnusson; +Cc: Junio C Hamano, Matt McCutchen, git
In-Reply-To: <237967ef0810110553r662df370ud1ec34de402bfe1c@mail.gmail.com>
"Mikael Magnusson" <mikachu@gmail.com> writes:
> 2008/10/11 Junio C Hamano <gitster@pobox.com>:
>> Perhaps the thinko was caused by discrepancy between the way internal
>> revision parser handles A..B and the way git-rev-parse parses it. While
>> the internal revision parser parses it into "A ^B", rev-parse gives them
>> in reverse order, i.e. "B ^A" (which is not going to change). In any
>> case, thanks for spotting this.
>
> Ehm, do you mean the internal parses it into "A ^B" and rev-parse gives "^B A"?
Sorry; a typo is in my description on the result from the internal parser.
For A..B, rev-parse gives B^ then A, and internal gives ^A in ent[0] and B
in ent[1].
^ permalink raw reply
* Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true
From: Junio C Hamano @ 2008-10-11 21:34 UTC (permalink / raw)
To: Mark Levedahl; +Cc: spearce, dpotapov, git
In-Reply-To: <1223751859-3540-1-git-send-email-mlevedahl@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
> Cygwin's POSIX emulation allows use of core.filemode true, unlike native
> Window's implementation of stat / lstat, and Cygwin/git users who have
> configured core.filemode true in various repositories will be very
> unpleasantly surprised to find that git is no longer honoring that option.
> So, this patch fores use of Cygwin's stat functions if core.filemode is
s/fores/forces/;
> static int native_stat = 1;
> +static int core_filemode = 0;
Makes me wonder why "trust_executable_bit" is unavailable here.
Perhaps git_cygwin_config() does not fall back to git_default_config()
for a reason?
> static int git_cygwin_config(const char *var, const char *value, void *cb)
> {
> + if (!strcmp(var, "core.filemode")) {
> + core_filemode = git_config_bool(var, value);
> + native_stat &= !core_filemode;
> + }
> if (!strcmp(var, "core.ignorecygwinfstricks"))
> - native_stat = git_config_bool(var, value);
> + native_stat = git_config_bool(var, value) &&
> + !core_filemode;
> return 0;
> }
If you can safely determine if you would want to use cygwin_stat or not
only after you have read both core.filemode and core.ignorecygwinfstricks,
perhaps keeping the config reader as is (this includes not falling back to
git_default_config()) and instead doing:
static int init_stat(void)
{
...
git_config(git_sygwin_config, NULL);
if (!core_filemode && native_stat) {
cygwin_stat_fn = cygwin_stat;
cygwin_lstat_fn = cygwin_lstat;
} else {
cygwin_stat_fn = stat;
cygwin_lstat_fn = lstat;
}
...
is less yucky?
^ permalink raw reply
* Re: [PATCH] Git.pm: release hash and blob handles
From: Petr Baudis @ 2008-10-11 21:23 UTC (permalink / raw)
To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0810111229u60c28671w1afd952200c6fa23@mail.gmail.com>
On Sun, Oct 12, 2008 at 03:29:28AM +0800, Ray Chuan wrote:
> the methods
>
> * hash_and_insert_object
> * cat_blob
>
> use bidirectional pipes to pull/push data from the various git
> utilities, but they don't close them after this is complete. this
> denies Git's subsequent attempts to use these resources, leading to
> failure.
>
> a simple, reproducible test case can be seen at
> http://rctay.spaces.live.com/blog/cns!59D3BFCD027B09E5!792.entry.
Hmm, I don't understand why git-svn does not work for you, but this
patch is not correct. The whole point of the infrastructure is to leave
the pipes open so that subsequent calls _reuse_ these pipes. This leads
to substantial speedup as you do not need to re-fork git all the time.
--
Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
^ permalink raw reply
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Junio C Hamano @ 2008-10-11 21:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alex Bennee, git
In-Reply-To: <alpine.DEB.1.00.0810111457300.22125@pacific.mpi-cbg.de.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Color me puzzled. You said in another mail that you think this is the
> task for the MUA.
Not really. I said that I think people usually do this in MUA with the
current system. I did not mean to say that I think such a partition of
jobs between commit and MUA is ideal.
>> This is a bit tangent, but perhaps rebase needs a hook so that users can
>> strip certain tags automatically from the commit log messages (e.g.
>> things like Reviewd-by: and Tested-by: become less trustworthy when you
>> rebase; S-o-b: becomes somewhat less trustworthy when you "edit" in
>> rebase-i; etc).
>
> Maybe. I am not really convinced of the S-o-b. You kept stressing that
> the SOB is not about validity, but a statement that the patch is
> intellectually proper or some such (IOW it means something like "Darl,
> forget it"). And the point of origin does not change, even if you rebase
> the commit.
The "somewhat less trustworthy" kicks in when you "edit" in rebase-i if
you change the tree that gets recorded. You are right that it is
irrelevant if you ran rebase-i to only edit the commit log message.
^ permalink raw reply
* [PATCH] Git.pm: release hash and blob handles
From: Ray Chuan @ 2008-10-11 19:29 UTC (permalink / raw)
To: git
the methods
* hash_and_insert_object
* cat_blob
use bidirectional pipes to pull/push data from the various git
utilities, but they don't close them after this is complete. this
denies Git's subsequent attempts to use these resources, leading to
failure.
a simple, reproducible test case can be seen at
http://rctay.spaces.live.com/blog/cns!59D3BFCD027B09E5!792.entry.
Signed-off-by: Tay Ray Chuan <rctay89 <at> gmail.com>
---
perl/Git.pm | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 6aab712..7e17f38 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -821,6 +821,7 @@ sub hash_and_insert_object {
throw Error::Simple("in pipe went bad");
}
+ $self->_close_hash_and_insert_object();
return $hash;
}
@@ -910,6 +911,7 @@ sub cat_blob {
throw Error::Simple("couldn't write to passed in filehandle");
}
+ $self->_close_cat_blob();
return $size;
}
--
1.6.0.2
^ permalink raw reply related
* [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true
From: Mark Levedahl @ 2008-10-11 19:04 UTC (permalink / raw)
To: spearce, dpotapov; +Cc: git, Mark Levedahl
In-Reply-To: <1223751280-2104-1-git-send-email-mlevedahl@gmail.com>
Cygwin's POSIX emulation allows use of core.filemode true, unlike native
Window's implementation of stat / lstat, and Cygwin/git users who have
configured core.filemode true in various repositories will be very
unpleasantly surprised to find that git is no longer honoring that option.
So, this patch fores use of Cygwin's stat functions if core.filemode is
set true, regardless of any other considerations.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Resend as I mangled Shawn's email address.
Documentation/config.txt | 4 +++-
compat/cygwin.c | 10 +++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7161597..a3a9495 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::
one hierarchy using Cygwin mount. If true, Git uses native Win32 API
whenever it is possible and falls back to Cygwin functions only to
handle symbol links. The native mode is more than twice faster than
- normal Cygwin l/stat() functions. True by default.
+ normal Cygwin l/stat() functions. True by default, unless core.filemode
+ is true, in which case ignoreCygwinFSTricks is ignored as Cygwin's
+ POSIX emulation is required to support core.filemode.
core.trustctime::
If false, the ctime differences between the index and the
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 423ff20..54028b3 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -91,13 +91,21 @@ static int cygwin_stat(const char *path, struct stat *buf)
* functions should be used. The choice is determined by core.ignorecygwinfstricks.
* Reading this option is not always possible immediately as git_dir may be
* not be set yet. So until it is set, use cygwin lstat/stat functions.
+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as
+ * the Windows stat fuctions do not determine posix filemode.
*/
static int native_stat = 1;
+static int core_filemode = 0;
static int git_cygwin_config(const char *var, const char *value, void *cb)
{
+ if (!strcmp(var, "core.filemode")) {
+ core_filemode = git_config_bool(var, value);
+ native_stat &= !core_filemode;
+ }
if (!strcmp(var, "core.ignorecygwinfstricks"))
- native_stat = git_config_bool(var, value);
+ native_stat = git_config_bool(var, value) &&
+ !core_filemode;
return 0;
}
--
1.6.0.2.535.g47d45.dirty
^ permalink raw reply related
* [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true
From: Mark Levedahl @ 2008-10-11 18:54 UTC (permalink / raw)
To: spearc, dpotapov; +Cc: git, Mark Levedahl
Cygwin's POSIX emulation allows use of core.filemode true, unlike native
Window's implementation of stat / lstat, and Cygwin/git users who have
configured core.filemode true in various repositories will be very
unpleasantly surprised to find that git is no longer honoring that option.
So, this patch fores use of Cygwin's stat functions if core.filemode is
set true, regardless of any other considerations.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Documentation/config.txt | 4 +++-
compat/cygwin.c | 10 +++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7161597..a3a9495 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::
one hierarchy using Cygwin mount. If true, Git uses native Win32 API
whenever it is possible and falls back to Cygwin functions only to
handle symbol links. The native mode is more than twice faster than
- normal Cygwin l/stat() functions. True by default.
+ normal Cygwin l/stat() functions. True by default, unless core.filemode
+ is true, in which case ignoreCygwinFSTricks is ignored as Cygwin's
+ POSIX emulation is required to support core.filemode.
core.trustctime::
If false, the ctime differences between the index and the
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 423ff20..54028b3 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -91,13 +91,21 @@ static int cygwin_stat(const char *path, struct stat *buf)
* functions should be used. The choice is determined by core.ignorecygwinfstricks.
* Reading this option is not always possible immediately as git_dir may be
* not be set yet. So until it is set, use cygwin lstat/stat functions.
+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as
+ * the Windows stat fuctions do not determine posix filemode.
*/
static int native_stat = 1;
+static int core_filemode = 0;
static int git_cygwin_config(const char *var, const char *value, void *cb)
{
+ if (!strcmp(var, "core.filemode")) {
+ core_filemode = git_config_bool(var, value);
+ native_stat &= !core_filemode;
+ }
if (!strcmp(var, "core.ignorecygwinfstricks"))
- native_stat = git_config_bool(var, value);
+ native_stat = git_config_bool(var, value) &&
+ !core_filemode;
return 0;
}
--
1.6.0.2.535.g47d45.dirty
^ permalink raw reply related
* [ANNOUNCE] CGIT 0.8.1
From: Lars Hjemli @ 2008-10-11 18:46 UTC (permalink / raw)
To: Git Mailing List
cgit-0.8.1, another webinterface for git, is now available.
clone: git://hjemli.net/pub/git/cgit
browse: http://hjemli.net/git/cgit
This is a minor feature-release which adds support for extracting
snapshot revision (i.e. tag name) from the snapshot name instead of
relying on querystring parameters, i.e. the following urls will
download the expected revisons:
http://hjemli.net/git/cgit/snapshot/cgit-0.8.1.tar.gz
http://hjemli.net/git/cgit/snapshot/cgit-0.8.tar.gz
---
Shortlog since v0.8:
Lars Hjemli (5):
ui-shared: specify correct css class for summary tab
Add cgit_query.nohead flag
ui-snapshot: add dwimmery
Makefile: enable compilation on uclibc
CGIT 0.8.1
^ permalink raw reply
* Re: User Authentication ?
From: Jakub Narebski @ 2008-10-11 18:28 UTC (permalink / raw)
To: Neshama Parhoti; +Cc: git
In-Reply-To: <912ec82a0810110941t33343fe1mfe1bce58739f79fa@mail.gmail.com>
"Neshama Parhoti" <pneshama@gmail.com> writes:
> I want to setup a git server on the web but I need user
> authentication.
>
> From what I understand, currently git-daemon does not support
> authentication.
The purpose of git-daemon is to allow fast (and bandwidth-saving)
anonymous read-only (fetch) access to git repositories. The ability
to push via git-daemon was added later, and is turned off by default
because it should be used only in special situation.
> Is there any way to achieve that ?
The reason behind git-daemon not supporting authentication is that
re-implementing authentication poorly is a bad idea.
If you need authentication there is SSH that provides authentication
(for ssh:// protocol), or WebDAV (for HTTP push protocol). Perhaps
also future "smart" HTTP server would support some kind of
authentification...
> I really don't want to give ssh logins for people who I just want to
> be able to access my repository...
First, you can always set git-shell as shell for those git only
accounts. Second, you can set up Gitosis, which IIRC needs only single
account, and handles authentication by itself; I have heard also of
ssh_acl in this context...
I don't know if there is some other equivalent of Gitosis...
HTH
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* User Authentication ?
From: Neshama Parhoti @ 2008-10-11 16:41 UTC (permalink / raw)
To: git
Hi All,
I want to setup a git server on the web but I need user authentication.
>From what I understand, currently git-daemon does not support authentication.
Is there any way to achieve that ?
I really don't want to give ssh logins for people who I just want to
be able to access my repository...
Thank you,
Pnesh.
^ permalink raw reply
* [PATCH] print an error message for invalid path
From: Dmitry Potapov @ 2008-10-11 16:39 UTC (permalink / raw)
To: Johannes Sixt, Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <48EAFBC2.7020305@viscovery.net>
If verification of path failed, it is always better to print an error message
saying this than relying on the caller function to print a meaningful error
message (especially when the callee already prints error message for another
situation).
Because the callers of add_index_entry_with_check() did not print any error
message, it resulted that the user would not notice the problem when checkout
if an invalid path failed.
Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
On Tue, Oct 07, 2008 at 08:03:46AM +0200, Johannes Sixt wrote:
>
> Look at the original patch. You did not change the behavior except to
> write more error messages. Maybe I misunderstand the words "to error out".
> I understand them as "to detect an error and return early", but not "write
> an error message".
For me, to "error out" means to show an error to the user. Usually, it
implies that the program will return after that, though not necessary
immediately. (Like gcc may print an error but it continues to parse the
program and may report more errors).
You are right that I have not changed anything in terms of exiting
earlier, and because I am aware about any commonly accepted definition
of what "error out" means, I have replaced the comment with less
ambiguous and detail description.
builtin-update-index.c | 2 +-
read-cache.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 417f972..3a2291b 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -218,7 +218,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
struct cache_entry *ce;
if (!verify_path(path))
- return -1;
+ return error("Invalid path '%s'", path);
len = strlen(path);
size = cache_entry_size(len);
diff --git a/read-cache.c b/read-cache.c
index 901064b..aff6390 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -591,8 +591,10 @@ struct cache_entry *make_cache_entry(unsigned int mode,
int size, len;
struct cache_entry *ce;
- if (!verify_path(path))
+ if (!verify_path(path)) {
+ error("Invalid path '%s'", path);
return NULL;
+ }
len = strlen(path);
size = cache_entry_size(len);
@@ -874,7 +876,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
if (!ok_to_add)
return -1;
if (!verify_path(ce->name))
- return -1;
+ return error("Invalid path '%s'", ce->name);
if (!skip_df_check &&
check_file_directory_conflict(istate, ce, pos, ok_to_replace)) {
--
1.6.0.2.447.g64b01
^ permalink raw reply related
* Re: [PATCH v2] correct verify_path for Windows
From: Dmitry Potapov @ 2008-10-11 16:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Joshua Juran, Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <48EAFF23.1020607@viscovery.net>
On Tue, Oct 07, 2008 at 08:18:11AM +0200, Johannes Sixt wrote:
> Dmitry Potapov schrieb:
> > +#if defined(_WIN32) || defined(__CYGWIN__)
>
> I think that for consistency you should use __MINGW32__ instead of _WIN32.
I like Alex's suggestion more to use FILESYSTEM_CASEINSENSITIVE.
>
> > + /* On Windows, file names are case-insensitive */
> > + case 'G':
> > + if ((rest[1]|0x20) != 'i')
> > + break;
> > + if ((rest[2]|0x20) != 't')
> > + break;
>
> We have tolower().
I am aware of that, but I am not sure what we gain by using it. It seems
it makes only code bigger and slow. As to readability, I don't see much
improvement... Isn't obvious what this code does, especially with the
above comment?
Dmitry
^ 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