* Re: possible Improving diff algoritm
From: Javier Domingo @ 2012-12-14 12:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Geert Bosch, Morten Welinder, Kevin, git
In-Reply-To: <7vzk1izcv6.fsf@alter.siamese.dyndns.org>
I think the idea of being preferable to have a blank line at the end
of the added/deleted block is key in this case.
Javier Domingo
2012/12/13 Junio C Hamano <gitster@pobox.com>:
> Geert Bosch <bosch@adacore.com> writes:
>
>> It would seem that just looking at the line length (stripped) of
>> the last line, might be sufficient for cost function to minimize.
>> Here the some would be 3 vs 0. In case of ties, use the last
>> possibility with minimum cost.
>
> -- 8< --
> #ifdef A
>
> some stuff
> about A
>
> #endif
> #ifdef Z
>
> some more stuff
> about Z
>
> #endif
> -- >8 --
>
> If you insert a block for M following the existing formatting
> convention in the middle, your heuristics will pick the blank line
> after "about A" as having minimum cost, no?
>
> You inherently have to know the nature of the payload, as your eyes
> that judge the result use that knowledge when doing so, I am afraid.
> I think your "define a function that gives a good score to lines
> that are likely to be good breaking points" idea has merit, but I
> think that should be tied to the content type, most likely via the
> attribute mechanism.
>
> In any case, I consider this as a low-impact (as Michael Haggerty
> noted, it is impossible to introduce a bug that subtly break the
> output; your result is either totally borked or is correct) and
> low-hanging fruit (it can be done as a postprocessing phase after
> the xdiff machinery has done the heavy-lifting of computing LCA), if
> somebody wants to experiment and implement one. As long as the new
> heuristics is hidden behind an explicit command line option to avoid
> other "consequences", I wouldn't discourage interested parties from
> working on it. It is not just my itch, though.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] git.c: add --index-file command-line option.
From: Manlio Perillo @ 2012-12-14 11:23 UTC (permalink / raw)
To: git; +Cc: Manlio Perillo
Unlike other environment variables (e.g. GIT_WORK_TREE,
GIT_NAMESPACE), it was not possible to set the GIT_INDEX_FILE
environment variable using the command line.
Add a new --index-file command-line option.
Update the t7500-commit test to include --index-file option coverage.
The tests have been adapted from the existing
'using alternate GIT_INDEX_FILE (1)' and
'using alternate GIT_INDEX_FILE (2)' tests.
Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
Documentation/git.txt | 10 +++++++++-
git.c | 17 ++++++++++++++++-
t/t7500-commit.sh | 29 +++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index e643683..5a582dd 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -12,7 +12,8 @@ SYNOPSIS
'git' [--version] [--help] [-c <name>=<value>]
[--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
[-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
- [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
+ [--git-dir=<path>] [--work-tree=<path>] [--index-file=<path>]
+ [--namespace=<name>]
<command> [<args>]
DESCRIPTION
@@ -408,6 +409,12 @@ help ...`.
variable (see core.worktree in linkgit:git-config[1] for a
more detailed discussion).
+--index-file=<path>::
+ Set the path to the index file. It can be an absolute path
+ or a path relative to the current working directory.
+ This can also be controlled by setting the GIT_INDEX_FILE
+ environment variable.
+
--namespace=<path>::
Set the git namespace. See linkgit:gitnamespaces[7] for more
details. Equivalent to setting the `GIT_NAMESPACE` environment
@@ -632,6 +639,7 @@ git so take care if using Cogito etc.
This environment allows the specification of an alternate
index file. If not specified, the default of `$GIT_DIR/index`
is used.
+ The '--index-file command-line option also sets this value.
'GIT_OBJECT_DIRECTORY'::
If the object storage directory is specified via this
diff --git a/git.c b/git.c
index d33f9b3..b0f473d 100644
--- a/git.c
+++ b/git.c
@@ -8,7 +8,8 @@
const char git_usage_string[] =
"git [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
" [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]\n"
- " [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n"
+ " [--git-dir=<path>] [--work-tree=<path>] [--index-file=<path>]\n"
+ " [--namespace=<name>]\n"
" [-c name=value] [--help]\n"
" <command> [<args>]";
@@ -121,6 +122,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
setenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
if (envchanged)
*envchanged = 1;
+ } else if (!strcmp(cmd, "--index-file")) {
+ if (*argc < 2) {
+ fprintf(stderr, "No path given for --index-file.\n" );
+ usage(git_usage_string);
+ }
+ setenv(INDEX_ENVIRONMENT, (*argv)[1], 1);
+ if (envchanged)
+ *envchanged = 1;
+ (*argv)++;
+ (*argc)--;
+ } else if (!prefixcmp(cmd, "--index-file=")) {
+ setenv(INDEX_ENVIRONMENT, cmd + 13, 1);
+ if (envchanged)
+ *envchanged = 1;
} else if (!strcmp(cmd, "--bare")) {
static char git_dir[PATH_MAX+1];
is_bare_repository_cfg = 1;
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 1c908f4..c405a78 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -168,6 +168,35 @@ test_expect_success 'using alternate GIT_INDEX_FILE (2)' '
cmp .git/index saved-index >/dev/null
'
+test_expect_success 'using alternate --index-file (1)' '
+
+ cp .git/index saved-index &&
+ (
+ echo some new content >file &&
+ index_file=.git/another_index &&
+ git --index-file=$index_file add file &&
+ git --index-file=$index_file commit -m "commit using another index" &&
+ git --index-file=$index_file diff-index --exit-code HEAD &&
+ git --index-file=$index_file diff-files --exit-code
+ ) &&
+ cmp .git/index saved-index >/dev/null
+
+'
+
+test_expect_success 'using alternate --index-file (2)' '
+
+ cp .git/index saved-index &&
+ (
+ rm -f .git/no-such-index &&
+ index_file=.git/no-such-index &&
+ git --index-file=$index_file commit -m "commit using nonexistent index" &&
+ test -z "$(git --index-file=$index_file ls-files)" &&
+ test -z "$(git --index-file=$index_file ls-tree HEAD)"
+
+ ) &&
+ cmp .git/index saved-index >/dev/null
+'
+
cat > expect << EOF
zort
--
1.8.1.rc1.17.g75ed918.dirty
^ permalink raw reply related
* Re: Build fixes for another obscure Unix
From: Joachim Schmitz @ 2012-12-14 7:54 UTC (permalink / raw)
To: git
In-Reply-To: <CAEvUa7nNNYREAsxc==tfg+e1XNZFbDVOpGXE6z-7+SfbqNrp6Q@mail.gmail.com>
David Michael wrote:
> Hi,
>
> On Thu, Dec 13, 2012 at 12:18 PM, Pyeron, Jason J CTR (US)
> <jason.j.pyeron.ctr@mail.mil> wrote:
>>> Would there be any interest in applying such individual
>>> compatibility fixes for this system, even if a full port doesn't
>>> reach completion?
>>
>> What are the down sides? Can your changes be shown to not impact
>> builds on other systems?
>
> I've pushed a handful of small compatibility patches to GitHub[1]
> which are enough to successfully compile the project. The default
> values of the new variables should make them unnoticeable to other
> systems.
>
> Are there any concerns with this type of change? If they would be
> acceptable, I can try sending the first four of those patches to the
> list properly. (I expect the last two may be tweaked as I continue
> working with the port.)
>
> I do have a concern with strings.h, though. That file will be
> included for most people who run ./configure, when it wasn't before.
> Do you think it's worth making a more detailed test to see if
> strcasecmp is still undefined after string.h is included, rather than
> just testing for the header's existence?
>
> Thanks.
>
> David
>
> [1] https://github.com/dm0-/git/commits
For what's it worth: I ACK your HP-NonStop patch (as you can see by my
comment in git-compat-util.h I was thinking along the same line)
https://github.com/dm0-/git/commit/933d72a5cfdc63fa9c3c68afa2f4899d9c3f791e
together with its prerequisit
https://github.com/dm0-/git/commit/301032c6488aeabb94ccc81bfb6d65ff2c23b924
ACKed by: Joachim Schmitz <jojo@schmitz-digital.de>
^ permalink raw reply
* [PATCH] Documentation/git-clean: Document --force --force
From: Soren Brinkmann @ 2012-12-14 1:46 UTC (permalink / raw)
To: git; +Cc: Soren Brinkmann
This patch documents the behavior of 'git clean' when
encountering nested git repositories.
Such repositories are only deleted if '-f' is passed twice
to 'git clean'.
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
Documentation/git-clean.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 9f42c0d..0b31454 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -23,6 +23,9 @@ example, be useful to remove all build products.
If any optional `<path>...` arguments are given, only those paths
are affected.
+Nested git repositories are not removed unless the '-f' option is
+passed to 'git clean' twice.
+
OPTIONS
-------
-d::
@@ -35,6 +38,8 @@ OPTIONS
--force::
If the git configuration variable clean.requireForce is not set
to false, 'git clean' will refuse to run unless given -f or -n.
+ Pass this option twice to 'git clean' in order to also remove
+ nested git repositories.
-n::
--dry-run::
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Felipe Contreras @ 2012-12-14 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vhanpwllh.fsf@alter.siamese.dyndns.org>
On Thu, Dec 13, 2012 at 5:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Perhaps. It's not really clear if we should update the working tree at
>>> all. A 'git push' doesn't update the working directory on the remote,
>>> but a 'bzr push' does. I thought it was better to leave this
>>> distinction clear, in case this becomes an issue later on.
>> ...
>> ... I don't mind rephrasing that four lines and amend the
>> log message of what I have in 'pu', if that is what you want.
>
> ... which may look like this.
>
> remote-bzr: update working tree upon pushing
>
> A 'git push' doesn't update the working directory on the remote, but
> a 'bzr push' does. Teach the remote helper for bzr to update the
> working tree on the bzr side upon pushing via the "export" command.
>
> Let me know if I grossly misinterpreted/misrepresented what you
> wanted to do in this patch.
Looks good to me.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-14 0:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsj79wmck.fsf@alter.siamese.dyndns.org>
On Thu, Dec 13, 2012 at 5:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> One of the review points were about this piece in the test:
>>>
>>> > +cmd=<<EOF
>>> > +import bzrlib
>>> > +bzrlib.initialize()
>>> > +import bzrlib.plugin
>>> > +bzrlib.plugin.load_plugins()
>>> > +import bzrlib.plugins.fastimport
>>> > +EOF
>>> > +if ! "$PYTHON_PATH" -c "$cmd"; then
>>>
>>> I cannot see how this could have ever worked.
>>>
>>> And I still don't see how your "would work just fine" can be true.
>>
>> As I have explained, all this code is the equivalent of python -c '',
>> or rather, it's a no-op. It works in the sense that it doesn't break
>> anything.
>
> Aren't you ashamed of yourself after having said this?
It is a fact.
>> The purpose of the code is to check for the fastimport plug-in, but
>> that plug-in is not used any more, it's vestigial code, it doesn't
>> matter if the check works or not, as long as it doesn't fail.
>
> If so, the final version that is suitable for merging would have
> that unused code stripped away, no?
To the users there's absolutely no difference.
>>> But it is totally a different matter to merge a crap with known
>>> breakage that is one easy fix away from the get-go. Allowing that
>>> means that all the times we spend on reviewing patches here go
>>> wasted, discouraging reviewers.
>>
>> There is no breakage.
>
> Unused code that burdens others to read through to make sure nothing
> is broken is already broken from maintenance point of view.
Remove the whole test then. I'm already doing way more than most of
the code in contrib by providing tests.
> Why are you wasting my time and everybody's bandwidth on this, when
> you are very well capable of rerolling the series with removal and
> style fixes in far shorter time?
I will do that, when I do that.
We have no time constraints, have we? This code is not getting in
1.8.1 either way.
Anyway, if you merge this code as it is, nothing bad will happen.
Nobody would get hurt, and in fact, very few, if anybody, would
notice.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: unclear documentation of git fetch --tags option and tagopt config
From: Junio C Hamano @ 2012-12-14 0:13 UTC (permalink / raw)
To: Philip Oakley; +Cc: 乙酸鋰, git
In-Reply-To: <084AB408ED4E4CF3B048B8615658F158@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> What would be the best way of updating the documentation to clarify the
> point? Given ch3cooli's previous surprise.
Oh, thanks for bringing it up. I was about to start another message
that begins with "Having said all that..." ;-)
I think the entire paragraph should be rewritten. The first long
sentence explains that you will get tags that point at the branches
and other stuff you follow by default, so there is no need to
explicitly ask for tags most of the time. While that is true, it is
secondary in describing what "--tags" is about. It is to grab all
tags and store them locally, and that needs to come at the very
beginning.
And that auto-following behaviour is already described in the
previous entry for -n/--no-tags. So how about something like this:
This is a short-hand for giving "refs/tags/*:refs/tags/*"
refspec from the command line, to ask all tags to be fetched
and stored locally.
Note that it is deliberate that the above does not mention tagopt
configuration at all. The variable was primarily meant to be used
with --no-tags, so that with this:
[remote "origin"] tagopt = --no-tags
you can "git fetch origin" to keep up with the development on
branches without having to fetch tags from there. Fetching tags and
only tags from a remote is almost always not what you want; in other
words, remote."origin".tagopt set to --tags is a misconfiguration
99% of the time, unless you are only interested in following tagged
release points.
^ permalink raw reply
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Junio C Hamano @ 2012-12-13 23:58 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vobhxwm3n.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> Perhaps. It's not really clear if we should update the working tree at
>> all. A 'git push' doesn't update the working directory on the remote,
>> but a 'bzr push' does. I thought it was better to leave this
>> distinction clear, in case this becomes an issue later on.
> ...
> ... I don't mind rephrasing that four lines and amend the
> log message of what I have in 'pu', if that is what you want.
... which may look like this.
remote-bzr: update working tree upon pushing
A 'git push' doesn't update the working directory on the remote, but
a 'bzr push' does. Teach the remote helper for bzr to update the
working tree on the bzr side upon pushing via the "export" command.
Let me know if I grossly misinterpreted/misrepresented what you
wanted to do in this patch.
Thanks.
^ permalink raw reply
* Re: unclear documentation of git fetch --tags option and tagopt config
From: Philip Oakley @ 2012-12-13 23:54 UTC (permalink / raw)
To: Junio C Hamano, 乙酸鋰; +Cc: git
In-Reply-To: <7v7golzta8.fsf@alter.siamese.dyndns.org>
From: "Junio C Hamano" <gitster@pobox.com> Sent: Thursday, December 13,
2012 6:44 PM
> 乙酸鋰 <ch3cooli@gmail.com> writes:
>
>> With git fetch --tags
>> or remote.origin.tagopt = --tags
>> git fetch only fetches tags, but not branches.
>> Current documentation does not mention that no branches are fetched /
>> pulled when --tags option or remote.origin.tagopt = --tags is
>> specified.
>
> In the canonical form you spell out what you want to fetch from
> where, and a lazy "git fetch" or "git fetch origin" that does not
> specify what are to be fetched are the special cases. Because they
> do not say what to fetch, they would become a no-op, which would not
> be very useful, if there is no special casing for them. Instead,
> they use sensible default, taking refspec from the configuration
> variable remote.$name.fetch.
>
> Giving refspecs or the "--tags" option from the command line is a
> way to explicitly override this default, hence:
>
> $ git fetch origin HEAD
>
> only fetches the history leading to the commit at HEAD at the
> remote, ignoring the configured refspecs. As "--tags" is a synonym
> to "refs/tags/*:refs/tags/*", "git fetch --tags origin" tells us to
> ignore refspecs and grab only the tags, i.e.:
>
> $ git fetch origin "refs/tags/*:refs/tags/*"
>
> which does not grab any branches.
>
> You can of course do:
>
> $ git fetch --tags origin
> refs/heads/master:refs/remotes/origin/master
>
> --
What would be the best way of updating the documentation to clarify the
point? Given ch3cooli's previous surprise.
^ permalink raw reply
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Junio C Hamano @ 2012-12-13 23:47 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <CAMP44s1ZdMK+0_pP3qkZUepOvkfMaXeY2BV0MFu5YOSV=40Dcw@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Wed, Nov 28, 2012 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>> contrib/remote-helpers/git-remote-bzr | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
>>> index 2c05f35..5b89a05 100755
>>> --- a/contrib/remote-helpers/git-remote-bzr
>>> +++ b/contrib/remote-helpers/git-remote-bzr
>>> @@ -571,6 +571,8 @@ def do_export(parser):
>>> repo.generate_revision_history(revid, marks.get_tip('master'))
>>> revno, revid = repo.last_revision_info()
>>> peer.import_last_revision_info_and_tags(repo, revno, revid)
>>> + wt = peer.bzrdir.open_workingtree()
>>> + wt.update()
>>> print "ok %s" % ref
>>> print
>>
>> Shouldn't this be squashed as part of one of the earlier patches?
>> The split between 1/7 (import first) and 2/7 (then support export)
>> makes sense, but this one looks more like "oops, we forgot to touch
>> the working tree while updating the history in the previous steps
>> and here is a fix" to me.
>
> Perhaps. It's not really clear if we should update the working tree at
> all. A 'git push' doesn't update the working directory on the remote,
> but a 'bzr push' does. I thought it was better to leave this
> distinction clear, in case this becomes an issue later on.
If you explained that in the log message, then we would have avoided
this exchange, and more importantly, if you had a in-code comment
there, it may help people who later want to add a knob to leave the
working tree intact. The seed you sow now to help others, who may
be less skillful and knowledgeable than you are, will help the
project in the longer term.
Thanks. I don't mind rephrasing that four lines and amend the
log message of what I have in 'pu', if that is what you want.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Junio C Hamano @ 2012-12-13 23:42 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s0qK6yNiPe0ERDJWK-wfm3DdXZYwRzisoCPJ7PjsdkObQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> One of the review points were about this piece in the test:
>>
>> > +cmd=<<EOF
>> > +import bzrlib
>> > +bzrlib.initialize()
>> > +import bzrlib.plugin
>> > +bzrlib.plugin.load_plugins()
>> > +import bzrlib.plugins.fastimport
>> > +EOF
>> > +if ! "$PYTHON_PATH" -c "$cmd"; then
>>
>> I cannot see how this could have ever worked.
>>
>> And I still don't see how your "would work just fine" can be true.
>
> As I have explained, all this code is the equivalent of python -c '',
> or rather, it's a no-op. It works in the sense that it doesn't break
> anything.
Aren't you ashamed of yourself after having said this?
> The purpose of the code is to check for the fastimport plug-in, but
> that plug-in is not used any more, it's vestigial code, it doesn't
> matter if the check works or not, as long as it doesn't fail.
If so, the final version that is suitable for merging would have
that unused code stripped away, no?
>> But it is totally a different matter to merge a crap with known
>> breakage that is one easy fix away from the get-go. Allowing that
>> means that all the times we spend on reviewing patches here go
>> wasted, discouraging reviewers.
>
> There is no breakage.
Unused code that burdens others to read through to make sure nothing
is broken is already broken from maintenance point of view.
Why are you wasting my time and everybody's bandwidth on this, when
you are very well capable of rerolling the series with removal and
style fixes in far shorter time?
^ permalink raw reply
* Re: Build fixes for another obscure Unix
From: David Michael @ 2012-12-13 22:30 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <871B6C10EBEFE342A772D1159D13208539FF9088@umechphg.easf.csd.disa.mil>
Hi,
On Thu, Dec 13, 2012 at 12:18 PM, Pyeron, Jason J CTR (US)
<jason.j.pyeron.ctr@mail.mil> wrote:
>> Would there be any interest in applying such individual compatibility
>> fixes for this system, even if a full port doesn't reach completion?
>
> What are the down sides? Can your changes be shown to not impact builds on other systems?
I've pushed a handful of small compatibility patches to GitHub[1]
which are enough to successfully compile the project. The default
values of the new variables should make them unnoticeable to other
systems.
Are there any concerns with this type of change? If they would be
acceptable, I can try sending the first four of those patches to the
list properly. (I expect the last two may be tweaked as I continue
working with the port.)
I do have a concern with strings.h, though. That file will be
included for most people who run ./configure, when it wasn't before.
Do you think it's worth making a more detailed test to see if
strcasecmp is still undefined after string.h is included, rather than
just testing for the header's existence?
Thanks.
David
[1] https://github.com/dm0-/git/commits
^ permalink raw reply
* Re: [PATCH] Fix sizeof usage in get_permutations
From: Felipe Contreras @ 2012-12-13 22:09 UTC (permalink / raw)
To: Joachim Schmitz; +Cc: git
In-Reply-To: <kacus2$mml$1@ger.gmane.org>
On Thu, Dec 13, 2012 at 10:13 AM, Joachim Schmitz
<jojo@schmitz-digital.de> wrote:
> Matthew Daley wrote:
>>
>> Currently it gets the size of an otherwise unrelated, unused variable
>> instead of the expected struct size.
>>
>> Signed-off-by: Matthew Daley <mattjd@gmail.com>
>> ---
>> builtin/pack-redundant.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
>> index f5c6afc..7544687 100644
>> --- a/builtin/pack-redundant.c
>> +++ b/builtin/pack-redundant.c
>> @@ -301,14 +301,14 @@ static void pll_free(struct pll *l)
>> */
>> static struct pll * get_permutations(struct pack_list *list, int n)
>> {
>> - struct pll *subset, *ret = NULL, *new_pll = NULL, *pll;
>> + struct pll *subset, *ret = NULL, *new_pll = NULL;
>>
>> if (list == NULL || pack_list_size(list) < n || n == 0)
>> return NULL;
>>
>> if (n == 1) {
>> while (list) {
>> - new_pll = xmalloc(sizeof(pll));
>> + new_pll = xmalloc(sizeof(struct pll));
>> new_pll->pl = NULL;
>> pack_list_insert(&new_pll->pl, list);
>> new_pll->next = ret;
>> @@ -321,7 +321,7 @@ static struct pll * get_permutations(struct
>> pack_list *list, int n) while (list->next) {
>> subset = get_permutations(list->next, n - 1);
>> while (subset) {
>> - new_pll = xmalloc(sizeof(pll));
>> + new_pll = xmalloc(sizeof(struct pll));
>
>
> Why not just
> new_pll = xmalloc(sizeof(*new_pll));
I prefer that one; it's Linux kernel style.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Felipe Contreras @ 2012-12-13 22:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vtxs9vda3.fsf@alter.siamese.dyndns.org>
On Wed, Nov 28, 2012 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> contrib/remote-helpers/git-remote-bzr | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
>> index 2c05f35..5b89a05 100755
>> --- a/contrib/remote-helpers/git-remote-bzr
>> +++ b/contrib/remote-helpers/git-remote-bzr
>> @@ -571,6 +571,8 @@ def do_export(parser):
>> repo.generate_revision_history(revid, marks.get_tip('master'))
>> revno, revid = repo.last_revision_info()
>> peer.import_last_revision_info_and_tags(repo, revno, revid)
>> + wt = peer.bzrdir.open_workingtree()
>> + wt.update()
>> print "ok %s" % ref
>> print
>
> Shouldn't this be squashed as part of one of the earlier patches?
> The split between 1/7 (import first) and 2/7 (then support export)
> makes sense, but this one looks more like "oops, we forgot to touch
> the working tree while updating the history in the previous steps
> and here is a fix" to me.
Perhaps. It's not really clear if we should update the working tree at
all. A 'git push' doesn't update the working directory on the remote,
but a 'bzr push' does. I thought it was better to leave this
distinction clear, in case this becomes an issue later on.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-13 22:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwxhycii.fsf@alter.siamese.dyndns.org>
On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>>> New remote helper for bzr (v3). With minor fixes, this may be ready
>>>>> for 'next'.
>>>>
>>>> What minor fixes?
>>>
>>> Lookng at the above (fixup), $gmane/210744 comes to mind
>>
>> That doesn't matter. The code and the tests would work just fine.
>
> One of us must be very confused. Perhaps you were looking at a
> wrong message (or I quoted a wrong one?).
>
> ... goes and double checks ...
>
> One of the review points were about this piece in the test:
>
> > +cmd=<<EOF
> > +import bzrlib
> > +bzrlib.initialize()
> > +import bzrlib.plugin
> > +bzrlib.plugin.load_plugins()
> > +import bzrlib.plugins.fastimport
> > +EOF
> > +if ! "$PYTHON_PATH" -c "$cmd"; then
>
> I cannot see how this could have ever worked.
>
> And I still don't see how your "would work just fine" can be true.
As I have explained, all this code is the equivalent of python -c '',
or rather, it's a no-op. It works in the sense that it doesn't break
anything.
The purpose of the code is to check for the fastimport plug-in, but
that plug-in is not used any more, it's vestigial code, it doesn't
matter if the check works or not, as long as it doesn't fail.
>>> but there may be others. It is the responsibility of a contributor to keep
>>> track of review comments others give to his or her patches and
>>> reroll them, so I do not recall every minor details, sorry.
>
> There may be others, but $gmane/210745 is also relevant, I think.
I'll comment on the patch, but I don't think it really prevents the merge.
>> There is nothing that prevents remote-bzr from being merged.
>
> What we merge may not be perfect. Bugs and misdesigns are often
> identified long after a topic is merged and it is perfectly normal
> we fix things up in-tree. There are even times where we say "OK, it
> is known to break if the user does something that pokes this and
> that obscure corners of this code, but the benefit of merging this
> 99% working code to help users that do not exercise the rare cases
> is greater than having them wait for getting the remaining 1% right,
> so let's merge it with known breakage documentation".
>
> But it is totally a different matter to merge a crap with known
> breakage that is one easy fix away from the get-go. Allowing that
> means that all the times we spend on reviewing patches here go
> wasted, discouraging reviewers.
There is no breakage.
> If you want others to take your patches with respect, please take
> reviewers' comments with the same respect you expect to be paid by
> others.
I don't need others to take my patches with respect, my patches are
not people, they don't have feelings.
That being said, I don't think I have disrespected any of your
comments. Yes, you are right that the above code is wrong and doesn't
do anything, what part of agreeing is disrespectful? But I don't agree
that it is a merge blocker. Disagreeing is not disrespecting.
This code was ready for 1.8.1, but it's not going to be there, so, I
don't see any hurry. As I said, I think the code is ready, and these
minor details can wait.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* asd
From: Philipp Bartsch @ 2012-12-13 21:25 UTC (permalink / raw)
To: git
subscribe git
^ permalink raw reply
* [PATCH] For git-subtree, when installing docs (make install-doc), create man1 folder first.
From: Jesper L. Nielsen @ 2012-12-13 20:09 UTC (permalink / raw)
To: git, gitster; +Cc: Jesper L. Nielsen
From: "Jesper L. Nielsen" <lyager@gmail.com>
Hi..
I installed Git subtree and discovered that the if the man1dir doesn't exist the man-page for Git Subtree is just called man1.
So, small patch to create the folder first in the Makefile. Hope everything is right with the patch and submitting of the patch.
Best Regards
Jesper
Signed-off-by: Jesper L. Nielsen <lyager@gmail.com>
---
contrib/subtree/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 05cdd5c..a341cf4 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -35,6 +35,7 @@ install: $(GIT_SUBTREE)
install-doc: install-man
install-man: $(GIT_SUBTREE_DOC)
+ mkdir -p $(man1dir)
$(INSTALL) -m 644 $^ $(man1dir)
$(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCHv2] Add directory pattern matching to attributes
From: Junio C Hamano @ 2012-12-13 20:08 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: git
In-Reply-To: <201212082104.39411.avila.jn@gmail.com>
"Jean-Noël AVILA" <avila.jn@gmail.com> writes:
> The manpage of gitattributes says: "The rules how the pattern
> matches paths are the same as in .gitignore files" and the gitignore
> pattern matching has a pattern ending with / for directory matching.
>
> This rule is specifically relevant for the 'export-ignore' rule used
> for git archive.
>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
> archive.c | 3 ++-
> attr.c | 32 ++++++++++++++++------
> t/t5002-archive-attr-pattern.sh | 57 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 83 insertions(+), 9 deletions(-)
> create mode 100644 t/t5002-archive-attr-pattern.sh
Looks nicely done.
> diff --git a/attr.c b/attr.c
> index 097ae87..cdba88a 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -564,17 +564,31 @@ static void bootstrap_attr_stack(void)
> attr_stack = elem;
> }
>
> +static const char *find_basename(const char *path)
> +{
> + char pathbuf[PATH_MAX];
> + int pathlen;
> + const char *cp;
> +
> + pathlen =strlen(path);
> + if (path[pathlen-1] != '/') {
> + cp =strrchr(path, '/');
> + return cp ? cp + 1: path;
> + } else {
> + strncpy(pathbuf, path, pathlen);
> + pathbuf[pathlen-1] = '\0';
> + cp =strrchr(pathbuf, '/');
> + return cp ? path + (cp - pathbuf) + 1 : path;
> + }
> +}
Let's do this function like this instead; shorter and equally easy
to understand.
static const char *find_basename(const char *path)
{
const char *cp, *last_slash = NULL;
for (cp = path; *cp; cp++) {
if (*cp == '/' && cp[1])
last_slash = cp;
}
return last_slash ? last_slash + 1 : path;
}
Thanks; will queue.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Junio C Hamano @ 2012-12-13 19:31 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s3uyC0V6ycTv78mG36_i7ugMLwwNk2cqNZatEJuL7Ee1w@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>>> New remote helper for bzr (v3). With minor fixes, this may be ready
>>>> for 'next'.
>>>
>>> What minor fixes?
>>
>> Lookng at the above (fixup), $gmane/210744 comes to mind
>
> That doesn't matter. The code and the tests would work just fine.
One of us must be very confused. Perhaps you were looking at a
wrong message (or I quoted a wrong one?).
... goes and double checks ...
One of the review points were about this piece in the test:
> +cmd=<<EOF
> +import bzrlib
> +bzrlib.initialize()
> +import bzrlib.plugin
> +bzrlib.plugin.load_plugins()
> +import bzrlib.plugins.fastimport
> +EOF
> +if ! "$PYTHON_PATH" -c "$cmd"; then
I cannot see how this could have ever worked.
And I still don't see how your "would work just fine" can be true.
>> but there may be others. It is the responsibility of a contributor to keep
>> track of review comments others give to his or her patches and
>> reroll them, so I do not recall every minor details, sorry.
There may be others, but $gmane/210745 is also relevant, I think.
> There is nothing that prevents remote-bzr from being merged.
What we merge may not be perfect. Bugs and misdesigns are often
identified long after a topic is merged and it is perfectly normal
we fix things up in-tree. There are even times where we say "OK, it
is known to break if the user does something that pokes this and
that obscure corners of this code, but the benefit of merging this
99% working code to help users that do not exercise the rare cases
is greater than having them wait for getting the remaining 1% right,
so let's merge it with known breakage documentation".
But it is totally a different matter to merge a crap with known
breakage that is one easy fix away from the get-go. Allowing that
means that all the times we spend on reviewing patches here go
wasted, discouraging reviewers.
If you want others to take your patches with respect, please take
reviewers' comments with the same respect you expect to be paid by
others.
^ permalink raw reply
* Re: [PATCH] Fix sizeof usage in get_permutations
From: Junio C Hamano @ 2012-12-13 19:11 UTC (permalink / raw)
To: Matthew Daley; +Cc: git
In-Reply-To: <1355405790-20302-1-git-send-email-mattjd@gmail.com>
Thanks; it shows how rarely this obscure tool is used these days.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-13 19:06 UTC (permalink / raw)
To: Max Horn; +Cc: Junio C Hamano, git
In-Reply-To: <BF9B1394-0321-4F1C-AD1B-F40D02DBE71A@quendi.de>
On Thu, Dec 13, 2012 at 6:04 AM, Max Horn <postbox@quendi.de> wrote:
>
> On 13.12.2012, at 11:08, Felipe Contreras wrote:
>
>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>>> New remote helper for bzr (v3). With minor fixes, this may be ready
>>>>> for 'next'.
>>>>
>>>> What minor fixes?
>>>
>>> Lookng at the above (fixup), $gmane/210744 comes to mind
>>
>> That doesn't matter. The code and the tests would work just fine.
>
>
> It doesn't matter? I find that statement hard to align with what the maintainer of git, and thus the person who decides whether your patch series gets merged or not, wrote just above? In fact, it seems to me that what Junio said matters a great deal...
So you think Junio knows more about remote-bzr than I do? I repeat; it
doesn't affect the tests, it doesn't affect the code, it doesn't cause
any problem. remote-bzr could be merged today, in fact, it could have
been merged a month ago.
You don't trust me? Here, look:
cmd=<<EOF
import bzrlib
bzrlib.initialize()
import bzrlib.plugin
bzrlib.plugin.load_plugins()
import bzrlib.plugins.fastimport
EOF
if ! "$PYTHON_PATH" -c "$cmd"; then
echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
skip_all='skipping remote-bzr tests; bzr-fastimport not available'
test_done
fi
All this code is a no-op, because, as Junio pointed out, cmd is null.
How is that a problem? It's not. The first version of remote-bzr
relied on the bazaar fastimport plug-in, so this check was needed, in
case you had bazaar, but not this particular plug-in, but today
remote-bzr doesn't need this plug-in, so this chunk of code should be
removed. The fact that this code does nothing (because python -c ''
does nothing) is *not a problem*.
In fact, even if that code failed 100% of the time, it wouldn't hurt
anybody, because 'make -C t' would work, everything would work, the
only thing that would fail is 'make -C contrib/remote-helpers/
test-bzr', which very very few people would consider a problem. But it
doesn't fail, it works.
Who benefits by delaying the merging of this code? Nobody. Who gets
hurt? The users, of course.
> This is a very strange attitude...
>
> In another email, you complained about nobody reviewing your patches respectively nobody voicing any constructive criticism. Yet Junio did just that, and again in $gmane/210745 -- and you replied to neither, and acted on neither (not even by refuting the points brought up), and now summarily dismiss them as irrelevant. I find that quite disturbing :-(.
I didn't say it was irrelevant, it should be fixed, but Junio said
"With minor fixes, this may be ready for 'next'." which is no true
IMO, it's ready *now*, it was ready one month ago. For 'next', this
problem doesn't matter.
The feedback is appreciated, but delaying the merging of this code for
no reason makes little sense to me. Junio, of course, can do whatever
he wants. The removal of this no-op code can wait, or it can be done
on top of v3, there's no need for re-roll, and Junio already
complained about the v3 re-roll.
And I didn't act because I was on vacations, git development is not my
only priority. And even if I had time, I don't see why I should
prioritize this fix, it's not important, the code is ready.
>>> but there may be others. It is the responsibility of a contributor to keep
>>> track of review comments others give to his or her patches and
>>> reroll them, so I do not recall every minor details, sorry.
>>
>> There is nothing that prevents remote-bzr from being merged.
>
> Well, I think that is up to Junio to decide in the end, though :-). He wrote
No. He can decide if the code gets merged, but he is not the voice of
truth. Nothing prevents him from merging the code, except himself.
There is no known issue with the code, that is a true fact.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2] git.txt: add missing info about --git-dir command-line option
From: Junio C Hamano @ 2012-12-13 18:54 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git
In-Reply-To: <1355421439-14024-1-git-send-email-manlio.perillo@gmail.com>
Thanks.
^ permalink raw reply
* Re: Build fixes for another obscure Unix
From: Junio C Hamano @ 2012-12-13 18:53 UTC (permalink / raw)
To: David Michael; +Cc: git
In-Reply-To: <CAEvUa7nn9M5np3wD=Z1152K4pwNGhSKkC=rS9U=yc=UcaOxMCw@mail.gmail.com>
David Michael <fedora.dm0@gmail.com> writes:
> I've been experimenting with git running on z/OS USS. It is not yet
> stable, but I have had to make a few fixes and generalizations in the
> build system to get it to compile.
>
> Would there be any interest in applying such individual compatibility
> fixes for this system, even if a full port doesn't reach completion?
If you post patches here, it may help you find like-minded people
who may want to join forces and help you complete the port, so by
all means.
If I pick up a partially completed series and carry it in my tree is
a separate matter. The patch has to be cleanly done (i.e. without
too many #ifdefs) for longer-term maintainability, and it has to be
clear that the changes do not affect other platforms negatively.
^ permalink raw reply
* Re: unclear documentation of git fetch --tags option and tagopt config
From: Junio C Hamano @ 2012-12-13 18:44 UTC (permalink / raw)
To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6Ti7yPFfhTb2qfSKE1+5n4Ftey4DQeqpm3SSL-bOfspUg@mail.gmail.com>
乙酸鋰 <ch3cooli@gmail.com> writes:
> With git fetch --tags
> or remote.origin.tagopt = --tags
> git fetch only fetches tags, but not branches.
> Current documentation does not mention that no branches are fetched /
> pulled when --tags option or remote.origin.tagopt = --tags is
> specified.
In the canonical form you spell out what you want to fetch from
where, and a lazy "git fetch" or "git fetch origin" that does not
specify what are to be fetched are the special cases. Because they
do not say what to fetch, they would become a no-op, which would not
be very useful, if there is no special casing for them. Instead,
they use sensible default, taking refspec from the configuration
variable remote.$name.fetch.
Giving refspecs or the "--tags" option from the command line is a
way to explicitly override this default, hence:
$ git fetch origin HEAD
only fetches the history leading to the commit at HEAD at the
remote, ignoring the configured refspecs. As "--tags" is a synonym
to "refs/tags/*:refs/tags/*", "git fetch --tags origin" tells us to
ignore refspecs and grab only the tags, i.e.:
$ git fetch origin "refs/tags/*:refs/tags/*"
which does not grab any branches.
You can of course do:
$ git fetch --tags origin refs/heads/master:refs/remotes/origin/master
^ permalink raw reply
* Re: [PATCH v3 2/2] cache-tree: remove dead i-t-a code in verify_cache()
From: Junio C Hamano @ 2012-12-13 18:34 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <1355362747-13474-2-git-send-email-pclouds@gmail.com>
Will replace the one in 'pu' with these two. Thanks.
^ 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