* Re: [RFD] should all merge bases be equal?
From: Michael Haggerty @ 2017-02-09 14:44 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqmvi2sj8f.fsf@gitster.mtv.corp.google.com>
On 10/18/2016 12:28 AM, Junio C Hamano wrote:
> [...]
> Being accustomed how fast my merges go, there is one merge that
> hiccups for me every once in a few days: merging back from 'master'
> to 'next'. [...]
>
> The reason why this merge is slow is because it typically have many
> merge bases. [...]
I overlooked this topic until just now :-(
I spent a lot of time looking at merge bases a couple of years ago [1],
originally motivated by the crappy diffs you get from
git diff master...branch
when the merge base is chosen poorly. In that email I include a lot of
data and suggest a different heuristic, namely to define the "best"
merge base $M to be the one that minimizes the number of non-merge
commits between $M and either of the branch tips (it doesn't matter
which one you choose); i.e., the one that minimizes
git rev-list --count --no-merges $M..$TIP
. The idea is that a merge base that is "closer" content-wise to the
tips will probably yield smaller diffs. I would expect that merge base
also to yield simpler merges, though I didn't test that. Relying on the
number of commits (rather than some other measure of how much the
content has been changed) is only a heuristic, but it seems to work well
and it can be implemented pretty cheaply.
We actually use an algorithm like the one I described at GitHub, though
it is implemented as a script rather than ever having been integrated
into git. And (for no particular reason) we include merge commits in the
commit count (it doesn't make much difference whether merges are
included or excluded).
Your idea to look at the first-parent histories of the two branch tips
is an interesting one and has the nice theoretical property that it is
based on the DAG topology rather than a count of commits. I'd be very
curious to see how the sizes of asymmetric diffs differ between your
method and mine, because for me smaller and more readable diffs are one
of the main benefits of better merge bases.
I would worry a bit that your proposed algorithm won't perform as well
for people who use less disciplined workflows than git.git or the Linux
kernel. For example, many people merge a lot more frequently and
chaotically, maybe even with the parents reversed from the canonical order.
Anyway, I mostly wanted to remind you of the earlier discussion of this
topic. There's a lot more information there.
Michael
[1] http://public-inbox.org/git/539A25BF.4060501@alum.mit.edu/
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Matthieu Moy @ 2017-02-09 12:28 UTC (permalink / raw)
To: Christian Couder
Cc: Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
Siddharth Kannan
In-Reply-To: <vpq7f4zmu3j.fsf@anie.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> I created a Git organization and invited you + Peff as admins. I'll
>> start cut-and-pasting to show my good faith ;-).
>
> I created this page based on last year's:
>
> https://git.github.io/SoC-2017-Org-Application/
>
> I filled-in the "org profile". "Org application" is still TODO.
PS: I didn't hear from the libgit2 folks, so I think it's reasonable to
consider that we don't apply for them.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Matthieu Moy @ 2017-02-09 12:25 UTC (permalink / raw)
To: Siddharth Kannan
Cc: Junio C Hamano, Git List, Pranit Bauva, Jeff King, pclouds,
brian m. carlson
In-Reply-To: <CAN-3QhoZN_wYvqbVdU_c1h4vUOaT5FOBFL7k+FemNpqkxjWDDA@mail.gmail.com>
Siddharth Kannan <kannan.siddharth12@gmail.com> writes:
> Hello Matthieu,
>
> On 8 February 2017 at 20:10, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>> In a previous discussion, I made an analogy with "cd -" (which is the
>> source of inspiration of this shorthand AFAIK): "-" did not magically
>> become "the last visited directory" for all Unix commands, just for
>> "cd". And in this case, I'm happy with it. For example, I never need
>> "mkdir -", and I'm happy I can't "rm -fr -" by mistake.
>>
>> So, it's debatable whether it's a good thing to have all commands
>> support "-". For example, forcing users to explicitly type "git branch
>> -d @{1}" and not providing them with a shortcut might be a good thing.
>
> builtin/branch.c does not call setup_revisions and remains unaffected
> by this patch :)
Right, I forgot this: in some place we need any revspec, but "branch -d"
needs a branch name explicitly.
> [...]
> 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.
Keep the arguments in mind when polishing the commit message.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Mike Rappazzo @ 2017-02-09 13:46 UTC (permalink / raw)
To: Duy Nguyen
Cc: Johannes Schindelin, Git Mailing List, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <CACsJy8CigsWjAq5cmJ=cbBmj=DdJtHdMKxmoifftuz9+9kqJiQ@mail.gmail.com>
On Thu, Feb 9, 2017 at 4:48 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Feb 8, 2017 at 7:17 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> In addition to making git_path() aware of certain file names that need
>> to be handled differently e.g. when running in worktrees, the commit
>> 557bd833bb (git_path(): be aware of file relocation in $GIT_DIR,
>> 2014-11-30) also snuck in a new option for `git rev-parse`:
>> `--git-path`.
>>
>> On the face of it, there is no obvious bug in that commit's diff: it
>> faithfully calls git_path() on the argument and prints it out, i.e. `git
>> rev-parse --git-path <filename>` has the same precise behavior as
>> calling `git_path("<filename>")` in C.
>>
>> The problem lies deeper, much deeper. In hindsight (which is always
>> unfair), implementing the .git/ directory discovery in
>> `setup_git_directory()` by changing the working directory may have
>> allowed us to avoid passing around a struct that contains information
>> about the current repository, but it bought us many, many problems.
>
> 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/
> --
> Duy
[1] http://public-inbox.org/git/20170203024829.8071-16-szeder.dev@gmail.com/
^ permalink raw reply
* Re: Cross-referencing the Git mailing list archive with their corresponding commits in `pu`
From: Lars Schneider @ 2017-02-09 14:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Josh Triplett, git
In-Reply-To: <xmqqwpd316f7.fsf@gitster.mtv.corp.google.com>
> On 06 Feb 2017, at 20:10, Junio C Hamano <gitster@pobox.com> wrote:
>
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> So I thought maybe the From: line (from the body, if available, otherwise
>> from the header) in conjunction with the "Date:" header would work.
>
> FYI, I use a post-applypatch hook to populate refs/notes/amlog notes
> tree when I queue a new patch; I am not sure how well the notes in
> it are preserved across rebases, but it could be a good starting
> point. The notes tree is mirrored at git://github.com/git/gitster
> repository.
>
> E.g.
>
> $ git show --notes=amlog --stat
That's super useful! Thanks for the pointer!
Wouldn't it make sense to push these notes to github.com/git/git ?
- Lars
^ permalink raw reply
* Re: Request re git status
From: Cornelius Weig @ 2017-02-07 1:15 UTC (permalink / raw)
To: Phil Hord, Ron Pero, Git
In-Reply-To: <CABURp0qbKMfngfsC5pQeO+qyRPxa21vi090hMWDtLd+BBH_3Jg@mail.gmail.com>
On 02/07/2017 01:45 AM, Phil Hord wrote:
> On Mon, Feb 6, 2017 at 3:36 PM Ron Pero <rpero@magnadev.com> wrote:
> Do you mean you almost pushed some changed history with "--force"
> which would have lost others' changes? Use of this option is
> discouraged on shared branches for this very reason. But if you do
> use it, the remote will tell you the hash of the old branch so you can
> undo the damage.
>
> But if you did not use --force, then you were not in danger of being
> bit. Git would have prevented the push in that case.
I totally agree with Phil. Besides, git-status should be fast. And
talking to a remote can be painfully slow. As Phil pointed out, even the
slow answer when talking to the remote can give you better guarantees
than the quick (local) answer. Therefore, I prefer the quick answer.
Since you pointed out the use of --force, I want to add the
--force-with-lease option of git-push. The idea is basically, that we
may force-push, if the remote end does indeed have the state we think it
has. This avoids those situations where somebody pushed to the remote
while you were typing 'git push --force' (which would then loose the
other contributor's work). For details have a look at 'git help push'.
>> Or change the message to tell what it really
>> did, e.g. "Your branch was up-to-date with 'origin/master' when last
>> checked at {timestamp}"? Or even just say, "Do a fetch to find out
>> whether your branch is up to date"?
>
> These are reasonable suggestions, but i don't think the extra wording
> adds anything for most users. Adding a timestamp seems generally
> useful, but it could get us into other trouble since we have to depend
> on outside sources for timestamps.
The date of the last update is actually stored in the reflogs for the
remote branches. That timestamp is "internal" and could be trusted.
However, I don't quite believe that it would avoid accidents. For that
you would have to remember the time when some other (!) contributor has
pushed to the remote AND recognize that its timestamp is after the date
printed.
I prefer being warned by git when I try to do something stupid.
^ permalink raw reply
* Re: [PATCH 6/7] completion: teach remote subcommands option completion
From: Cornelius Weig @ 2017-02-02 10:29 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: bitte.keine.werbung.einwerfen, thomas.braun, john, git
In-Reply-To: <20170202013759.25789-1-szeder.dev@gmail.com>
On 02/02/2017 02:37 AM, SZEDER Gábor wrote:
> The 'set-head' subcommand has '--auto' and '--delete' options, and
> 'set-branches' has '--add'.
Oops. Thanks for spotting this..
>> __git_complete_remote_or_refspec
>> ;;
>> - update)
>> + update,*)
>
> The 'update' subcommand has a '--prune' option.
>
..and that.
^ permalink raw reply
* Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree
From: Michael Haggerty @ 2017-02-09 14:03 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano, Stefan Beller
In-Reply-To: <CACsJy8Diy92CNbJ1OBn893VFFrSsxBFWSyQHjt_Dzq9x7jfibQ@mail.gmail.com>
On 02/09/2017 12:59 PM, Duy Nguyen wrote:
> On Thu, Feb 9, 2017 at 1:07 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> It is unquestionably a good goal to avoid parsing references outside of
>> `refs/files-backend.c`. But I'm not a fan of this approach.
>
> Yes. But in this context it was more of a guinea pig. I wanted
> something simple enough to code up show we can see what the approach
> looked like. Good thing I did it.
>
>>
>> There are two meanings of the concept of a "ref store", and I think this
>> change muddles them:
>>
>> 1. The references that happen to be *physically* stored in a particular
>> location, for example the `refs/bisect/*` references in a worktree.
>>
>> 2. The references that *logically* should be considered part of a
>> particular repository. This might require stitching together
>> references from multiple sources, for example `HEAD` and
>> `refs/bisect` from a worktree's own directory with other
>> references from the main repository.
>>
>> Either of these concepts can be implemented via the `ref_store` abstraction.
>>
>> The `ref_store` for a submodule should represent the references
>> logically visible from the submodule. The main program shouldn't care
>> whether the references are stored in a single physical location or
>> spread across multiple locations (for example, if the submodule were
>> itself a linked worktree).
>>
>> The `ref_store` that you want here for a worktree is not the worktree's
>> *logical* `ref_store`. You want the worktree's *physical* `ref_store`.
>
> Yep.
>
>> Mixing logical and physical reference stores together is a bad idea
>> (even if we were willing to ignore the fact that worktrees are not
>> submodules in the accepted sense of the word).
>>
>> ...
>>
>> I think the best solution would be to expose the concept of `ref_store`
>> in the public refs API. Then users of submodules would essentially do
>>
>> struct ref_store *refs = get_submodule_refs(submodule_path);
>> ... resolve_ref_recursively(refs, refname, 0, sha1, &flags) ...
>> ... for_each_ref(refs, fn, cb_data) ...
>>
>> whereas for a worktree you'd have to look up the `ref_store` instance
>> somewhere else (or maybe keep it as part of some worktree structure, if
>> there is one) but you would use it via the same API.
>
> Oh I was going to reply to Stefan about his comment to my (**)
> footnote. Something along the this line
>
> "Ideally we would introduce a new set of api, maybe with refs_ prefix,
> that takes a refs_store. Then submodule people can get a ref store
> somewhere and pass to it. Worktree people get maybe some other refs
> store for it. The "old" api like for_each_ref() is a thin wrapper
> around it, just like read_cache() vs read_index(&the_index). If the
> *_submodule does not see much use, we might as well kill it and use
> the generic refs_*".
>
> If I didn't misunderstood anything else, then I think we're on the same page.
>
> Now I need to see if I can get there in a reasonable time frame (so I
> can fix my "gc in worktree" problem properly) or I would need
> something temporary but not so hacky. I'll try to make this new api
> and see how it works out. If you think I should not do it right away,
> for whatever reason, stop me now.
Sounds good. A couple of hints and points to remember:
* I think `struct ref_store *` should remain opaque outside of the refs
code.
* The virtual function interface embodied in `struct ref_storage_be`
isn't meant to be an external interface (i.e., don't just expose it
and have external callers use it directly).
* One important distinction between the main reference store and
submodule reference stores is that we know the object store for
the former but not the latter. That implies that some operations
are, or should be, impossible for submodules (e.g., anything that
involves looking up objects, including peeling refs). However, we
*do* know the object store for linked worktrees. So they don't have
to be as dumbed-down as submodule ref stores. (This might be
irrelevant if you don't need any additional features for your
current purposes.)
Also, I just sent my submodule-hash patch series to the mailing list in
case you want to build on that.
Michael
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Duy Nguyen @ 2017-02-09 13:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <xmqqwpd0v15j.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 9, 2017 at 4:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
> With or without the patch in this thread, parse_pathspec() behaves
> the same way for either CWD or FULL if you feed a non-empty
> pathspec with at least one positive element. IOW, if a caller feeds
> a non-empty pathspec and there is no "negative" element involved, it
> does not matter if we feed CWD or FULL.
Yes. Now that you put it that way, it may make more sense to rename
the options to PATHSPEC_DEFAULT_* instead of PATHSPEC_PREFER_*.
> - builtin/checkout.c::cmd_checkout(), when argc==0, does not call
> parse_pathspec(). This codepath will get affected by Linus's
> change ("cd t && git checkout :\!perf" would try to check out
> everything except t/perf, but what is a reasonable definition of
> "everything" in the context of this command). We need to
> decide, and I am leaning towards preferring CWD for this case.
So far I have seen arguments with single negative pathspec as
examples. What about "cd t; git checkout :^perf :^../Documentation"?
CWD is probably not the best base to be excluded from. Maybe the
common prefix of all negative pathspecs? But things start to get a bit
unintuitive on that road. Or do will still bail out on multiple
negative pathspecs with no positive one?
Also, even with single negative pathspec, what about "cd t; git
checkout :^../ewah"?
> So, I am tempted to suggest us doing the following:
>
> * Leave a NEEDSWORK comment to archive.c::path_exists() that is
> used for checking the validation of pathspec elements. To fix it
> properly, we need to be able to skip a negative pathspec to be
> passed to this function by the caller, and to do so, we need to
> expose a helper from the pathspec API that gets a single string
> and returns what magic it has, but that is of lower priority.
Side note. There's something else I'm not happy with pathspec handling
in "git archive". Try "cd t; git archive HEAD -- ../Documentation" and
you'll get a very unfriendly error message. But well, no time for it.
> * Retire the PATHSPEC_PREFER_CWD bit and replace its use with the
> lack of the PATHSPEC_PREFER_FULL bit.
>
> * Keep most of the above callsites that currently do not pass
> CWD/FULL as they are, except the ones that should take FULL (see
> above).
>
> Comments?
This comes from my experience reading files-backend.c. I didn't
realize '0' in files_downcast(ref_store, 0, "pack_refs"); meant
'submodule not allowed' because apparently I was too lazy to read
prototype. But if was files_downcast(ref_store, NO_SUBMODULE,
"pack_refs"), it would save people's time.
With that in mind, should we keep _CWD the name, so people can quickly
see and guess that it prefers _CWD than _FULL? _CWD can be defined as
zero. It there's mostly as a friendly reminder.
Other than that, yes if killing one flag helps maintenance, I'm for it.
PS. You may notice I favored ^ over ! already. ! was a pain point for
me too but I was too lazy to bring it up and fight for it. Thanks
Linus.
--
Duy
^ permalink raw reply
* [PATCH 2/5] refs: push the submodule attribute down
From: Michael Haggerty @ 2017-02-09 13:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git, Michael Haggerty
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
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.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 9 --------
refs/files-backend.c | 61 ++++++++++++++++++++++++++++++++++++----------------
refs/refs-internal.h | 13 -----------
3 files changed, 43 insertions(+), 40 deletions(-)
diff --git a/refs.c b/refs.c
index 50d192c..723b4be 100644
--- a/refs.c
+++ b/refs.c
@@ -1404,11 +1404,8 @@ void base_ref_store_init(struct ref_store *refs,
if (main_ref_store)
die("BUG: main_ref_store initialized twice");
- refs->submodule = "";
main_ref_store = refs;
} else {
- refs->submodule = xstrdup(submodule);
-
if (!submodule_ref_stores.tablesize)
hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 20);
@@ -1473,12 +1470,6 @@ struct ref_store *get_ref_store(const char *submodule)
return refs;
}
-void assert_main_repository(struct ref_store *refs, const char *caller)
-{
- if (*refs->submodule)
- die("BUG: %s called for a submodule", caller);
-}
-
/* backend functions */
int pack_refs(unsigned int flags)
{
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c041d4b..6ed7e13 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -912,6 +912,14 @@ struct packed_ref_cache {
*/
struct files_ref_store {
struct ref_store base;
+
+ /*
+ * The name of the submodule represented by this object, or
+ * the empty string if it represents the main repository's
+ * reference store:
+ */
+ const char *submodule;
+
struct ref_entry *loose;
struct packed_ref_cache *packed;
};
@@ -974,10 +982,23 @@ static struct ref_store *files_ref_store_create(const char *submodule)
base_ref_store_init(ref_store, &refs_be_files, submodule);
+ refs->submodule = submodule ? xstrdup(submodule) : "";
+
return ref_store;
}
/*
+ * Die if refs is for a submodule (i.e., not for the main repository).
+ * caller is used in any necessary error messages.
+ */
+static void files_assert_main_repository(struct files_ref_store *refs,
+ const char *caller)
+{
+ if (*refs->submodule)
+ die("BUG: %s called for a submodule", caller);
+}
+
+/*
* Downcast ref_store to files_ref_store. Die if ref_store is not a
* files_ref_store. If submodule_allowed is not true, then also die if
* files_ref_store is for a submodule (i.e., not for the main
@@ -987,14 +1008,18 @@ static struct files_ref_store *files_downcast(
struct ref_store *ref_store, int submodule_allowed,
const char *caller)
{
+ struct files_ref_store *refs;
+
if (ref_store->be != &refs_be_files)
die("BUG: ref_store is type \"%s\" not \"files\" in %s",
ref_store->be->name, caller);
+ refs = (struct files_ref_store *)ref_store;
+
if (!submodule_allowed)
- assert_main_repository(ref_store, caller);
+ files_assert_main_repository(refs, caller);
- return (struct files_ref_store *)ref_store;
+ return refs;
}
/* The length of a peeled reference line in packed-refs, including EOL: */
@@ -1133,8 +1158,8 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
{
char *packed_refs_file;
- if (*refs->base.submodule)
- packed_refs_file = git_pathdup_submodule(refs->base.submodule,
+ if (*refs->submodule)
+ packed_refs_file = git_pathdup_submodule(refs->submodule,
"packed-refs");
else
packed_refs_file = git_pathdup("packed-refs");
@@ -1203,8 +1228,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
size_t path_baselen;
int err = 0;
- if (*refs->base.submodule)
- err = strbuf_git_path_submodule(&path, refs->base.submodule, "%s", dirname);
+ if (*refs->submodule)
+ err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
else
strbuf_git_path(&path, "%s", dirname);
path_baselen = path.len;
@@ -1244,10 +1269,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
} else {
int read_ok;
- if (*refs->base.submodule) {
+ if (*refs->submodule) {
hashclr(sha1);
flag = 0;
- read_ok = !resolve_gitlink_ref(refs->base.submodule,
+ read_ok = !resolve_gitlink_ref(refs->submodule,
refname.buf, sha1);
} else {
read_ok = !read_ref_full(refname.buf,
@@ -1358,8 +1383,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(&sb_path);
- if (*refs->base.submodule)
- strbuf_git_path_submodule(&sb_path, refs->base.submodule, "%s", refname);
+ if (*refs->submodule)
+ strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
else
strbuf_git_path(&sb_path, "%s", refname);
@@ -1540,7 +1565,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
int ret = TRANSACTION_GENERIC_ERROR;
assert(err);
- assert_main_repository(&refs->base, "lock_raw_ref");
+ files_assert_main_repository(refs, "lock_raw_ref");
*type = 0;
@@ -2006,7 +2031,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
int attempts_remaining = 3;
int resolved;
- assert_main_repository(&refs->base, "lock_ref_sha1_basic");
+ files_assert_main_repository(refs, "lock_ref_sha1_basic");
assert(err);
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -2152,7 +2177,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
static int timeout_value = 1000;
struct packed_ref_cache *packed_ref_cache;
- assert_main_repository(&refs->base, "lock_packed_refs");
+ files_assert_main_repository(refs, "lock_packed_refs");
if (!timeout_configured) {
git_config_get_int("core.packedrefstimeout", &timeout_value);
@@ -2190,7 +2215,7 @@ static int commit_packed_refs(struct files_ref_store *refs)
int save_errno = 0;
FILE *out;
- assert_main_repository(&refs->base, "commit_packed_refs");
+ files_assert_main_repository(refs, "commit_packed_refs");
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2223,7 +2248,7 @@ static void rollback_packed_refs(struct files_ref_store *refs)
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(refs);
- assert_main_repository(&refs->base, "rollback_packed_refs");
+ files_assert_main_repository(refs, "rollback_packed_refs");
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2397,7 +2422,7 @@ static int repack_without_refs(struct files_ref_store *refs,
struct string_list_item *refname;
int ret, needs_repacking = 0, removed = 0;
- assert_main_repository(&refs->base, "repack_without_refs");
+ files_assert_main_repository(refs, "repack_without_refs");
assert(err);
/* Look for a packed ref */
@@ -2930,7 +2955,7 @@ static int commit_ref_update(struct files_ref_store *refs,
const unsigned char *sha1, const char *logmsg,
struct strbuf *err)
{
- assert_main_repository(&refs->base, "commit_ref_update");
+ files_assert_main_repository(refs, "commit_ref_update");
clear_loose_ref_cache(refs);
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 0, err)) {
@@ -3560,7 +3585,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
int ret;
struct ref_lock *lock;
- assert_main_repository(&refs->base, "lock_ref_for_update");
+ files_assert_main_repository(refs, "lock_ref_for_update");
if ((update->flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
update->flags |= REF_DELETING;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 4ed5f89..97f275b 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -627,13 +627,6 @@ extern struct ref_storage_be refs_be_files;
struct ref_store {
/* The backend describing this ref_store's storage scheme: */
const struct ref_storage_be *be;
-
- /*
- * The name of the submodule represented by this object, or
- * the empty string if it represents the main repository's
- * reference store:
- */
- const char *submodule;
};
/*
@@ -675,10 +668,4 @@ struct ref_store *lookup_ref_store(const char *submodule);
*/
struct ref_store *get_ref_store(const char *submodule);
-/*
- * Die if refs is for a submodule (i.e., not for the main repository).
- * caller is used in any necessary error messages.
- */
-void assert_main_repository(struct ref_store *refs, const char *caller);
-
#endif /* REFS_REFS_INTERNAL_H */
--
2.9.3
^ permalink raw reply related
* [PATCH 5/5] read_loose_refs(): read refs using resolve_ref_recursively()
From: Michael Haggerty @ 2017-02-09 13:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git, Michael Haggerty
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
There is no need to call read_ref_full() or resolve_gitlink_ref() from
read_loose_refs(), because we already have a ref_store object in hand.
So we can call resolve_ref_recursively() ourselves. Happily, this
unifies the code for the submodule vs. non-submodule cases.
This requires resolve_ref_recursively() to be exposed to the refs
subsystem, though not to non-refs code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 8 ++++----
refs/files-backend.c | 18 ++++--------------
refs/refs-internal.h | 5 +++++
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index 6012f67..210a453 100644
--- a/refs.c
+++ b/refs.c
@@ -1235,10 +1235,10 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
}
/* This function needs to return a meaningful errno on failure */
-static const char *resolve_ref_recursively(struct ref_store *refs,
- const char *refname,
- int resolve_flags,
- unsigned char *sha1, int *flags)
+const char *resolve_ref_recursively(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ unsigned char *sha1, int *flags)
{
static struct strbuf sb_refname = STRBUF_INIT;
int unused_flags;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 069a8ee..f087a99 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1267,20 +1267,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
create_dir_entry(refs, refname.buf,
refname.len, 1));
} else {
- int read_ok;
-
- if (refs->submodule) {
- hashclr(sha1);
- flag = 0;
- read_ok = !resolve_gitlink_ref(refs->submodule,
- refname.buf, sha1);
- } else {
- read_ok = !read_ref_full(refname.buf,
- RESOLVE_REF_READING,
- sha1, &flag);
- }
-
- if (!read_ok) {
+ if (!resolve_ref_recursively(&refs->base,
+ refname.buf,
+ RESOLVE_REF_READING,
+ sha1, &flag)) {
hashclr(sha1);
flag |= REF_ISBROKEN;
} else if (is_null_sha1(sha1)) {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 73281f5..af9e5fe 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -673,4 +673,9 @@ struct ref_store *lookup_ref_store(const char *submodule);
*/
struct ref_store *get_ref_store(const char *submodule);
+const char *resolve_ref_recursively(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ unsigned char *sha1, int *flags);
+
#endif /* REFS_REFS_INTERNAL_H */
--
2.9.3
^ permalink raw reply related
* [PATCH 4/5] files_ref_store::submodule: use NULL for the main repository
From: Michael Haggerty @ 2017-02-09 13:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git, Michael Haggerty
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
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.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs/files-backend.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 794b88c..069a8ee 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -915,8 +915,8 @@ struct files_ref_store {
/*
* The name of the submodule represented by this object, or
- * the empty string if it represents the main repository's
- * reference store:
+ * NULL if it represents the main repository's reference
+ * store:
*/
const char *submodule;
@@ -982,7 +982,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
base_ref_store_init(ref_store, &refs_be_files);
- refs->submodule = submodule ? xstrdup(submodule) : "";
+ refs->submodule = xstrdup_or_null(submodule);
return ref_store;
}
@@ -994,7 +994,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
static void files_assert_main_repository(struct files_ref_store *refs,
const char *caller)
{
- if (*refs->submodule)
+ if (refs->submodule)
die("BUG: %s called for a submodule", caller);
}
@@ -1158,7 +1158,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
{
char *packed_refs_file;
- if (*refs->submodule)
+ if (refs->submodule)
packed_refs_file = git_pathdup_submodule(refs->submodule,
"packed-refs");
else
@@ -1228,7 +1228,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
size_t path_baselen;
int err = 0;
- if (*refs->submodule)
+ if (refs->submodule)
err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
else
strbuf_git_path(&path, "%s", dirname);
@@ -1269,7 +1269,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
} else {
int read_ok;
- if (*refs->submodule) {
+ if (refs->submodule) {
hashclr(sha1);
flag = 0;
read_ok = !resolve_gitlink_ref(refs->submodule,
@@ -1383,7 +1383,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(&sb_path);
- if (*refs->submodule)
+ if (refs->submodule)
strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
else
strbuf_git_path(&sb_path, "%s", refname);
--
2.9.3
^ permalink raw reply related
* [PATCH 1/5] refs: store submodule ref stores in a hashmap
From: Michael Haggerty @ 2017-02-09 13:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git, Michael Haggerty
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
Aside from scaling better, this means that the submodule name needn't be
stored in the ref_store instance anymore (which will be changed in a
moment). This, in turn, will help loosen the strict 1:1 relationship
between ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 61 ++++++++++++++++++++++++++++++++++++++++------------
refs/refs-internal.h | 6 ------
2 files changed, 47 insertions(+), 20 deletions(-)
diff --git a/refs.c b/refs.c
index cd36b64..50d192c 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,7 @@
*/
#include "cache.h"
+#include "hashmap.h"
#include "lockfile.h"
#include "refs.h"
#include "refs/refs-internal.h"
@@ -1357,11 +1358,42 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
return 0;
}
+struct submodule_hash_entry
+{
+ struct hashmap_entry ent; /* must be the first member! */
+
+ struct ref_store *refs;
+
+ /* NUL-terminated name of submodule: */
+ char submodule[FLEX_ARRAY];
+};
+
+static int submodule_hash_cmp(const void *entry, const void *entry_or_key,
+ const void *keydata)
+{
+ const struct submodule_hash_entry *e1 = entry, *e2 = entry_or_key;
+ const char *submodule = keydata;
+
+ return strcmp(e1->submodule, submodule ? submodule : e2->submodule);
+}
+
+static struct submodule_hash_entry *alloc_submodule_hash_entry(
+ const char *submodule, struct ref_store *refs)
+{
+ size_t len = strlen(submodule);
+ struct submodule_hash_entry *entry = malloc(sizeof(*entry) + len + 1);
+
+ hashmap_entry_init(entry, strhash(submodule));
+ entry->refs = refs;
+ memcpy(entry->submodule, submodule, len + 1);
+ return entry;
+}
+
/* A pointer to the ref_store for the main repository: */
static struct ref_store *main_ref_store;
-/* A linked list of ref_stores for submodules: */
-static struct ref_store *submodule_ref_stores;
+/* A hashmap of ref_stores, stored by submodule name: */
+static struct hashmap submodule_ref_stores;
void base_ref_store_init(struct ref_store *refs,
const struct ref_storage_be *be,
@@ -1373,16 +1405,17 @@ void base_ref_store_init(struct ref_store *refs,
die("BUG: main_ref_store initialized twice");
refs->submodule = "";
- refs->next = NULL;
main_ref_store = refs;
} else {
- if (lookup_ref_store(submodule))
+ refs->submodule = xstrdup(submodule);
+
+ if (!submodule_ref_stores.tablesize)
+ hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 20);
+
+ if (hashmap_put(&submodule_ref_stores,
+ alloc_submodule_hash_entry(submodule, refs)))
die("BUG: ref_store for submodule '%s' initialized twice",
submodule);
-
- refs->submodule = xstrdup(submodule);
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
}
}
@@ -1402,17 +1435,17 @@ struct ref_store *ref_store_init(const char *submodule)
struct ref_store *lookup_ref_store(const char *submodule)
{
- struct ref_store *refs;
+ struct submodule_hash_entry *entry;
if (!submodule || !*submodule)
return main_ref_store;
- for (refs = submodule_ref_stores; refs; refs = refs->next) {
- if (!strcmp(submodule, refs->submodule))
- return refs;
- }
+ if (!submodule_ref_stores.tablesize)
+ hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 20);
- return NULL;
+ entry = hashmap_get_from_hash(&submodule_ref_stores,
+ strhash(submodule), submodule);
+ return entry ? entry->refs : NULL;
}
struct ref_store *get_ref_store(const char *submodule)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 25444cf..4ed5f89 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -634,12 +634,6 @@ struct ref_store {
* reference store:
*/
const char *submodule;
-
- /*
- * Submodule reference store instances are stored in a linked
- * list using this pointer.
- */
- struct ref_store *next;
};
/*
--
2.9.3
^ permalink raw reply related
* [PATCH 3/5] register_ref_store(): new function
From: Michael Haggerty @ 2017-02-09 13:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git, Michael Haggerty
In-Reply-To: <cover.1486629195.git.mhagger@alum.mit.edu>
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.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 19 +++++++++++++------
refs/files-backend.c | 2 +-
refs/refs-internal.h | 15 ++++++++++-----
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 723b4be..6012f67 100644
--- a/refs.c
+++ b/refs.c
@@ -1395,11 +1395,8 @@ static struct ref_store *main_ref_store;
/* A hashmap of ref_stores, stored by submodule name: */
static struct hashmap submodule_ref_stores;
-void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule)
+void register_ref_store(struct ref_store *refs, const char *submodule)
{
- refs->be = be;
if (!submodule) {
if (main_ref_store)
die("BUG: main_ref_store initialized twice");
@@ -1416,18 +1413,28 @@ void base_ref_store_init(struct ref_store *refs,
}
}
+void base_ref_store_init(struct ref_store *refs,
+ const struct ref_storage_be *be)
+{
+ refs->be = be;
+}
+
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);
+
+ register_ref_store(refs, submodule);
+ return refs;
}
struct ref_store *lookup_ref_store(const char *submodule)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6ed7e13..794b88c 100644
--- 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) : "";
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 97f275b..73281f5 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -481,7 +481,7 @@ struct ref_store;
* Initialize the ref_store for the specified submodule, or for the
* main repository if submodule == NULL. These functions should call
* base_ref_store_init() to initialize the shared part of the
- * ref_store and to record the ref_store for later lookup.
+ * ref_store.
*/
typedef struct ref_store *ref_store_init_fn(const char *submodule);
@@ -630,12 +630,17 @@ struct ref_store {
};
/*
- * Fill in the generic part of refs for the specified submodule and
- * add it to our collection of reference stores.
+ * Register the specified ref_store to be the one that should be used
+ * for submodule (or the main repository if submodule is NULL). It is
+ * a fatal error to call this function twice for the same submodule.
+ */
+void register_ref_store(struct ref_store *refs, const char *submodule);
+
+/*
+ * Fill in the generic part of refs.
*/
void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule);
+ const struct ref_storage_be *be);
/*
* Create, record, and return a ref_store instance for the specified
--
2.9.3
^ permalink raw reply related
* Re: Fwd: Possibly nicer pathspec syntax?
From: Duy Nguyen @ 2017-02-09 13:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <xmqqy3xgwpiq.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 9, 2017 at 12:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Feb 8, 2017 at 12:12 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> Two-patch series to follow.
>>
>> glossary-content.txt update for both patches would be nice.
>
> I am no longer worried about it as I saw somebody actually sent
> follow-up patches on this, but I want to pick your brain on one
> thing that is related to this codepath.
>
> We have PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL bits in flags,
> added at fc12261fea ("parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
> flags", 2013-07-14), and I think the intent is some commands when
> given no pathspec work on all paths in the current subdirectory
> while others work on the full tree, regardless of where you are.
> "grep" is in the former camp, "log" is in the latter. And there is
> a check to catch a bug in a caller that sets both.
>
> I am wondering about this hunk (this is from the original commit
> that added it):
>
> if (!entry) {
> static const char *raw[2];
>
> + if (flags & PATHSPEC_PREFER_FULL)
> + return;
> +
> + if (!(flags & PATHSPEC_PREFER_CWD))
> + die("BUG: PATHSPEC_PREFER_CWD requires arguments");
> +
> pathspec->items = item = xmalloc(sizeof(*item));
> memset(item, 0, sizeof(*item));
> item->match = prefix;
> ... returns a single entry pathspec to cover cwd ...
>
> The BUG message is given when
>
> - The command got no pathspec from the caller; and
> - PATHSPEC_PREFER_FULL is not set; and
> - PATHSPEC_PREFER_CWD is NOT set.
>
> but the message says that the caller must have args when it sets
> prefer-cwd. Is this a simple typo? If so what should it say?
>
> die("BUG: one of PATHSPEC_PREFER_FULL or _CWD must be set");
Without reading through your next mail, I'd say "BUG: this command
requires arguments".
> Does this third possibility (i.e. a caller is allowed to pass
> "flags" that does not prefer either) exist to support a command
> where the caller MUST have at least one pathspec? If that were the
> case, this wouldn't be a BUG but an end-user error, e.g.
>
> die("at least one pathspec element is required");
Or this. Yes. I might have just been defensive at then and kept the
third option open.
> If you know offhand which callers pass neither of the two
> PATHSPEC_PREFER_* bits and remember for what purpose you allowed
> them to do so, please remind me. I'll keep digging in the meantime.
I don't usually remember what I ate yesterday and this commit was from
2013 :D But I'll see if your findings spark anything in my brain.
--
Duy
^ permalink raw reply
* [PATCH 0/5] Store submodules in a hash, not a linked list
From: Michael Haggerty @ 2017-02-09 13:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, git, Michael Haggerty
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()
refs.c | 93 ++++++++++++++++++++++++++++++++++------------------
refs/files-backend.c | 77 +++++++++++++++++++++++++------------------
refs/refs-internal.h | 37 ++++++++-------------
3 files changed, 122 insertions(+), 85 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)
From: Johannes Schindelin @ 2017-02-09 12:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqzihzymn3.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 6 Feb 2017, Junio C Hamano wrote:
> * sf/putty-w-args (2017-02-01) 5 commits
> - SQUASH???
> - connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
> - git_connect(): factor out SSH variant handling
> - connect: rename tortoiseplink and putty variables
> - connect: handle putty/plink also in GIT_SSH_COMMAND
>
> The command line options for ssh invocation needs to be tweaked for
> some implementations of SSH (e.g. PuTTY plink wants "-P <port>"
> while OpenSSH wants "-p <port>" to specify port to connect to), and
> the variant was guessed when GIT_SSH environment variable is used
> to specify it. Extend the guess to the command specified by the
> newer GIT_SSH_COMMAND and also core.sshcommand configuration
> variable, and give an escape hatch for users to deal with
> misdetected cases.
>
> Stalled?
> cf. <alpine.DEB.2.20.1702012319460.3496@virtualbox>
The latest messages in that thread are
- your claim that you never said correctness is pused to a back seat (when
an earlier, detailed mail listed four priorities of your patch review,
none of which is said correctness, so I did not bother to answer), and
- my answer that suggested to take a break because the conversation turned
less rational: I had to point out that your objection was not really
valid in this case.
I now see that you added a SQUASH commit (that was news to me, thank you
very much), and that you seem to still insist that the code should prepare
for possible future changes in the config settings that may actually never
materialize. (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.)
In short: unless I read any convincing argument in favor of said SQUASH
commit, I will remain convinced that v3, as submitted, is actually the
best way forward.
Thank you for your attention,
Johannes
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Pranit Bauva @ 2017-02-09 12:18 UTC (permalink / raw)
To: Jeff King
Cc: Matthieu Moy, git, Lars Schneider, Christian Couder,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer
In-Reply-To: <20170125204504.ebw2sa4uokfwwfnt@sigill.intra.peff.net>
Hey everyone,
On Thu, Jan 26, 2017 at 2:15 AM, Jeff King <peff@peff.net> wrote:
> I do not mind doing the administrative stuff. But the real work is in
> polishing up the ideas list and microprojects page. So I think the first
> step, if people are interested in GSoC, is not just to say "yes, let's
> do it", but to actually flesh out these pages:
I will help with adding more ideas to the microprojects list. But
since I am not quite familiar with the whole code base, I will need
some help with verifying those whether they are in the scope or not. I
am not sure whether I would be able to help with actual project ideas
but I will try. I will do it within a week or so.
Regards,
Pranit Bauva
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Matthieu Moy @ 2017-02-09 12:22 UTC (permalink / raw)
To: Christian Couder
Cc: Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
Siddharth Kannan
In-Reply-To: <vpqlgtfmun0.fsf@anie.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> I created a Git organization and invited you + Peff as admins. I'll
> start cut-and-pasting to show my good faith ;-).
I created this page based on last year's:
https://git.github.io/SoC-2017-Org-Application/
I filled-in the "org profile". "Org application" is still TODO.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Matthieu Moy @ 2017-02-09 12:11 UTC (permalink / raw)
To: Christian Couder
Cc: Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
Siddharth Kannan
In-Reply-To: <CAP8UFD1V=WD-EHkBkAVET9ztvsHZr_S5GVBWrQ6F1e0LwJoksQ@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> Well Peff wrote in reply to your email:
>
>> I did co-admin last year and the year before, but I made Matthieu do all
>> the work. :)
>>
>> I do not mind doing the administrative stuff. But the real work is in
>> polishing up the ideas list and microprojects page.
>
> So I thought Peff would be ok to be the admin (do "the administrative
> stuff").
There are several things the admins need to do:
1) "administrative stuff" about money with Conservancy (aka SFC). As I
understand it, really not much to do since Google and Conservancy
work directly with each other for most stuff.
2) Filling-in the application, i.e. essentially copy-past from the
website.
3) Then, make sure things that must happen do happen (reviewing
applications, start online or offline discussions when needed, ...).
Last year Peff did 1) and I did most of 2+3). My understanding of Peff's
reply was "OK to continue doing 1)".
I think you (Christian) could do 2+3). It's much, much less work than
being a mentor. Honnestly I felt like I did nothing and then Peff said I
did all the work :o). I can help, but as I said I'm really short in time
budget and I'd like to spend it more on coding+reviewing.
> I don't think emails in this thread is what really counts.
> I worked on the Idea page starting some months ago, and as I wrote I
> reviewed it again and found it not too bad.
OK, so giving up now seems unfair to you indeed.
I created a Git organization and invited you + Peff as admins. I'll
start cut-and-pasting to show my good faith ;-).
> About the janitoring part, as I previously said I am reluctant to do
> that as I don't know what Dscho would be ok to mentor.
> And I also think it's not absolutely necessary to do it before
> applying as an org.
Right.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree
From: Duy Nguyen @ 2017-02-09 11:59 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Git Mailing List, Junio C Hamano, Stefan Beller
In-Reply-To: <37fe2024-0378-a974-a28d-18a89d3e2312@alum.mit.edu>
On Thu, Feb 9, 2017 at 1:07 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> It is unquestionably a good goal to avoid parsing references outside of
> `refs/files-backend.c`. But I'm not a fan of this approach.
Yes. But in this context it was more of a guinea pig. I wanted
something simple enough to code up show we can see what the approach
looked like. Good thing I did it.
>
> There are two meanings of the concept of a "ref store", and I think this
> change muddles them:
>
> 1. The references that happen to be *physically* stored in a particular
> location, for example the `refs/bisect/*` references in a worktree.
>
> 2. The references that *logically* should be considered part of a
> particular repository. This might require stitching together
> references from multiple sources, for example `HEAD` and
> `refs/bisect` from a worktree's own directory with other
> references from the main repository.
>
> Either of these concepts can be implemented via the `ref_store` abstraction.
>
> The `ref_store` for a submodule should represent the references
> logically visible from the submodule. The main program shouldn't care
> whether the references are stored in a single physical location or
> spread across multiple locations (for example, if the submodule were
> itself a linked worktree).
>
> The `ref_store` that you want here for a worktree is not the worktree's
> *logical* `ref_store`. You want the worktree's *physical* `ref_store`.
Yep.
> Mixing logical and physical reference stores together is a bad idea
> (even if we were willing to ignore the fact that worktrees are not
> submodules in the accepted sense of the word).
>
> ...
>
> I think the best solution would be to expose the concept of `ref_store`
> in the public refs API. Then users of submodules would essentially do
>
> struct ref_store *refs = get_submodule_refs(submodule_path);
> ... resolve_ref_recursively(refs, refname, 0, sha1, &flags) ...
> ... for_each_ref(refs, fn, cb_data) ...
>
> whereas for a worktree you'd have to look up the `ref_store` instance
> somewhere else (or maybe keep it as part of some worktree structure, if
> there is one) but you would use it via the same API.
Oh I was going to reply to Stefan about his comment to my (**)
footnote. Something along the this line
"Ideally we would introduce a new set of api, maybe with refs_ prefix,
that takes a refs_store. Then submodule people can get a ref store
somewhere and pass to it. Worktree people get maybe some other refs
store for it. The "old" api like for_each_ref() is a thin wrapper
around it, just like read_cache() vs read_index(&the_index). If the
*_submodule does not see much use, we might as well kill it and use
the generic refs_*".
If I didn't misunderstood anything else, then I think we're on the same page.
Now I need to see if I can get there in a reasonable time frame (so I
can fix my "gc in worktree" problem properly) or I would need
something temporary but not so hacky. I'll try to make this new api
and see how it works out. If you think I should not do it right away,
for whatever reason, stop me now.
--
Duy
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Christian Couder @ 2017-02-09 10:49 UTC (permalink / raw)
To: Siddharth Kannan
Cc: Matthieu Moy, Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer
In-Reply-To: <CAN-3QhotVm-LmOJ4cuKCa2txYxFJMHY1aqbX1GznieQx57AR+A@mail.gmail.com>
On Thu, Feb 9, 2017 at 11:28 AM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> On 9 February 2017 at 15:45, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>>
>> A non-quoted but yet important part of my initial email was:
>>
>> | So, as much as possible, I'd like to avoid being an org admin this
>> | year. It's not a lot of work (much, much less than being a mentor!),
>> | but if I manage to get some time to work for Git, I'd rather do that
>> | on coding and reviewing this year.
>>
>> and for now, no one stepped in to admin.
>
> I would like to point everyone to this reply from Jeff King on the
> original post: [1]
> (In case this was lost in the midst of other emails) It sounds like
> Jeff King is okay
> with taking up the "admin" role.
>
> I do not mind doing the administrative stuff. But the real work is in
> polishing up the ideas list and microprojects page. So I think the first
> step, if people are interested in GSoC, is not just to say "yes, let's
> do it", but to actually flesh out these pages:
Yeah it was also my impression based on the above that Peff would be
ok to take up the admin role.
Now if he doesn't want for some reason to take it, then I am ok with
us not applying, but again it would have been better to be clearer
about that before the eve of the deadline.
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Christian Couder @ 2017-02-09 10:45 UTC (permalink / raw)
To: Matthieu Moy
Cc: Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
Siddharth Kannan
In-Reply-To: <vpqzihvpt41.fsf@anie.imag.fr>
On Thu, Feb 9, 2017 at 11:15 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> On Wed, Feb 8, 2017 at 3:54 PM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>> Jeff King <peff@peff.net> writes:
>>>
>>>> On Mon, Jan 23, 2017 at 04:02:02PM +0100, Matthieu Moy wrote:
>>>>
>>>>> * We need to write the application, i.e. essentially polish and update
>>>>> the text here: https://git.github.io/SoC-2016-Org-Application/ and
>>>>> update the list of project ideas and microprojects :
>>>>> https://git.github.io/SoC-2017-Ideas/
>>>>> https://git.github.io/SoC-2016-Microprojects/
>>>>
>>>> That can be done incrementally by people who care (especially mentors)
>>>> over the next week or so, and doesn't require any real admin
>>>> coordination. If it happens and the result looks good, then the
>>>> application process is pretty straightforward.
>>>>
>>>> If it doesn't, then we probably ought not to participate in GSoC.
>>>
>>> OK, it seems the last message did not raise a lot of enthousiasm (unless
>>> I missed some off-list discussion at Git-Merge?).
>>
>> I think having 2 possible mentors or co-mentors still shows some
>> enthousiasm even if I agree it's unfortunate there is not more
>> enthousiasm.
>
> A non-quoted but yet important part of my initial email was:
>
> | So, as much as possible, I'd like to avoid being an org admin this
> | year. It's not a lot of work (much, much less than being a mentor!),
> | but if I manage to get some time to work for Git, I'd rather do that
> | on coding and reviewing this year.
>
> and for now, no one stepped in to admin.
Well Peff wrote in reply to your email:
> I did co-admin last year and the year before, but I made Matthieu do all
> the work. :)
>
> I do not mind doing the administrative stuff. But the real work is in
> polishing up the ideas list and microprojects page.
So I thought Peff would be ok to be the admin (do "the administrative stuff").
> Other non-negligible sources of work are reviewing microprojects and
> applications. Having a few more messages in this thread would have been
> a good hint that we had volunteers to do that.
I don't think emails in this thread is what really counts.
I worked on the Idea page starting some months ago, and as I wrote I
reviewed it again and found it not too bad.
>> Someone steps in to do what exactly?
>
> First we need an admin. Then as you said a bit of janitoring work on
> the web pages.
About the janitoring part, as I previously said I am reluctant to do
that as I don't know what Dscho would be ok to mentor.
And I also think it's not absolutely necessary to do it before
applying as an org.
If you just want Peff or someone else to apply, then please just say
it and hopefully Peff will do it and be the admin.
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Siddharth Kannan @ 2017-02-09 10:28 UTC (permalink / raw)
To: Matthieu Moy
Cc: Christian Couder, Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer
In-Reply-To: <vpqzihvpt41.fsf@anie.imag.fr>
On 9 February 2017 at 15:45, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>
> A non-quoted but yet important part of my initial email was:
>
> | So, as much as possible, I'd like to avoid being an org admin this
> | year. It's not a lot of work (much, much less than being a mentor!),
> | but if I manage to get some time to work for Git, I'd rather do that
> | on coding and reviewing this year.
>
> and for now, no one stepped in to admin.
I would like to point everyone to this reply from Jeff King on the
original post: [1]
(In case this was lost in the midst of other emails) It sounds like
Jeff King is okay
with taking up the "admin" role.
I do not mind doing the administrative stuff. But the real work is in
polishing up the ideas list and microprojects page. So I think the first
step, if people are interested in GSoC, is not just to say "yes, let's
do it", but to actually flesh out these pages:
>
>> Someone steps in to do what exactly?
>
> First we need an admin. Then as you said a bit of janitoring work on
> the web pages.
[1]: https://public-inbox.org/git/20170125204504.ebw2sa4uokfwwfnt@sigill.intra.peff.net/
--
Best Regards,
- Siddharth.
^ permalink raw reply
* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Matthieu Moy @ 2017-02-09 10:15 UTC (permalink / raw)
To: Christian Couder
Cc: Jeff King, git, Pranit Bauva, Lars Schneider,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
Siddharth Kannan
In-Reply-To: <CAP8UFD3aygSf5U2abnpCfRzEf-hH5fSNuzFBBtgCjSQC3F8c5A@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Wed, Feb 8, 2017 at 3:54 PM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Jeff King <peff@peff.net> writes:
>>
>>> On Mon, Jan 23, 2017 at 04:02:02PM +0100, Matthieu Moy wrote:
>>>
>>>> * We need to write the application, i.e. essentially polish and update
>>>> the text here: https://git.github.io/SoC-2016-Org-Application/ and
>>>> update the list of project ideas and microprojects :
>>>> https://git.github.io/SoC-2017-Ideas/
>>>> https://git.github.io/SoC-2016-Microprojects/
>>>
>>> That can be done incrementally by people who care (especially mentors)
>>> over the next week or so, and doesn't require any real admin
>>> coordination. If it happens and the result looks good, then the
>>> application process is pretty straightforward.
>>>
>>> If it doesn't, then we probably ought not to participate in GSoC.
>>
>> OK, it seems the last message did not raise a lot of enthousiasm (unless
>> I missed some off-list discussion at Git-Merge?).
>
> I think having 2 possible mentors or co-mentors still shows some
> enthousiasm even if I agree it's unfortunate there is not more
> enthousiasm.
A non-quoted but yet important part of my initial email was:
| So, as much as possible, I'd like to avoid being an org admin this
| year. It's not a lot of work (much, much less than being a mentor!),
| but if I manage to get some time to work for Git, I'd rather do that
| on coding and reviewing this year.
and for now, no one stepped in to admin.
Other non-negligible sources of work are reviewing microprojects and
applications. Having a few more messages in this thread would have been
a good hint that we had volunteers to do that.
> Someone steps in to do what exactly?
First we need an admin. Then as you said a bit of janitoring work on
the web pages.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ 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