* Re: [PATCH v4 2/2] push: teach --recurse-submodules the on-demand option
From: Phil Hord @ 2011-12-12 23:50 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Heiko Voigt, Fredrik Gustafsson, git
In-Reply-To: <4EE6805D.7020708@web.de>
On Mon, Dec 12, 2011 at 5:29 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 12.12.2011 22:16, schrieb Phil Hord:
>> On Tue, Oct 18, 2011 at 4:58 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>>> Am 18.10.2011 00:33, schrieb Junio C Hamano:
>>>> We could even declare that the gitlink for such a
>>>> submodule should record 0{40} SHA-1 in the superproject, but I do not
>>>> think that is necessary.
>>>
>>> Me neither, e.g. the SHA-1 which was the submodules HEAD when it was added
>>> should do nicely. And that would avoid referencing a non-existing commit
>>> in case you later want to turn a floating submodule into an exact one.
>>
>>
>> I'm sorry I missed this comment before.
>>
>> I hope we can allow storing the actual gitlink in the superproject for
>> each commit even when we're using floating submodules.
>
> I think you misread my statement, I was just talking about the initial
> commit containing the newly added submodule, not any subsequent ones.
> Floating makes differences between the original SHA-1 and the current
> tip of the branch invisible, so there is nothing to commit.
>
>> I thought-experimented with this a bit last year and came to the
>> conclusion that I should be able to 'float' to tips (developer
>> convenience) and also to store the SHA-1 of each gitlink through
>> history (automated maybe; as-needed).
>
> Which means that after "git submodule update" floated a submodule branch
> further, you would have to commit that in the superproject.
Sadly, yes. Currently I have my CI-server do this for me after it
verifies each new submodule commit is able to build successfully.
>> The problem with "float-only" is that it loses history so, for
>> example, git-bisect doesn't work.
>
> Yep. And different developers can have the same superproject commit
> checked out but their submodules can be quite different.
>> The problem with "float + gitlinks", of course, is that it looks like
>> "not floating" to the developers (git-status is dirty unless
>> overridden, etc.)
>
> Yeah. But what if each "git submodule update" would update the tip of
> the submodule branch and add that to the superproject? You could follow
> a tip but still produce reproducible trees.
Yes, and that's what I want.
Not what it sounded like was being suggested before, which (to my
eyes) implied that the submodule gitlinks were useless noise.
Phil
^ permalink raw reply
* Re: [PATCH v3 0/3] grep attributes and multithreading
From: Junio C Hamano @ 2011-12-12 23:44 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, René Scharfe, Jeff King, Eric Herman
In-Reply-To: <cover.1323723759.git.trast@student.ethz.ch>
Thanks; sign-off?
^ permalink raw reply
* Re: Auto update submodules after merge and reset
From: Phil Hord @ 2011-12-12 23:43 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Andreas T.Auer, git
In-Reply-To: <4EE682A3.8070704@web.de>
On Mon, Dec 12, 2011 at 5:39 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 11.12.2011 22:15, schrieb Andreas T.Auer:
>> In http://thread.gmane.org/gmane.comp.version-control.git/183837 was discussed whether the gitlink in the superproject should be set to all-zero if updates follow the tip or maybe use the SHA1 of the commit when the submodule was added. I think the gitlink should be updated everytime when a new commit in the superproject is created.
>
> Nope, only when "git submodule update" is run. Otherwise you'll spray the
> history with submodule updates totally unrelated to the commits in the
> superproject, which is rather confusing.
And this is why my superproject is a makefile, a .gitmodules file and
a bunch of gitlinks. We only use it to track the advancement of
submodule activity.
So yes, I want my superproject's gitlinks to update automatically. I
just don't know a smart way to make that happen.
Phil
^ permalink raw reply
* Re: [PATCH v2] Update documentation for stripspace
From: Junio C Hamano @ 2011-12-12 23:41 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git
In-Reply-To: <1323728909-7847-1-git-send-email-conrad.irwin@gmail.com>
Conrad Irwin <conrad.irwin@gmail.com> writes:
> ...
> I've moved the *NOTE* into a SEE ALSO section where I think it reads
> less opinionatedly ― is that better?
I think it looks a lot worse.
At least your original hinted that some people may confuse the two and the
NOTE was there for such people; other people who would not even dream of
such a confusion won't find the existence of the note a "Huh?". But the
updated patch with a link in SEE ALSO section without any explanation
would be a definite "Huh?" for those of us who find that stripspace does
not have anything to do with what "apply --whitespace=fix" does.
The new example section looks good. Perhaps we can just drop the extra SEE
ALSO and resurrect the *NOTE* from your v1 patch.
Thanks.
^ permalink raw reply
* Re: Breakage (?) in configure and git_vsnprintf()
From: Brandon Casey @ 2011-12-12 23:25 UTC (permalink / raw)
To: Jeff King
Cc: Andreas Schwab, Michael Haggerty, Junio C Hamano,
git discussion list, Michal Rokos
In-Reply-To: <20111212142550.GA23875@sigill.intra.peff.net>
FYI: all tests passed on IRIX (not that I suspected otherwise).
-Brandon
On 12/12/2011 08:25 AM, Jeff King wrote:
> On Mon, Dec 12, 2011 at 10:30:27AM +0100, Andreas Schwab wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> if (maxsize > 0) {
>>> - ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
>>> + va_copy(cp, ap);
>>> + ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
>>
>> You also need to call va_end on the copy.
>
> Silly me. Thanks for catching.
>
> Junio, here's a corrected version.
>
> -- >8 --
> Subject: [PATCH] compat/snprintf: don't look at va_list twice
>
> If you define SNPRINTF_RETURNS_BOGUS, we use a special
> git_vsnprintf wrapper assumes that vsnprintf returns "-1"
> instead of the number of characters that you would need to
> store the result.
>
> To do this, it invokes vsnprintf multiple times, growing a
> heap buffer until we have enough space to hold the result.
> However, this means we evaluate the va_list parameter
> multiple times, which is generally a bad thing (it may be
> modified by calls to vsnprintf, yielding undefined
> behavior).
>
> Instead, we must va_copy it and hand the copy to vsnprintf,
> so we always have a pristine va_list.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> compat/snprintf.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/compat/snprintf.c b/compat/snprintf.c
> index e1e0e75..42ea1ac 100644
> --- a/compat/snprintf.c
> +++ b/compat/snprintf.c
> @@ -19,11 +19,14 @@
> #undef vsnprintf
> int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
> {
> + va_list cp;
> char *s;
> int ret = -1;
>
> if (maxsize > 0) {
> - ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
> + va_copy(cp, ap);
> + ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
> + va_end(cp);
> if (ret == maxsize-1)
> ret = -1;
> /* Windows does not NUL-terminate if result fills buffer */
> @@ -42,7 +45,9 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
> if (! str)
> break;
> s = str;
> - ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
> + va_copy(cp, ap);
> + ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
> + va_end(cp);
> if (ret == maxsize-1)
> ret = -1;
> }
^ permalink raw reply
* Re: [PATCH 2/2] Makefile: optionally exclude code that needs Unix sockets
From: Junio C Hamano @ 2011-12-12 23:31 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, git
In-Reply-To: <20111212213951.GB9754@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> In theory we should also disable the documentation for credential-cache.
> But that means surgery not only to Documentation/Makefile, but figuring
> out how to pass the flags down to the actual asciidoc process (since
> gitcredentials(7) mentions it in the text). Certainly possible, but I
> don't know if it's worth the effort or not.
I do not think it matters that much. We've been shipping documentation for
stuff like remote archiver and daemon without conditional compilation, no?
^ permalink raw reply
* Re: [PATCH 1/3] test-terminal: give the child an empty stdin TTY
From: Jonathan Nieder @ 2011-12-12 23:25 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Thomas Rast, git, Michael Haggerty
In-Reply-To: <20111212191602.GA14061@sigill.intra.peff.net>
Jeff King wrote:
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -198,6 +198,9 @@ else
> exec 4>/dev/null 3>/dev/null
> fi
>
> +exec 6<&0
> +exec 0</dev/null
> +
> test_failure=0
> test_count=0
> test_fixed=0
Independently of the changes to explicitly pass HEAD to "git shortlog"
in t7006 (we should) and make test_terminal make stdin into a tty, too
(if tests still make sure "git log --stdin" launches a pager with
stdin not a tty, then it should be a fine change), this looks good to
me.
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Phil Hord @ 2011-12-12 22:56 UTC (permalink / raw)
To: Leif Gruenwoldt; +Cc: Andreas T.Auer, Junio C Hamano, git
In-Reply-To: <CALFF=ZRB7qjj7VMhzr12ySdHmZsySoqceu5brFht8rX1+W3NPg@mail.gmail.com>
On Mon, Dec 12, 2011 at 2:13 PM, Leif Gruenwoldt <leifer@gmail.com> wrote:
> On Mon, Dec 12, 2011 at 1:42 PM, Andreas T.Auer
> <andreas.t.auer_gtml_37453@ursus.ath.cx> wrote:
>
>> The next question is: Wouldn't you like to have the new stable branch only
>> pulled in, when the projectX (as the superproject) is currently on that new
>> development branch (maybe master)?
>>
>> But if you checkout that fixed released version 1.2.9.8, wouldn't it be
>> better that in that case the gitlinked version of the submodule is checked
>> out instead of some unrelated new version? I mean, when the gitlinks are
>> tracked with the projectX commits, this should work well.
>>
>> And what about a maintenance branch, which is not a fixed version but a
>> quite stable branch which should only have bugfixes. Shouldn't the auto-pull
>> be disabled in that case, too?
>>
>> I think the "auto-pull" behavior should depend on the currently checked out
>> branch. So the configuration options should allow the definition of one or
>> more mappings.
>
> Yes. I think you nailed it. The floating behaviour would best be
> configured per branch.
Yes, I think you nailed it too. I've been thinking the same thing for
a while now, but I didn't know how to express it completely. Some of
the discussion on here last week gelled the last bits in my mind.
To wit, I think I would want something like this in my project:
Use gitlinks when the superproject HEAD is one of these:
refs/heads/maint/*
refs/heads/svn/* (historic branches)
refs/tags/*
<SHA1> (detached)
Float on the rest, using the branch given in .gitmodules (which may be
* to mean "use the same branch as the superproject".)
But maybe it is foolish of me to keep branches where I really want
lightweight tags. If so, I could get away with this:
Float if .git/HEAD begins with "refs/heads"
Else, use the SHA1.
> An aside. Would this mean a "git pull" on the product repo would
> automatically do a pull (git submodule update) on the submodule too?
Good question.
I want to say "eventually, but not yet." But someone else may
disagree. "git pull --recurse-submodules=yes" does not do this yet.
A separate git-submodule-update is still required. But I think this
is a separate issue from floating submodules.
Phil
^ permalink raw reply
* Re: Auto update submodules after merge and reset
From: Jens Lehmann @ 2011-12-12 22:39 UTC (permalink / raw)
To: Andreas T.Auer; +Cc: Phil Hord, git
In-Reply-To: <4EE51D7B.7020806@ursus.ath.cx>
Am 11.12.2011 22:15, schrieb Andreas T.Auer:
>
>
> On 10.12.2011 00:57 Phil Hord wrote:
>> On Thu, Dec 8, 2011 at 4:13 AM,
>> <andreas.t.auer_gtml_37453@ursus.ath.cx> wrote:
>>
>> Yes, but maybe you can update this information in the .gitmodules
>> file easily with a command. Maybe it could be something simpler
>> than "git sync-gitmodules-branches", but that is essentially what it
>> would do: it would save the current branch in each submodule as the
>> "tracked" branch in the .gitmodules file.
>
> Ok, I have read a better description of the "floating submodule" model now, so it is a different use case and somehow it makes sense. In that case there are probably just a few branches that you would like to follow, maybe an "unstable" for the newest development or "stable" for the current release or some maintenance branches.
>
>> Now this makes sense. I want the same thing. I want to preserve
>> history on "old" commits, but I want to "advance to tip" on "new"
>> commits.
>>
>> The trouble, I think, is in telling the difference between "old" and
>> "new". I think it means there is a switch, like --auto-follow (or
>> --no-auto-follow for the alternate if core.auto_follow is set). But
>> having a config option as the default is likely to break lots of
>> scripts.
>
> In my use case the branches on the submodules follow the superproject and (mostly) versions that are committed there, it just adds the possibility to keep on working without checking out a branch after an update and without colliding with existing branchnames in the submodule.
Using superproject branch names in the submodules make no sense for a
lot of use cases.
> The other use case wants to follow the commits of that other submodule without checking the corresponding gitlinks into the superproject. But wouldn't it also make sense here to define actually a mapping in the .gitmodule that says "if the branch 'develop' is checkedout in the supermodule then with every submodule update automatically pull the newest 'unstable' commit from the submodule"? Or for "master" follow "stable" or for the "maint" branch follow updates in the "bugfixes" branch.
>
> For example
>
> [submodule "commonlib"]
> update = heads develop:unstable master:stable maint:bugfixes
Having that configured with "branch=unstable", "branch=stable" etc. in
.gitmodules on the superprojects branches would be simpler and achieve
the same functionality.
> So whenever a defined branch is checked out in the superproject the mapped branch will be checked out in the submodule ("new" commit), but if a (e.g. tagged) commit is checked out ("old" commit) then the gitlink in the superproject is used to check out the referenced commit in the submodule.
I think checkout should only use the submodule commit recorded in the
superproject and a subsequent "git submodule update" should be needed
to update the submodule to tip. Otherwise you record SHA-1 but still
won't be able to bisect ...
> In http://thread.gmane.org/gmane.comp.version-control.git/183837 was discussed whether the gitlink in the superproject should be set to all-zero if updates follow the tip or maybe use the SHA1 of the commit when the submodule was added. I think the gitlink should be updated everytime when a new commit in the superproject is created.
Nope, only when "git submodule update" is run. Otherwise you'll spray the
history with submodule updates totally unrelated to the commits in the
superproject, which is rather confusing.
^ permalink raw reply
* Re: [PATCH 4/2] grep: turn off threading for non-worktree
From: René Scharfe @ 2011-12-12 22:37 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: Thomas Rast, git, Eric Herman, Junio C Hamano
In-Reply-To: <20111210131305.GA13344@arf.padd.com>
Am 10.12.2011 14:13, schrieb Pete Wyckoff:
> rene.scharfe@lsrfire.ath.cx wrote on Wed, 07 Dec 2011 18:00 +0100:
>> Am 07.12.2011 09:12, schrieb Thomas Rast:
>>> René Scharfe wrote:
>>>> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
>>>> index 47ac188..e981a9b 100644
>>>> --- a/Documentation/git-grep.txt
>>>> +++ b/Documentation/git-grep.txt
>>>> @@ -228,8 +228,9 @@ OPTIONS
>>>> there is a match and with non-zero status when there isn't.
>>>>
>>>> --threads<n>::
>>>> + Run<n> search threads in parallel. Default is 8 when searching
>>>> + the worktree and 0 otherwise. This option is ignored if git was
>>>> + built without support for POSIX threads.
>>> [...]
>>>> - nr_threads = (online_cpus()> 1) ? THREADS : 0;
>>>> + nr_threads = (online_cpus()> 1&& !list.nr) ? THREADS : 0;
>>>
>>> It would be more consistent to stick to the pack.threads convention
>>> where 0 means "all of my cores", so to disable threading the user
>>> would set the number of threads to 1. Or were you trying to measure
>>> the contention between the worker thread and the add_work() thread?
>>
>> Yes, indeed, the cost for the threading overhead does interest me. The
>> documentation should perhaps mention --no-threads explicitly to avoid
>> confusion.
>>
>> Currently there is no way to specify "as many threads as there are
>> cores" here. Previous measurements indicated that it wasn't too useful,
>> however, because I/O parallelism was beneficial even for machines with
>> less than eight cores and more threads didn't pay off.
>
> Right. Even for single CPU machines this is true, so the
> nr_threads calculation above should still use all 8 THREADS
> regardless of the number of online_cpus().
That makes sense. However, in a quick test with a simple regex against
a cache-warm Linux repo threading increased the runtime of git grep by
30% on a single-core virtual machine. Let's keep that check until we
understand this better..
René
^ permalink raw reply
* Re: [PATCH v3 0/3] grep attributes and multithreading
From: René Scharfe @ 2011-12-12 22:37 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Junio C Hamano, Jeff King, Eric Herman
In-Reply-To: <cover.1323723759.git.trast@student.ethz.ch>
Am 12.12.2011 22:16, schrieb Thomas Rast:
> I think we should finish up these three patches for the next release.
> I already posted a bunch of POC patches, but doing the timing manually
> has been getting on my nerves lately. So I would first like to
> formalize some of the performance testing, perhaps along lines already
> drawn up by Michael Hagger, perhaps not. Then we can revisit the
> issue of grep performance. But I would prefer not to block the -W fix
> and two easy and confirmed speedups on that.
Yes, that's a good idea. The three patches are uncontroversial
incremental improvements.
> I dropped this part entirely:
>
>> How about adding a parameter to control the number of threads
>> (--threads?) instead that defaults to eight (or five) for the worktree
>> and one for the rest? That would also make benchmarking easier.
>
> It does make testing a lot easier, but the interface is IMHO not fit
> for users and I have a feeling that the "right" for-debugging
> interface will end up falling out of the performance testing work
> (probably an environment variable). The end-user option should be a
> config setting, if any.
Agreed; users shouldn't need to specify such a parameter -- our
heuristic should be good enough.
René
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jens Lehmann @ 2011-12-12 22:31 UTC (permalink / raw)
To: Leif Gruenwoldt; +Cc: Andreas T.Auer, Junio C Hamano, git
In-Reply-To: <CALFF=ZRB7qjj7VMhzr12ySdHmZsySoqceu5brFht8rX1+W3NPg@mail.gmail.com>
Am 12.12.2011 20:13, schrieb Leif Gruenwoldt:
> On Mon, Dec 12, 2011 at 1:42 PM, Andreas T.Auer
> <andreas.t.auer_gtml_37453@ursus.ath.cx> wrote:
>
>> The next question is: Wouldn't you like to have the new stable branch only
>> pulled in, when the projectX (as the superproject) is currently on that new
>> development branch (maybe master)?
>>
>> But if you checkout that fixed released version 1.2.9.8, wouldn't it be
>> better that in that case the gitlinked version of the submodule is checked
>> out instead of some unrelated new version? I mean, when the gitlinks are
>> tracked with the projectX commits, this should work well.
>>
>> And what about a maintenance branch, which is not a fixed version but a
>> quite stable branch which should only have bugfixes. Shouldn't the auto-pull
>> be disabled in that case, too?
>>
>> I think the "auto-pull" behavior should depend on the currently checked out
>> branch. So the configuration options should allow the definition of one or
>> more mappings.
>
> Yes. I think you nailed it. The floating behaviour would best be
> configured per branch.
Why not use .gitmodules and make the "branch" setting work without having to
sync it?
> An aside. Would this mean a "git pull" on the product repo would
> automatically do a pull (git submodule update) on the submodule too?
Me thinks that only "git submodule update" should do that. Or what should
happen for people not using pull but just doing fetch and checkout?
^ permalink raw reply
* Re: [PATCH v4 2/2] push: teach --recurse-submodules the on-demand option
From: Jens Lehmann @ 2011-12-12 22:29 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, Heiko Voigt, Fredrik Gustafsson, git
In-Reply-To: <CABURp0okOmsk4JV9Ku5pHJb5vT-kr_fmweNNBKZ_OoRyfZan=Q@mail.gmail.com>
Am 12.12.2011 22:16, schrieb Phil Hord:
> On Tue, Oct 18, 2011 at 4:58 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 18.10.2011 00:33, schrieb Junio C Hamano:
>>> We could even declare that the gitlink for such a
>>> submodule should record 0{40} SHA-1 in the superproject, but I do not
>>> think that is necessary.
>>
>> Me neither, e.g. the SHA-1 which was the submodules HEAD when it was added
>> should do nicely. And that would avoid referencing a non-existing commit
>> in case you later want to turn a floating submodule into an exact one.
>
>
> I'm sorry I missed this comment before.
>
> I hope we can allow storing the actual gitlink in the superproject for
> each commit even when we're using floating submodules.
I think you misread my statement, I was just talking about the initial
commit containing the newly added submodule, not any subsequent ones.
Floating makes differences between the original SHA-1 and the current
tip of the branch invisible, so there is nothing to commit.
> I thought-experimented with this a bit last year and came to the
> conclusion that I should be able to 'float' to tips (developer
> convenience) and also to store the SHA-1 of each gitlink through
> history (automated maybe; as-needed).
Which means that after "git submodule update" floated a submodule branch
further, you would have to commit that in the superproject.
> The problem with "float-only" is that it loses history so, for
> example, git-bisect doesn't work.
Yep. And different developers can have the same superproject commit
checked out but their submodules can be quite different.
> The problem with "float + gitlinks", of course, is that it looks like
> "not floating" to the developers (git-status is dirty unless
> overridden, etc.)
Yeah. But what if each "git submodule update" would update the tip of
the submodule branch and add that to the superproject? You could follow
a tip but still produce reproducible trees.
^ permalink raw reply
* [PATCH v2] Update documentation for stripspace
From: Conrad Irwin @ 2011-12-12 22:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Conrad Irwin
In-Reply-To: <7vy5ui5h0k.fsf@alter.siamese.dyndns.org>
On Sun, Dec 11, 2011 at 10:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Conrad Irwin <conrad.irwin@gmail.com> writes:
>
> The original says "remove" and new one says "normalize*s*". I think we
> tend to say things in imperative mood (i.e. without the trailing "s").
Good point, fixed.
>
> I do not think 'user-provided metadata' is a good wording. This is just a
> simple text clean-up filter and you can use it to clean your text files
> that you mean to store in the repository as well.
Changed just to "text", as that seems simpler. I've left the example
uses as is, they were the prime reason for that sentence existing.
>
> The last one is a bit funny, though.
>
> By definition, you cannot end the last line with more than one '\n' (upon
> seeing the second '\n', you would realize immediately that the line you
> saw was _not_ the last line). I think you meant the file does not end with
> an incomplete line, i.e. "ensures the output does not end with an
> incomplete line by adding '\n' at the end if needed".
Hmm, I'm not sure that's the best way of describing it — I've gone with:
"add a missing '\n' to the last line if necessary.".
>
>> +In the case where the input consists entirely of whitespace characters, no
>> +output will be produced.
>> +
>> +*NOTE*: This is intended for cleaning metadata, prefer the `--whitespace=fix`
>> +mode of linkgit:git-apply[1] for correcting whitespace of patches or files in
>> +the repository.
>
> I can tell that these three lines were the _primary_ thing you wanted to
> add with this patch, having never seen anybody got confused between the
> whitespace breakage fix and text cleaning, I wonder if this is adding
> clarity or giving users an impression that git can do too many things than
> they can wrap their mind around and forcing them to wonder if they have to
> learn everything git can do for them.
The motivation for this patch was an old post to the Cairo mailing list
[1]. There they were using (previously undocumented) behaviour of
git stripspace, instead of git apply --whitespace=fix (it may be that
--whitespace=fix didn't exist at that point?).
I've moved the *NOTE* into a SEE ALSO section where I think it reads
less opinionatedly — is that better?
>
>> OPTIONS
>> -------
>> -s::
>> --strip-comments::
>> - In addition to empty lines, also strip lines starting with '#'.
>> + Also remove all lines starting with '#'.
[snip]
>
> If I were touching this description, I probably would say something like
> "Treat lines starting with a '#' as if they are empty lines".
>
If only it were that simple! If you have a commented line between two
non-commented lines, then no empty line results. I've added an example
to the re-rolled patch that show-cases the behaviour of comment
stripping in both cases. I went with "skip and remove" as the verb, to
imply that the lines are ignored by the previous transformations.
Thanks for the detailed feedback.
Conrad
[1] http://lists.freedesktop.org/archives/cairo/2006-June/007062.html
----8<----
Tell the user what this command is intended for, and expand the
description of what it does.
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
---
Documentation/git-stripspace.txt | 69 ++++++++++++++++++++++++++++++++++---
builtin/stripspace.c | 2 +-
2 files changed, 64 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt
index b78f031..a0a6ea4 100644
--- a/Documentation/git-stripspace.txt
+++ b/Documentation/git-stripspace.txt
@@ -3,26 +3,83 @@ git-stripspace(1)
NAME
----
-git-stripspace - Filter out empty lines
+git-stripspace - Remove unnecessary whitespace
SYNOPSIS
--------
[verse]
-'git stripspace' [-s | --strip-comments] < <stream>
+'git stripspace' [-s | --strip-comments] < input
DESCRIPTION
-----------
-Remove multiple empty lines, and empty lines at beginning and end.
+
+Clean the input in the manner used by 'git' for text such as commit
+messages, notes, tags and branch descriptions.
+
+With no arguments, this will:
+
+- remove trailing whitespace from all lines
+- collapse multiple consecutive empty lines into one empty line
+- remove blank lines from the beginning and end of the input
+- add a missing '\n' to the last line if necessary.
+
+In the case where the input consists entirely of whitespace characters, no
+output will be produced.
OPTIONS
-------
-s::
--strip-comments::
- In addition to empty lines, also strip lines starting with '#'.
+ Skip and remove all lines starting with '#'.
+
+EXAMPLES
+--------
+
+Given the following noisy input with '$' indicating the end of a line:
+
+--------
+|A brief introduction $
+| $
+|$
+|A new paragraph$
+|# with a commented-out line $
+|explaining lots of stuff.$
+|$
+|# An old paragraph, also commented-out. $
+| $
+|The end.$
+| $
+---------
+
+Use 'git stripspace' with no arguments to obtain:
+
+--------
+|A brief introduction$
+|$
+|A new paragraph$
+|# with a commented-out line$
+|explaining lots of stuff.$
+|$
+|# An old paragraph, also commented-out.$
+|$
+|The end.$
+---------
-<stream>::
- Byte stream to act on.
+Use 'git stripspace --strip-comments' to obtain:
+
+--------
+|A brief introduction$
+|$
+|A new paragraph$
+|explaining lots of stuff.$
+|$
+|The end.$
+---------
+
+SEE ALSO
+--------
+The `--whitespace=fix` mode of linkgit:git-apply[1].
GIT
---
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 1288ffc..f16986c 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -75,7 +75,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
!strcmp(argv[1], "--strip-comments")))
strip_comments = 1;
else if (argc > 1)
- usage("git stripspace [-s | --strip-comments] < <stream>");
+ usage("git stripspace [-s | --strip-comments] < input");
if (strbuf_read(&buf, 0, 1024) < 0)
die_errno("could not read the input");
--
1.7.8.164.g00d7e
^ permalink raw reply related
* Re: Question about commit message wrapping
From: Frans Klaver @ 2011-12-12 22:16 UTC (permalink / raw)
To: Holger Hellmuth
Cc: Andrew Ardill, Jakub Narebski, Sidney San Martín, git
In-Reply-To: <4EE62DB9.8030406@ira.uka.de>
On Mon, 12 Dec 2011 17:37:13 +0100, Holger Hellmuth <hellmuth@ira.uka.de>
wrote:
>> I am starting to think that we need to somehow keep the current
>> behavior, but override at smaller widths. Maybe even use format=flowed
>> in format-patch.
>
> Could you explain what overriding at smaller widths would achieve?
> Supporting format=flowed would be nice though.
I specifically meant trying to detect pre-wrapped text, removing the
new-lines where we think it is because of wrapping at 80 characters. So
the result would be: perfect on screens up to 80 characters wide, and ok
on anything wider.
The implementation of that however could affect code in the mail if it
isn't done properly.
>
>> On the other hand, the fundamental use with git is to
>> communicate code, and I'm not sure how that [cw]ould be handled.
>
> I prefer wrapped code to code that is cut of at a specific column.
> Wrapped code has much less possibility for misinterpretation. Python
> programmers might disagree?
Wrapped code as in auto-wrapped? Or as in manually wrapped? Python
programmers have significant white space, but you can still hard wrap
stuff, as long as the next statement is properly indented.
>
> I see your proposal mainly as an improvement to the display of texts,
> not code.
Me too.
--
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
^ permalink raw reply
* Re: Breakage (?) in configure and git_vsnprintf()
From: Jonathan Nieder @ 2011-12-12 21:56 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, Junio C Hamano, git discussion list,
Michal Rokos, Brandon Casey
In-Reply-To: <20111212064305.GA16511@sigill.intra.peff.net>
Jeff King wrote:
> I'll leave the issue of "-std=c89" triggering SNPRINTF_RETURNS_BOGUS to
> people who know and care about autoconf. My gut is to say "don't do
> that". Git is not actually pure c89. We typically target systems that
> are _at least_ c89, but it's more important to match and run well on
> real-world systems than what was defined in the standard. So we don't
> depend on c99, but we do depend on quirks and features that were
> prominent in mid-90's Unix variants.
Using vsnprintf isn't even wrong from the standards-pedant point of
view --- vsnprintf has been in POSIX for a long time. The problem is
that the configure script is not setting appropriate feature test
macros such as _XOPEN_SOURCE (like git-compat-util.h does) during its
feature tests.
Maybe it would be possible to hook the appropriate magic into
AC_LANG_PROGRAM.
^ permalink raw reply
* [PATCHv2 5/5] mv: be quiet about overwriting
From: Jeff King @ 2011-12-12 21:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jari Aalto, git
In-Reply-To: <20111212215238.GD9754@sigill.intra.peff.net>
When a user asks us to force a mv and overwrite the
destination, we print a warning. However, since a typical
use would be:
$ git mv one two
fatal: destination exists, source=one, destination=two
$ git mv -f one two
warning: overwriting 'two'
this warning is just noise. We already know we're
overwriting; that's why we gave -f!
This patch silences the warning unless "--verbose" is given.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/mv.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index 8dd5a45..2a144b0 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -177,7 +177,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
* check both source and destination
*/
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
- warning(_("overwriting '%s'"), dst);
+ if (verbose)
+ warning(_("overwriting '%s'"), dst);
bad = NULL;
} else
bad = _("Cannot overwrite");
--
1.7.8.13.g74677
^ permalink raw reply related
* [PATCHv2 4/5] mv: improve overwrite warning
From: Jeff King @ 2011-12-12 21:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jari Aalto, git
In-Reply-To: <20111212215238.GD9754@sigill.intra.peff.net>
When we try to "git mv" over an existing file, the error
message is fairly informative:
$ git mv one two
fatal: destination exists, source=one, destination=two
When the user forces the overwrite, we give a warning:
$ git mv -f one two
warning: destination exists; will overwrite!
This is less informative, but still sufficient in the simple
rename case, as there is only one rename happening.
But when moving files from one directory to another, it
becomes useless:
$ mkdir three
$ touch one two three/one
$ git add .
$ git mv one two three
fatal: destination exists, source=one, destination=three/one
$ git mv -f one two three
warning: destination exists; will overwrite!
The first message is helpful, but the second one gives us no
clue about what was overwritten. Let's mention the name of
the destination file:
$ git mv -f one two three
warning: overwriting 'three/one'
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/mv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index ae6c30c..8dd5a45 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -177,7 +177,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
* check both source and destination
*/
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
- warning(_("%s; will overwrite!"), bad);
+ warning(_("overwriting '%s'"), dst);
bad = NULL;
} else
bad = _("Cannot overwrite");
--
1.7.8.13.g74677
^ permalink raw reply related
* Re: [PATCH 4/5] mv: improve overwrite warning
From: Jeff King @ 2011-12-12 21:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jari Aalto, git
In-Reply-To: <7vobvd36ms.fsf@alter.siamese.dyndns.org>
On Mon, Dec 12, 2011 at 11:57:31AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > This message looks overly long to me, but I wanted to match the existing
> > messages. Another option would be just:
> >
> > warning: overwriting 'three/one'
>
> Yes, I think it makes perfect sense to drop the ugly "source=one destination=two"
> cruft, both for single-source and multiple-source cases.
Yeah, the more I look at the message in the patch I sent, the uglier it
gets. Here's a re-rolled 4 and 5 with the nicer format.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] Makefile: Windows lacks /dev/tty
From: Johannes Sixt @ 2011-12-12 21:52 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111212211801.GA9754@sigill.intra.peff.net>
Am 12.12.2011 22:18, schrieb Jeff King:
> The most recent version of the prompt series has platforms opting into
> the replacement with HAVE_DEV_TTY.
I see. Obviously, I've tested an earlier iteration. This patch can be
dropped then.
-- Hannes
^ permalink raw reply
* Re: [PATCH 2/5] mv: honor --verbose flag
From: Jeff King @ 2011-12-12 21:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jari Aalto, git
In-Reply-To: <7vwra136tj.fsf@alter.siamese.dyndns.org>
On Mon, Dec 12, 2011 at 11:53:28AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > The code for a verbose flag has been here since "git mv" was
> > converted to C many years ago, but actually getting the "-v"
> > flag from the command line was accidentally lost in the
> > transition.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> > This has been broken since 2006, so I guess nobody really cares. But
> > it's simple to fix.
>
> Heh. It means nobody exercised the codepaths that are inside "if (verbose)",
> so it may uncover old bugs, no?
I thought that at first, too, but actually there is only one code path
currently enabled by "verbose", and it is to print "Renaming ...". You
can also exercise that code path with "--dry-run" (and the whole path
consists of only a single printf, so hopefully we didn't manage to
squeeze any bugs in there).
Once upon a time, the verbose flag was passed on to add_file_to_index,
but that was dropped when the code switched to using
rename_index_entry_at in 81dc230 (git-mv: Keep moved index entries
inact, 2008-07-21).
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] Makefile: optionally exclude code that needs Unix sockets
From: Jeff King @ 2011-12-12 21:39 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4EE66E58.6040404@kdbg.org>
On Mon, Dec 12, 2011 at 10:12:56PM +0100, Johannes Sixt wrote:
> Introduce a configuration option NO_UNIX_SOCKETS to exclude code that
> depends on Unix sockets and use it in MSVC and MinGW builds.
Makes sense, thanks.
> Notice that unix-socket.h was missing from LIB_H before; fix that, too.
Oops, thanks.
> All or most of the tests introduced by the series fail on Windows.
> What is the preferred way to exclude them? Mark the all with a
> prerequisite? Or exit early at the beginning of the test file?
The cache daemon is the only thing that uses sockets. I would suggest
just exiting early from its script (patch below).
The rest of the tests should pass under Windows. If they don't, then
they should be fixed (I'm happy to work on that, with the limitation
that I don't actually have a Windows box, and if at all possible I'd
like to keep it that way).
In theory we should also disable the documentation for credential-cache.
But that means surgery not only to Documentation/Makefile, but figuring
out how to pass the flags down to the actual asciidoc process (since
gitcredentials(7) mentions it in the text). Certainly possible, but I
don't know if it's worth the effort or not.
---
diff --git a/Makefile b/Makefile
index e3ee884..6e7e190 100644
--- a/Makefile
+++ b/Makefile
@@ -2220,6 +2220,7 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
@echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease >>$@
endif
@echo GETTEXT_POISON=\''$(subst ','\'',$(subst ','\'',$(GETTEXT_POISON)))'\' >>$@
+ @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
### Detect Tck/Tk interpreter path changes
ifndef NO_TCLTK
diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
index 3a65f99..82c8411 100755
--- a/t/t0301-credential-cache.sh
+++ b/t/t0301-credential-cache.sh
@@ -4,6 +4,11 @@ test_description='credential-cache tests'
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-credential.sh
+test -z "$NO_UNIX_SOCKETS" || {
+ skip_all='skipping credential-cache tests, unix sockets not available'
+ test_done
+}
+
# don't leave a stale daemon running
trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT
^ permalink raw reply related
* Re: [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)
From: Junio C Hamano @ 2011-12-12 21:31 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Sixt, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
Overall, I think this series is a more correct and faithful implementation
of the design we discussed earlier in the thread
http://thread.gmane.org/gmane.comp.version-control.git/179304/focus=179625
I saw a few minor nits in the log messages but otherwise nothing
objectionable jumped at me from my initial reading of the series.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] Makefile: Windows lacks /dev/tty
From: Jeff King @ 2011-12-12 21:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4EE66DAB.5070407@kdbg.org>
On Mon, Dec 12, 2011 at 10:10:03PM +0100, Johannes Sixt wrote:
> diff --git a/Makefile b/Makefile
> index 2c506b3..5b7f6a8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1139,6 +1139,7 @@ ifeq ($(uname_S),Windows)
> NO_CURL = YesPlease
> NO_PYTHON = YesPlease
> BLK_SHA1 = YesPlease
> + NO_DEV_TTY = YesPlease
> NO_POSIX_GOODIES = UnfortunatelyYes
> NATIVE_CRLF = YesPlease
>
> @@ -1232,6 +1233,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> ETAGS_TARGET = ETAGS
> NO_INET_PTON = YesPlease
> NO_INET_NTOP = YesPlease
> + NO_DEV_TTY = YesPlease
> NO_POSIX_GOODIES = UnfortunatelyYes
> COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/win32
> COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
Unless I've bungled something, these should be no-ops, shouldn't they?
The most recent version of the prompt series has platforms opting into
the replacement with HAVE_DEV_TTY.
-Peff
^ permalink raw reply
* Re: [PATCH v4 2/2] push: teach --recurse-submodules the on-demand option
From: Phil Hord @ 2011-12-12 21:16 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Heiko Voigt, Fredrik Gustafsson, git
In-Reply-To: <4E9DE883.9050105@web.de>
On Tue, Oct 18, 2011 at 4:58 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 18.10.2011 00:33, schrieb Junio C Hamano:
>> The very first step for floating submodules support would be relatively
>> simple. We could declare that an entry in the .gitmodules file in the
>> superproject can optionally specify which branch needs to be checked out
>> with something like:
>>
>> [submodule "libfoo"]
>> branch = master
>> path = include/foo
>> url = git://foo.com/git/lib.git
>>
>> and when such an entry is defined, a command at the superproject level
>> would largely ignore what is at include/foo in the tree object recorded in
>> the superproject commit and in the index. When we show "git status" in the
>> superproject, instead of using the commit bound to the superproject, we
>> would use include/foo/.git/HEAD as the basis for detecting "local" changes
>> to the submodule.
>
> Yup. And the presence of the "branch" config could tell "git submodule
> update" to fetch and advance that branch to the tip every time it is run.
> And it could tell the diff machinery (which is also used by status) to
> ignore the differences between a submodule's HEAD and the SHA-1 in the
> superproject (while still allowing to silence the presence of untracked
> and/or modified files by using the diff.ignoreSubmodules option) and
> fetch would just stop doing any on-demand action for such submodules.
> Anything I missed?
>
>> We could even declare that the gitlink for such a
>> submodule should record 0{40} SHA-1 in the superproject, but I do not
>> think that is necessary.
>
> Me neither, e.g. the SHA-1 which was the submodules HEAD when it was added
> should do nicely. And that would avoid referencing a non-existing commit
> in case you later want to turn a floating submodule into an exact one.
I'm sorry I missed this comment before.
I hope we can allow storing the actual gitlink in the superproject for
each commit even when we're using floating submodules. I
thought-experimented with this a bit last year and came to the
conclusion that I should be able to 'float' to tips (developer
convenience) and also to store the SHA-1 of each gitlink through
history (automated maybe; as-needed).
The problem with "float-only" is that it loses history so, for
example, git-bisect doesn't work.
The problem with "float + gitlinks", of course, is that it looks like
"not floating" to the developers (git-status is dirty unless
overridden, etc.)
Is there a deeper reason this wouldn't be possible?
Phil
^ 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