* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Junio C Hamano @ 2017-02-09 22:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1702092135050.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > (And that would have to be handled at a different point, as I had
>> > pointed out, so that suggested preparation would most likely not help
>> > at all.)
>>
>> I did not think "that would have to be handled at a different point"
>> is correct at all, if by "a point" you meant "a location in the
>> code" [*1*].
>
> Yes, I mean the location in the code.
>
> But since you keep insisting that you are right and I am wrong,...
There is no "insisting". Didn't we just see how wrong you were
about "different point"? An extended syntax of override would be
handled in the new helper to handle override, the same point in the
code as other overrides are handled.
> ... and even
> go so far as calling your patch reverting my refactoring a hot-fix, why
> don't you just go ahead and merge the result over my objections?
At this point, you are simply being silly.
Isn't "Putty is not a command but is also handled as if it is a
valid implementation of SSH" a bug? Isn't making the code not to be
confused like so a fix?
A different approach to fix the issue would be to declare that the
command names and overrides are not in separate namespaces.
If you prefer to go that route, the documentation can use an update
to make it not mention "putty" as a valid override (the users have
to spell "plink"), mention "plink.exe" is also accepted, etc. and
make it clear that the override environment and configuration
variables are the way to tell Git: "The ssh implementation I have
behaves the same way as this well-known implementation, so treat it
as such without actually looking at the path to the command in the
ssh.command string".
That may limit the freedom for future enhancement of overrides, but
is an acceptable short-cut. After all, the overrides are merely an
escape hatch and we expect them to be used only rarely.
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Stefan Beller @ 2017-02-09 21:47 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Christian Couder, Matthieu Moy, Jeff King, git, Pranit Bauva,
Lars Schneider, Carlos Martín Nieto, Thomas Gummerer,
Siddharth Kannan
In-Reply-To: <alpine.DEB.2.20.1702092236500.3496@virtualbox>
On Thu, Feb 9, 2017 at 1:38 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Christian,
>
> On Thu, 9 Feb 2017, Christian Couder wrote:
>
>> I just had a look and the microproject and idea pages for this year are
>> ok. They are not great sure, but not much worse than the previous
>> years. What should probably be done is to remove project ideas where is
>> no "possible mentor" listed.
>>
>> But I am reluctant to do that as I don't know what Dscho would be ok to
>> mentor.
>
> I am okay to mentor (except anything that touches submodules).
I am okay to mentor anything (preferrably submodules).
Thanks,
Stefan
>
> Ciao,
> Johannes
^ permalink raw reply
* Re: Bug with fixup and autosquash
From: Junio C Hamano @ 2017-02-09 21:21 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Ashutosh Bapat, git, Michael Haggerty, Michael J Gruber,
Matthieu Moy
In-Reply-To: <alpine.DEB.2.20.1702092142020.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> This prefix match also happens to introduce a serious performance problem,
> which is why I "fixed" this issue in the rebase--helper already (which is
> the case if you are using Git for Windows, whose master branch builds on
> Linux and MacOSX as well). I quoted "fix" because my motivation was to fix
> the performance problem, not the "incorrect match" problem.
In other words, regardless of your motivation, you "fix"ed both,
which is very nice ;-)
^ permalink raw reply
* Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Junio C Hamano @ 2017-02-09 21:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <alpine.DEB.2.20.1702092201080.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Wed, 8 Feb 2017, Junio C Hamano wrote:
>
>> How long has "rev-parse --git-path" been around? Had scripts in the
>> wild chance to learn to live with the "output is relative to the top of
>> the working tree" reality? I think the answers are "since 2.5" and
>> "yes".
>
> Correct. And the third question is: How did the scripts work around this
> feature?
>
> The answer is obvious: by switching back to `$(git rev-parse
> --git-dir)/filename`.
>
> This is literally on what I spent the better part of Wednesday.
>
> There is just no way you can "fix" this otherwise. As an occasional shell
> scripter, you may be tempted to use `$(git rev-parse --show-cdup)$(git
> rev-parse --git-path filename)`, but of course that breaks in worktrees
> and if you do not use worktrees you would not even know that your
> workaround introduced another bug.
In case it is not clear, I understand all of the above.
I was just worried about the people who do *NOT* use worktrees and
did the obvious "concatenate --cdup with --git-path" and thought
their script were working happily and well. By prepending the path
to the (real) location of the .git in the updated --git-path output
ourselves, they will complain, our update broke their script.
If we introduced the fix gently, by (1) warn when "--git-path" is
used but give the current output anyway, while adding the "fixed"
one as another new option, and then (2) remove "--git-path" after
several releases, they will not have to complain, even though they
will see warning until they stop using "--git-path".
There may be gentler alternative ways to transition, and I do not
worry about the specifics of them too much. I just think we cannot
do this in a single step without harming existing users.
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Johannes Schindelin @ 2017-02-09 21:38 UTC (permalink / raw)
To: Christian Couder
Cc: Matthieu Moy, Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Thomas Gummerer, Siddharth Kannan
In-Reply-To: <CAP8UFD3aygSf5U2abnpCfRzEf-hH5fSNuzFBBtgCjSQC3F8c5A@mail.gmail.com>
Hi Christian,
On Thu, 9 Feb 2017, Christian Couder wrote:
> I just had a look and the microproject and idea pages for this year are
> ok. They are not great sure, but not much worse than the previous
> years. What should probably be done is to remove project ideas where is
> no "possible mentor" listed.
>
> But I am reluctant to do that as I don't know what Dscho would be ok to
> mentor.
I am okay to mentor (except anything that touches submodules).
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Johannes Schindelin @ 2017-02-09 21:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqh944wmcs.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 8 Feb 2017, Junio C Hamano wrote:
> How long has "rev-parse --git-path" been around? Had scripts in the
> wild chance to learn to live with the "output is relative to the top of
> the working tree" reality? I think the answers are "since 2.5" and
> "yes".
Correct. And the third question is: How did the scripts work around this
feature?
The answer is obvious: by switching back to `$(git rev-parse
--git-dir)/filename`.
This is literally on what I spent the better part of Wednesday.
There is just no way you can "fix" this otherwise. As an occasional shell
scripter, you may be tempted to use `$(git rev-parse --show-cdup)$(git
rev-parse --git-path filename)`, but of course that breaks in worktrees
and if you do not use worktrees you would not even know that your
workaround introduced another bug.
The current handling of --git-path is just plain wrong. The fact that I am
the first person to report this here merely shows how much it is used in
the wild.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 4/5] files_ref_store::submodule: use NULL for the main repository
From: Stefan Beller @ 2017-02-09 21:36 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Haggerty, Nguyễn Thái Ngọc Duy,
Johannes Schindelin, David Turner, git@vger.kernel.org
In-Reply-To: <xmqq1sv7oyei.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 9, 2017 at 1:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> if (!submodule || !*submodule)
>> - return be->init(NULL);
>> + refs = be->init(NULL);
>> else
>> - return be->init(submodule);
>> + refs = be->init(submodule);
>
> Can't we also lose this "if !*submodule, turn it to NULL"?
That was my suggestion as well, but that did not look nicer per se.
That is because at this point of the series we still handle "" just fine.
>> refs->submodule = submodule ? xstrdup(submodule) : "";
>
> Also, can't we use xstrdup_or_null(submodule) with this step?
>
That is done in 4/5; here we must keep a "" around instead of NULL IIUC.
^ permalink raw reply
* Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Junio C Hamano @ 2017-02-09 21:33 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <CACsJy8CigsWjAq5cmJ=cbBmj=DdJtHdMKxmoifftuz9+9kqJiQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Relevant thread in the past [1] which fixes both --git-path and
> --git-common-dir. I think the author dropped it somehow (or forgot
> about it, I know I did). Sorry can't comment on that thread, or this
> patch, yet.
>
> [1] http://public-inbox.org/git/1464261556-89722-1-git-send-email-rappazzo@gmail.com/
Thanks for a pointer. I see Mike responded to this message (I
haven't had a chance to read and think about it yet), so I trust
that you three can figure out if these are the same issues and what
the final solution in the longer term should be.
I have no strong opinion for or against a "longer term" solution
that makes "rev-parse --git-path" behave differently from how it
behaves today, but I am not yet convinced that we can reach that
longer term goal without a transition period, as I suspect there are
existing users that know and came to expect how it behaves, based on
its today's behaviour. Other than that I do not have suggestion on
this topic at the moment.
Thanks.
^ permalink raw reply
* Re: [PATCH 0/5] Store submodules in a hash, not a linked list
From: Michael Haggerty @ 2017-02-09 21:23 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Stefan Beller, Johannes Schindelin, David Turner, git
In-Reply-To: <20170209195812.dbbmko4sas3wtdy5@sigill.intra.peff.net>
On 02/09/2017 08:58 PM, Jeff King wrote:
> On Thu, Feb 09, 2017 at 02:26:57PM +0100, Michael Haggerty wrote:
> [...]
>> A `files_ref_store` would be perfectly happy to represent, say, the
>> references *physically* stored in a linked worktree (e.g., `HEAD`,
>> `refs/bisect/*`, etc) even though that is not the complete collection
>> of refs that are *logically* visible from that worktree (which
>> includes references from the main repository, too). But the old code
>> was confusing the two things by storing "submodule" in every
>> `ref_store` instance.
>>
>> So push the submodule attribute down to the `files_ref_store` class
>> (but continue to let the `ref_store`s be looked up by submodule).
>
> I'm not sure I understand all of the ramifications here. It _sounds_ like
> pushing this down into the files-backend code would make it harder to
> have mixed ref-backends for different submodules. Or is this just
> pushing down an implementation detail of the files backend, and future
> code is free to have as many different ref_stores as it likes?
I don't understand how this would make it harder, aside from the fact
that a new backend class might also need a path member and have to
maintain its own copy rather than using one that the base class provides.
I consider it an implementation detail of the files backend that it
needs to keep a permanent record of its submodule path in
files_ref_store. Some hypothetical future backend might instead need,
say, an IP number and port to connect to a MySQL server. A hypothetical
pure packed-refs backend might just store the path of the packed-refs file.
A more likely imminent change is that backends need a path, but that the
path needn't correspond to the git_dir of the repository that contains
the corresponding objects, for example in the case of a linked worktree.
You might ask for the ref_store for a worktree-submodule, and end up
getting a compound object that delegates to one ref_store pointing at
its git_dir and one at its common_dir.
Michael
^ permalink raw reply
* Re: [PATCH 4/5] files_ref_store::submodule: use NULL for the main repository
From: Junio C Hamano @ 2017-02-09 21:19 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git
In-Reply-To: <111d663c0fd3e9669e7c28537f581833488ca4a6.1486629195.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> The old practice of storing the empty string in this member for the main
> repository was a holdover from before 00eebe3 (refs: create a base class
> "ref_store" for files_ref_store, 2016-09-04), when the submodule was
> stored in a flex array at the end of `struct files_ref_store`. Storing
> NULL for this case is more idiomatic and a tiny bit less code.
Yes. I noticed this bit in 3/5 and wondered about it, knowing this
step comes next:
> struct ref_store *ref_store_init(const char *submodule)
> {
> const char *be_name = "files";
> struct ref_storage_be *be = find_ref_storage_backend(be_name);
> + struct ref_store *refs;
>
> if (!be)
> die("BUG: reference backend %s is unknown", be_name);
>
> if (!submodule || !*submodule)
> - return be->init(NULL);
> + refs = be->init(NULL);
> else
> - return be->init(submodule);
> + refs = be->init(submodule);
Can't we also lose this "if !*submodule, turn it to NULL"?
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -980,7 +980,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
> struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
> struct ref_store *ref_store = (struct ref_store *)refs;
>
> - base_ref_store_init(ref_store, &refs_be_files, submodule);
> + base_ref_store_init(ref_store, &refs_be_files);
>
> refs->submodule = submodule ? xstrdup(submodule) : "";
Also, can't we use xstrdup_or_null(submodule) with this step?
^ permalink raw reply
* Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Johannes Schindelin @ 2017-02-09 21:11 UTC (permalink / raw)
To: Mike Rappazzo
Cc: Duy Nguyen, Git Mailing List, Junio C Hamano, SZEDER Gábor
In-Reply-To: <CANoM8SWv+KD92T263gS0Uxxi2ekNQdo0aNGx3AmweVasXk3GbA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]
Hi Mike,
On Thu, 9 Feb 2017, Mike Rappazzo wrote:
> On Thu, Feb 9, 2017 at 4:48 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>
> > Relevant thread in the past [1] which fixes both --git-path and
> > --git-common-dir. I think the author dropped it somehow (or forgot
> > about it, I know I did). Sorry can't comment on that thread, or this
> > patch, yet.
>
> I didn't exactly forget it (I have it sitting in a branch), I wasn't
> sure what else was needed (from a v5 I guess), so it has stagnated.
>
> There was also another patch [1] at the time done by SZEDER Gábor trying
> to speed up the completion scripts by adding `git rev-parse
> --absolute-git-dir` option to deal with this case as well.
>
> > [1] http://public-inbox.org/git/1464261556-89722-1-git-send-email-rappazzo@gmail.com/
>
> [1] http://public-inbox.org/git/20170203024829.8071-16-szeder.dev@gmail.com/
Ah, so I was not the only person reporting this bug, but I am seemingly
having as much luck getting a fix in.
I had a quick look at your v4:
http://public-inbox.org/git/1464261556-89722-3-git-send-email-rappazzo@gmail.com/
It seems you replaced the git_path() by a combination of git_dir() and
relative_path(), but that would break the use case where git_path()
handles certain arguments specially, e.g. "objects" which knows that the
.git/objects/ path can be overridden via the environment.
I tried very hard to keep that working in my patch, essentially by
emulating what git_path() does already when being called in a worktree's
subdirectory: make the path absolute.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 3/5] register_ref_store(): new function
From: Junio C Hamano @ 2017-02-09 21:16 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git
In-Reply-To: <ce326e17822184eff434b957d28f2233795162db.1486629195.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Move the responsibility for registering the ref_store for a submodule
> from base_ref_store_init() to a new function, register_ref_store(). Call
> the latter from ref_store_init().
>
> This means that base_ref_store_init() can lose its submodule argument,
> further weakening the 1:1 relationship between ref_stores and
> submodules.
OK. I think I am starting to get it. I've always found it
disturbing that for_each_*ref*() has a "submodule" variant.
Instead, the plan (outlined in the discussion from yesterday that
triggered your posting this series) is to give an API to request a
"ref-store", and then ask that object to iterate over refs, and
these steps get us closer to that goal, because the "to enumerate
these" won't have to know what set of refs a ref-store contains. If
you want to iterate over refs in a submodule, you grab a ref-store
for the submodule and ask it to iterate. Iterating over refs in
another worktree, you grab a different ref-store and ask it to
iterate using the same API.
Sounds like a good direction to go.
^ permalink raw reply
* Re: [PATCH 2/5] refs: push the submodule attribute down
From: Junio C Hamano @ 2017-02-09 21:07 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git
In-Reply-To: <8958e7e26cc8bf11a76672eb8ea98bc9ba662fdc.1486629195.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Push the submodule attribute down from ref_store to files_ref_store.
> This is another step towards loosening the 1:1 connection between
> ref_stores and submodules.
The update seems to retain the externally visible effects, but what
does this change mean for future backend writers whose code will sit
next to files_ref_store? They need to have "submodule" field in their
implementation of the backend and keep track of it?
Granted that the primary thing that looks at ->submodule field in
the code before this change all live in refs/files-backend.c, but I
am not sure if that is an artifact of us having only one backend at
this moment, or a sign that future backends would benefit from extra
freedom to choose how they exactly implement their submodule
support.
^ permalink raw reply
* Re: Bug with fixup and autosquash
From: Johannes Schindelin @ 2017-02-09 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ashutosh Bapat, git, Michael Haggerty, Michael J Gruber,
Matthieu Moy
In-Reply-To: <xmqqbmucuwb0.fsf@gitster.mtv.corp.google.com>
Hi Ashutosh and Junio,
On Wed, 8 Feb 2017, Junio C Hamano wrote:
> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
>
> > I have been using git rebase heavily these days and seem to have found
> > a bug.
> >
> > If there are two commit messages which have same prefix e.g.
> > yyyyyy This is prefix
> > xxxxxx This is prefix and message
> >
> > xxxxxx comitted before yyyyyy
> >
> > Now I commit a fixup to yyyyyy using git commit --fixup yyyyyy
> > zzzzzz fixup! This is prefix
> >
> > When I run git rebase -i --autosquash, the script it shows me looks like
> > pick xxxxxx This is prefix and message
> > fixup zzzzzz fixup! This is prefix
> > pick yyyyyy This is prefix
> >
> > I think the correct order is
> > pick xxxxxx This is prefix and message
> > pick yyyyyy This is prefix
> > fixup zzzzzz fixup! This is prefix
> >
> > Is that right?
>
> [...]
>
> Unfortunately, "rebase -i --autosquash" reorders the entries by
> identifying the commit by its title, and it goes with prefix match so
> that fix-up commits created without using --fixup option but manually
> records the title's prefix substring can also work.
This prefix match also happens to introduce a serious performance problem,
which is why I "fixed" this issue in the rebase--helper already (which is
the case if you are using Git for Windows, whose master branch builds on
Linux and MacOSX as well). I quoted "fix" because my motivation was to fix
the performance problem, not the "incorrect match" problem.
The rebase--helper code (specifically, the patch moving autosquash logic
into it: https://github.com/dscho/git/commit/7d0831637f) tries to match
exact onelines first, and falls back to prefix matching only after that.
Now that the sequencer-i patch series is in `master`, the next step is to
send the patch series introducing the rebase--helper. The patch series
including the fix discussed above relies on that one. Meaning that it will
take a while to get through the mill.
So please do not hold your breath until this feature/fix hits an official
Git version. If you need it[*1*] faster, feel free to build Git for
Windows' master and run with that for a while.
Ciao,
Johannes
Footnote: By "it" I mean "the feature/fix", not "an official Git version"
nor "your breath".
^ permalink raw reply
* gitk bug: file select in the tree mode
From: Anatoly Borodin @ 2017-02-09 20:44 UTC (permalink / raw)
To: git
Hi All!
There is a bug in gitk (e.g. 2.11.0):
1) Choose a repository with files in a subdir (git's repo for example).
2) `cd` to a subdir (e.g. `xdiff`).
3) Run `gitk`.
4) Select 'Tree' in the 'Patch / Tree' panel.
5) Select any file with 'Highlight this too' or 'Highlight this only'
(e.g `xmerge.c`).
6) See the short file name (`xmerge.c`) instead of the full path
(`xdiff/xmerge.c`) in the 'Find commit touching path:' edit field. No
commits touching the file can be found.
--
Mit freundlichen Grüßen,
Anatoly Borodin
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Johannes Schindelin @ 2017-02-09 20:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqfujns2li.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 9 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > (And that would have to be handled at a different point, as I had
> > pointed out, so that suggested preparation would most likely not help
> > at all.)
>
> I did not think "that would have to be handled at a different point"
> is correct at all, if by "a point" you meant "a location in the
> code" [*1*].
Yes, I mean the location in the code.
But since you keep insisting that you are right and I am wrong, and even
go so far as calling your patch reverting my refactoring a hot-fix, why
don't you just go ahead and merge the result over my objections?
Ciao,
Johannes
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Junio C Hamano @ 2017-02-09 19:24 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <2f67fc21-92f9-a03e-1b09-a237af6dbc46@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 02/06/2017 11:34 PM, Junio C Hamano wrote:
>> [...]
>> --------------------------------------------------
>> [Stalled]
>> [...]
>> * mh/ref-remove-empty-directory (2017-01-07) 23 commits
>> - files_transaction_commit(): clean up empty directories
>> - try_remove_empty_parents(): teach to remove parents of reflogs, too
>> - try_remove_empty_parents(): don't trash argument contents
>> - try_remove_empty_parents(): rename parameter "name" -> "refname"
>> - delete_ref_loose(): inline function
>> - delete_ref_loose(): derive loose reference path from lock
>> - log_ref_write_1(): inline function
>> - log_ref_setup(): manage the name of the reflog file internally
>> - log_ref_write_1(): don't depend on logfile argument
>> - log_ref_setup(): pass the open file descriptor back to the caller
>> - log_ref_setup(): improve robustness against races
>> - log_ref_setup(): separate code for create vs non-create
>> - log_ref_write(): inline function
>> - rename_tmp_log(): improve error reporting
>> - rename_tmp_log(): use raceproof_create_file()
>> - lock_ref_sha1_basic(): use raceproof_create_file()
>> - lock_ref_sha1_basic(): inline constant
>> - raceproof_create_file(): new function
>> - safe_create_leading_directories(): set errno on SCLD_EXISTS
>> - safe_create_leading_directories_const(): preserve errno
>> - t5505: use "for-each-ref" to test for the non-existence of references
>> - refname_is_safe(): correct docstring
>> - files_rename_ref(): tidy up whitespace
>>
>> Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
>> once there no longer is any other branch whose name begins with
>> "foo/", but we didn't do so so far. Now we do.
>>
>> Expecting a reroll.
>> cf. <5051c78e-51f9-becd-e1a6-9c0b781d6912@alum.mit.edu>
>
> I think you missed v4 of this patch series [1], which is the re-roll
> that you were waiting for. And I missed that you missed it...
>
> Michael
>
> [1] http://public-inbox.org/git/cover.1483719289.git.mhagger@alum.mit.edu/
Actually it was worse than that. What the above lists *is* v4; I
just failed to update "Expecting a reroll" note when I updated the
topic with your rerolled patches, and left it there trusting the
now-stale note of mine.
Sorry, and a HUGE thanks for noticing the mistake.
^ permalink raw reply
* Re: [PATCH 0/5] Store submodules in a hash, not a linked list
From: Jeff King @ 2017-02-09 19:58 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Stefan Beller, Johannes Schindelin, David Turner, git
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
On Thu, Feb 09, 2017 at 02:26:57PM +0100, Michael Haggerty wrote:
> I have mentioned this patch series on the mailing list a couple of
> time [1,2] but haven't submitted it before. I just rebased it to
> current master. It is available from my Git fork [3] as branch
> "submodule-hash".
>
> The first point of this patch series is to optimize submodule
> `ref_store` lookup by storing the `ref_store`s in a hashmap rather
> than a linked list. But a more interesting second point is to weaken
> the 1:1 relationship between submodules and `ref_stores` a little bit
> more.
Sounds good. I remember this had been discussed before due to
performance issues with resolve_gitlink_ref(), and we took a different
route (not populating non-submodule entries). I think it's nice to have
both optimizations, though, as they hit different use cases.
> A `files_ref_store` would be perfectly happy to represent, say, the
> references *physically* stored in a linked worktree (e.g., `HEAD`,
> `refs/bisect/*`, etc) even though that is not the complete collection
> of refs that are *logically* visible from that worktree (which
> includes references from the main repository, too). But the old code
> was confusing the two things by storing "submodule" in every
> `ref_store` instance.
>
> So push the submodule attribute down to the `files_ref_store` class
> (but continue to let the `ref_store`s be looked up by submodule).
I'm not sure I understand all of the ramifications here. It _sounds_ like
pushing this down into the files-backend code would make it harder to
have mixed ref-backends for different submodules. Or is this just
pushing down an implementation detail of the files backend, and future
code is free to have as many different ref_stores as it likes?
-Peff
^ permalink raw reply
* Re: [PATCH 0/5] Store submodules in a hash, not a linked list
From: Junio C Hamano @ 2017-02-09 19:48 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I have mentioned this patch series on the mailing list a couple of
> time [1,2] but haven't submitted it before. I just rebased it to
> current master. It is available from my Git fork [3] as branch
> "submodule-hash".
>
> The first point of this patch series is to optimize submodule
> `ref_store` lookup by storing the `ref_store`s in a hashmap rather
> than a linked list. But a more interesting second point is to weaken
> the 1:1 relationship between submodules and `ref_stores` a little bit
> more.
>
> A `files_ref_store` would be perfectly happy to represent, say, the
> references *physically* stored in a linked worktree (e.g., `HEAD`,
> `refs/bisect/*`, etc) even though that is not the complete collection
> of refs that are *logically* visible from that worktree (which
> includes references from the main repository, too). But the old code
> was confusing the two things by storing "submodule" in every
> `ref_store` instance.
>
> So push the submodule attribute down to the `files_ref_store` class
> (but continue to let the `ref_store`s be looked up by submodule).
>
> The last commit is relatively orthogonal to the others; it simplifies
> read_loose_refs() by calling resolve_ref_recursively() directly using
> the `ref_store` instance that it already has in hand, rather than
> indirectly via the public wrappers.
>
> Michael
>
> [1] http://public-inbox.org/git/341999fc-4496-b974-c117-c18a2fca1358@alum.mit.edu/
> [2] http://public-inbox.org/git/37fe2024-0378-a974-a28d-18a89d3e2312@alum.mit.edu/
> [3] https://github.com/mhagger/git
>
> Michael Haggerty (5):
> refs: store submodule ref stores in a hashmap
> refs: push the submodule attribute down
> register_ref_store(): new function
> files_ref_store::submodule: use NULL for the main repository
> read_loose_refs(): read refs using resolve_ref_recursively()
Thanks. Will queue on mh/submodule-hash forked from 'maint'.
^ permalink raw reply
* Re: Bug with fixup and autosquash
From: Junio C Hamano @ 2017-02-09 19:46 UTC (permalink / raw)
To: Michael J Gruber
Cc: Ashutosh Bapat, git, Johannes Schindelin, Michael Haggerty,
Matthieu Moy
In-Reply-To: <07eb2367-9509-afb0-2494-f02a44304bc4@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Junio C Hamano venit, vidit, dixit 08.02.2017 23:55:
>
>> Let's hear from some of those (Cc'ed) who were involved in an
>> earlier --autosquash thread.
>>
>> https://public-inbox.org/git/cover.1259934977.git.mhagger@alum.mit.edu/
>>
>>
>> [Footnote]
>>
>> *1* "rebase -i --autosquash" does understand "fixup! yyyyyy", so if
>> you are willing to accept the consequence of not being able to
>> rebase twice, you could instead do
>>
>> $ git commit -m "fixup! yyyyyy"
>>
>> I would think.
>
> Doesn't this indicate that rebase is fine as is?
Not really, unless you ignore "if you are willing to accept" part,
which actually is a big downside. And --fixup-fixed will make it
worse, unfortunately.
> - teach "git commit --fixup=<rev>" to check for duplicates (same prefix,
> maybe only in "recent" history) and make it issue a warning, say:
This is a very good idea worth pursuing, and (I didn't think things
through, though) we may even be able to bound "recent" without
heuristics---scanning between <rev> and the tip for duplicates might
be sufficient.
> Additionally, we could teach commit --fixup-fixed to commit -m "fixup!
> <sha1> <prefix>" so that we have both uniqueness and verbosity in the
> rebase-i-editor. This would allow "rebase -i" to fall back to the old
> mode when "<sha1>" is not in the range it operates on.
This is also a possibility, but it needs cooperation between both
"commit" and "rebase -i".
I personally do not think rewriting "fixup! yyyyyy" on the title
during rebase is worth doing, but that is not because I have a
concrete reason against it but it just sounds like too much magic to
my gut feeling. Perhaps it can be done reliably with minimal change
to the code. I dunno.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/5] refs: store submodule ref stores in a hashmap
From: Jeff King @ 2017-02-09 19:32 UTC (permalink / raw)
To: Michael Haggerty
Cc: Stefan Beller, Junio C Hamano,
Nguyễn Thái Ngọc Duy, Johannes Schindelin,
David Turner, git@vger.kernel.org
In-Reply-To: <4a21dba7-76ef-6aec-b326-c1046f3daad2@alum.mit.edu>
On Thu, Feb 09, 2017 at 06:40:29PM +0100, Michael Haggerty wrote:
> On 02/09/2017 05:58 PM, Stefan Beller wrote:
> >> @@ -1402,17 +1435,17 @@ struct ref_store *ref_store_init(const char *submodule)
> >>
> >> struct ref_store *lookup_ref_store(const char *submodule)
> >> {
> >
> >> + if (!submodule_ref_stores.tablesize)
> >> + hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 20);
> >
> >
> > So we can lookup a submodule even before we initialized the subsystem?
> > Does that actually happen? (It sounds like a bug to me.)
> >
> > Instead of initializing, you could return NULL directly here.
> [...]
> I suppose this code path could be changed to return NULL without
> initializing the hashmap, but the hashmap will be initialized a moment
> later by ref_store_init(), so I don't see much difference either way.
I faced a similar issue when adding the oidset API recently:
http://public-inbox.org/git/20170208205307.uvgj3saf3cyrvtan@sigill.intra.peff.net/
I came to the same conclusion that it doesn't matter much in practice. A
nice thing about "return NULL" is that you do not have to duplicate the
initialization which happens on the "write" side (so if you ever changed
submodule_hash_cmp, for example, it needs changed in both places).
I also used the "cmpfn" member to check whether the table had been
initialized, which matches existing uses elsewhere. But I do notice that
the documentation explicitly says "tablesize" is good for this purpose,
so it's probably a better choice.
-Peff
^ permalink raw reply
* Re: [PATCH v3 00/27] Revamp the attribute system; another round
From: Junio C Hamano @ 2017-02-09 19:31 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, pclouds
In-Reply-To: <20170209171804.GA61274@google.com>
Brandon Williams <bmwill@google.com> writes:
> At least v3 gets the attribute system to a state where further
> improvements should be relatively easy to make. And now as long as each
> thread has a unique attr_check structure, multiple callers can exist
> inside the attribute system at the same time. There is still more work
> to be done on it though. Still my biggest complaint is the "direction"
> aspect of the system. I would love to also eliminate that as global
> state at some point though I'm not sure how at this point.
We are in agreement 100% ;-) The "direction" was the last thorn I
was fighting with (without successfully coming up with a usable
solution) when I stopped working on my original series before Stefan
took it over.
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Junio C Hamano @ 2017-02-09 19:18 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <2f67fc21-92f9-a03e-1b09-a237af6dbc46@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 02/06/2017 11:34 PM, Junio C Hamano wrote:
>> [...]
>> --------------------------------------------------
>> [Stalled]
>> [...]
>> * mh/ref-remove-empty-directory (2017-01-07) 23 commits
>>
>> Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
>> once there no longer is any other branch whose name begins with
>> "foo/", but we didn't do so so far. Now we do.
>>
>> Expecting a reroll.
>> cf. <5051c78e-51f9-becd-e1a6-9c0b781d6912@alum.mit.edu>
>
> I think you missed v4 of this patch series [1], which is the re-roll
> that you were waiting for. And I missed that you missed it...
>
> Michael
>
> [1] http://public-inbox.org/git/cover.1483719289.git.mhagger@alum.mit.edu/
Great. Thanks.
^ permalink raw reply
* Re: Google Doc about the Contributors' Summit
From: Junio C Hamano @ 2017-02-09 19:09 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
In-Reply-To: <alpine.DEB.2.20.1702021007460.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I just started typing stuff up in a Google Doc, and made it editable to
> everyone, feel free to help and add things:
>
> https://docs.google.com/document/d/1KDoSn4btbK5VJCVld32he29U0pUeFGhpFxyx9ZJDDB0/edit?usp=sharing
Thanks for a write-up, Dscho.
List of bullet points just make non-attendees envious, imagining
that attendees had all the fun discussing what is behind these
bullet points, without being able to know what was discussed, if
they reached consensus and what the consensus was, but it is OK ;-)
A few items caught my attention, not because I found them more
important than other items on the page, but because they seem to
want my input without directly asking me ;-)
* If Junio would accept patches by replying to the sender with an
ID and/or a patch name. He picks this (branch) name when he gets
your patch.
I am not sure what exactly I am asked to "accept" here. I sometimes
forget to respond with "Thanks, will queue." to the patch message
and whoever said this wants me to consistently do so? I never say
"... will queue as js/difftool-builtin topic." mainly because I do
not know what the name of the topic branch should be at the point of
reading the patch. All I know is that decided that it may be worth
queuing, so it is a bit harder to arrange. But I can certainly try
if it makes the lives of contributors easier.
* Junio has a script for the todo branch which we can use to
generate the what's cooking branch. Perhaps we could continuously
generate this onto a webpage.
I have no objection, but I doubt that people find such an auto
generated version all that useful, as "git log --first-parent
origin/master..origin/pu" would tell exactly the same story. It
will lack the "topic summary" comment I have yet to write, if the
final 'pu' of the day that was pushed out was before my local update
to the draft of the next issue of "What's cooking" report [*1*], and
would not have any update on the next action (e.g. "Will merge to
...") relative to the latest issue of "What's cooking" report. IOW,
such an auto-generated report lacks all the added value over "git log"
output.
* Making the actual workflow more publicly known, e.g. document how
to generate the cooking email, to learn about the state of a
generation.
The exact mechanics of "how to generate" may be of less importance
than "how the information contained therein relates to their own
work" to the contributors, and I think MaintNotes that is sent out
at key milestones more or less covers the mechanics, but here is how
the sausage is made these days:
- I find a patch series on the list that is in good enough shape to
be in 'pu'. Perhaps it was already discussed and redone a few
times without hitting my tree. Or it may be the first attempt of
a new topic. I come up with a topic name, decide where the topic
should fork at [*2*], create the topic branch and queue the
patches. I may or may not test the topic in isolation at this
point.
- I may find an updated patch series of what has been queued. I go
to the existing topic branch and replace it (I try to keep it
forked at the same commit) after inspecting what got updated.
"git tbdiff" is a useful program to help this step. I may or may
not test the topic in isolation at this point.
- I repeat the above two for various topics during the day, and at
some point between 14:00-15:00 my time, stop taking new patches.
The day's integration cycle starts.
- If there are topics that have cooked long enough in 'next' and
planned to graduate to 'master', merge [*3*] them to 'master',
update the draft Release notes. Otherwise skip this step.
- If there are topics that have cooked long enough in 'pu' and
planned to graduate to 'next', merge them to 'next'. Otherwise
skip this step.
- If I updated 'master', merge its tip to 'next' (this should update
the draft release notes and nothing else, unless I took something
directly to 'master').
- Then I recreate 'jch', which is a point between 'master' and 'pu'.
This is the version I use for my own work, contains all topics in
'next' but a bit more. "git checkout -B jch master" begins it,
and then the topics that were in 'jch' are merged on top. The
latest version of updated topics that were already in 'jch' are
incorporated at this point, and "git diff jch@{20.hours}" would
show the effect of their interdiff (plus RelNotes update, if
'master' was updated during this integration cycle).
- I ran "git branch --no-merged pu --sort=-committerdate" to see the
topics that are new; the top of this list shows new topics and
updated topics (note that I just updated 'jch' but not 'pu' yet at
this point). Among them, I pick the ones that I am willing to be
a guinea-pig for before they hit 'next' and merge them to 'jch'.
Other topics that used to be in 'pu' may also be merged at this
point, when they turn out to be "interesting" enough.
- 'pu' is recreated on top of 'jch' [*4*]. "git checkout -B pu jch"
begins it, the topics that were already in 'pu' are merged on top
(some of them may have been merged to updated 'jch' in the above
step), together with the new topics we acquired.
- All integration branches are then tested (and installed for my own
use) at this point.
- While the tests are running, the draft "What's cooking" report is
updated. Here is where I write topic summary for new topics and
update topic summary for updated topics. The next action
(e.g. "Will merge to ...") may also be updated at this point.
- If the test notices problems, I go back to redo the day's merges
[*5*]. The first thing to do is to rewind the integration
branches back to the state before the day's integration started.
Trivial breakages are fixed-up in place with "rebase -i" or
queuing "SQUASH???" fix on top of the offending topic before
merging them back to 'master', 'next', 'jch' or 'pu'. A more
involved ones may force me to punt and eject the topic from the
integration branches. Or I may leave it in 'pu' so that others
who are more clueful in the area the offending topic touches can
notice the breakage and send in fix-up patches [*6*].
- If I have time, I redo 'jch' and 'pu' at this point once again,
because the merge commit for new topics we just tested do not have
their merge log message in sync with the topic summary in the
draft "What's cooking" report at this point [*6*]. I run the
tests for integration branches again, but the tree objects at the
tips of the integration branches are expected to be the same as
what was tested earlier, so it becomes a no-op.
- I push out the integration branches to the usual places [*7*].
- Then I go back to the very beginning. I start taking patches for
the next day's integration cycle.
Interested people are welcome to condense the above to update
MaintNotes in my 'todo' branch.
[Footnote]
*1* Merges that appear early in the "--first-parent master..pu"
carry the topic summary I have in the draft of "What's cooking"
report as its merge message.
*2* Sometimes a maint-worthy patch series is done in such a way that
it only applies to 'next'. I try to tweak such a patch series
to apply 'maint' and either use the result of my tweak, or ask
the submitter to base it on 'maint', depending on how confident
I am with the tweaked result.
*3* When I do this and any other "merge" of a topic branch into an
integration branch, the topic description in the draft "What's
cooking" report is used as the merge log message, if one already
exists.
*4* Readers would notice that 'jch' and 'pu' are not decendants of
'next'; this is very much deliberate, and it has helped me to
spot mismerges of tricky topics when they graduate to 'master' a
few times.
*5* IOW, I go back to the step "around 14:00-15:00".
*6* This is why the tip of 'pu' sometimes may not even build. IOW,
a broken 'pu' is an invitation for help, not complaint.
*7* Remember *3* above and also the fact that "What's cooking" draft
is updated only after everything is merged to 'pu'; there is a
small chicken and egg problem here and redoing 'jch/pu' will fix
it.
*8* Broken-out individual topics are sent to github.com/gitster/git
^ permalink raw reply
* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Siddharth Kannan @ 2017-02-09 18:21 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Git List, Pranit Bauva, Jeff King, pclouds,
brian m. carlson
In-Reply-To: <vpqwpczlfe5.fsf@anie.imag.fr>
On 9 February 2017 at 17:55, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>
>> [...]
>> As you might notice, in this list, most commands are not of the `rm` variety,
>> i.e. something that would delete stuff.
>
> OK, I think I'm convinced.
I am glad! :)
>
> Keep the arguments in mind when polishing the commit message.
I will definitely do that. I am working on a good commit message for
this by looking at some past changes to sha1_name.c which have
affected multiple commands.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
--
Best Regards,
- Siddharth.
^ 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