From owner-svn-ports-head@freebsd.org Mon Jan 30 13:50:49 2017 Return-Path: Delivered-To: svn-ports-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 69EFECC7CDB; Mon, 30 Jan 2017 13:50:49 +0000 (UTC) (envelope-from freebsd@toco-domains.de) Received: from toco-domains.de (mail.toco-domains.de [176.9.39.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 00D43BB6; Mon, 30 Jan 2017 13:50:48 +0000 (UTC) (envelope-from freebsd@toco-domains.de) Received: from [0.0.0.0] (mail.toco-domains.de [IPv6:2a01:4f8:150:50a5::6]) by toco-domains.de (Postfix) with ESMTPA id 3579D1AAF056; Mon, 30 Jan 2017 14:50:46 +0100 (CET) Subject: Re: svn commit: r432561 - head/lang/php71 To: John Marino , marino@freebsd.org, ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org References: <201701271852.v0RIqOvW033749@repo.freebsd.org> <6aebcd6a-0439-f23a-be9d-e06f46d3511b@toco-domains.de> <4dd6d654-4525-c440-83b1-6a22215f6020@toco-domains.de> <5725262c-4152-c711-e53b-a509742bcba1@marino.st> Cc: Mathieu Arnold From: Torsten Zuehlsdorff Message-ID: <23d56a5a-f7a0-5d03-cbef-c08bdafa417b@toco-domains.de> Date: Mon, 30 Jan 2017 14:50:46 +0100 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <5725262c-4152-c711-e53b-a509742bcba1@marino.st> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-ports-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the ports tree for head List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Jan 2017 13:50:49 -0000 On 30.01.2017 14:28, John Marino wrote: > On 1/30/2017 05:15, Torsten Zuehlsdorff wrote: >> >> >> On 29.01.2017 19:55, John Marino wrote: >>>> On 27.01.2017 19:52, John Marino wrote: >>>>> Author: marino >>>>> Date: Fri Jan 27 18:52:24 2017 >>>>> New Revision: 432561 >>>>> URL: https://svnweb.freebsd.org/changeset/ports/432561 >>>>> >>>>> Log: >>>>> lang/php71: Bring DTRACE exclude for DF from lang/php70 >>>>> >>>>> Maybe this would have been caught with SVN copy? Oh well. >>>>> >>>>> Approved by: just-fix-it >>>> >>>> Its funny how "just-fix-it" often translate in "just-broke-it" - see >>>> r432567. >>>> >>>> But beside that back to being serious: why did nobody suggest a fix? If >>>> repo-copy is this needed, we can do it right now? >>>> svn rm lang/php71 && svn cp -r432321 lang/php70 lang/php71 && some >>>> files >>>> copied would be all. >>>> I took mat into CC because i want to know if something speaks >>>> against it >>>> or if just nobody thought about it. >>>> >>>> Also fun note: with an svn copy the DTRACE option wouldn't be included >>>> too. I finished the patch 2 month ago, long before DTRACE was >>>> introduced. So it wouldn't be there. >>> >>> Which would be an error of process. >>> - svn cp php70 >>> - overwrite with your working copy >>> - view with svn diff. The new additions in the last 2 months would be >>> apparent. Thus the 3rd step wasn't done. >> >> I did exactly this. 2 months ago there was no DTRACE in php70. So >> copying the working-copy of php71 over the repo-copy would have not >> included the DTRACE option. Therefore the repo-copy wouldn't have had >> helped. > > EXACTLY. > Your mistake is leaving this at two months ago. > I meant check differences the day of committing, not check differences 2 > months ago. If you had not assumed that zero changes happened to php70 > that would affect php71 in a two month period, then you would rechecked > the diff before you actually commit and seen them. I would have noticed > the changes because I always run "svn diff" before committing. I do a svn diff before every commit. Sadly i did not see this mistake within this some hundred lines. >>> You misunderstood r432567. >>> That wasn't to correct my commit. >>> That was to add the second missing item, DTRACE for aarch. >>> >>>> I just noted in this 2 month how hard it is to find somebody for review >>>> and second how hard it is to get such a patch reviewed. First the >>>> review.freebsd.org didn't work for me because it contains a bug with >>>> umlaut for years. And second a repocopy could not be displayed. >>> >>> Yes, the diff would have been displayed. >>> That's the benefit of repocopy. >> >> Maybe i wasn't clear enough: the repo-copy could not displayed in >> review.freebsd.org. If you say: its possible. Okay, than its just the >> umlaut-bug preventing the display of my patch. > > I am not talking about review.freebsd.org. > I'm talking about svn. Read my statement again. I explicitly talk about reviews.freebsd.org and also about its incapability of displaying a repo-copy. >> But - no, you are not right this time. Doing a repo-copy without >> changing the copy will cause a svn diff to just list the files. The diff >> didn't even contain the revision copied from. To bring a path like this >> into the review tool you need at least the --show-copies-as-adds param, >> which defeats the purpose of a repo-copy in review. > > I don't think anybody was talking about review. I was - i explicitly wrote it. > If you do repocopy correctly, we all get a commit mail that shows the > differences between the new version and the version it was copied from. > In this case, we got what looked like complete files added which is how > mat know that svncopy wasn't used. I know. You missed my point: making an error with repo-copy is hard to catch - even with the help of others. reviews.freebsd.org is not able to display a repo-copy. "svn diff" doesn't even show the revision of the copy used. So you need to do it timely (with is hard with such a big patch) or you need to track everything in the meantime to decide if its needed to do all the work again. And again. And again... Greetings, Torsten