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=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 439CF2047F for ; Tue, 1 Aug 2017 20:50:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752741AbdHAUuf (ORCPT ); Tue, 1 Aug 2017 16:50:35 -0400 Received: from mx2.suse.de ([195.135.220.15]:50718 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752496AbdHAUue (ORCPT ); Tue, 1 Aug 2017 16:50:34 -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 024C2ABB8; Tue, 1 Aug 2017 20:50:33 +0000 (UTC) Date: Tue, 01 Aug 2017 22:50:32 +0200 Message-ID: From: Takashi Iwai To: =?UTF-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason Cc: Junio C Hamano , Git Mailing List , Andreas Stieger Subject: Re: [PATCH] hash: Allow building with the external sha1dc library In-Reply-To: <87ini7m5a6.fsf@gmail.com> References: <87ini7m5a6.fsf@gmail.com> 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 21:55:45 +0200, Ævar Arnfjörð Bjarmason wrote: > > > On Tue, Aug 01 2017, Takashi Iwai jotted: > > > 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, > > It's just like this because when I hacked up that facility I was making > the bare minimum change needed to not make local modifications to the > upstream code. If there's better ways to do this in the presence of > DC_SHA1_EXTERNAL & without that would be most welcome. Thanks. Can't it be like the code below? sha1.h is included only via hash.h, and sha1dc_git.h doesn't matter for the compile of sha1dc/*.c. Or am I missing something...? > > #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" thanks, Takashi > > > > In sha1dc_git.h, we'd need another ifdef for DC_SHA1_EXTERNAL to > > define the own git_SHA1DCInit(), but it's trivial. > > > > > > Takashi >