* Re: [PATCH v1 3/3] git-gui hooks for new config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 18:58 UTC (permalink / raw)
To: Johannes Schindelin, Shawn O Pearce
Cc: Junio C Hamano, Jeff King, Teemu Likonen, git
In-Reply-To: <alpine.DEB.1.00.0901251918230.14855@racer>
On Sun, 25 Jan 2009, Johannes Schindelin wrote:
> Rather than storing the information about how to call "git diff" as
> diff.primer, why don't you store that information in a config variable
> gui.whiteSpaceMode and teach "git gui" to call "git diff" accordingly?
>
> That would have the further advantage of not breaking other people's
> setups...
This wasn't just for the GUI. Using Git for a project I imported from CVS, I
wanted --ignore-space-at-eol to be in effect at all times on the command line.
Of course, the "alias.dff" approach suggested yesterday by Teemu would work for
that. But I got the feeling this is a more general need. I'll name my primary
inspiration: ExamDiff (a Windows program). ExamDiff lets you specify a wide
range of options that remain in effect over all invocations. Seems like
something a lot of users would find natural. Please comment.
> Please submit git-gui patches without the git-gui prefix, as it makes it
> harder on the maintainer of git-gui, Shawn (who you did not Cc: BTW).
Sorry Shawn, I should have Cc'd you. Please let me know if I can improve the
git-gui code in any way.
-- Keith
^ permalink raw reply
* [PATCH] Makefile: Make 'configure --with-expat=path' actually work
From: Serge van den Boom @ 2009-01-25 19:24 UTC (permalink / raw)
To: git
The prefix specified with the --with-expat option of configure was not
actually used.
Signed-off-by: Serge van den Boom <svdb@stack.nl>
---
diff --git a/Makefile b/Makefile
index b4d9cb4..e7218cb 100644
--- a/Makefile
+++ b/Makefile
@@ -849,7 +849,12 @@ else
endif
endif
ifndef NO_EXPAT
- EXPAT_LIBEXPAT = -lexpat
+ ifdef EXPATDIR
+ BASIC_CFLAGS += -I$(EXPATDIR)/include
+ EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
+ else
+ EXPAT_LIBEXPAT = -lexpat
+ endif
endif
endif
^ permalink raw reply related
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Johannes Schindelin @ 2009-01-25 19:30 UTC (permalink / raw)
To: Keith Cascio; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <alpine.GSO.2.00.0901251033160.12651@kiwi.cs.ucla.edu>
Hi,
On Sun, 25 Jan 2009, Keith Cascio wrote:
> On Sun, 25 Jan 2009, Johannes Schindelin wrote:
>
> > That would break existing scripts using "git diff" rather badly. We
> > already did not allow something like "git config alias.diff ..." from
> > changing the behavior of "git diff", so I cannot find a reason why we
> > should let diff.primer (a misnomer BTW) override the behavior.
>
> I took special care to protect all core scripts from the effects.
What about my scripts I have here locally? Do you want to change them,
too?
> I fact, by introducing the cpp macro DIFF_MACHINE_FRIENDLY() and the
> command-line options "--machine-friendly" and "--no-primer", I made such
> protection declarative. Don't you find it preferable that existing
> programs and scripts would explicitly declare their desire for
> machine-friendly output?
No. We made a promise long time ago that plumbing (and "git diff" is
pretty much plumbing, except for the configurable colorness) would not
change behind scripts' backs.
And since Shawn uses plumbing for that very reason, your diff.primer patch
would not be allowed to make a difference. Ever.
Now, if you would have changed only the UI diff things (i.e. git diff, but
not git diff-files), I could have accepted the diff.primer patch for
different applications than "git gui", but from cursory reading of your
patch it does not appear so.
Speaking of appearance (or for that matter, explaining why it was only a
cursory reading): did it not occur to you that your coding style is
utterly different from the surrounding code?
Just to number a few things that would definitely prohibit this patch from
being applied:
- space instead of tabs,
- horrible lengths of spaces within the line,
- no space after if, but after the parenthesis.
Now, this could be good explanation why you need the patch (to ignore
white-space), but that is not a reason of letting us suffer, too.
Besides, it seems you did a lot of "fixes" on the side that I do not like
at all. Simple example: if the original code cleared the
DIRSTAT_CUMULATIVE flag, it is not acceptable for you to introduce an
unnecessary if(), testing if the CUMULATIVE flag was set to begin with.
Ciao,
Dscho
^ permalink raw reply
* [PATCH 1/3] git-svn: add --ignore-paths option for fetching
From: Vitaly "_Vi" Shukela @ 2009-01-25 19:49 UTC (permalink / raw)
To: trast; +Cc: git, Vitaly "_Vi" Shukela
Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
---
git-svn.perl | 43 +++++++++++++++++++++++++++----------------
1 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index d4cb538..40b0e9e 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -70,7 +70,8 @@ my ($_stdin, $_help, $_edit,
$Git::SVN::_follow_parent = 1;
my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
'config-dir=s' => \$Git::SVN::Ra::config_dir,
- 'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache );
+ 'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache,
+ 'ignore-paths=s' => \$SVN::Git::Fetcher::ignore_regex );
my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
'authors-file|A=s' => \$_authors,
'repack:i' => \$Git::SVN::_repack,
@@ -3245,6 +3246,21 @@ use warnings;
use Carp qw/croak/;
use File::Temp qw/tempfile/;
use IO::File qw//;
+use vars qw/ $ignore_regex/;
+
+# returns true if a given path is inside a ".git" directory
+sub in_dot_git($) {
+ $_[0] =~ m{(?:^|/)\.git(?:/|$)};
+}
+
+# 0 -- don't ignore, 1 -- ignore
+sub is_path_ignored($) {
+ my ($path) = @_;
+ return 1 if in_dot_git($path);
+ return 0 unless defined($ignore_regex);
+ return 1 if $path =~ m!$ignore_regex!o;
+ return 0;
+}
# file baton members: path, mode_a, mode_b, pool, fh, blob, base
sub new {
@@ -3292,11 +3308,6 @@ sub _mark_empty_symlinks {
\%ret;
}
-# returns true if a given path is inside a ".git" directory
-sub in_dot_git {
- $_[0] =~ m{(?:^|/)\.git(?:/|$)};
-}
-
sub set_path_strip {
my ($self, $path) = @_;
$self->{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path;
@@ -3322,7 +3333,7 @@ sub git_path {
sub delete_entry {
my ($self, $path, $rev, $pb) = @_;
- return undef if in_dot_git($path);
+ return undef if is_path_ignored($path);
my $gpath = $self->git_path($path);
return undef if ($gpath eq '');
@@ -3352,7 +3363,7 @@ sub open_file {
my ($self, $path, $pb, $rev) = @_;
my ($mode, $blob);
- goto out if in_dot_git($path);
+ goto out if is_path_ignored($path);
my $gpath = $self->git_path($path);
($mode, $blob) = (command('ls-tree', $self->{c}, '--', $gpath)
@@ -3372,7 +3383,7 @@ sub add_file {
my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
my $mode;
- if (!in_dot_git($path)) {
+ if (!is_path_ignored($path)) {
my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
delete $self->{empty}->{$dir};
$mode = '100644';
@@ -3383,7 +3394,7 @@ sub add_file {
sub add_directory {
my ($self, $path, $cp_path, $cp_rev) = @_;
- goto out if in_dot_git($path);
+ goto out if is_path_ignored($path);
my $gpath = $self->git_path($path);
if ($gpath eq '') {
my ($ls, $ctx) = command_output_pipe(qw/ls-tree
@@ -3407,7 +3418,7 @@ out:
sub change_dir_prop {
my ($self, $db, $prop, $value) = @_;
- return undef if in_dot_git($db->{path});
+ return undef if is_path_ignored($db->{path});
$self->{dir_prop}->{$db->{path}} ||= {};
$self->{dir_prop}->{$db->{path}}->{$prop} = $value;
undef;
@@ -3415,7 +3426,7 @@ sub change_dir_prop {
sub absent_directory {
my ($self, $path, $pb) = @_;
- return undef if in_dot_git($pb->{path});
+ return undef if is_path_ignored($path);
$self->{absent_dir}->{$pb->{path}} ||= [];
push @{$self->{absent_dir}->{$pb->{path}}}, $path;
undef;
@@ -3423,7 +3434,7 @@ sub absent_directory {
sub absent_file {
my ($self, $path, $pb) = @_;
- return undef if in_dot_git($pb->{path});
+ return undef if is_path_ignored($path);
$self->{absent_file}->{$pb->{path}} ||= [];
push @{$self->{absent_file}->{$pb->{path}}}, $path;
undef;
@@ -3431,7 +3442,7 @@ sub absent_file {
sub change_file_prop {
my ($self, $fb, $prop, $value) = @_;
- return undef if in_dot_git($fb->{path});
+ return undef if is_path_ignored($fb->{path});
if ($prop eq 'svn:executable') {
if ($fb->{mode_b} != 120000) {
$fb->{mode_b} = defined $value ? 100755 : 100644;
@@ -3447,7 +3458,7 @@ sub change_file_prop {
sub apply_textdelta {
my ($self, $fb, $exp) = @_;
- return undef if (in_dot_git($fb->{path}));
+ return undef if is_path_ignored($fb->{path});
my $fh = $::_repository->temp_acquire('svn_delta');
# $fh gets auto-closed() by SVN::TxDelta::apply(),
# (but $base does not,) so dup() it for reading in close_file
@@ -3494,7 +3505,7 @@ sub apply_textdelta {
sub close_file {
my ($self, $fb, $exp) = @_;
- return undef if (in_dot_git($fb->{path}));
+ return undef if is_path_ignored($fb->{path});
my $hash;
my $path = $self->git_path($fb->{path});
--
1.6.1.288.gff3b4
^ permalink raw reply related
* [PATCH 3/3] git-svn: Add test for --ignore-paths parameter
From: Vitaly "_Vi" Shukela @ 2009-01-25 19:49 UTC (permalink / raw)
To: trast; +Cc: git, Vitaly "_Vi" Shukela
In-Reply-To: <1232912944-27076-2-git-send-email-public_vi@tut.by>
Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
---
t/t9134-git-svn-ignore-paths.sh | 97 +++++++++++++++++++++++++++++++++++++++
1 files changed, 97 insertions(+), 0 deletions(-)
create mode 100755 t/t9134-git-svn-ignore-paths.sh
diff --git a/t/t9134-git-svn-ignore-paths.sh b/t/t9134-git-svn-ignore-paths.sh
new file mode 100755
index 0000000..094cb6a
--- /dev/null
+++ b/t/t9134-git-svn-ignore-paths.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Eric Wong
+#
+
+test_description='git svn property tests'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup test repository' '
+ svn co "$svnrepo" s &&
+ (
+ cd s &&
+ mkdir qqq www &&
+ echo test_qqq > qqq/test_qqq.txt &&
+ echo test_www > www/test_www.txt &&
+ svn add qqq &&
+ svn add www &&
+ svn commit -m "create some files" &&
+ svn up &&
+ echo hi >> www/test_www.txt &&
+ svn commit -m "modify www/test_www.txt" &&
+ svn up
+ )
+'
+
+test_expect_success 'clone an SVN repository with ignored www directory' '
+ git svn clone --ignore-paths="^www" "$svnrepo" g &&
+ echo test_qqq > expect &&
+ for i in g/*/*.txt; do cat $i >> expect2; done &&
+ test_cmp expect expect2
+'
+
+test_expect_success 'SVN-side change outside of www' '
+ (
+ cd s &&
+ echo b >> qqq/test_qqq.txt &&
+ svn commit -m "SVN-side change outside of www" &&
+ svn up &&
+ svn log -v | fgrep "SVN-side change outside of www"
+ )
+'
+
+test_expect_success 'update git svn-cloned repo' '
+ (
+ cd g &&
+ git svn rebase --ignore-paths="^www" &&
+ echo -e "test_qqq\nb" > expect &&
+ for i in */*.txt; do cat $i >> expect2; done &&
+ test_cmp expect2 expect &&
+ rm expect expect2
+ )
+'
+
+test_expect_success 'SVN-side change inside of ignored www' '
+ (
+ cd s &&
+ echo zaq >> www/test_www.txt
+ svn commit -m "SVN-side change inside of www/test_www.txt" &&
+ svn up &&
+ svn log -v | fgrep "SVN-side change inside of www/test_www.txt"
+ )
+'
+
+test_expect_success 'update git svn-cloned repo' '
+ (
+ cd g &&
+ git svn rebase --ignore-paths="^www" &&
+ echo -e "test_qqq\nb" > expect &&
+ for i in */*.txt; do cat $i >> expect2; done &&
+ test_cmp expect2 expect &&
+ rm expect expect2
+ )
+'
+
+test_expect_success 'SVN-side change in and out of ignored www' '
+ (
+ cd s &&
+ echo cvf >> www/test_www.txt
+ echo ygg >> qqq/test_qqq.txt
+ svn commit -m "SVN-side change in and out of ignored www" &&
+ svn up &&
+ svn log -v | fgrep "SVN-side change in and out of ignored www"
+ )
+'
+
+test_expect_success 'update git svn-cloned repo again' '
+ (
+ cd g &&
+ git svn rebase --ignore-paths="^www" &&
+ echo -e "test_qqq\nb\nygg" > expect &&
+ for i in */*.txt; do cat $i >> expect2; done &&
+ test_cmp expect2 expect &&
+ rm expect expect2
+ )
+'
+
+test_done
--
1.6.1.288.gff3b4
^ permalink raw reply related
* [PATCH 2/3] git-svn: documented --ignore-paths
From: Vitaly "_Vi" Shukela @ 2009-01-25 19:49 UTC (permalink / raw)
To: trast; +Cc: git, Vitaly "_Vi" Shukela
In-Reply-To: <1232912944-27076-1-git-send-email-public_vi@tut.by>
Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
---
Documentation/git-svn.txt | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 63d2f5e..2215fcb 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -103,6 +103,19 @@ repository to be able to interoperate with someone else's local Git
repository, either don't use this option or you should both use it in
the same local timezone.
+--ignore-paths=<regex>;;
+ This allows one to specify Perl regular expression that will
+ cause skipping of all matching paths from checkout from SVN.
+ Examples:
+
+ --ignore-paths="^doc" - skip "doc*" directory for every fetch.
+
+ --ignore-paths="^[^/]+/(?:branches|tags)" - skip "branches"
+ and "tags" of first level directories.
+
+ Regular expression is not persistent, you should specify
+ it every time when fetching.
+
'clone'::
Runs 'init' and 'fetch'. It will automatically create a
directory based on the basename of the URL passed to it;
--
1.6.1.288.gff3b4
^ permalink raw reply related
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 20:14 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Shawn O Pearce
In-Reply-To: <alpine.DEB.1.00.0901252016590.14855@racer>
On Sun, 25 Jan 2009, Johannes Schindelin wrote:
> What about my scripts I have here locally? Do you want to change them,
> too?
Someone who already wrote local scripts probably won't use the diff.primer
feature, or if he does, he'll be thoughtful about the consequences and modify
his scripts. diff.primer has no effect unless you use it. It is totally
personal.
> > I fact, by introducing the cpp macro DIFF_MACHINE_FRIENDLY() and the
> > command-line options "--machine-friendly" and "--no-primer", I made such
> > protection declarative. Don't you find it preferable that existing programs
> > and scripts would explicitly declare their desire for machine-friendly
> > output?
> No. We made a promise long time ago that plumbing (and "git diff" is
> pretty much plumbing, except for the configurable colorness) would not
> change behind scripts' backs.
>
> And since Shawn uses plumbing for that very reason, your diff.primer patch
> would not be allowed to make a difference. Ever.
I fixed up every single place where gitk and git-gui internally call diff.
Before I ever got there, Shawn already protected himself by passing --no-color
to "git diff" (git-gui/lib/diff.tcl line 279). My changes are in a good spirit
because they enhance the declarativeness and explicitness of the code. I added
meaningful information.
> Now, if you would have changed only the UI diff things (i.e. git diff, but
> not git diff-files), I could have accepted the diff.primer patch for
> different applications than "git gui", but from cursory reading of your
> patch it does not appear so.
>
> Speaking of appearance (or for that matter, explaining why it was only a
> cursory reading): did it not occur to you that your coding style is
> utterly different from the surrounding code?
>
> Just to number a few things that would definitely prohibit this patch from
> being applied:
>
> - space instead of tabs,
> - horrible lengths of spaces within the line,
> - no space after if, but after the parenthesis.
You are right, I should mimic the existing conventions. If we reach consensus
about the functionality of the patch, I would be happy to redo all the white
space to match what's already there. In case you are curious, I put whitespace
in lines in the name of readability, to make tokens line up with each other.
Again, it is right to mimic local convention and I have no problem making that
revision.
> Besides, it seems you did a lot of "fixes" on the side that I do not like at
> all. Simple example: if the original code cleared the DIRSTAT_CUMULATIVE
> flag, it is not acceptable for you to introduce an unnecessary if(), testing
> if the CUMULATIVE flag was set to begin with.
I appreciate that you noticed this little detail. You are extremely thorough
and I take your criticism very seriously. The reason for that little if() is
that, with the enhanced declarativeness of the code in diff.h, calling the cpp
macro DIFF_OPT_SET() is now more meaningful that just flipping a bit. It is now
the equivalent of saying "I intend to set this bit, and I want my intention
recorded and honored later." The purpose of the piece of code where I
introduced the if() is only to setup the right defaults, not to declare user
intention. Therefore, IOW, if the default is already in place, do nothing.
c is only one of many languages I write. Designers of newer languages emphasize
meangingfulness, explicitness, declarativeness of code, e.g. the DRY principle,
etc. c is very flexible, so it does not prevent us from following these
beneficial practices in c. After all, the first c++ compilers simply emitted c
and then called a c compiler, right?
With thanks,
Keith
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-25 20:34 UTC (permalink / raw)
To: Keith Cascio; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1232904657-31831-2-git-send-email-keith@cs.ucla.edu>
Keith Cascio <keith@cs.ucla.edu> writes:
> Introduce config variable "diff.primer".
> Allows user to specify arbitrary options
> to pass to diff on every invocation,
> including internal invocations from other
> programs, e.g. git-gui.
> Introduce diff command-line options:
> --no-primer, --machine-friendly
> Protect git-format-patch, git-apply,
> git-am, git-rebase, git-gui and gitk
> from inapplicable options.
>
> Signed-off-by: Keith Cascio <keith@cs.ucla.edu>
Your Subject is good; in a shortlog output that will be taken 3 months
down the road, it will still tell us what this patch was about, among 100
other patches that are about different topics.
The proposed commit log message describes what the patch does, but it does
not explain what problem it solves, nor why the approach the patch takes
to solve that problem is good. Lines that are too short and too dense
without paragraph breaks do not help readability either.
> ---
> Documentation/config.txt | 14 +++++++
> Documentation/diff-options.txt | 13 ++++++
> Makefile | 2 +
> builtin-log.c | 1 +
> diff.c | 83 +++++++++++++++++++++++++++++++++++-----
> diff.h | 15 ++++++-
> git-gui/lib/diff.tcl | 8 +++-
> gitk-git/gitk | 16 ++++----
> 8 files changed, 129 insertions(+), 23 deletions(-)
You can work around the backward incompatibility you are introducing for
known users that you broke with your patch, by including updates to them,
and that is what your patches to git-gui and gitk are, but that is a sure
sign that the approach is flawed.
The point of lowlevel plumbing (e.g. diff-{files,index,tree}) is to give
people's scripts an interface that they can rely on. It is not about
giving a magic interface that all the users are somehow magically upgraded
without change, when the underlying git is upgraded.
If a script X does not use "ignore whitespace" without an explicit request
from the end user when it runs diff-tree internally, installing a new
version of diff-tree should *NOT* magically make script X to run it with
"ignore whitespace", because you do not know what the script X uses
diff-tree output for and how, even when the end user sets diff.primer to
get "ignore whitespace" applied to his command line invocation of "git
diff" Porcelain. Imagine a case where the operation of that script X
relies on seeing at least two context lines around the hunk in order to
correctly parse textual diff output from "diff-index -p", and the user
sets "-U1" in diff.primer --- you would break the script and it is not
fair to blame the script for not explicitly passing -U3 and relying on the
default.
Scriptability by definition means you do not know how scripts written by
people around plumbing use the output; I do not think you can sensibly say
"this should not be turned on in a machine friendly output, but this is
safe to use".
I would not be opposed to an enhancement to the plumbing that the scripts
can use to say "I am willing to take any option (or perhaps "these
options") given to me via diff.primer". Some scripts may want to be just
a pass-thru of whatever the underlying git-diff-* command outputs, and it
may be a handy way to magically upgrade them to allow their invocation of
lowlevel plumbing to be affected by what the end-user configured. But
that magic upgrade has to be an opt/in process.
There are funny indentation to align the same variable names on two
adjacent lines and such; please don't.
^ permalink raw reply
* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Junio C Hamano @ 2009-01-25 20:34 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Johannes Schindelin, Hannu Koivisto, git
In-Reply-To: <94a0d4530901240604o5ae0d321h17dc6aabeefe9d53@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> It's much easier for everyone to just use git config --global than
> explain how create and edit the .gitconfig file. If this is explained
> it shouldn't be in the "Telling git your name" section.
I think it is Ok to have, and it probably is helpful to have, a document
that accompanies the generic manual, "Platform supplement", to explain
things like "On this platform, what is referred to as $HOME in the generic
manual is implemented as %HOMEDRIVE%%HOMEPATH%", to cover peculiarities of
each platform.
But I think it is going backwards to butcher the description in the main
manual in order to make the platform supplement shorter. Even Windows is
not a good enough reason to break the logical ordering of how things
should be best taught to the readers of the manual, which is not specific
to a platform.
^ permalink raw reply
* Re: [PATCH] http-push: refactor request url creation
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
To: Ray Chuan; +Cc: git, Johannes Schindelin
In-Reply-To: <be6fef0d0901242208p635264e5jc1f95d784cd51450@mail.gmail.com>
Ray Chuan <rctay89@gmail.com> writes:
> Currently, functions that deal with objects on the remote repository
> have to allocate and do strcpys to generate the URL.
>
> This patch saves them this trouble, by providing a function that
> returns a URL: either the object's 2-digit hex directory (eg.
> /objects/a1/) or the complete object location (eg. /objects/a1/b2).
>
> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
If you wrote it, and then Johannes vetted it, then these two should be in
the opposite order.
> +static char *get_remote_object_url(const char *url, const char *hex,
> int only_two_digit_prefix) {
Linewrapped.
> + struct strbuf buf = STRBUF_INIT;
> +
> + strbuf_addf(&buf, "%sobjects/%.*s/", url, 2, hex);
> + if(!only_two_digit_prefix)
"if (..."
> @@ -304,17 +312,7 @@ static void start_fetch_loose(struct
> transfer_request *request)
>
> git_SHA1_Init(&request->c);
>
> - url = xmalloc(strlen(remote->url) + 50);
> ...
> - strcpy(request->url, url);
> + request->url = get_remote_object_url(remote->url, hex, 0);
> ...
> - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> + curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
The original code gave a separate "url" to setop() but this gives the same
string. Does curl_easy_setop() copies the given string away? IOW is this
change safe?
> @@ -550,21 +540,11 @@ static void start_put(struct transfer_request *request)
>
> request->buffer.buf.len = stream.total_out;
>
> - request->url = xmalloc(strlen(remote->url) +
> ...
> - strcpy(posn, request->lock->token);
> + strbuf_addf(&url_buf, "Destination: %s",
> get_remote_object_url(remote->url, hex, 0));
Linewrapped.
The return value from get_remote_object_url() leaks here.
> + request->dest = strbuf_detach(&url_buf, NULL);
> +
> + strbuf_addf(&url_buf, "%s_%s", get_remote_object_url(remote->url,
> hex, 0), request->lock->token);
The return value from get_remote_object_url() leaks here, too.
^ permalink raw reply
* Re: [PATCH 0/2] Add submodule-support to git archive
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Nanako Shiraishi, Johannes Schindelin, git
In-Reply-To: <8c5c35580901250018x6827291cj36e6bcb10afa9b27@mail.gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
> On Sun, Jan 25, 2009 at 05:53, Nanako Shiraishi <nanako3@lavabit.com> wrote:
>> What would I do to try this new series? Fork a branch from Junio's master branch,
>> apply your new patches, and merge the result to Junio's next?
>
> Yes, that sounds right (btw: the series is buildt on top of 5dc1308562
> (Merge branch 'js/patience-diff') and can be pulled from
> git://hjemli.net/pub/git/git lh/traverse-gitlinks).
>
> But before merging with 'next', you'll need to `git revert -m 1 bdf31cbc00`.
Yuck, that is too much to ask for regular testers and users.
Could we switch to incremental refinements once a series hits next, pretty
please?
^ permalink raw reply
* Re: [PATCH next] t1505: remove debugging cruft
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Johannes Schindelin
In-Reply-To: <1232835794-22176-1-git-send-email-trast@student.ethz.ch>
Thanks.
^ permalink raw reply
* The lifecycle of a patch and the maintainer involvement
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
To: Brent Goodrick; +Cc: Johannes Schindelin, Daniel Barkalow, Mike Ralphson, git
In-Reply-To: <18811.32772.728276.923430@hungover.brentg.com>
Brent Goodrick <bgoodr@gmail.com> writes:
> While I'm at it, what is the standard procedure for submitting git
> patches for review once I've cooked up and validated it on my end? I'm
> guessing posting the patch into this mailing list is part of the
> answer to that question.
Yes, a guideline is in Documentation/SubmittingPatches for the initial
submission. After that, the lifecycle of a patch submitted on the list
goes like this:
(1) A patch is shown to the list participants.
(2) People may like it, or may have issues with it, and responds with
their comments describing problems, suggestions for improvements,
etc. People who are not interested in the topic may stay silent.
(3) The original author responds with updated patch. Sometimes people
who commented on in step 2 may even send "here is how I would do this
one; don't you think this is better?", and the original author may
say "Yeah, let's use yours instead".
(4) After steps 2 and 3 repeats zero or more times, the latest patch may
become one that everyone likes, or at least nobody has trouble with
inclusion. The author sends such a patch saying "this is meant for
inclusion based on discussion and refinements in these threads...".
(5) The maintainer picks it up when it looks polished enough.
Your patch may appear in the periodical "What's cooking" or "What's in"
summary with zero iteration of steps 2 and 3 if it is obvious enough.
I act as just one of the list participant during steps 1-3. I may stay
silent during this period but that only means the topic is not interesting
to me and nothing more. It does not mean that the topic has no chance of
getting included.
I act as the maintainer for steps 4 and 5. If you do not hear from me
after step 4, then I am either being lazy, busy, or sick, or the patch got
lost in the noise and I need a reminder. Note that I may reject or ask
further refinement at step 4 to ensure overall quality throughout the
system even in areas I am not interested in and didn't say anything during
steps 1-3.
^ permalink raw reply
* Re: Heads up: major rebase -i -p rework coming up
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Sverre Rabbelier, Jakub Narebski, git
In-Reply-To: <alpine.DEB.1.00.0901251622310.14855@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> So maybe I answered my question myself:
>
> merge parents $sha1 [$sha1...] original $sha1 $msg
When you are reparenting, how would original commit get in the picture?
You wouldn't want the resulting merge to claim it merged X (which would be
what's in original's commit log) when in fact it now merged Y because the
user reparented it, would you?
^ permalink raw reply
* Re: [PATCH v1 0/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
To: Keith Cascio; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1232904657-31831-1-git-send-email-keith@cs.ucla.edu>
Keith Cascio <keith@cs.ucla.edu> writes:
> Future work: Extend the gitattributes mechanism so it supports
> all [diff] config variables, including e.g. diff.mnemonicprefix
> and diff.primer.
I am puzzled.
The gitattributes mechanism is about per-path settings, but I do not think
a mnemonicprefix that is per-path makes much sense.
^ permalink raw reply
* Re: [PATCH 1/3] git-svn: add --ignore-paths option for fetching
From: Sverre Rabbelier @ 2009-01-25 20:41 UTC (permalink / raw)
To: Vitaly _Vi Shukela; +Cc: trast, git
In-Reply-To: <1232912944-27076-1-git-send-email-public_vi@tut.by>
On Sun, Jan 25, 2009 at 20:49, Vitaly _Vi Shukela <public_vi@tut.by> wrote:
>
> Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
You still did not explain _in your commit message_ why it would be
useful to have, which is what Thomas asked for ;).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH v1 0/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 20:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr62rcee5.fsf@gitster.siamese.dyndns.org>
On Sun, 25 Jan 2009, Junio C Hamano wrote:
> I am puzzled.
>
> The gitattributes mechanism is about per-path settings, but I do not think a
> mnemonicprefix that is per-path makes much sense.
That was just an example (perhaps poorly chosen). What I meant to suggest is
making gitattributes consistent with gitconfig WRT at least the [diff] section.
But maybe that's not appropriate. Thanks for the insight.
^ permalink raw reply
* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-25 20:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Jakub Narebski, git
In-Reply-To: <7vwscjceec.fsf@gitster.siamese.dyndns.org>
Hi,
On Sun, 25 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > So maybe I answered my question myself:
> >
> > merge parents $sha1 [$sha1...] original $sha1 $msg
>
> When you are reparenting, how would original commit get in the picture?
> You wouldn't want the resulting merge to claim it merged X (which would be
> what's in original's commit log) when in fact it now merged Y because the
> user reparented it, would you?
Oh yes, I would! Example:
A - B - C
/
X - Y
If I merged X into B by accident, but actually meant to merge Y, then C'
should still come after B', no?
Ciao,
Dscho
^ permalink raw reply
* Re: diff settings
From: Ted Pavlic @ 2009-01-25 21:02 UTC (permalink / raw)
To: Keith Cascio; +Cc: Teemu Likonen, git
In-Reply-To: <alpine.GSO.2.00.0901241159050.23073@kiwi.cs.ucla.edu>
>> git config --global alias.dff "diff -w"
>>
>> Then "git dff" is your new "git diff -w". :-)
>
> That feature is gonna come in handy. It doesn't 100% fulfill my needs here but
> I like it! Thanks, I did not know about alias.*
Mercurial has a "defaults.*" that does exactly what you want. That is,
you can say
[defaults]
diff = -w
and "hg diff" will automatically do "hg diff -w". Such a feature might
be a nice addition to git.
--Ted
--
Ted Pavlic <ted@tedpavlic.com>
Please visit my ALS association page:
http://web.alsa.org/goto/tedpavlic
My family appreciates your support in the fight to defeat ALS.
^ permalink raw reply
* Re: diff settings
From: Keith Cascio @ 2009-01-25 21:11 UTC (permalink / raw)
To: Ted Pavlic; +Cc: Teemu Likonen, git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <497CD352.2060402@tedpavlic.com>
On Sun, 25 Jan 2009, Ted Pavlic wrote:
> Mercurial has a "defaults.*" that does exactly what you want. That is, you can
> say
>
> [defaults]
> diff = -w
>
> and "hg diff" will automatically do "hg diff -w". Such a feature might be a
> nice addition to git.
Thank you, that is very interesting. I just submitted a patch this morning that
does exactly what you describe, but I called it "primer" instead of "defaults"
because it seemed more explicit. Check it out.
-- Keith
^ permalink raw reply
* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Felipe Contreras @ 2009-01-25 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Hannu Koivisto, git
In-Reply-To: <7vvds3dszy.fsf@gitster.siamese.dyndns.org>
On Sun, Jan 25, 2009 at 10:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> It's much easier for everyone to just use git config --global than
>> explain how create and edit the .gitconfig file. If this is explained
>> it shouldn't be in the "Telling git your name" section.
>
> I think it is Ok to have, and it probably is helpful to have, a document
> that accompanies the generic manual, "Platform supplement", to explain
> things like "On this platform, what is referred to as $HOME in the generic
> manual is implemented as %HOMEDRIVE%%HOMEPATH%", to cover peculiarities of
> each platform.
>
> But I think it is going backwards to butcher the description in the main
> manual in order to make the platform supplement shorter. Even Windows is
> not a good enough reason to break the logical ordering of how things
> should be best taught to the readers of the manual, which is not specific
> to a platform.
That's why I think there should be a whole section regarding git
configuration before "Telling git your name".
However, my last proposal was to have both the git config --global
*and* the $HOME/.gitconfig description. Is there any argument against
that?
--
Felipe Contreras
^ permalink raw reply
* [PATCH] Mention "local convention" rule in the CodingGuidelines
From: Nanako Shiraishi @ 2009-01-25 21:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The document suggests to imitate the existing code, but didn't
say which existing code it should imitate. This clarifies.
Signed-off-by: しらいしななこ <nanako3@lavabit.com>
---
Quoting Junio C Hamano <gitster@pobox.com>:
> It is always preferable to match the _local_ convention. I'd expect a new
> script added to git suite to match my preference (the one I showed you in
> my comments to you that is used in git-am, which is what you suggested
> above), but I'd expect a modification to mergetool to match the style
> mergetool already uses.
Documentation/CodingGuidelines | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index f628c1f..664c6c2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -21,8 +21,13 @@ code. For git in general, three rough rules are:
As for more concrete guidelines, just imitate the existing code
(this is a good guideline, no matter which project you are
-contributing to). But if you must have a list of rules,
-here they are.
+contributing to). It is always preferable to match the _local_
+convention. A new code added to git suite is expected to match
+the overall style of existing code. A modification to an existing
+code is expected to match the style the surrounding code already
+uses (even if it doesn't match the overall style of existing code).
+
+But if you must have a list of rules, here they are.
For shell scripts specifically (not exhaustive):
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Jeff King @ 2009-01-25 21:44 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, Johannes Schindelin, Hannu Koivisto, git
In-Reply-To: <94a0d4530901251312q4201d51btd806fe860a12afd6@mail.gmail.com>
On Sun, Jan 25, 2009 at 11:12:57PM +0200, Felipe Contreras wrote:
> However, my last proposal was to have both the git config --global
> *and* the $HOME/.gitconfig description. Is there any argument against
> that?
This is like the fifth time you have asked, and for some reason, nobody
seems to have said yes or no. So I will go ahead and say: yes, I think
that is a fine idea.
I think there should also be some explanatory text that indicates they
are totally interchangeable for the rest of the document. Something
like: "When we show configuration in the rest of this document, we will
use format X [I think probably "git config $VAR $VALUE"]. But you can
use whichever method you are most comfortable with."
-Peff
^ permalink raw reply
* Re: Heads up: major rebase -i -p rework coming up
From: Jakub Narebski @ 2009-01-25 22:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sverre Rabbelier, git
In-Reply-To: <7vwscjceec.fsf@gitster.siamese.dyndns.org>
On Sun, 25 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > So maybe I answered my question myself:
> >
> > merge parents $sha1 [$sha1...] original $sha1 $msg
>
> When you are reparenting, how would original commit get in the picture?
> You wouldn't want the resulting merge to claim it merged X (which would be
> what's in original's commit log) when in fact it now merged Y because the
> user reparented it, would you?
Well, the subject part of merge (with merged branches names) shouldn't,
I guess, change. The summary (shortlog) part might, or perhaps even
should following rewrite (if it was present here).
But there is one issue I am wondering about: could we pick up _merge_
_resolution_? So if you have evil merge, and the change is for example
splitting commits without visible final changes, or just changing some
commit message before merge, it would get recreated without problems?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v1 0/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-25 22:07 UTC (permalink / raw)
To: Keith Cascio; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.GSO.2.00.0901251239000.12651@kiwi.cs.ucla.edu>
On Sun, Jan 25, 2009 at 12:41:21PM -0800, Keith Cascio wrote:
> > I am puzzled.
> >
> > The gitattributes mechanism is about per-path settings, but I do not
> > think a mnemonicprefix that is per-path makes much sense.
>
> That was just an example (perhaps poorly chosen). What I meant to
> suggest is making gitattributes consistent with gitconfig WRT at least
> the [diff] section. But maybe that's not appropriate. Thanks for the
> insight.
I don't think you want it entirely consistent. What would
diff.renamelimit mean in the context of a gitattribute? But I do think
it makes sense for some (like specific diff options such as whitespace
handling).
Also, if you're going to have options that apply to gitattributes diff
drivers _and_ as a general fallback, I think we need to define when the
fallback kicks in. That is, let's say I have a gitattributes file like
this:
*.c diff=c
and my config says:
[diff]
opt1 = val1_default
opt2 = val2_default
[diff "c"]
opt1 = val1_c
Now obviously if I want to use opt1 for my C files, it should be val1_c.
But if I want to use opt2, what should it use? There are two reasonable
choices, I think:
1. You use val2_default. The rationale is that the "c" diff driver did
not define an opt2, so you fall back to the global default.
2. It is unset. The rationale is that you are using the "c" diff
driver, and it has left the value unset. The default then means "if
you have no diff driver setup".
I suspect "1" is what people would want most of the time, but "2" is
actually more flexible (since there is otherwise no way to say "I
explicitly left diff.c.opt2 unset").
If (2) is desired, I think it makes more sense to put such "default"
options into their own diff driver section. Like:
[diff "default"]
opt2 = whatever
And then it is more clear that once you have selected the "c" diff
driver, the values in the other "default" are not relevant.
I don't think this is a huge issue overall, but it occurs to me that we
have just added diff.wordRegex and diff.*.wordRegex. So it makes sense
to think for a minute which behavior we want before it ships and we are
stuck with backwards compatibility forever.
-Peff
^ 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