From patchwork Fri Sep 19 10:41:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 391219 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B44AF14018C for ; Fri, 19 Sep 2014 20:42:46 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=qHcrK9/jjpmGxo3bh AykXjNL/Dp1sKyosuz8GjllZnsq1IM9Mh2wUb6aKWSU+tTmS4Gv6FnB8XQMEZgAj +2vuJHUtFqJHx07TBttXQG7uoQVP+bvCHmTLhK9zjoZvoFaPmdVgYOIwYqMuKVLQ IB9YxfXBx2leGVm23Dg2jpGEzk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=TX2Yv3+w37g55MKnezNNt73 TG24=; b=N4RBYyIKFQZaI/fqRVgCS8O2aBcvcrNmGIk2gQitqofHCSl9b6xRTra 0NHbJ90j0rcXsCkJw2qx8BDgbKHpiy2EsZQjx9qkpqDRDU7/ELKIfq5Nipa/OXTd YMznURDz2ObV6jGTI5ZWEiORoAM/wnKuUWfMRp5v8Jr35T/vZGTQ= Received: (qmail 2027 invoked by alias); 19 Sep 2014 10:42:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 2011 invoked by uid 89); 19 Sep 2014 10:42:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Sep 2014 10:42:36 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1XUve1-0007NP-1N from Tom_deVries@mentor.com ; Fri, 19 Sep 2014 03:42:29 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.181.6; Fri, 19 Sep 2014 11:42:22 +0100 Message-ID: <541C0871.4020407@mentor.com> Date: Fri, 19 Sep 2014 12:41:53 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Diego Novillo CC: Yury Gribov , GCC Patches , Geoff Keating , Trevor Saunders , Segher Boessenkool Subject: Re: [PATCH][PING] Keep patch file permissions in mklog References: <5389890F.2050501@mentor.com> <53DB15C5.6070206@samsung.com> <53DB391C.7050808@mentor.com> <53DB3F2A.5070800@samsung.com> <53DD3AB2.9040901@mentor.com> <53DF2BF4.8090806@samsung.com> <53DF40F7.6050909@mentor.com> <541AF2A3.60600@samsung.com> In-Reply-To: On 18-09-14 19:46, Diego Novillo wrote: > On Thu, Sep 18, 2014 at 10:56 AM, Yury Gribov wrote: >> >> On 08/04/2014 12:14 PM, Tom de Vries wrote: >>> >>> On 04-08-14 08:45, Yury Gribov wrote: >>>> >>>> Thanks! My 2 (actually 4) cents below. >>>> >>> >>> Hi Yuri, >>> >>> thanks for the review. >>> >>>> > +if ($#ARGV == 1 && ("$ARGV[0]" eq "-i" || "$ARGV[0]" eq >>>> "--inline")) { >>>> > + $diff = $ARGV[1]; >>>> >>>> Can we shift here and then just set $diff to $ARGV[0] unconditionally? >>>> >>> >>> Done. >>> >>>> > + if ($diff eq "-") { >>>> > + die "Reading from - and using -i are not compatible"; >>>> > + } >>>> >>>> Hm, can't we dump ChangeLog to stdout in that case? >>>> The limitation looks rather strange. >>>> >>> >>> My original idea here was that --inline means 'in the patch file', which >>> is not possible if the patch comes from stdin. >>> >>> I've now interpreted it such that --inline prints to stdout what it >>> would print to the patch file otherwise, that is, both log and patch. >>> Printing just the log to stdout can be already be achieved by not using >>> --inline. >>> >>>> > + open (FILE1, '>', $tmp) or die "Could not open temp file"; >>>> >>>> Could we use more descriptive name? >>>> >>> >>> I've used the slightly more descriptive 'OUTPUTFILE'. >>> >>>> > + system ("cat $diff >>$tmp") == 0 >>>> > + or die "Could not append patch to temp file"; >>>> > ... >>>> > + unlink ($tmp) == 1 or die "Could not remove temp file"; >>>> >>>> The checks look like an overkill given that we don't check for result >>>> of mktemp... >>>> >>> >>> I've added a check for the result of mktemp, and removed the unlink >>> result check. I've left in the "Could not append patch to temp file" >>> check because the patch file might be read-only. >>> >>> OK for trunk? >>> >>> Thanks, >>> - Tom >>> >> >> Pinging the patch for Tom. > > > Apologies for the delay. Could someone post the latest patch. I see > it's gone through a cycle of reviews and changes. > > Hi Diego, here it is. I've followed up on the explanation by Segher about 2.15 File module version and fixed the comment. I've not added the 2.15 file module version check on copy Segher also mentioned, since I'm not sure about that one. AFAIU the tradeoff is: - without the check the mklog still works ok for older versions of perl, apart from the permissions (the situation we're in for all versions of perl without the patch) - with the check the script won't work at all for older perl version. So it's a question of predictability (always do the same or do nothing) vs. robustness (do as much as you can given the circumstances). I'm not sure which one is better in this case. Thanks, - Tom 2014-08-11 Tom de Vries * mklog: Add --inline option. diff --git a/contrib/mklog b/contrib/mklog index 3d17dc5..1352e99 100755 --- a/contrib/mklog +++ b/contrib/mklog @@ -26,6 +26,9 @@ # Author: Diego Novillo and # Cary Coutant +use File::Temp; +use File::Copy qw(cp mv); + # Change these settings to reflect your profile. $username = $ENV{'USER'}; $name = `finger $username | grep -o 'Name: .*'`; @@ -56,14 +59,22 @@ if (-d "$gcc_root/.git") { # Program starts here. You should not need to edit anything below this # line. #----------------------------------------------------------------------------- -if ($#ARGV != 0) { +$inline = 0; +if ($#ARGV == 1 && ("$ARGV[0]" eq "-i" || "$ARGV[0]" eq "--inline")) { + shift; + $inline = 1; +} elsif ($#ARGV != 0) { $prog = `basename $0`; chop ($prog); print <', $tmp) or die "Could not open temp file: $!"; +} else { + *OUTPUTFILE = STDOUT; +} + +# Print the log foreach my $clname (keys %cl_entries) { - print "$clname:\n\n$hdrline\n\n$cl_entries{$clname}\n"; + print OUTPUTFILE "$clname:\n\n$hdrline\n\n$cl_entries{$clname}\n"; +} + +if ($inline) { + # Append the patch to the log + foreach (@diff_lines) { + print OUTPUTFILE "$_\n"; + } +} + +if ($inline && $diff ne "-") { + # Close $tmp + close(OUTPUTFILE); + + # Write new contents to $diff atomically + mv $tmp, $diff or die "Could not move temp file to patch file: $!"; } exit 0; -- 1.9.1