From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id C42831FF9C for ; Mon, 26 Oct 2020 22:28:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394412AbgJZW2h (ORCPT ); Mon, 26 Oct 2020 18:28:37 -0400 Received: from mail-oi1-f196.google.com ([209.85.167.196]:41730 "EHLO mail-oi1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394061AbgJZW2h (ORCPT ); Mon, 26 Oct 2020 18:28:37 -0400 Received: by mail-oi1-f196.google.com with SMTP id k65so11581688oih.8 for ; Mon, 26 Oct 2020 15:28:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=d+ZvLhQpM+r8+0x5f/A4v0DDqBeY3J2uVZ+UHsxf4u4=; b=vP8yoT09Bt2srfPrEp7ZgfBroHwg6BqmowOo52E3bSsGQh33ChC0b6RKz253cm4qww PLwZYx+y2xLUiFU9bEaSrcjpCefHdcUpOJKnZSIPQmnFwNqE0R2NMnfCVsDu9FLFUyZy lsUhXu4a32M+tNi5VS7VOsNI8WRdKNeFZq3PoiDRqpfBygGHMzy+ekPkzZRYTlVgZ18i e/emStkxt/nxi0HdBu+i7jZBBX9sGWy0yo4nIf8Gf5nzD/Ut/wWZvbOkKcqzxqCNrekC V/q0PGlGzBeXB3EFzq+2JNWFsznBaMbRE0NHSEnMcgUMhRIJMCk2TJIzDQMEkxdYrMI+ g4Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=d+ZvLhQpM+r8+0x5f/A4v0DDqBeY3J2uVZ+UHsxf4u4=; b=UlQAwjJEfx9pqcsQcHzkQKZnanA7TPuwc1nFSrpZrv0tHth58TRlgdr8SFrPvLhR0m yMWik/xxZ+0GsJGOsNcJoIV0hPX2KsdBAI2sE5wHE1AH3T39UelPA0VNxgkhIxVeg77C pikCCVwPjMrTAA/dG9ZsgF0xxH6JSQcaha4G7D7mGtBd6xI/iU/y+xBXNZTOYu064RVY mBTGzskBYZAbfc2awWuNw1pvDMZavS0suZ8GoP7nSnPffRR2NVw0VLrOLoHfdq2Ik/yi NFS1lNi/mLhgRrg4L2HsPgWcUztzh2fi0oqvE836jQuf5aEXcS5y3wFSzBgWfezVPSKf 3Z/A== X-Gm-Message-State: AOAM533pSzRW5CMc/BYlv4xkydZWjxNVyjF4EFNTbMgpt1Q68cKtuJxl O3TPjYMocMZ4QEGhrWUppfjhbqWvSoPiWA9jiHs= X-Google-Smtp-Source: ABdhPJz5GFMhJHeTPINBDtYQG6sVZ2Dmk0EyQYrZ/OIbfVVLGVyNLe0D4huQxTrv7LsOwMbSDOAyMwKqOrOlmlcXY04= X-Received: by 2002:a05:6808:17:: with SMTP id u23mr12058766oic.31.1603751316046; Mon, 26 Oct 2020 15:28:36 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Elijah Newren Date: Mon, 26 Oct 2020 15:28:24 -0700 Message-ID: Subject: Re: [PATCH v2 1/4] merge-ort: barebones API of new merge strategy with empty implementation To: Junio C Hamano Cc: Elijah Newren via GitGitGadget , Git Mailing List , Taylor Blau , Peter Baumann Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Mon, Oct 26, 2020 at 3:10 PM Junio C Hamano wrote: > > Elijah Newren writes: > >> > + /* > >> > + * Additional metadata used by merge_switch_to_result() or future calls > >> > + * to merge_inmemory_*(). Not for external use. > >> > + */ > >> > + void *priv; > >> > + unsigned ate; > >> > >> I'd prefer to see this named not so cute. Will we hang random > >> variations of things, or would this be better to be made into a > >> pointer to union, with an enum that tells us which kind it is in > >> use? > > > > I don't understand the union suggestion. Both fields are used. > > I thought "priv" shouldn't be "anything goes, so it is 'void *'" but > is probably a "union { ... } priv;" with associated enum next to it > that tells which one of the possibilities in the union is in effect. I guess I'm still not following where these "possibilities" come from or what I could have said that would have suggested possibilities. priv is a pointer to a private data structure. The data structure is defined and used only in merge-ort.c and not exposed to callers. That data does need to be passed from merge_incore_*() to merge_switch_to_result() (or to merge_finalize()). Since those two are separate function calls invoked by the caller, and since callers shouldn't touch any of this data in priv, it's passed as an opaque field within merge_result. Thus void*. > > Would you object if 'ate' was named '_'? > > Either is horrible name. I'll just rip it out, for now. It isn't relevant to early versions of merge-ort anyway, and when I re-introduce it with yet another horrible name, there will at least be more context for others to suggest a better name for me. Besides, the variable isn't at all necessary for the algorithm; it exists solely as a way to catch a small gotcha in API usage of later versions of merge-ort, which I otherwise didn't have a clean way of detecting. > >> > +/* rename-detecting three-way merge with recursive ancestor consolidation. */ > >> > +void merge_inmemory_recursive(struct merge_options *opt, > >> > + struct commit_list *merge_bases, > >> > + struct commit *side1, > >> > + struct commit *side2, > >> > + struct merge_result *result); > >> > >> I've seen "incore" spelled as a squashed-into-a-single-word, but not > >> "in_memory". > > > > I can add an underscore. Or switch to incore. Preference? > > Anything shorter would get my vote. Sounds good, will do. > > Yes, your reading is correct. We don't touch the index (or any index, > > or any cache_entry) at all. Among other things, data that can be used > > to update the index are in the "priv" field. > > > > I'll try to add some notes to the file. > > Sounds good. :-)