From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.2 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 8B49C2047F for ; Tue, 1 Aug 2017 16:13:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751778AbdHAQM4 (ORCPT ); Tue, 1 Aug 2017 12:12:56 -0400 Received: from mx2.suse.de ([195.135.220.15]:52566 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751295AbdHAQML (ORCPT ); Tue, 1 Aug 2017 12:12:11 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 97A72AD16; Tue, 1 Aug 2017 16:12:10 +0000 (UTC) Date: Tue, 01 Aug 2017 18:12:10 +0200 Message-ID: From: Takashi Iwai To: Junio C Hamano Cc: =?UTF-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Git Mailing List , Andreas Stieger Subject: Re: [PATCH] hash: Allow building with the external sha1dc library In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Tue, 01 Aug 2017 17:56:00 +0200, Junio C Hamano wrote: > > Takashi Iwai writes: > > > On Fri, 28 Jul 2017 17:58:14 +0200, > > Ævar Arnfjörð Bjarmason wrote: > >> ... > >> * We now have much of the same header code copy/pasted between > >> sha1dc_git.h and sha1dc_git_ext.h, did you consider just always > >> including the former but making what it's doing conditional on > >> DC_SHA1_EXTERNAL? I don't know if it would be worth it from a cursory > >> glance, but again your commit message doesn't list that among options > >> considered & discarded. > > > > I don't mind either way, there is no perfect solution in this case. > > As you know, many people think the ifdef ugly no matter how. > > > > I leave the decision to maintainer. Just let me know which option is > > preferred. > > Yeah, I also found it somewhat confusing to have these two headers > that look quite similar to each other at the top-level of the tree. > > What's the "conditional" part between the two headers? Is it just > whether the header for underlying library is included? I wonder if > it's just the matter of adjusting "hash.h" to read like this > > ... > #if defined(DC_SHA1_EXTERNAL) > -#include "sha1dc_git_ext.h" > +#include > +#include "sha1dc_git.h" > #elif defined(DC_SHA1_SUBMODULE) > ... > > or are there heavier tweaks needed that won't be solved by continuing > along the same line? As _ext.h variant is included only at this place, > if we can do with minimum tweaks around here without introducing it, > it may be ideal, I would think. Well, a tricky part is that currently sha1dc_git.h is included from sha1dc/sha1.h implicitly by SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H definition. IMO, we should stop this, and use the standard inclusion instead, i.e. in hash.h, #if defined(DC_SHA1_EXTERNAL) #include #elif defined(DC_SHA1_SUBMODULE) #include "sha1collisiondetection/lib/sha1.h" #else #include "sha1dc/sha1.h" #endif #include "sha1dc_git.h" In sha1dc_git.h, we'd need another ifdef for DC_SHA1_EXTERNAL to define the own git_SHA1DCInit(), but it's trivial. Takashi