* Re: [PATCH] reset: --unmerge
From: Junio C Hamano @ 2016-10-27 16:23 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <87d1il3hhs.fsf@linux-m68k.org>
Andreas Schwab <schwab@linux-m68k.org> writes:
> On Okt 25 2016, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Somebody with a bright idea decided that vc-git-resolve-conflicts
>> variable should be on by default in Emacs 25.1 X-<
>
> This is consistent with the behaviour of the other VC backends, where it
> isn't even customizable.
The problem I had was "M-x save-buffer" (after resolving the
conflicts in it manually) running "git add" on the resulting file,
robbing from "git diff" an opportunity to help the user to see how
the result looks relative to both branches.
Do you mean that VC mode broke the same feature equally other SCMs,
too? Do other SCM supported by VC backends take advantage of
unmerged stages in the index (until you say "$scm add") by allowing
you to verify the combined diff with "$scm diff"?
^ permalink raw reply
* Re: [PATCH v1 16/19] read-cache: unlink old sharedindex files
From: Junio C Hamano @ 2016-10-27 16:13 UTC (permalink / raw)
To: Duy Nguyen
Cc: Christian Couder, Git Mailing List,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <CACsJy8BPe085Qu4GpJ3MLTPeZM_7pbVrX31enQrGQn0oyjSrdg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Christian, if we assume to go with Junio's suggestion to disable
> split-index on temporary files, the only files left we have to take
> care of are index and index.lock. I believe pruning here in this code
> will have an advantage over in "git gc --auto" because when this is
> executed, we know we're holding index.lock, so nobody else is updating
> the index, it's race-free.
>
> All we need to do is peek in $GIT_DIR/index
> to see what shared index file it requires and keep it alive too, the
> remaining of shared index files can be deleted safely. We don't even
> need to fall back to mtime.
Yes, that exactly was why I wondered if we can afford to limit
splitting only to the primary index, because it makes things a
lot simpler.
But I suspect that temporary index is where split-index shines most,
e.g. while creating a partial commit. The mechanism penalizes the
read performance by making the format more complex in order to favor
the write performance, which is very much suited for temporary one
that is read only once after it is written before it gets discarded
(on the other hand, splitting the primary index will penalize reads
that happen a lot more than writes).
While I still find it attractive at the conceptual level to limit
splitting only to the primary index for the resulting simplicity,
I doubt it is a good way to go, as I meant to say in
<xmqqeg33ccjj.fsf@gitster.mtv.corp.google.com>
> git-gc just can't match this because while it's running, somebody else
> may be updating $GIT_DIR/index. Handling races would be a lot harder.
It could attempt to take a lock on the primary index while it runs,
and refrain to do anything if it can't take the lock ("gc --auto"
may want to silently retry), and then the race is no longer an
issue, no?
^ permalink raw reply
* Re: [PATCH] reset: --unmerge
From: Andreas Schwab @ 2016-10-27 16:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <xmqqmvhsc8kn.fsf@gitster.mtv.corp.google.com>
On Okt 25 2016, Junio C Hamano <gitster@pobox.com> wrote:
> Somebody with a bright idea decided that vc-git-resolve-conflicts
> variable should be on by default in Emacs 25.1 X-<
This is consistent with the behaviour of the other VC backends, where it
isn't even customizable.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #06; Mon, 24)
From: Johannes Schindelin @ 2016-10-27 15:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Stefan Beller, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <23f15627-4964-7ba9-45c3-899366d621bd@kdbg.org>
Hi Hannes,
On Tue, 25 Oct 2016, Johannes Sixt wrote:
> Am 25.10.2016 um 20:13 schrieb Stefan Beller:
> > On Tue, Oct 25, 2016 at 10:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > > - the "off-by-one fix" part of sb/submodule-ignore-trailing-slash
> > > needs to be in the upcoming release but the "trailing /. in base
> > > should not affect the resolution of ../relative/path" part that
> > > is still under discussion can wait. Which means we'd need a few
> > > more !MINGW prerequisites in the tests by -rc0.
> > >[...]
> >
> > So maybe instead of adding !MINGW we rather want to apply
> > https://public-inbox.org/git/2908451e-4273-8826-8989-5572263cc283@kdbg.org/
> > instead for now?
>
> I was about to submit this very patch again, and only then saw your message.
> So, yes, that's what I propose, too.
>
> Dscho, does this patch fix the test failures that you observed, too?
> Unfortunately, it goes against our endeavor to reduce subshells.
I am fine with the patch, even if I did not have a chance to test it yet
(ran out of time today).
Ciao,
Dscho
^ permalink raw reply
* Re: Expanding Includes in .gitignore
From: Jeff King @ 2016-10-27 10:50 UTC (permalink / raw)
To: Aaron Pelly; +Cc: git
In-Reply-To: <80919456-7563-2c16-ba23-ce4fcc2777de@pelly.co>
On Thu, Oct 27, 2016 at 01:22:43PM +1300, Aaron Pelly wrote:
> The use case for this is where I did not write my own rules, but I want
> to keep them updated. https://github.com/github/gitignore is a damn good
> resource, but I want to pull it and include relevant bits project by
> project and/or system wide. I don't want to have to update many projects
> manually if that, or any other, repo changes.
That seems like a reasonable thing to want.
> A very brief look at dir.c would indicate that a recursive call from
> add_excludes to itself when it parses some sort of include tag would do
> it within a file. I'm sure it'd be pretty straight forward to hook into
> something in dir.c to parse directories too.
>
> I'm thinking something like ". path/to/include/file" in an ignore file,
> and/or creating .gitignore.d and/or allowing $HOME/.config/git/ignore
> and $GIT_DIR/info/exclude to be directories. Or some sane and consistent
> mixture of these things.
I'd shy away from an actual include directive, as it raises a lot of
complications:
- as you noted, cycles in the include graph need to be detected and
broken
- we parse possibly-hostile .gitignore files from cloned repositories.
What happens when I include ask to include /etc/passwd? Probably
nothing, but there are setups where it might matter (e.g., something
like Travis that auto-builds untrusted repositories, and you could
potentially leak the contents of files via error messages). It's
nice to avoid the issue entirely.
- finding a backwards-compatible syntax
Whereas letting any of the user- or repo-level exclude files be a
directory, and simply reading all of the files inside, seems simple and
obvious. If you go that route, it probably makes sense to teach
gitattributes the same trick.
> In the case of a directory the plan would be to add links to files
> stored/sourced elsewhere. This does pose a precedence question which I
> haven't thought about yet, but probably makes it too hard for the
> limited value it brings.
I think the normal behavior in such "foo.d" directory is to just sort
the contents lexically and read them in order, as if they were all
concatenated together, and with no recursion. I.e., behave "as if" the
user had run "cat $dir/*".
That lets you handle precedence via the filenames (or symlink names). It
can't handle all cases (some items in "00foo" want precedence over "01bar"
and vice versa), but I don't think there's an easy solution. That's a
good sign that one or more of the files should be broken up.
-Peff
^ permalink raw reply
* Re: Expanding Includes in .gitignore
From: Aaron Pelly @ 2016-10-27 9:51 UTC (permalink / raw)
Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kasr6ubuOm01rLtdK7pfWqriZE1vioLyxAr2G2Hd1xGhg@mail.gmail.com>
On 27/10/16 15:22, Stefan Beller wrote:
>> The use case for this is where I did not write my own rules, but I want
>> to keep them updated. https://github.com/github/gitignore is a damn good
>> resource, but I want to pull it and include relevant bits project by
>> project and/or system wide. I don't want to have to update many projects
>> manually if that, or any other, repo changes.
>
> .git/info/exclude could be a (sym)link to an up to date version
> of the gitignore repo as a hack?
>
Using links isn't a bad idea, but you still end up at some stage
combining the contents of several files that already exist. Well, in my
example, anyway.
I accept that I'm being pretty trivial, and once it's set up there's
never any pressing need to change anything, but it still irks me.
Even with a linked .gitignore, or .git/info/exclude there will be
sections that are project, language, editor, machine, whatever,
specific. So I still need to copy stuff from one file to another by
hand. By allowing includes, I only have to have a link to each file
describing the data types for each component of the environment.
And they are community maintained, so I don't have to google every time
I try a new editor.
note to self: reply-to isn't the list.
^ permalink raw reply
* Re: [PATCH v1 16/19] read-cache: unlink old sharedindex files
From: Duy Nguyen @ 2016-10-27 10:25 UTC (permalink / raw)
To: Christian Couder
Cc: Git Mailing List, Junio C Hamano,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <CACsJy8C4Qd76LSYxk9BJtN3n3Knv_RCDgBOO-ybkNHFZK61ahQ@mail.gmail.com>
On Tue, Oct 25, 2016 at 5:43 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> static int write_shared_index(struct index_state *istate,
>> @@ -2211,8 +2269,11 @@ static int write_shared_index(struct index_state *istate,
>> }
>> ret = rename_tempfile(&temporary_sharedindex,
>> git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
>> - if (!ret)
>> + if (!ret) {
>> hashcpy(si->base_sha1, si->base->sha1);
>> + clean_shared_index_files(sha1_to_hex(si->base->sha1));
>
> This operation is technically garbage collection and should belong to
> "git gc --auto", which is already called automatically in a few
> places. Is it not called often enough that we need to do the cleaning
> up right after a new shared index is created?
Christian, if we assume to go with Junio's suggestion to disable
split-index on temporary files, the only files left we have to take
care of are index and index.lock. I believe pruning here in this code
will have an advantage over in "git gc --auto" because when this is
executed, we know we're holding index.lock, so nobody else is updating
the index, it's race-free. All we need to do is peek in $GIT_DIR/index
to see what shared index file it requires and keep it alive too, the
remaining of shared index files can be deleted safely. We don't even
need to fall back to mtime.
git-gc just can't match this because while it's running, somebody else
may be updating $GIT_DIR/index. Handling races would be a lot harder.
--
Duy
^ permalink raw reply
* Re: Expanding Includes in .gitignore
From: Aaron Pelly @ 2016-10-27 10:33 UTC (permalink / raw)
Cc: git
In-Reply-To: <CALhvvbYqeWw+q=TPxTpve6JKoy0URYeWxj2vVOnzrA_g3Z3esA@mail.gmail.com>
On 27/10/16 21:19, Alexei Lozovsky wrote:
>> I'm thinking something like ". path/to/include/file" in an ignore file,
>> and/or creating .gitignore.d and/or allowing $HOME/.config/git/ignore
>> and $GIT_DIR/info/exclude to be directories. Or some sane and consistent
>> mixture of these things.
>
> I think the rc.d-like approach with directories is better as it
> does not add new magical filenames (what if I absolutely do need
> to name my directories ". path", with a space? :)
Yes. Another alternative I thought of was #include path/to/include/file.
That'd be backwards compatible, which is a good thing, but would involve
parsing comments, which obviously could start with #include. Or lines
like ^Include /path/file$ In the case of finding an invalid file,
passing it over and issuing a simple warning should surface any issues
with existing gitignores. Anyway, this conversation is why I bring it up
on the list.
Coming back to this, maybe :(include)/path/file might be more git-like
>> In the case of a directory the plan would be to add links to files
>> stored/sourced elsewhere. This does pose a precedence question which I
>> haven't thought about yet, but probably makes it too hard for the
>> limited value it brings.
>
> Now, if we consider the case of multiple .gitignore files, it
> could be unexpected and possibly annoying for negative patterns
> in one file to affect the patterns added by some other files.
That is a concern. It is non obvious; the worst kind of annoying.
> I would find it more conceptually simple to apply individual
> .gitignores one by one, as opposed to parsing them all and
> creating one giant exclusion rule. (In technical terms, this
> means keeping one struct exclude_list for each .gitignore,
> not merging them all into one single list.)
I agree. I haven't looked, but that sounds like touching significantly
more code though. Actually, thinking about it for 20 seconds more, it
shouldn't be too hard, should it?
> In this case there should be no precendence problems as applied
> gitignores only add new ignored files, without un-ignoring
> anything previously ignored by other files.
Again, I haven't looked yet, but there is still an issue of precedence
with other gitignore files in $HOME and the repo.
> However, if we allow textual inclusion, then it means that we
> can put a gitignore into our gitignore so that we can unignore
> while we ignore, which again brings us the question of whether
> it is actually needed and expected.
Gah! Yes. One way or the other.
>> I would like to know the desirability/practicality/stupidity of such a
>> feature as I believe it is within my skillset to implement it.
>
> However, I do not recall any precendent of git using rc.d-like
> configs.
Many things have adopted this technique recently. Well, the last 15
years. It is common, understood, and fairly simple. I see no issue with it.
> And some can argue that your goal can be achieved by
> generating the .gitignore by some external means and symlinking
> the result into .git/info/exclude, so this is not Git's problem
> and we should not be overcomplicating things with something as
> simple as a list exclude patterns. This line of argument also
> can be used to opposes any textual inclusion as well, because
> it can be expanded into 'why don't we add a Turing-complete
> programming language then to specify the patterns to ignore'.
I know. Another reason to bring the idea to the list. I sort of have
this attitude myself. My main objection to it is that I can't think of a
hook to automate it with.
But: What about some kind of :(exec) that executes a script and returns
a gitignore file? You write the script; you're responsible. And the
behaviour is obvious. I haven't thought that through. It just came to me
then, and might present security issues, but could greatly simplify things.
^ permalink raw reply
* Re: [PATCH] rebase: add --forget to cleanup rebase, leave HEAD untouched
From: Duy Nguyen @ 2016-10-27 10:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqq60ofcavd.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 26, 2016 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> There are occasions when you decide to abort an in-progress rebase and
>> move on to do something else but you forget to do "git rebase --abort"
>> first. Or the rebase has been in progress for so long you forgot about
>> it. By the time you realize that (e.g. by starting another rebase)
>> it's already too late to retrace your steps. The solution is normally
>>
>> rm -r .git/<some rebase dir>
>>
>> and continue with your life. But there could be two different
>> directories for <some rebase dir> (and it obviously requires some
>> knowledge of how rebase works), and the ".git" part could be much
>> longer if you are not at top-dir, or in a linked worktree. And
>> "rm -r" is very dangerous to do in .git, a mistake in there could
>> destroy object database or other important data.
>>
>> Provide "git rebase --forget" for this exact use case.
>
> Two and a half comments.
>
> - The title says "leave HEAD untouched". Are my working tree files
> and my index also safe from this operation, or is HEAD the only
> thing that is protected?
Everything is protected. I will rephrase the title a bit. The option
is basically a safe form of "rm -r .git/rebase-{apply,merge}".
> - I think I saw a variant of this gotcha for an unconcluded
> cherry-pick that was left behind, which the bash-prompt script
> did not notice but the next "git cherry-pick" did by complaining
> "you are in the middle" or something like that. Perhaps we would
> want to have a similarly sounding option to help that case, too,
> not in this patch but as another patch on the same theme?
That would be nice. I don't put lots of git info on my shell prompt
though, so it does not help me. And it's probably difficult to report
the right thing too. Sometimes in the middle of rebase I would switch
to another branch, look or do stuff, then "git checkout -" back. I
don't think we can make the prompt script clever enough to see my
intention.
> - Would it have helped if bash-prompt were in use? I am not saying
> that this patch becomes unnecessary if you use it; I am trying to
> see if it helps its users by reminding them what state they are
> in.
Since I don't use it because I want to keep shell prompt short and
light, it doe not help me. But it looks like git-prompt.sh does print
rebase in progress and others (only checked the code, didn't test it).
--
Duy
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Jeff King @ 2016-10-27 10:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, git, Lars Schneider, Eric Wong,
Johannes Schindelin
In-Reply-To: <xmqqd1imbymi.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 26, 2016 at 02:15:33PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Wed, Oct 26, 2016 at 10:52:41AM -0700, Junio C Hamano wrote:
> >
> >> > I actually wonder if it is worth carrying around the O_NOATIME hack at
> >> > all.
> >>
> >> Yes, I share the thought. We no longer have too many loose objects
> >> to matter.
> >>
> >> I do not mind flipping the order, but I'd prefer to cook the result
> >> even longer. I am tempted to suggest we take two step route:
> >>
> >> - ship 2.11 with the "atime has been there and we won't regress it"
> >> shape, while cooking the "cloexec is semantically more
> >> important" version in 'next' during the feature freeze
> >>
> >> - immediately after 2.11 merge it to 'master' for 2.12 to make sure
> >> there is no fallout.
> >
> > That sounds reasonable, though I'd consider jumping straight to "NOATIME
> > is not worth it; drop it" as the patch for post-2.11.
>
> That endgame is fine by me too. Thanks for a sanity-check.
So here's that endgame patch. My main concern with it was that there
might be non-Linux systems that could be affected. But when I dug into
it, I found that this code was never activated anywhere besides Linux in
the first place. So I really doubt this will have any negative impact at
all. I certainly don't mind cooking it until post-2.11, though.
+cc Linus as the original author of 144bde78e9 in case there is
something subtle I'm missing, but this really just seems like it's
an outdated optimization.
-- >8 --
Subject: [PATCH] sha1_file: stop opening files with O_NOATIME
When we open object files, we try to do so with O_NOATIME.
This dates back to 144bde78e9 (Use O_NOATIME when opening
the sha1 files., 2005-04-23), which is an optimization to
avoid creating a bunch of dirty inodes when we're accessing
many objects. But a few things have changed since then:
1. In June 2005, git learned about packfiles, which means
we would do a lot fewer atime updates (rather than one
per object access, we'd generally get one per packfile).
2. In late 2006, Linux learned about "relatime", which is
generally the default on modern installs. So
performance around atimes updates is a non-issue there
these days.
All the world isn't Linux, but as it turns out, Linux
is the only platform to implement O_NOATIME in the
first place.
So it's very unlikely that this code is helping anybody
these days.
It's not a particularly large amount of code, but the
fallback-retry creates complexity. E.g., we do a similar
fallback for CLOEXEC; which one should take precedence, or
should we try all possible combinations? Dropping O_NOATIME
makes those questions go away.
Signed-off-by: Jeff King <peff@peff.net>
---
sha1_file.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 09045df1dc..6f02a57d8b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -27,14 +27,6 @@
#include "list.h"
#include "mergesort.h"
-#ifndef O_NOATIME
-#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
-#define O_NOATIME 01000000
-#else
-#define O_NOATIME 0
-#endif
-#endif
-
#define SZ_FMT PRIuMAX
static inline uintmax_t sz_fmt(size_t s) { return s; }
@@ -1561,7 +1553,7 @@ int check_sha1_signature(const unsigned char *sha1, void *map,
int git_open(const char *name)
{
- static int sha1_file_open_flag = O_NOATIME | O_CLOEXEC;
+ static int sha1_file_open_flag = O_CLOEXEC;
for (;;) {
int fd;
@@ -1577,11 +1569,6 @@ int git_open(const char *name)
continue;
}
- /* Might the failure be due to O_NOATIME? */
- if (errno != ENOENT && (sha1_file_open_flag & O_NOATIME)) {
- sha1_file_open_flag &= ~O_NOATIME;
- continue;
- }
return -1;
}
}
--
2.10.1.916.g0d2035c
^ permalink raw reply related
* Re: [PATCH v1 16/19] read-cache: unlink old sharedindex files
From: Christian Couder @ 2016-10-27 12:14 UTC (permalink / raw)
To: Duy Nguyen
Cc: Git Mailing List, Junio C Hamano,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <CACsJy8BPe085Qu4GpJ3MLTPeZM_7pbVrX31enQrGQn0oyjSrdg@mail.gmail.com>
On Thu, Oct 27, 2016 at 12:25 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, Oct 25, 2016 at 5:43 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>>> static int write_shared_index(struct index_state *istate,
>>> @@ -2211,8 +2269,11 @@ static int write_shared_index(struct index_state *istate,
>>> }
>>> ret = rename_tempfile(&temporary_sharedindex,
>>> git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
>>> - if (!ret)
>>> + if (!ret) {
>>> hashcpy(si->base_sha1, si->base->sha1);
>>> + clean_shared_index_files(sha1_to_hex(si->base->sha1));
>>
>> This operation is technically garbage collection and should belong to
>> "git gc --auto", which is already called automatically in a few
>> places. Is it not called often enough that we need to do the cleaning
>> up right after a new shared index is created?
>
> Christian, if we assume to go with Junio's suggestion to disable
> split-index on temporary files, the only files left we have to take
> care of are index and index.lock. I believe pruning here in this code
> will have an advantage over in "git gc --auto" because when this is
> executed, we know we're holding index.lock, so nobody else is updating
> the index, it's race-free. All we need to do is peek in $GIT_DIR/index
> to see what shared index file it requires and keep it alive too, the
> remaining of shared index files can be deleted safely. We don't even
> need to fall back to mtime.
>
> git-gc just can't match this because while it's running, somebody else
> may be updating $GIT_DIR/index. Handling races would be a lot harder.
Yeah, I agree that if we disable split-index on temporary files and on
commands like "git read-tree --index-output=<file>" then it is the
right thing to do it in write_shared_index(). (In fact when I wrote
the previous RFC series I didn't think about those special cases, and
that was the reason why I did it like this. So I just need to go back
to the implementation that was in the previous RFC series.)
I am just still wondering if disabling split-index on temporary files
could not have a bad performance impact for some use cases, but I
guess we could always come back to problem again if that happens.
Thanks,
Christian.
^ permalink raw reply
* Re: Expanding Includes in .gitignore
From: Alexei Lozovsky @ 2016-10-27 8:19 UTC (permalink / raw)
To: Aaron Pelly; +Cc: git
In-Reply-To: <80919456-7563-2c16-ba23-ce4fcc2777de@pelly.co>
> I'm thinking something like ". path/to/include/file" in an ignore file,
> and/or creating .gitignore.d and/or allowing $HOME/.config/git/ignore
> and $GIT_DIR/info/exclude to be directories. Or some sane and consistent
> mixture of these things.
I think the rc.d-like approach with directories is better as it
does not add new magical filenames (what if I absolutely do need
to name my directories ". path", with a space? :) keeping the
syntax of gitignores themselves simple, and allowing us to think
of files as still being separate entities. This can be useful
for the following case:
> In the case of a directory the plan would be to add links to files
> stored/sourced elsewhere. This does pose a precedence question which I
> haven't thought about yet, but probably makes it too hard for the
> limited value it brings.
As I understand, the precedence only matters for negative patterns
(the ones that start with an exclamation mark, !). For example,
suppose you have files 'foo1' and 'foo2'. This .gitignore
foo*
!foo1
will ignore foo2, but will show foo1. However, if the lines are
swapped:
!foo1
foo*
then both foo1 and foo2 will be ignored.
Now, if we consider the case of multiple .gitignore files, it
could be unexpected and possibly annoying for negative patterns
in one file to affect the patterns added by some other files.
I would find it more conceptually simple to apply individual
.gitignores one by one, as opposed to parsing them all and
creating one giant exclusion rule. (In technical terms, this
means keeping one struct exclude_list for each .gitignore,
not merging them all into one single list.)
In this case there should be no precendence problems as applied
gitignores only add new ignored files, without un-ignoring
anything previously ignored by other files.
However, if we allow textual inclusion, then it means that we
can put a gitignore into our gitignore so that we can unignore
while we ignore, which again brings us the question of whether
it is actually needed and expected.
> I would like to know the desirability/practicality/stupidity of such a
> feature as I believe it is within my skillset to implement it.
In my mind, this feature definitely has utility and it can be
implemented in backwards-compatible way, so why not.
However, I do not recall any precendent of git using rc.d-like
configs. And some can argue that your goal can be achieved by
generating the .gitignore by some external means and symlinking
the result into .git/info/exclude, so this is not Git's problem
and we should not be overcomplicating things with something as
simple as a list exclude patterns. This line of argument also
can be used to opposes any textual inclusion as well, because
it can be expanded into 'why don't we add a Turing-complete
programming language then to specify the patterns to ignore'.
^ permalink raw reply
* Re: [PATCH] Documentation/git-diff: document git diff with 3+ commits
From: Michael J Gruber @ 2016-10-27 9:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jacob Keller
In-Reply-To: <xmqqh97zask7.fsf@gitster.mtv.corp.google.com>
Junio C Hamano venit, vidit, dixit 26.10.2016 20:11:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> That one is difficult to discover but super useful, so document it:
>> Specifying 3 or more commits makes git diff switch to combined diff.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>
>> Notes:
>> Note that we have the following now:
>> ...
>> 'git diff A..B' equivalent to 'git diff A B'
>> in contrast to 'git log A..B' listing commits between M and B only
>> (without the commits between M and A unless they are "in" B).
>
> The standard answer is:
>
> Do not use two-dot form with 'git diff', if you find it
> confusing. Diff is about two endpoints, not about a range
> between two.
>
> The reason why we do not reject can be easily guessed by any
> intelligent person when some historical background is given, I
> think.
That is very well true. I'm more concerned with the presence, though,
that is: How easy to use is git now? Users choose git because they care
about the history of their project, not necessarily that of Git ;)
> - In the beginning A...B did not exist. A..B was the only "range"
> notation.
>
> - "git log A..B" was in wide use. Remember, "git log A...B" did
> not exist.
>
> - People started mistyping "git diff A..B", which looked as if the
> user typed "git diff ^A B" to the internal.
>
> - Git _could_ have rejected that as a bogus request to diff two
> points, ^A (what is that???) and B, but "What else could the user
> have meant with 'git diff A..B' other than 'git diff A B'?" was
> an argument to favor doing _something_ useful rather than
> erroring out. Remember, "A...B" did not exist when this
> happened.
It did not exist, but even at that point in time, "git log A..B" listed
only commits between the merge base and B, not those which are only in A
and not in B. Whereas "git diff A B" shows the differences between the
endpoints A and B.
>> @@ -12,6 +12,7 @@ SYNOPSIS
>> 'git diff' [options] [<commit>] [--] [<path>...]
>> 'git diff' [options] --cached [<commit>] [--] [<path>...]
>> 'git diff' [options] <commit> <commit> [--] [<path>...]
>> +'git diff' [options] <commit> <commit> <commit> [<commit>...]
>
> Made me wonder "is [<A>...] 0-or-more As or 1-or-more As?".
0-or-more, at least that's the way it is used in all lines here.
> Don't we allow pathspecs in this case?
Yes, the combinded diff mode kicks in only with no blobs (not: no
pathspec, which I had misread) and N>=3 commits. Maybe I should update
the code comments in builtin/diff.c to describe this, too.
Michael
^ permalink raw reply
* Re: [PATCH] Update git rebase documentation to clarify HEAD behavior
From: Junio C Hamano @ 2016-10-27 6:47 UTC (permalink / raw)
To: Cody Sehl; +Cc: git
In-Reply-To: <010201580457bdd2-99237b54-5e36-4430-bb8d-7e9088aed522-000000@eu-west-1.amazonses.com>
Cody Sehl <cody.sehl@gmail.com> writes:
> The first few paragraphs in the git-rebase.txt documentation lay out the steps git takes during a rebase:
> 1. everything from `<upstream>..HEAD` is saved to a temporary area
> 2. `HEAD` is set to `<upstream>`
> 3. the changes held in the temporary area are applied one by one in order on top of the new `HEAD`
>
> The second step was described using the phrase `The current branch is reset to <upstream>`, which is true (because `HEAD` == current branch), but not clear.
> ---
Please wrap your lines to reasonable lengths like 70 columns or so.
Please sign off your patch.
> Documentation/git-rebase.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index de222c8..c47ca11 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -33,7 +33,7 @@ of commits that would be shown by `git log <upstream>..HEAD`; or by
> description on `--fork-point` below); or by `git log HEAD`, if the
> `--root` option is specified.
>
> -The current branch is reset to <upstream>, or <newbase> if the
> +HEAD is reset to <upstream>, or <newbase> if the
> --onto option was supplied. This has the exact same effect as
> `git reset --hard <upstream>` (or <newbase>). ORIG_HEAD is set
> to point at the tip of the branch before the reset.
This is describing an ancient behaviour before 6fd2f5e60d ("rebase:
operate on a detached HEAD", 2007-11-08) in v1.5.4 timeframe. We
apparently failed to update the description.
This depends on the desired technical detail of the description, but
a correct rewrite would be "HEAD is detached at <upstream>, or
<newbase>, and ORIG_HEAD is set to point at the tip of the branch
before this happens". Detaching the HEAD at <upstream> no longer
has the same effect as "git reset --hard <upstream>" (or <newbase>),
so that sentence must go. It was the primary point of the ancient
change at 6fd2f5e60d after all.
And then there is a new step (to be numbered 4. in your description
in the proposed log message), which updates the tip of the branch to
the resulting HEAD (after replaying all these changes) and check the
branch out, which needs to be added. Perhaps after "one by one, in
order." Oh, the mention of "reapplied to the current branch" also
needs to be updated to "reapplied to the detached HEAD", too.
On the other hand, if we do not aim for that deep level of technical
correctness, but want to tell a white lie to make it easier to
understand at the conceptual level to new readers who haven't
grasped the detached HEAD, then the current description is fine. By
bringing up "HEAD", you seem to be aiming for techincal correctness
(which I tend to agree is a good direction to go in this part of the
documentation), so the existing text needs a bit more work than your
patch to be brought to the modern world.
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Junio C Hamano @ 2016-10-27 6:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Stefan Beller, Johannes.Schindelin, git, simon, peff
In-Reply-To: <xmqqh97y8g74.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>>> As many codepaths may not even need access to the attributes, I
>>> doubt that would be a very productive direction to go.
>>
>> So, what is productive then? Pessimizing one (not exactly minor) platform?
>
> Lazy on-demand initialization as needed, perhaps? The on-demand
> initialization mechanism may become no-op on some platforms that can
> do static initialization.
Ah, I think I misunderstood your "please rewrite". Did you mean to
add "void attr_start(void)" helper function to attr.c that does
series of pthread_mutex_init() calls as needed? That function can
be called from main() of platforms that cannot statically initialize
mutices, while on other platforms it can be a no-op as long as the
variables are statically initialized? If so, that would not pessimize
any platform, I would think.
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Sixt @ 2016-10-27 6:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Johannes.Schindelin, git, simon, peff
In-Reply-To: <xmqqh97y8g74.fsf@gitster.mtv.corp.google.com>
Am 27.10.2016 um 08:21 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>>> As many codepaths may not even need access to the attributes, I
>>> doubt that would be a very productive direction to go.
>>
>> So, what is productive then? Pessimizing one (not exactly minor) platform?
>
> Lazy on-demand initialization as needed, perhaps? The on-demand
> initialization mechanism may become no-op on some platforms that can
> do static initialization.
This is the pessimization that I am talking about. I would not mind at
all if it were only for the attribute subsystem, but the proposed patch
would pessimize *all* uses of pthread_mutex_lock.
-- Hannes
^ permalink raw reply
* [PATCH] Update git rebase documentation to clarify HEAD behavior
From: Cody Sehl @ 2016-10-27 4:13 UTC (permalink / raw)
To: git
The first few paragraphs in the git-rebase.txt documentation lay out the steps git takes during a rebase:
1. everything from `<upstream>..HEAD` is saved to a temporary area
2. `HEAD` is set to `<upstream>`
3. the changes held in the temporary area are applied one by one in order on top of the new `HEAD`
The second step was described using the phrase `The current branch is reset to <upstream>`, which is true (because `HEAD` == current branch), but not clear.
---
Documentation/git-rebase.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index de222c8..c47ca11 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -33,7 +33,7 @@ of commits that would be shown by `git log <upstream>..HEAD`; or by
description on `--fork-point` below); or by `git log HEAD`, if the
`--root` option is specified.
-The current branch is reset to <upstream>, or <newbase> if the
+HEAD is reset to <upstream>, or <newbase> if the
--onto option was supplied. This has the exact same effect as
`git reset --hard <upstream>` (or <newbase>). ORIG_HEAD is set
to point at the tip of the branch before the reset.
--
https://github.com/git/git/pull/301
^ permalink raw reply related
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Junio C Hamano @ 2016-10-27 6:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Stefan Beller, Johannes.Schindelin, git, simon, peff
In-Reply-To: <67e38b43-0264-12f2-cca8-4b718ed7dc9d@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
>> As many codepaths may not even need access to the attributes, I
>> doubt that would be a very productive direction to go.
>
> So, what is productive then? Pessimizing one (not exactly minor) platform?
Lazy on-demand initialization as needed, perhaps? The on-demand
initialization mechanism may become no-op on some platforms that can
do static initialization.
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Sixt @ 2016-10-27 6:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Johannes.Schindelin, git, simon, peff
In-Reply-To: <xmqqlgxa8h3a.fsf@gitster.mtv.corp.google.com>
Am 27.10.2016 um 08:02 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Am 26.10.2016 um 23:57 schrieb Stefan Beller:
>>> In Windows it is not possible to have a static initialized mutex as of
>>> now, but that seems to be painful for the upcoming refactoring of the
>>> attribute subsystem, as we have no good place to put the initialization
>>> of the attr global lock.
>>
>> Please rewrite the attribute system such that it can have a dynamic
>> initialization. If you find a global initialization in main() too
>> gross (I would agree) then setup_git_directory() might be the right
>> place.
>
> As many codepaths may not even need access to the attributes, I
> doubt that would be a very productive direction to go.
So, what is productive then? Pessimizing one (not exactly minor) platform?
-- Hannes
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Junio C Hamano @ 2016-10-27 6:02 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Stefan Beller, Johannes.Schindelin, git, simon, peff
In-Reply-To: <93be5d21-6cb6-ee2b-9f4f-c2fe7c690d6c@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 26.10.2016 um 23:57 schrieb Stefan Beller:
>> In Windows it is not possible to have a static initialized mutex as of
>> now, but that seems to be painful for the upcoming refactoring of the
>> attribute subsystem, as we have no good place to put the initialization
>> of the attr global lock.
>
> Please rewrite the attribute system such that it can have a dynamic
> initialization. If you find a global initialization in main() too
> gross (I would agree) then setup_git_directory() might be the right
> place.
As many codepaths may not even need access to the attributes, I
doubt that would be a very productive direction to go.
^ permalink raw reply
* Re: "git subtree --squash" interacts poorly with revert, merge, and rebase
From: Junio C Hamano @ 2016-10-27 6:00 UTC (permalink / raw)
To: Peter Williams; +Cc: Stefan Beller, Matt McCutchen, git
In-Reply-To: <xmqqtwby8hu8.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Peter Williams <pwil3058@bigpond.net.au> writes:
>
>> However, for git commands such as diff/status whose job is to display
>> information it would be nice if they had a --recursive option to
>> override the default submodule diff/status and show details of the
>> changes in the submodules. Sometimes you want to see the big picture
>> in detail.
>
> I won't disagree. My comment was only on this part from the original:
>
>>> - We have to make separate commits and manage corresponding topic
>>> branches for the superproject and subprojects.
>
> and on this point, we seem to be in agreement.
Oh, and as Stefan mentioned, a "git diff" that recurses into the
submodules to give you detailed big picture has been in 'next'
(perhaps aready in 'master' as of tonight, but I am not sure
offhand) to be tested, together with many other fixes and
enhancements that all are waiting to be included in future releases.
The more people try and give feedback to these branches early, the
more solid release with better support for more goodies you'd want
we will be able to give you. Early adopters are always appreciated
but especially in time like this before the feature freeze for the
upcoming release (see tinyurl.com/gitCal for the schedule), they are
of great help.
Start by cloning from any one of these places
git://git.kernel.org/pub/scm/git/git.git/
https://kernel.googlesource.com/pub/scm/git/git
git://repo.or.cz/alt-git.git/
https://github.com/git/git/
and then
$ git checkout -b next origin/next
: read INSTALL to figure out if any custom options are needed
: in the following 'make' invocations for your environment
$ make && make install
$ PATH=$HOME/bin:$PATH
to join the fun.
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Sixt @ 2016-10-27 5:49 UTC (permalink / raw)
To: Stefan Beller; +Cc: Johannes.Schindelin, git, simon, peff
In-Reply-To: <20161026215732.16411-1-sbeller@google.com>
Am 26.10.2016 um 23:57 schrieb Stefan Beller:
> In Windows it is not possible to have a static initialized mutex as of
> now, but that seems to be painful for the upcoming refactoring of the
> attribute subsystem, as we have no good place to put the initialization
> of the attr global lock.
Please rewrite the attribute system such that it can have a dynamic
initialization. If you find a global initialization in main() too gross
(I would agree) then setup_git_directory() might be the right place.
-- Hannes
^ permalink raw reply
* Re: "git subtree --squash" interacts poorly with revert, merge, and rebase
From: Junio C Hamano @ 2016-10-27 5:46 UTC (permalink / raw)
To: Peter Williams; +Cc: Stefan Beller, Matt McCutchen, git
In-Reply-To: <f07745f8-d0ff-c41f-fd44-0812757fbd43@bigpond.net.au>
Peter Williams <pwil3058@bigpond.net.au> writes:
> However, for git commands such as diff/status whose job is to display
> information it would be nice if they had a --recursive option to
> override the default submodule diff/status and show details of the
> changes in the submodules. Sometimes you want to see the big picture
> in detail.
I won't disagree. My comment was only on this part from the original:
>> - We have to make separate commits and manage corresponding topic
>> branches for the superproject and subprojects.
and on this point, we seem to be in agreement.
^ permalink raw reply
* Re: [PATCHv2 1/2] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-27 5:44 UTC (permalink / raw)
To: Stefan Beller
Cc: Duy Nguyen, git@vger.kernel.org, Johannes Schindelin,
Johannes Sixt, Jeff King, Brandon Williams, Simon Ruderich
In-Reply-To: <xmqq37jia0p8.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Stefan Beller <sbeller@google.com> writes:
>
>> Yeah, I can make it work without exposing struct git_attr.
>
> You completely misunderstood me. "struct git_attr" MUST be visible
> to the users so that they can ask for the name in git_check.attr[0].
>
> What would be nice to hide if you can is the function to intern a
> string into a pointer to struct git_attr, i.e. git_attr() function.
Even though that was not the primary point of my suggestion, I
actually think it is OK to make "struct git_attr" a structure that
is opaque to the users of the API if you wanted to. The attr_check
structure will have an array of pointers to "struct git_attr", and
the structure definition may be visible to the public attr.h header,
but the API users won't have to be able to dereference the pointer
"struct git_attr *". git_check.attr[0] would be a pointer to an
opaque structure from API users' point of view, that can be passed
to API function git_attr_name() to read its string.
What is nice to hide is the constructor of the structure. What it,
i.e. "struct git_attr *git_attr(const char *)", needs to do is to
(1) see if the attribute object with the same name already exists in
the table of "all known attributes in the universe", and if there
is, return that instance, (2) otherwise create a new attribute
object, register it to the table and return it. And it needs to do
it in a way that is thread-safe.
If we have to give access to it to the API users, then we'd need to
acquire and release the Big Attr Lock per each call.
The calls to git_attr() you need to make in your implementation will
be made from two codepaths:
* check_initl() acquires the Big Attr Lock, creates a check struct,
makes multiple calls to git_attr() to construct the necessary
git_attr instances to fill the array and then releases the lock,
so the git_attr() constructor does not have to be protected for
concurrent access.
* check_attr() acquires the Big Attr Lock, calls down to
prepare_attr_stack() as necessary to parse .gitattributes files
found in the directory hierarchy, which makes calls to git_attr()
to record the attributes found in the file. Then it does the
matching to fill results[] array and releases the lock. Again,
git_attr() constructors are called under the lock, so there is no
need for a separate lock.
If these are the only callpaths that reach git_attr() to construct
new attribute objects, it would mean that you can make this private
to attr subsystem and hide it from the users of the API.
Otherwise, you would need to rename the git_attr() constructor that
used internally under the Big Lock to
static struct git_attr *git_attr_locked(const char *);
that is defined inside attr.c, and then provide the external version
as a thin wrapper that calls it under the Big Lock, i.e.
struct git_attr *git_attr(const char *s)
{
struct git_attr *attr;
take_big_attr_lock();
attr = git_attr_locked(s);
release_big_attr_lock();
return attr;
}
That will have to make the big attr lock busier, and it would be
good if we can avoid it. That is where my "can we hide git_attr()
constructor?" comes from.
^ permalink raw reply
* Re: "git subtree --squash" interacts poorly with revert, merge, and rebase
From: Peter Williams @ 2016-10-27 4:23 UTC (permalink / raw)
To: Junio C Hamano, Stefan Beller; +Cc: Matt McCutchen, git
In-Reply-To: <xmqqk2cuach3.fsf@gitster.mtv.corp.google.com>
On 27/10/16 09:59, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> - We have to make separate commits and manage corresponding topic
>>> branches for the superproject and subprojects.
>>
>> Well yeah, that is how submodule work on a conceptual level.
>> While having multiple commits may seem like overhead, note
>> the subtle difference for these commits. One if deep down in the
>> stack patching one of the submodules, the other is a high level
>> commit advancing the submodule pointer.
>>
>> Note that the target audience of these two commit messages
>> might be vastly different, hence can be worded differently.
>> (The submodule describing how you fixed e.g. a memleak or race condition
>> and the superproject describes on why you needed to include that submodule,
>> e.g. because you switched your toplevel application to use threads.)
>
> Both good points.
>
> Another thing to keep in mind is that in a well-organized project,
> it is expected that you would have multiple commits in a submodule,
> solving one single issue that is needed by the superproject in a
> finer grained way, before the resulting submodule tip is recorded in
> the tree of the superproject in one commit. IOW, between the time
> the superproject's history moves by one commit, the submodule may
> have multiple commits in order for the submodule to become ready to
> be consumed by the superproject.
>
>
I'm a relatively new user of submodules and I quite like them (having
tried a few other strategies for sharing common code between multiple
projects and found them quite painful) and find them fairly easy to use.
I especially like the fact that the submodule command isn't very
complicated and that the best method for managing commits, etc in the
submodule is to cd into their root directory and then treat them like
any other git repository (greatly reducing the amount of new stuff that
you have to learn in order to use them). Also, from my experience so
far, I see three different types of work going on within my workspaces
that include submodules:
1. I'm working on changes to the submodule and using the superproject
that it's checked out in to test those changes in which case most of the
change is occurring in the submodule with changes in the superproject
usually being small one related to API changes in the submodule.
2. I'm working on changes in the superproject and the only changes that
get made in the submodules are to fix bugs uncovered by the work in the
superproject.
3. I'm modifying a superproject to accommodate changes to a submodule
that's changed as a result of having changes pulled from another repository.
In none of these cases do I feel the desire/need to commit the changes
to the superproject and submodule(s) with a single commit command which
more or less agrees with your points.
However, for git commands such as diff/status whose job is to display
information it would be nice if they had a --recursive option to
override the default submodule diff/status and show details of the
changes in the submodules. Sometimes you want to see the big picture in
detail.
^ 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