* Re: [PATCH v3 13/25] sequencer: prepare for rebase -i's commit functionality
From: Junio C Hamano @ 2016-10-12 16:55 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Jakub Narębski, Johannes Sixt, Michael Haggerty
In-Reply-To: <alpine.DEB.2.20.1610121356510.3492@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Hmph, didn't we recently add parse_key_value_squoted() to build
>> read_author_script() in builtin/am.c on top of it, so that this
>> piece of code can also take advantage of and share the parser?
>
> I already pointed out that the author-script file may *not* be quoted.
I think my puzzlement comes from here. What makes it OK for "am" to
expect the contents of author-script file to be quoted but it is not
OK to expect the same here? What makes it not quoted for _this_
reader, in other words?
I am not sure what you meant by "nominally related", but the purpose
of the author-script in these two codepaths is the same, isn't it?
Somebody leaves the author information from the source (either from
an e-mailed patch or an existing commit), so that a later step can
use that pieces of information left in the file when (re)creating a
commit to record the tree made by using pieces of information from
the source.
Are our use in the author-script in these two codepaths _already_
inconsistent? IOW, "am" never writes malformed unquoted values,
while the sequencer writes out in a way that is randomly quoted or
not quoted, iow, if you fed such an author-file to "am", it wouldn't
understand it?
I fully support your position to use different codepaths, if the
file that has the same name and that is used for the same purpose
uses different format in these two separate codepaths and the users
already expect them to be different. We obviously need to have two
separate parsers.
But if that is not the case, IOW, if "am"'s author-script shares the
same issue (i.e. "'am' initially writes the file properly quoted,
but this or that can happen to change its quoting and we need to
read from such a file"), then perhaps sharing needs to happen the
other way around? This patch may prepare "rebase -i" side for the
"this or that" (I still do not know what they are) to allow the
resulting file read correctly, but the same "this or that" can break
what "am" has used and is in use there if that is the case, no?
What makes it OK for "am" to expect the contents of author-script
file to be quoted but it is not OK to expect the same here? What
makes it not quoted for _this_ reader, and doesn't "am" share the
same issue?
>> > +/*
>>
>> Offtopic: this line and the beginning of the new comment block that
>> begins with "Read the author-script" above show a suboptimal marking
>> of what is added and what is left. I wonder "diff-indent-heuristic"
>> topic by Michael can help to make it look better.
>
> Maybe. I'll try to look into that once the more serious questions about
> this patch series have been addressed.
You do not have to; the remark was meant for Michael (newly cc'ed in
the message you are responding to).
^ permalink raw reply
* [PATCH] worktree: allow the main brach of a bare repository to be checked out
From: Dennis Kaarsemaker @ 2016-10-12 16:41 UTC (permalink / raw)
To: git; +Cc: pclouds, mtutty, rappazzo
In-Reply-To: <CACsJy8CmgmGLEi0xQUY9Eo-4FkA4eDNk9WJ2LtEDVFQBjbFdCA@mail.gmail.com>
In bare repositories, get_worktrees() still returns the main repository,
so git worktree list can show it. ignore it in find_shared_symref so we
can still check out the main branch.
Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
---
t/t2025-worktree-add.sh | 8 ++++++++
worktree.c | 2 ++
2 files changed, 10 insertions(+)
On Sun, 2016-10-09 at 17:52 +0700, Duy Nguyen wrote:
> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker > <dennis@kaarsemaker.net> wrote:
> > On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
> > >
> > > The only exception seems to be merging to master. When I do git
> > > worktree add /tmp/path/to/worktree master I get an error:
> > >
> > > [fatal: 'master' is already checked out at '/path/to/bare/repo']
> > >
> >
> > The worktree code treats the base repo as a worktree, even if it's
> > bare. For the purpose of being able to do a checkout of the main branch
> > of a bare repo, this patch should do:
> >
> --snip--
>
> You're fast :) I'm still studying 8d9fdd7 (worktree.c: check whether
> branch is rebased in another worktree - 2016-04-22). But yeah that
> should fix it.
OK, so here it is as a proper patch.
D.
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335..2996c38 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without "add"' '
)
'
++test_expect_success '"add" default branch of a bare repo' '
+ (
+ git clone --bare . bare2 &&
+ cd bare2 &&
+ git worktree add ../there3 master
+ )
+'
+
test_expect_success 'checkout with grafts' '
test_when_finished rm .git/info/grafts &&
test_commit abc &&
diff --git a/worktree.c b/worktree.c
index 5acfe4c..35e95b7 100644
--- a/worktree.c
+++ b/worktree.c
@@ -345,6 +345,8 @@ const struct worktree *find_shared_symref(const char *symref,
for (i = 0; worktrees[i]; i++) {
struct worktree *wt = worktrees[i];
+ if(wt->is_bare)
+ continue;
if (wt->is_detached && !strcmp(symref, "HEAD")) {
if (is_worktree_being_rebased(wt, target)) {
--
2.10.1-356-g947a599
--
Dennis Kaarsemaker <dennis@kaarsemaker.net>
http://twitter.com/seveas
^ permalink raw reply related
* Re: [PATCH] t1512: become resilient to GETTEXT_POISON build
From: Jeff King @ 2016-10-12 16:32 UTC (permalink / raw)
To: Vasco Almeida; +Cc: git
In-Reply-To: <20161012122533.32223-1-vascomalmeida@sapo.pt>
On Wed, Oct 12, 2016 at 12:25:33PM +0000, Vasco Almeida wrote:
> The concerned message was marked for translation by 0c99171
> ("get_short_sha1: mark ambiguity error for translation", 2016-09-26).
Thanks, this looks obviously correct.
-Peff
^ permalink raw reply
* Re: Bug with git merge-base and a packed ref
From: Jeff King @ 2016-10-12 16:32 UTC (permalink / raw)
To: Stepan Kasal; +Cc: John Keeping, git
In-Reply-To: <20161012103716.GA31533@ucw.cz>
On Wed, Oct 12, 2016 at 12:37:16PM +0200, Stepan Kasal wrote:
> A reproducer would look like that
>
> # in repo1:
> git checkout tmp
> cd ..
> git clone repo1 repo2
> cd repo1
> git rebase elsewhere tmp
> cd ../repo2
> # edit
> git commit -a -m 'Another commit'
> git pull -r
>
> The last command performs something like
> git rebase new-origin/tmp
> instead of
> git rebase --onto new-origin/tmp old-origin/tmp
>
> I'm using git version 2.10.1.windows.1
>
>
> I tried to debug the issue:
> I found that the bug happens only at the very first pull after clone.
> I was able to reproduce it with git-pull.sh
>
> The problem seems to be that command
> git merge-base --fork-point refs/remotes/origin/tmp refs/heads/tmp
> returns nothing, because the refs are packed.
The --fork-point option looks in the reflog to notice that the upstream
branch has been rebased. I don't think clone actually writes reflog
entries, though, which would explain why it happens only on the first
pull after clone.
I suspect the necessary information _is_ there, though. When we update
the tracking branch, the new reflog entry will show it going from sha1
X to sha1 Y. So my guess is that --fork-point is looking for the entry
where it became "X" (which doesn't exist, because clone did not write
it), but it _could_ find that we came from "X" in the very first reflog
entry.
That's all without looking at the code, though. I don't have time to
examine it now, but maybe that can point somebody in the right
direction.
> Could you please fix merge-base so that it understands packed refs?
I think the packed-refs thing is probably a red herring. If merge-base
didn't understand packed refs, a huge chunk of git would be horribly
broken.
-Peff
^ permalink raw reply
* Re: [PATCH v3 12/25] sequencer: remember the onelines when parsing the todo file
From: Junio C Hamano @ 2016-10-12 16:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <alpine.DEB.2.20.1610121347361.3492@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > + const char *arg;
>> > + int arg_len;
>> > size_t offset_in_buf;
>>
>> micronit: you can make it to size_t and lose the cast below, no?
>
> No. The primary users of arg_len call a printf() style function with %.*s,
> expecting an int. So your suggestion would lose one cast, but introduce at
> least four casts in return.
Actually my point was not the number of casts required, but more
about using the correct type to store things. Granted, I do not
expect each of the lines would ever get too long to exceed "int"
(but fit in "size_t") in practice, and from that point of view, one
may be able to argue that "int" and "size_t" are both correct types,
but that argument applies equally to offset_in_buf, so...
^ permalink raw reply
* Re: interactive rebase should better highlight the not-applying commit
From: Johannes Schindelin @ 2016-10-12 16:24 UTC (permalink / raw)
To: Joshua N Pritikin; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <20161012132740.dvyofl36qtualxgk@droplet>
Hi Joshua,
On Wed, 12 Oct 2016, Joshua N Pritikin wrote:
> On Tue, Oct 11, 2016 at 01:55:22PM -0700, Stefan Beller wrote:
> > On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > > I assume somebody familiar with GIT's code base could make this
> > > change in about 10 minutes.
> >
> > Can you elaborate how you come to that estimate?
>
> Hm, a false belief in the general awesomeness of GIT developers?
No, a false belief in your own shortcomings, as you thought it would be
easier to address your wishes for somebody else than you.
> On Tue, Oct 11, 2016 at 02:25:19PM -0700, Stefan Beller wrote:
> > On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > > As of GIT 2.8.1, if you do an interactive rebase and get some conflict
> > > in the stack of patches then the commit with the conflict is buried in
> > > 4-5 lines of output. It is visually difficult to immediately pick out
> > > which commit did not apply cleanly. I suggest highlighting the 1 line
> > > commit summary in red or green or some color to help it stand out from
> > > all the other output.
> > >
> > > I decided to suggest this change after I realized that I probably
> > > skipped a commit during an interactive rebase instead of resolving the
> > > conflict. I knew I had to skip some commit so I assumed that I just need
> > > to skip without reading the commit summary carefully. Now it is 7-15
> > > days after I did the erroneous rebase. I had to spend a few hours today
> > > with GIT's archaeology tools to find the lost code.
> >
> > Looking at the actual code, this is not as easy as one might assume,
> > because rebase is written in shell. (One of the last remaining large
> > commands in shell), and there is no color support in the die(..)
> > function.
>
> I'm sorry to hear that.
>
> > However IIUC currently rebase is completely rewritten/ported to C
> > where it is easier to add color support as we do have some color
> > support in there already.
>
> Sounds great. Is there a beta release that I can try out?
There is no release as such, unless you count Git for Windows v2.10.0.
But you can try the `interactive-rebase` branch of
https://github.com/dscho/git; please note, though, that my main aim was to
be as faithful as possible in the conversion (modulo speed, of course).
> Also, I have another wishlist item for (interactive) rebase.
Hmm. You know, I cannot say that I am a fan of wishlists for Git, unless
the originator of said wishlist takes on their responsibility as an Open
Source user to make their wishes come true.
But maybe I read it all wrong and you do want to make this happen
yourself, and you simply want a little advice how to go about it?
> Sometimes I do a rebase to fix some tiny thing 10-15 commits from HEAD.
> Maybe only 1 file is affected and there are no merge conflicts, but when
> rebase reapplies all the commits, the timestamps of lots of unmodified
> files change even though they are unmodified compared to before the
> rebase.
Well, they *were* modified, right?
A workaround would be to create a new worktree using the awesome `git
worktree` command, perform the rebase there (on an unnamed branch -- AKA
"detached HEAD", no relation to Helloween), and then come back to the
original worktree and reset --hard to the new revision. That reset would
detect that there are actually no changes required to said files.
> Since the modification times are used by 'make' to compute dependencies,
> this creates a lot of useless recompilation that slows things down. It
> would be great if rebase only changed the timestamps of files that were
> actually modified.
Rebase will always have to change those timestamps. Because it really
changes those files. So the mtimes *need* to be updated. As far as rebase
is concerned, it does not matter that the final contents are identical to
*some* previous version...
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCHv2] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-12 16:20 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <CAGZ79kZrNSmPAQ6SmBzFDJtSmdCbqKcgQu4KDLfoYVkSXvo-og@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> I am not sure if the updates to the callers fulfill that purpose.
>> For example, look at this hunk.
>>
>>> @@ -111,6 +111,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
>>> struct archiver_args *args = c->args;
>>> write_archive_entry_fn_t write_entry = c->write_entry;
>>> static struct git_attr_check *check;
>>> + static struct git_attr_result result;
>>
>> As we discussed, this caller, even when threaded, will always want
>> to ask for a fixed two attributes, so "check" being static and
>> shared across threads is perfectly fine. But we do not want to see
>> "result" shared, do we?
>
> Well all of the hunks in the patch are not threaded, so they
> don't follow a threading pattern, but the static pattern to not be
> more expensive than needed.
Is it too invasive a change to make them as if they are thread-ready
users of API that happen to know their callers are not threading?
It would be ideal if we can prepare them so that the way they
interact with the attr subsystem will not have to change after this
step.
>> In other words, ideally, I think this part of the patch should
>> rather read like this:
>>
>> static struct git_attr_check *check;
>> struct git_attr_result result[2];
>>
>> ...
>> git_attr_check_initl(&check, "export-ignore", "export-subst", NULL);
>> if (!git_check_attr(path_without_prefix, check, result)) {
>> ... use result[0] and result[1] ...
>>
>> For sanity checking, it is OK to add ARRAY_SIZE(result) as the final
>> and extra parameter to git_check_attr() so that the function can
>> make sure it matches (or exceeds) check->nr.
>
> That seems tempting from a callers perspective; I'll look into that.
For callers that prepare "check" and "result" before asking
check-attr about the attributes in "check" for many paths, it is OK
to use your "allocate with attr_result_init()" pattern. The "result"
still needs to be made non-static, though.
But many callers do not follow that; rather they do
loop to iterate over paths {
call a helper func to learn attr X for path
use the value of attr X
}
using a callchain that embeds a helper function deep inside, and
"check" is kept in the helper, check-attr function is called from
there, and "result" is not passed from the caller to the helper
(obviously, because it does not exist in the current API). See the
callchain that leads down to convert.c::convert_attrs() for a
typical example. When converted to the new API, it needs to have a
new "result" structure every time it is called, and cannot reuse the
one that was used in its previous call.
^ permalink raw reply
* 2 directories same spelling one directory is camel cased
From: David Brown @ 2016-10-12 16:05 UTC (permalink / raw)
To: git
Howdy git gurus,
I have the dubious distinction of working with a remote repo (master)
that has a class loader run-time error when cloned, built and executed.
The reason for the runtime issue is a directory hierarchical path has to
directories (folders) with the same name spelling but one of the
directories is camel-cased. The package names are the same.
The compiler doesn't care but the run-time class loader has an issue
with the 2 'same like named' classes.
How to remove the offending directories and files at the locally cloned
repo but not push 'deleted' directories and files back to origin/master
the remote repo?
Please advise.
Regards.
^ permalink raw reply
* Re: interactive rebase should better highlight the not-applying commit
From: Johannes Schindelin @ 2016-10-12 16:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: Joshua N Pritikin, git@vger.kernel.org
In-Reply-To: <CAGZ79kYg3sZ42W-PEE7MgCDvt_h7hEQ7KWZsVKMb3DY=x5VK+w@mail.gmail.com>
Hi Stefan,
On Tue, 11 Oct 2016, Stefan Beller wrote:
> On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > I assume somebody familiar with GIT's code base could make this change
> > in about 10 minutes.
>
> Can you elaborate how you come to that estimate?
Why do you ask? He obviously has "a very good brain" ;-)
Seriously again, Git's source code is not that hard to read, and the Git
developer community is pretty helpful when anybody asks for pointers what
code to change.
Having said that, I did reimplement some parts of the shell script that is
git-rebase--interactive.sh [*1*] in C and am in the process of getting
those integrated into the next (or hopefully not a *much* later) version
of Git.
So what I'd like to see is an *exact* copy-paste of a message in question,
and a *concrete* proposal how it should look like instead.
Ciao,
Johannes
Footnote *1*:
https://github.com/git/git/blob/master/git-rebase--interactive.sh
^ permalink raw reply
* Re: Formatting problem send_mail in version 2.10.0
From: Larry Finger @ 2016-10-12 15:59 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jeff King, Mathieu Lienard--Mayor, Remi Lespinet, git
In-Reply-To: <vpq8tttr2ps.fsf@anie.imag.fr>
On 10/12/2016 10:45 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> On 10/12/2016 02:36 AM, Matthieu Moy wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>
>>>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>>
>>>>>> That added information at the end is intended to be passed on to the
>>>>>> stable group. In this case, the patch needs to be applied to kernel
>>>>>> versions 4.8 and later.
>>>>>
>>>>> OK, but where do people fetch this information from?
>>>>
>>>> This format is used in a patch for the kernel. When the patch is
>>>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>>>> a copy of the original patch. Maintainers of the indicated systems
>>>> then merge the patch with their stable version.
>>>
>>> Sorry, but this does not answer my question. I'll rephrase: when
>>> people behind stable@vger.kernel.org get the message, how do they know
>>> which version of the kernel they should apply it to?
>>>
>>>> I do not want it in the body of the message. I just want to pass a
>>>> hint to the stable maintainer(s).
>>>
>>> If it's not in the body of the message, then where is it?
>>
>> This point is clarified in the thread
>> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
>> with my upstream maintainer.
>
> Which explicitly states that the syntax is not [$number], but # $number,
> right?
Yes it does; however v2.10.0 mangles this into
Stable <stable@vger.kernel.org#4.8+>
and my mailer refuses to send that.
Larry
^ permalink raw reply
* Re: Formatting problem send_mail in version 2.10.0
From: Matthieu Moy @ 2016-10-12 15:45 UTC (permalink / raw)
To: Larry Finger; +Cc: Jeff King, Mathieu Lienard--Mayor, Remi Lespinet, git
In-Reply-To: <5978b674-02eb-59ad-9468-93203533575f@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 10/12/2016 02:36 AM, Matthieu Moy wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>
>>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>
>>>>> That added information at the end is intended to be passed on to the
>>>>> stable group. In this case, the patch needs to be applied to kernel
>>>>> versions 4.8 and later.
>>>>
>>>> OK, but where do people fetch this information from?
>>>
>>> This format is used in a patch for the kernel. When the patch is
>>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>>> a copy of the original patch. Maintainers of the indicated systems
>>> then merge the patch with their stable version.
>>
>> Sorry, but this does not answer my question. I'll rephrase: when
>> people behind stable@vger.kernel.org get the message, how do they know
>> which version of the kernel they should apply it to?
>>
>>> I do not want it in the body of the message. I just want to pass a
>>> hint to the stable maintainer(s).
>>
>> If it's not in the body of the message, then where is it?
>
> This point is clarified in the thread
> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
> with my upstream maintainer.
Which explicitly states that the syntax is not [$number], but # $number,
right?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Formatting problem send_mail in version 2.10.0
From: Matthieu Moy @ 2016-10-12 15:40 UTC (permalink / raw)
To: Larry Finger; +Cc: Jeff King, Mathieu Lienard--Mayor, Remi Lespinet, git
In-Reply-To: <02cfe81b-9f6a-ddbd-3411-2839e8a58b1e@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 10/12/2016 02:36 AM, Matthieu Moy wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>
>>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>
>>>>> That added information at the end is intended to be passed on to the
>>>>> stable group. In this case, the patch needs to be applied to kernel
>>>>> versions 4.8 and later.
>>>>
>>>> OK, but where do people fetch this information from?
>>>
>>> This format is used in a patch for the kernel. When the patch is
>>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>>> a copy of the original patch. Maintainers of the indicated systems
>>> then merge the patch with their stable version.
>>
>> Sorry, but this does not answer my question. I'll rephrase: when
>> people behind stable@vger.kernel.org get the message, how do they know
>> which version of the kernel they should apply it to?
>>
>>> I do not want it in the body of the message. I just want to pass a
>>> hint to the stable maintainer(s).
>>
>> If it's not in the body of the message, then where is it?
>
> That information is supplied in the "Cc: Stable" line at the end.
OK, so it *is* in the body of the email message.
If I understand correctly, the important is that:
* "git send-email" should accept sending a message for a commit
containing a Cc: <address@host> [number] line.
* The message should be sent to <address@host>
But I still don't get why using the (4.8+) notation wouldn't work. It
would still end up in the Cc: line in the body of the message, and the
message would still be sent to the same address. It is *not* dropped
from the message, just from the email headers of the message being sent.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Formatting problem send_mail in version 2.10.0
From: Larry Finger @ 2016-10-12 15:40 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jeff King, Mathieu Lienard--Mayor, Remi Lespinet, git
In-Reply-To: <vpq60oy589b.fsf@anie.imag.fr>
On 10/12/2016 02:36 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>
>>>> That added information at the end is intended to be passed on to the
>>>> stable group. In this case, the patch needs to be applied to kernel
>>>> versions 4.8 and later.
>>>
>>> OK, but where do people fetch this information from?
>>
>> This format is used in a patch for the kernel. When the patch is
>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>> a copy of the original patch. Maintainers of the indicated systems
>> then merge the patch with their stable version.
>
> Sorry, but this does not answer my question. I'll rephrase: when
> people behind stable@vger.kernel.org get the message, how do they know
> which version of the kernel they should apply it to?
>
>> I do not want it in the body of the message. I just want to pass a
>> hint to the stable maintainer(s).
>
> If it's not in the body of the message, then where is it?
This point is clarified in the thread
http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is with my
upstream maintainer.
Larry
^ permalink raw reply
* Re: [PATCH 1/5] trailer: use singly-linked list, not doubly
From: Christian Couder @ 2016-10-12 15:38 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Junio C Hamano
In-Reply-To: <8e12e0954f0a23d7c7905c58a3f7d8084d9338be.1476232683.git.jonathantanmy@google.com>
On Wed, Oct 12, 2016 at 3:23 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
> Use singly-linked lists (instead of doubly-linked lists) in trailer to
> keep track of arguments (whether implicit from configuration or explicit
> from the command line) and trailer items.
>
> This change significantly reduces the code length and simplifies the code.
It's true that the code can be simplified a lot by using a
singly-linked list, but if we already had and used some generic
functions or macros to handle doubly-linked list, I doubt there would
be a significant simplification (as the generic code could not be
deleted in this case).
> There are now fewer pointers to be manipulated, but most trailer
> manipulations now require seeking from beginning to end, so there might
> be a slight net decrease in performance; however the number of trailers
> is usually small (10 to 15 at the most) so this should not cause a big
> impact.
By default we append new trailers at the end of the trailer list, so a
singly-linked list is theoretically not well suited at all for
handling trailer lists.
You say that at most there is a small number of trailers, and that may
be true indeed for the Linux kernel and most projects these days, but
I am not sure it is a good assumption to make in general.
For example if some projects use or start using "CC: *" trailers and
tools to automatically append such trailers (perhaps using 'git
interpret-trailers' for that purpose by the way) based on people who
touched the same code, then it could very well be a common thing to
have 20 or more trailers on patches/commits for these projects.
There could also be automated testing tools that add their own
"Tested-by: *" trailers, and projects that require many people to add
their "Reviewed-by: *" trailers to each patch/commit. And in general
with millions of users, it is not very safe to make assumptions that
they will all be "reasonable" in the way they use a feature.
Another thing is that when Git started, I doubt many people would have
thought that there would often be more than just one or two
"Signed-off-by: *" trailer, and now 10 or 15 doesn't seem unthinkable.
In fact 'git interpret-trailers' has been made precisely because there
are more and more trailers, so there is more and more a need for a
tool to properly add and manage them.
We recently had a discussion on the list to increase the default
abbreviation length because it was not foreseen at the beginning of
Git that people would need a larger number of characters as projects
grow.
So even for the Linux kernel, I am not sure that it is safe to assume
that the number of trailers will not grow much over the years,
especially if we work on the tool that can make it easy to easily and
automatically add them.
Thanks,
Christian.
^ permalink raw reply
* Re: Formatting problem send_mail in version 2.10.0
From: Larry Finger @ 2016-10-12 15:27 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jeff King, Mathieu Lienard--Mayor, Remi Lespinet, git
In-Reply-To: <vpq60oy589b.fsf@anie.imag.fr>
On 10/12/2016 02:36 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>
>>>> That added information at the end is intended to be passed on to the
>>>> stable group. In this case, the patch needs to be applied to kernel
>>>> versions 4.8 and later.
>>>
>>> OK, but where do people fetch this information from?
>>
>> This format is used in a patch for the kernel. When the patch is
>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>> a copy of the original patch. Maintainers of the indicated systems
>> then merge the patch with their stable version.
>
> Sorry, but this does not answer my question. I'll rephrase: when
> people behind stable@vger.kernel.org get the message, how do they know
> which version of the kernel they should apply it to?
>
>> I do not want it in the body of the message. I just want to pass a
>> hint to the stable maintainer(s).
>
> If it's not in the body of the message, then where is it?
That information is supplied in the "Cc: Stable" line at the end. That is the
point for the "[4.8+]" notation. That means apply to kernel 4.8 and later.
Without this information, the patch author has to monitor the mainline kernel
for the application of the main patch, and then send a separate E-mail to Stable
indicating the version info. When the info is included, then the process is
automatic. I hope you understand now.
Larry
^ permalink raw reply
* problem with git worktree and git svn
From: Mathieu Arnold @ 2016-10-12 14:45 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 974 bytes --]
Hi,
I'm a FreeBSD ports developer, and while our main repository is in
Subversion, some of us use git-svn a lot for development.
I discovered git worktree earlier this week, and I found it a great
asset to be able to have more than one branch of my worktree accessible
at the same time...
Anyway, back to my problem, the way git-svn works, is that it looks for
a directory named "svn" in its gitdir and if it is not present, decide
the repository is using git-svn version 1 (whatever that is) and goes to
parse all the revisions to recreate the svn directory.
So I can only use git svn commands in my main worktree, the one with the
real gitdir.
To fix that, all I had to do is to add a symlink named svn in each
worktree's gitdir and pointing to ../../svn.
I think all that needs to happen is that when adding a new worktree, if
the main git directory has a "svn" directory, add a symlink to it in the
worktree's gitdir.
--
Mathieu Arnold
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]
^ permalink raw reply
* Re: git branches & merge
From: Anatoly Borodin @ 2016-10-12 14:43 UTC (permalink / raw)
To: webmaster; +Cc: git
In-Reply-To: <1240187899.41938.1476280216470.JavaMail.open-xchange@app08.ox.hosteurope.de>
Hi,
the IP will not be overwritten, you'll still have the new IP in
master. Nothing to worry about :)
--
Mit freundlichen Grüßen,
Anatoly Borodin
^ permalink raw reply
* Re: [PATCH] sequencer: mark a file-local symbol static
From: Johannes Schindelin @ 2016-10-12 14:17 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <376d8514-fe01-cc37-691d-6e101af564a5@ramsayjones.plus.com>
Hi Ramsay!
On Wed, 12 Oct 2016, Ramsay Jones wrote:
> If you need to re-roll your 'js/prepare-sequencer' branch, could you
> please squash this into commit 53f8024e ("sequencer: completely revamp
> the "todo" script parsing", 10-10-2016).
Will do!
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Johannes Schindelin @ 2016-10-12 13:30 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, venv21, dennis
In-Reply-To: <20161010175611.1058-1-sbeller@google.com>
Hi Stefan,
On Mon, 10 Oct 2016, Stefan Beller wrote:
> Before 63e95beb0 (2016-04-15, submodule: port resolve_relative_url from
> shell to C), it did not matter if the superprojects URL had a trailing
> slash or not. It was just chopped off as one of the first steps
> (The "remoteurl=${remoteurl%/}" near the beginning of
> resolve_relative_url(), which was removed in said commit).
>
> When porting this to the C version, an off-by-one error was introduced
> and we did not check the actual last character to be a slash, but the
> NULL delimiter.
It is a NUL delimiter, not a NULL delimiter.
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 444ec06..a7841a5 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -95,6 +95,8 @@ static int chop_last_dir(char **remoteurl, int is_relative)
> * NEEDSWORK: This works incorrectly on the domain and protocol part.
> * remote_url url outcome expectation
> * http://a.com/b ../c http://a.com/c as is
> + * http://a.com/b/ ../c http://a.com/c same as previous line, but
> + * ignore trailing slash in url
> * http://a.com/b ../../c http://c error out
> * http://a.com/b ../../../c http:/c error out
> * http://a.com/b ../../../../c http:c error out
> @@ -113,8 +115,8 @@ static char *relative_url(const char *remote_url,
> struct strbuf sb = STRBUF_INIT;
> size_t len = strlen(remoteurl);
>
> - if (is_dir_sep(remoteurl[len]))
> - remoteurl[len] = '\0';
> + if (is_dir_sep(remoteurl[len-1]))
> + remoteurl[len-1] = '\0';
>
> if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
> is_relative = 0;
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index bf2deee..82b98f8 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -319,6 +319,7 @@ test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
> test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
>
> test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
> +test_submodule_relative_url "(null)" "../foo/bar/" "../sub/a/b/c" "../foo/sub/a/b/c"
> test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
> test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
> test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
I see that this already made it to `next`. I saw that because it breaks
the build of Git for Windows (this was not noticed earlier because other
compile failures prevented the tests from running), as now the test cases
173 and 177 of t0060 fail (*not* the newly introduced 163).
Here is the output with -v -x:
-- snip --
[...]
expecting success:
actual=$(git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' '../.') &&
test "$actual" = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.'
+++ git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' ../.
++ actual=C:/git-sdk-64/usr/src/git/wip/t/.
++ test C:/git-sdk-64/usr/src/git/wip/t/. = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.'
error: last command exited with $?=1
not ok 172 - test_submodule_relative_url: (null) /usr/src/git/wip/t/trash directory.t0060-path-utils/. ../. => C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.
#
# actual=$(git submodule--helper
# resolve-relative-url-test '(null)'
# '/usr/src/git/wip/t/trash
# directory.t0060-path-utils/.' '../.') &&
# test "$actual" =
# 'C:/git-sdk-64/usr/src/git/wip/t/trash
# directory.t0060-path-utils/.'
#
[...]
expecting success:
actual=$(git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' '../submodule') &&
test "$actual" = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/submodule'
+++ git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' ../submodule
++ actual=C:/git-sdk-64/usr/src/git/wip/t/submodule
++ test C:/git-sdk-64/usr/src/git/wip/t/submodule = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/submodule'
error: last command exited with $?=1
not ok 176 - test_submodule_relative_url: (null) /usr/src/git/wip/t/trash directory.t0060-path-utils/. ../submodule => C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/submodule
#
# actual=$(git submodule--helper
# resolve-relative-url-test '(null)'
# '/usr/src/git/wip/t/trash
# directory.t0060-path-utils/.' '../submodule') &&
# test "$actual" =
# 'C:/git-sdk-64/usr/src/git/wip/t/trash
# directory.t0060-path-utils/submodule'
#
[...]
-- snap --
For comparison, this is how it succeeds in an Ubuntu VM:
-- snap --
expecting success:
actual=$(git submodule--helper resolve-relative-url-test '(null)' '/home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.' '../.') &&
test "$actual" = '/home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.'
+++ git submodule--helper resolve-relative-url-test '(null)' '/home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.' ../.
++ actual='/home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.'
++ test '/home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.' = '/home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 173 - test_submodule_relative_url: (null) /home/virtualbox/git/wip/t/trash directory.t0060-path-utils/. ../. => /home/virtualbox/git/wip/t/trash directory.t0060-path-utils/.
-- snap --
The reason that this fails on Windows is that the POSIX->Windows path
mangling of the MSYS2 shell strips the trailing . from "/some/directory/."
when converting it to "C:/git-sdk-64/some/directory", and for a good
reason: most Windows programs do not handle the trailing "." very well.
One very, very ugly workaround for this newly-introduced breakage would be
this:
-- snip --
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 82b98f8..abd82e9 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -328,11 +328,11 @@ test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
+test_submodule_relative_url "(null)" "$(pwd)/." "../." "$(pwd)/."
test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
test_submodule_relative_url "(null)" "$PWD" "./ " "$(pwd)/ "
-test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$(pwd)/." "../submodule" "$(pwd)/submodule"
test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
-- snap --
The reasons this is ugly: we specifically test for *Unixy* paths when we
use $PWD, as opposed to *Windowsy* paths when using $(pwd). We do this to
ensure a certain level of confidence that running things such as
git clone --recurse-submodules /z/project/.
work. And now that does not work anymore.
So where to go from here?
Ciao,
Dscho
^ permalink raw reply related
* git branches & merge
From: webmaster @ 2016-10-12 13:50 UTC (permalink / raw)
To: git
Hi.
I have created a release_x branch to work on a new release. The branch is based
on master.
Later I needed to create a hotfix_x branch to work on a hotfix. This branch is
also based on master.
In the hotfix_x branch I needed to correct an IP address to communicate with.
After doing so, I merged the hotfix_x branch with master again and made my
deployment.
Now in the release_x branche the IP address is still the old one.
I am afraid of committing and merging my release_x branch since I think that my
newly defined IP address (now on master) will be overridden when merging.
Am I right? If yes, what is the best way to solve this?
Thanks
-fuz
^ permalink raw reply
* Re: git diff
From: webmaster @ 2016-10-12 13:51 UTC (permalink / raw)
To: Mike Rappazzo; +Cc: git
In-Reply-To: <CANoM8SUtrV8CD8M=4CUB8xr9gQj_hKzGOdjQZHuz4+36y_UfJw@mail.gmail.com>
Thanks, Mike. Now I understood.
-fuz
> Mike Rappazzo <rappazzo@gmail.com> hat am 12. Oktober 2016 um 13:06
> geschrieben:
>
> On Wed, Oct 12, 2016 at 6:50 AM, <webmaster@peter-speer.de> wrote:
> > Hi.
> >
> > I created a new branch named hotfix from master.
> > I switched to the branch, changed 1 file.
> >
> > Now I want to see the diff from the both using
> >
> > git diff hotfix master
> >
> > I do not see any output (difference).
> > When I do a git status I see my file with status mofified, not staged for
> > commit.
>
> Since you just created the branch, and did not add any content, there
> is no difference to see. A branch is just a pointer to a commit. You
> now have two pointers pointing at the same commit.
>
> If you want to see the difference between your changes and the master
> branch, you can omit the first reference:
>
> git diff master
>
> When you start adding commits to your hotfix branch, you will be able
> to see the diff between that and master with the command that you
> gave. However, your arguments may be in the reverse order than what
> you expect. You want to specify master first because that is the
> mainline branch (I presume).
>
> When you have several commits on your hotfix branch, you can refer to
> older commits to diff against. There are several ways to refer back,
> but the simplest is to use a tilde '~' followed by a number to count
> back. For example 'hotfix~1' refers to the parent commit on the
> hotfix branch. There is a lot in the documentation[1], so take a look
> there for more info.
>
> Good luck.
> _Mike
>
> [1] https://git-scm.com/doc
>
> > Also, I can see that I am working with the correct branch, hotfix
> >
> > What am I doing wrong?
> >
> > -fuz
>
> On Wed, Oct 12, 2016 at 6:50 AM, <webmaster@peter-speer.de> wrote:
> > Hi.
> >
> > I created a new branch named hotfix from master.
> > I switched to the branch, changed 1 file.
> >
> > Now I want to see the diff from the both using
> >
> > git diff hotfix master
> >
> > I do not see any output (difference).
> > When I do a git status I see my file with status mofified, not staged for
> > commit.
> > Also, I can see that I am working with the correct branch, hotfix
> >
> > What am I doing wrong?
> >
> > -fuz
>
^ permalink raw reply
* [PATCH v2 2/2] convert.c: stream and fast search for binary
From: tboegi @ 2016-10-12 13:47 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
In-Reply-To: <20161009095649.1886-1-tboegi@web.de>
From: Torsten Bögershausen <tboegi@web.de>
When statistics are done for the autocrlf handling, the search in
the content can be stopped, if e.g
- a search for binary is done, and a NUL character is found
- a search for CRLF is done, and the first CRLF is found.
Similar when statistics for binary vs non-binary are gathered:
Whenever a lone CR or NUL is found, the search can be aborted.
When checking out files in "auto" mode, any file that has a "lone CR"
or a CRLF will not be converted, so the search can be aborted early.
Add the new bit, CONVERT_STAT_BITS_ANY_CR,
which is set for either lone CR or CRLF.
Many binary files have a NUL very early and it is often not necessary
to load the whole content of a file or blob into memory.
Split gather_stats() into gather_all_stats() and gather_stats_partly()
to do a streaming handling for blobs and files in the worktree.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
convert.c | 191 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 129 insertions(+), 62 deletions(-)
diff --git a/convert.c b/convert.c
index 077f5e6..2396fe5 100644
--- a/convert.c
+++ b/convert.c
@@ -3,6 +3,7 @@
#include "run-command.h"
#include "quote.h"
#include "sigchain.h"
+#include "streaming.h"
/*
* convert.c - convert a file when checking it out and checking it in.
@@ -13,10 +14,12 @@
* translation when the "text" attribute or "auto_crlf" option is set.
*/
-/* Stat bits: When BIN is set, the txt bits are unset */
#define CONVERT_STAT_BITS_TXT_LF 0x1
#define CONVERT_STAT_BITS_TXT_CRLF 0x2
#define CONVERT_STAT_BITS_BIN 0x4
+#define CONVERT_STAT_BITS_ANY_CR 0x8
+
+#define STREAM_BUFFER_SIZE (1024*16)
enum crlf_action {
CRLF_UNDEFINED,
@@ -31,30 +34,36 @@ enum crlf_action {
struct text_stat {
/* NUL, CR, LF and CRLF counts */
- unsigned nul, lonecr, lonelf, crlf;
+ unsigned stat_bits, lonecr, lonelf, crlf;
/* These are just approximations! */
unsigned printable, nonprintable;
};
-static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
+static void gather_stats_partly(const char *buf, unsigned long size,
+ struct text_stat *stats, unsigned search_only)
{
unsigned long i;
- memset(stats, 0, sizeof(*stats));
-
+ if (!buf || !size)
+ return;
for (i = 0; i < size; i++) {
unsigned char c = buf[i];
if (c == '\r') {
+ stats->stat_bits |= CONVERT_STAT_BITS_ANY_CR;
if (i+1 < size && buf[i+1] == '\n') {
stats->crlf++;
i++;
- } else
+ stats->stat_bits |= CONVERT_STAT_BITS_TXT_CRLF;
+ } else {
stats->lonecr++;
+ stats->stat_bits |= CONVERT_STAT_BITS_BIN;
+ }
continue;
}
if (c == '\n') {
stats->lonelf++;
+ stats->stat_bits |= CONVERT_STAT_BITS_TXT_LF;
continue;
}
if (c == 127)
@@ -67,7 +76,7 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *
stats->printable++;
break;
case 0:
- stats->nul++;
+ stats->stat_bits |= CONVERT_STAT_BITS_BIN;
/* fall through */
default:
stats->nonprintable++;
@@ -75,6 +84,8 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *
}
else
stats->printable++;
+ if (stats->stat_bits & search_only)
+ break; /* We found what we have been searching for */
}
/* If file ends with EOF then don't count this EOF as non-printable. */
@@ -86,41 +97,62 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *
* The same heuristics as diff.c::mmfile_is_binary()
* We treat files with bare CR as binary
*/
-static int convert_is_binary(unsigned long size, const struct text_stat *stats)
+static void convert_nonprintable(struct text_stat *stats)
{
- if (stats->lonecr)
- return 1;
- if (stats->nul)
- return 1;
if ((stats->printable >> 7) < stats->nonprintable)
- return 1;
- return 0;
+ stats->stat_bits |= CONVERT_STAT_BITS_BIN;
}
-static unsigned int gather_convert_stats(const char *data, unsigned long size)
+static void gather_all_stats(const char *buf, unsigned long size,
+ struct text_stat *stats, unsigned search_only)
{
+ memset(stats, 0, sizeof(*stats));
+ gather_stats_partly(buf, size, stats, search_only);
+ convert_nonprintable(stats);
+}
+
+
+static unsigned get_convert_stats_sha1(unsigned const char *sha1,
+ unsigned search_only)
+{
+ struct git_istream *st;
struct text_stat stats;
- int ret = 0;
- if (!data || !size)
- return 0;
- gather_stats(data, size, &stats);
- if (convert_is_binary(size, &stats))
- ret |= CONVERT_STAT_BITS_BIN;
- if (stats.crlf)
- ret |= CONVERT_STAT_BITS_TXT_CRLF;
- if (stats.lonelf)
- ret |= CONVERT_STAT_BITS_TXT_LF;
+ enum object_type type;
+ unsigned long sz;
- return ret;
+ if (!sha1)
+ return 0;
+ memset(&stats, 0, sizeof(stats));
+ st = open_istream(sha1, &type, &sz, NULL);
+ if (!st) {
+ return 0;
+ }
+ if (type != OBJ_BLOB)
+ goto close_and_exit_i;
+ for (;;) {
+ char buf[STREAM_BUFFER_SIZE];
+ ssize_t readlen = read_istream(st, buf, sizeof(buf));
+ if (readlen < 0)
+ break;
+ if (!readlen)
+ break;
+ gather_stats_partly(buf, (unsigned long)readlen, &stats, search_only);
+ if (stats.stat_bits & search_only)
+ break; /* We found what we have been searching for */
+ }
+close_and_exit_i:
+ close_istream(st);
+ convert_nonprintable(&stats);
+ return stats.stat_bits;
}
-static const char *gather_convert_stats_ascii(const char *data, unsigned long size)
+static const char *convert_stats_ascii(unsigned convert_stats)
{
- unsigned int convert_stats = gather_convert_stats(data, size);
-
+ const unsigned eol_bits = CONVERT_STAT_BITS_TXT_LF |
+ CONVERT_STAT_BITS_TXT_CRLF;
if (convert_stats & CONVERT_STAT_BITS_BIN)
return "-text";
- switch (convert_stats) {
+ switch (convert_stats & eol_bits) {
case CONVERT_STAT_BITS_TXT_LF:
return "lf";
case CONVERT_STAT_BITS_TXT_CRLF:
@@ -132,24 +164,45 @@ static const char *gather_convert_stats_ascii(const char *data, unsigned long si
}
}
+static unsigned get_convert_stats_wt(const char *path)
+{
+ struct text_stat stats;
+ unsigned search_only = CONVERT_STAT_BITS_BIN;
+ int fd;
+ memset(&stats, 0, sizeof(stats));
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ return 0;
+ for (;;) {
+ char buf[STREAM_BUFFER_SIZE];
+ ssize_t readlen = read(fd, buf, sizeof(buf));
+ if (readlen < 0)
+ break;
+ if (!readlen)
+ break;
+ gather_stats_partly(buf, (unsigned long)readlen, &stats, search_only);
+ if (stats.stat_bits & search_only)
+ break; /* We found what we have been searching for */
+ }
+ close(fd);
+ convert_nonprintable(&stats);
+ return stats.stat_bits;
+}
+
const char *get_cached_convert_stats_ascii(const char *path)
{
- const char *ret;
- unsigned long sz;
- void *data = read_blob_data_from_cache(path, &sz);
- ret = gather_convert_stats_ascii(data, sz);
- free(data);
- return ret;
+ unsigned convert_stats;
+ unsigned search_only = CONVERT_STAT_BITS_BIN;
+ convert_stats = get_convert_stats_sha1(get_sha1_from_cache(path),
+ search_only);
+ return convert_stats_ascii(convert_stats);
}
const char *get_wt_convert_stats_ascii(const char *path)
{
- const char *ret = "";
- struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(&sb, path, 0) >= 0)
- ret = gather_convert_stats_ascii(sb.buf, sb.len);
- strbuf_release(&sb);
- return ret;
+ unsigned convert_stats;
+ convert_stats = get_convert_stats_wt(path);
+ return convert_stats_ascii(convert_stats);
}
static int text_eol_is_crlf(void)
@@ -213,16 +266,10 @@ static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
static int has_cr_in_index(const char *path)
{
- unsigned long sz;
- void *data;
- int has_cr;
-
- data = read_blob_data_from_cache(path, &sz);
- if (!data)
- return 0;
- has_cr = memchr(data, '\r', sz) != NULL;
- free(data);
- return has_cr;
+ unsigned convert_stats;
+ convert_stats = get_convert_stats_sha1(get_sha1_from_cache(path),
+ CONVERT_STAT_BITS_ANY_CR);
+ return convert_stats & CONVERT_STAT_BITS_ANY_CR;
}
static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
@@ -234,13 +281,13 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
if (!stats->lonelf)
return 0;
- if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
+ if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_CRLF) {
/* If we have any CR or CRLF line endings, we do not touch it */
/* This is the new safer autocrlf-handling */
if (stats->lonecr || stats->crlf)
return 0;
- if (convert_is_binary(len, stats))
+ if (stats->stat_bits & CONVERT_STAT_BITS_BIN)
return 0;
}
return 1;
@@ -253,7 +300,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
{
struct text_stat stats;
char *dst;
- int convert_crlf_into_lf;
+ int has_crlf_to_convert;
+ unsigned search_only = 0;
if (crlf_action == CRLF_BINARY ||
(src && !len))
@@ -266,12 +314,16 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
if (!buf && !src)
return 1;
- gather_stats(src, len, &stats);
+ if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF)
+ search_only = CONVERT_STAT_BITS_BIN;
+
+ gather_all_stats(src, len, &stats, search_only);
+
/* Optimization: No CRLF? Nothing to convert, regardless. */
- convert_crlf_into_lf = !!stats.crlf;
+ has_crlf_to_convert = !!stats.crlf;
if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
- if (convert_is_binary(len, &stats))
+ if (stats.stat_bits & CONVERT_STAT_BITS_BIN)
return 0;
/*
* If the file in the index has any CR in it, do not convert.
@@ -280,24 +332,35 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
if (checksafe == SAFE_CRLF_RENORMALIZE)
checksafe = SAFE_CRLF_FALSE;
else if (has_cr_in_index(path))
- convert_crlf_into_lf = 0;
+ has_crlf_to_convert = 0;
}
if (checksafe && len) {
struct text_stat new_stats;
memcpy(&new_stats, &stats, sizeof(new_stats));
/* simulate "git add" */
- if (convert_crlf_into_lf) {
+ if (has_crlf_to_convert) {
new_stats.lonelf += new_stats.crlf;
new_stats.crlf = 0;
+ /* all crlf, if any, are gone. Update the bits */
+ new_stats.stat_bits = stats.stat_bits & CONVERT_STAT_BITS_BIN;
+ if (new_stats.lonelf)
+ new_stats.stat_bits |= CONVERT_STAT_BITS_TXT_LF;
+ if (new_stats.lonecr)
+ new_stats.stat_bits |= CONVERT_STAT_BITS_ANY_CR;
}
/* simulate "git checkout" */
if (will_convert_lf_to_crlf(len, &new_stats, crlf_action)) {
new_stats.crlf += new_stats.lonelf;
new_stats.lonelf = 0;
+ new_stats.stat_bits = stats.stat_bits & CONVERT_STAT_BITS_BIN;
+ if (new_stats.crlf)
+ new_stats.stat_bits |= CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_ANY_CR;
+ if (new_stats.lonecr)
+ new_stats.stat_bits |= CONVERT_STAT_BITS_ANY_CR;
}
check_safe_crlf(path, crlf_action, &stats, &new_stats, checksafe);
}
- if (!convert_crlf_into_lf)
+ if (!has_crlf_to_convert)
return 0;
/*
@@ -338,11 +401,15 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
{
char *to_free = NULL;
struct text_stat stats;
+ unsigned search_only = 0;
if (!len || output_eol(crlf_action) != EOL_CRLF)
return 0;
- gather_stats(src, len, &stats);
+ if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_CRLF)
+ search_only = CONVERT_STAT_BITS_ANY_CR | CONVERT_STAT_BITS_BIN;
+
+ gather_all_stats(src, len, &stats, search_only);
if (!will_convert_lf_to_crlf(len, &stats, crlf_action))
return 0;
--
2.10.0
^ permalink raw reply related
* [PATCH v2 0/2] Stream and fast search
From: tboegi @ 2016-10-12 13:47 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
In-Reply-To: <20161009095649.1886-1-tboegi@web.de>
From: Torsten Bögershausen <tboegi@web.de>
Changes since v1:
- Rename earlyout into search_only
- Increase buffer from 2KiB to 16KiB
- s/mask/eol_bits/
- Reduce the "noise"
- Document "split gather_stats() into gather_all_stats()/gather_stats_partly()
Torsten Bögershausen (2):
read-cache: factor out get_sha1_from_index() helper
convert.c: stream and fast search for binary
cache.h | 3 +
convert.c | 191 ++++++++++++++++++++++++++++++++++++++++-------------------
read-cache.c | 29 +++++----
3 files changed, 150 insertions(+), 73 deletions(-)
--
2.10.0
^ permalink raw reply
* [PATCH v2 1/2] read-cache: factor out get_sha1_from_index() helper
From: tboegi @ 2016-10-12 13:47 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
In-Reply-To: <20161009095649.1886-1-tboegi@web.de>
From: Torsten Bögershausen <tboegi@web.de>
Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().
This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.
Add a wrapper definition get_sha1_from_cache().
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
cache.h | 3 +++
read-cache.c | 29 ++++++++++++++++++-----------
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/cache.h b/cache.h
index 1604e29..04de209 100644
--- a/cache.h
+++ b/cache.h
@@ -380,6 +380,7 @@ extern void free_name_hash(struct index_state *istate);
#define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
#define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
#define read_blob_data_from_cache(path, sz) read_blob_data_from_index(&the_index, (path), (sz))
+#define get_sha1_from_cache(path) get_sha1_from_index (&the_index, (path))
#endif
enum object_type {
@@ -1089,6 +1090,8 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
}
+const unsigned char *get_sha1_from_index(struct index_state *istate, const char *path);
+
/*
* This internal function is only declared here for the benefit of
* lookup_replace_object(). Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index 38d67fa..5a1df14 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2290,13 +2290,27 @@ int index_name_is_other(const struct index_state *istate, const char *name,
void *read_blob_data_from_index(struct index_state *istate, const char *path, unsigned long *size)
{
- int pos, len;
+ const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
- len = strlen(path);
- pos = index_name_pos(istate, path, len);
+ sha1 = get_sha1_from_index(istate, path);
+ if (!sha1)
+ return NULL;
+ data = read_sha1_file(sha1, &type, &sz);
+ if (!data || type != OBJ_BLOB) {
+ free(data);
+ return NULL;
+ }
+ if (size)
+ *size = sz;
+ return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const char *path)
+{
+ int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
* We might be in the middle of a merge, in which
@@ -2312,14 +2326,7 @@ void *read_blob_data_from_index(struct index_state *istate, const char *path, un
}
if (pos < 0)
return NULL;
- data = read_sha1_file(istate->cache[pos]->oid.hash, &type, &sz);
- if (!data || type != OBJ_BLOB) {
- free(data);
- return NULL;
- }
- if (size)
- *size = sz;
- return data;
+ return istate->cache[pos]->oid.hash;
}
void stat_validity_clear(struct stat_validity *sv)
--
2.10.0
^ permalink raw reply related
* Re: [PATCH v2 3/3] batch check whether submodule needs pushing into one call
From: Heiko Voigt @ 2016-10-12 13:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Stefan Beller, git, Jens.Lehmann, Fredrik Gustafsson,
Leandro Lucarella
In-Reply-To: <xmqqlgxvbype.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 10, 2016 at 03:56:13PM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
>
> > -static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
> > +static int check_has_hash(const unsigned char sha1[20], void *data)
> > {
> > - if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> > + int *has_hash = (int *) data;
> > +
> > + if (!lookup_commit_reference(sha1))
> > + *has_hash = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
> > +{
> > + int has_hash = 1;
> > +
> > + if (add_submodule_odb(path))
> > + return 0;
> > +
> > + sha1_array_for_each_unique(hashes, check_has_hash, &has_hash);
> > + return has_hash;
> > +}
> > +
> > +static int submodule_needs_pushing(const char *path, struct sha1_array *hashes)
> > +{
> > + if (!submodule_has_hashes(path, hashes))
> > return 0;
>
> Same comment about naming.
>
> What do check-has-hash and submodule-has-hashes exactly mean by
> "hash" in their names? Because I think what is checked here is
> "does the local submodule repository have _all_ the commits
> referenced from the superproject commit we are pushing?", so I'd
> prefer to see "commit" in their names.
>
> If we do not even have these commits locally, then there is no point
> attempting to push, so returning 0 (i.e. it is not "needs pushing"
> situation) is correct but it is a but subtle. It's not "we know
> they already have them", but it is "even if we tried to push, it
> won't do us or the other side any good." A single-liner in-code
> comment may help.
First the naming part. How about:
submodule_has_commits()
?
Second as mentioned a previous answer[1] to this part: I would actually
like to have a die() here instead of blindly proceeding. Since the user
either specified --recurse-submodules=... at the commandline or it was
implicitly enabled because we have submodules in the tree we should be
careful and not push revisions referencing submodules that are not
available at a remote. If we can not properly figure it out I would
suggest to stop and tell the user how to solve the situation. E.g.
either she clones the appropriate submodules or specifies
--no-recurse-submodules on the commandline to tell git that she does not
care.
Returning 0 here means: "No push needed" but the correct answer would
be: "We do not know". Question is what we should do here which I am
planning to address in a separate patch series since that will be
changing behavior.
So how about:
if (!submodule_has_hashes(path, hashes))
/* NEEDSWORK: The correct answer here is "We do not
* know" instead of "No". We currently proceed pushing
* here as if the submodules commits are available on a
* remote, which is not always correct. */
return 0;
What do you think?
Cheers Heiko
[1] http://public-inbox.org/git/20160919195812.GC62429@book.hvoigt.net/
^ 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