From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61631 invoked by alias); 2 Sep 2018 08:31:46 -0000 Mailing-List: contact cygwin-apps-help@cygwin.com; run by ezmlm Precedence: bulk Sender: cygwin-apps-owner@cygwin.com List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Mail-Followup-To: cygwin-apps@cygwin.com Received: (qmail 60763 invoked by uid 89); 2 Sep 2018 08:30:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=knew, Care, weekend, conference X-HELO: vsmx011.vodafonemail.xion.oxcs.net Received: from Unknown (HELO vsmx011.vodafonemail.xion.oxcs.net) (153.92.174.89) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 02 Sep 2018 08:30:02 +0000 Received: from vsmx003.vodafonemail.xion.oxcs.net (unknown [192.168.75.197]) by mta-5-out.mta.xion.oxcs.net (Postfix) with ESMTP id 31D923E049B for ; Sun, 2 Sep 2018 08:29:54 +0000 (UTC) Received: from Gertrud (unknown [87.185.215.3]) by mta-7-out.mta.xion.oxcs.net (Postfix) with ESMTPA id 0A1ED300A22 for ; Sun, 2 Sep 2018 08:29:51 +0000 (UTC) From: Achim Gratz To: cygwin-apps@cygwin.com Subject: Re: Zstandard support for setup References: <874lg0d6l8.fsf@Rainer.invalid> <267037c7-7757-25ec-4b80-7ffcbc5768d6@dronecode.org.uk> Date: Sun, 02 Sep 2018 08:31:00 -0000 In-Reply-To: <267037c7-7757-25ec-4b80-7ffcbc5768d6@dronecode.org.uk> (Jon Turney's message of "Wed, 29 Aug 2018 17:42:43 +0100") Message-ID: <87r2icl2th.fsf@Rainer.invalid> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-09/txt/msg00000.txt.bz2 Jon Turney writes: > This should check for ztd in configure.ac using PKG_CHECK_MODULES, > rather than just assuming -lzstd is going to work. Sure. Waiting for your patch to actually use pkg_config to rebase it onto. > The setup executables are cross-built on Fedora, so a > mingw{32,64}-zstd-static package will need to become available. I don't have any contacts into the Fedora community, so I'd appreciate if maybe Yaakov could help? I have not yet found a distro that already has packages for MinGW. > compress_zstd.c: > > + using namespace std; > > Please don't That's a copy from the compress_xz files and it's used in many files elsewhere. Care to explain why you'd want it removed? If there's a good reason I'd tend to move this to a clean-up commit that touches all those files. We should do that before hauling Zstdandard in. > compress_zstd.h: > > +/* this is the parent class for all compress IO operations. > + */ > > Comment is incorrect Another copy-paste, so that cleanup also needs to be applied to more files (gz and xz). > I'd suggest removing all the 'virtual' since this class is final, but > since all the compress subclasses have it, don't bother. Same as above, so we should clean up there first as well. >> The debug output statements are still in the code (although commented), >> I'll let that sit a while and see if I find something else I want to >> clean up before I submit it for the upstream repo. > > I'd suggest keeping potentially useful ones under #ifdef DEBUG/#endif > rather than just removing them all. At the moment I'd rather remove them all as they were just meant for me to check if things were working as I had assumed. These produce a lot more output than the usual debugging statements and you don't really benefit if I keep just a subset enabled. If we had a debugging facility that knew about subsystems it would be a different matter (it's long been on my TODO list, near the bottom unfortunately). I'm attending a conference most of next week, so I need to postpone any real work until the next weekend at the minimum. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Factory and User Sound Singles for Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#WaldorfSounds