Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 30 Oct 2014 17:44:18 +0100
From:      Enrico Maria Crisostomo <enrico.m.crisostomo@gmail.com>
To:        Mathieu Arnold <mat@FreeBSD.org>
Cc:        freebsd-doc@FreeBSD.org, Matthew Seaman <matthew@freebsd.org>
Subject:   Re: Suggestion about a correction in Porter' HandBook, Chapter 10
Message-ID:  <4B0FBD68-B6F7-4C35-955F-EFCA88C3C6BA@gmail.com>
In-Reply-To: <D05DEAB77A61D8FB2D1338FC@ogg.in.absolight.net>
References:  <C318C257-B660-491E-B9E0-1E8602C5747F@gmail.com> <D05DEAB77A61D8FB2D1338FC@ogg.in.absolight.net>
next in thread | previous in thread | raw e-mail | index | archive | help 

[-- Attachment #1 --]
Hi Mathieu,

> On 30 Oct 2014, at 17:30, Mathieu Arnold <mat@FreeBSD.org> wrote:
> 
> 
> 
> +--On 30 octobre 2014 12:50:18 +0100 Enrico Maria Crisostomo
> <enrico.m.crisostomo@gmail.com> wrote:
> | Hi all,
> | 
> | This morning I was notified by Matthew Seaman about a problem in a port I
> | maintain and finally we discovered that `svn diff` does **not** output
> | information about files that have "history scheduled with commit": that
> | is, files which seem to be new files, but in reality are not.  The
> | typical case is files "added" as a result of a `svn mv` operation.  This
> | behaviour led to the problem that affected the port.
> | 
> | The Porter's Handbook implicitly implies that `svn diff` is equivalent to
> | `diff -ruN` but in fact it is not: using `svn diff` may lead to the
> | aforementioned problem.  svn 1.7 added the `--show-copies-as-adds` option
> | to the `svn diff` command which forces the expected behaviour.
> | Therefore, I suggest Chapter 10, Section 1, "Using Subversion to Make
> | Patches" to be amended in order to describe this behaviour.
> 
> Unless mistaken, the PHB says that if you move files around, you *must* say
> so in the PR you open, so that the developper can replicate the commands
> before applying the patch.

Indeed, it says so, and I did said so in the PR, adding the full `svn st` output showing what was going on.

> 
> Because if you add --show-copies-as-adds, the diff will be in such a way
> that files will get created without its ancestry taken into account, which
> we *do not* want.

Good point, that's the show-stopper.

> 
> The good thing about Subversion is that it has a way to know that foo has
> been copied to bar without having to fiddle around in the repository doing
> repo-copies like we used to do with CVS...
> 
> If the PHB is not clear about that (or I just dreamt I read/wrote that bit)
> do tell me, I'll fix it :-)

IMHO it's not that clear, but that may be subject to interpretation.  The relevant bits are these:

Chapter 10:

    Please mention any added or deleted files in the message, as they have to be explicitly specified to svn(1) when doing a commit.

Chapter 10, Section 10.1:

    While in the port directory, make any changes that are needed. If adding, moving, or removing a file, use svn to track these changes:

        % svn add new_file
        % svn move old_name new_name
        % svn remove deleted_file

[...snip...]

    The last step is to make a unified diff(1) of the changes:

        % svn diff > ../`make -VPKGNAME`.diff

    Note: 
    Any files that have been removed have to be explicitly mentioned in the PR, because file removal may not be obvious to the committer.

As you can see, the first citation talks about added and deleted files, but in the context of a manual diff.  The second citation, coming from the section about Subversion diff, only cites deletions and makes an example with the `svn move` command.

In my opinion, we could improve adding the important bits of your explanations (citing additions and additions coming from move operations and specifying that the committer has to manually perform those operations before applying the patch).

In fact, if I followed literally what's written in the PHB to my understanding, I'd only cite *deleted* files, and the committer will have no information about moved ones (that is what happened to us).

Cheers,
-- 
Enrico

> 
> Regards,
> 
> -- 
> Mathieu Arnold


[-- Attachment #2 --]
0	*H
010	+0	*H
0400
	*H
0}10	UIL10U

StartCom Ltd.1+0)U"Secure Digital Certificate Signing1)0'U StartCom Certification Authority0
071024210155Z
171024210155Z010	UIL10U

StartCom Ltd.1+0)U"Secure Digital Certificate Signing1806U/StartCom Class 1 Primary Intermediate Client CA0"0
	*H
0
	-).2AUGo#G
B|NDRpM-B=o-we5JQpa>O.#._<V
[~**pz~3WG.ᘟMlr[<Ce6fqO"uxfWN#uicgkv$Lb%y`_{`xK'GN00U00U0USr풜\|~5NԸQ0U#0N@[i04hCA0f+Z0X0'+0http://ocsp.startssl.com/ca0-+0!http://www.startssl.com/sfsca.crt0[UT0R0'%#!http://www.startssl.com/sfsca.crl0'%#!http://crl.startssl.com/sfsca.crl0U y0w0u+70f0.+"http://www.startssl.com/policy.pdf04+(http://www.startssl.com/intermediate.pdf0
	*H

}x,\c^#wMq}>UK/^yX֏y	frMIŲB61ymQ󸟆ҨݬZ0&;@#13qۑ&	̢o	6r_;GO>*I(	74XS1r3)!LJy6Kotˆ#
_wSr
;B
ADp(fs䰷6%.W0J3:bC<8t X1<Cn=t==wST~\wkBf|15zUP)(IjVB!OfI=bb\4-*em/нSJm7N[]'@ڽD9Kr>R7/|o^I@ټ'Pa$ z9a'L)(
I}vcH]۸D*W}
m>Q|C.(,lQ0E0-[0
	*H
010	UIL10U

StartCom Ltd.1+0)U"Secure Digital Certificate Signing1806U/StartCom Class 1 Primary Intermediate Client CA0
131211152739Z
141212092201Z0V1&0$Uenrico.m.crisostomo@gmail.com1,0*	*H
	enrico.m.crisostomo@gmail.com0"0
	*H
0
Ob>v߭v0vt5T`%HRɸ?q>DƄ	rN>Um%טůչQ1ZLlFK.'UG٤w+B-CVQpozPxa_c^8G%Gq7Eg/Yc4VIK@nAjiGÙFUap5+U襺Ķl
W*qWzhu`
A00	U00U0U%0++0UTB-X&{'"Q~0U#0Sr풜\|~5NԸQ0(U!0enrico.m.crisostomo@gmail.com0LU C0?0;+70*0.+"http://www.startssl.com/policy.pdf0+00' StartCom Certification Authority0This certificate was issued according to the Class 1 Validation requirements of the StartCom CA policy, reliance only for the intended purpose in compliance of the relying party obligations.06U/0-0+)'%http://crl.startssl.com/crtu1-crl.crl0+009+0-http://ocsp.startssl.com/sub/class1/client/ca0B+06http://aia.startssl.com/certs/sub.class1.client.ca.crt0#U0http://www.startssl.com/0
	*H
zJf0>oᙆڜC!ߋMXO uIAmL2М)XtU9H{zǚLITu8a8?"C-s׌S'_tZ9+GiElĄuvX5U#hswcS{O{I
GM`Y@Iz,VV@;&Qe[c72Z|좍1o0k0010	UIL10U

StartCom Ltd.1+0)U"Secure Digital Certificate Signing1806U/StartCom Class 1 Primary Intermediate Client CA[0	+0	*H
	1	*H
0	*H
	1
141030164419Z0#	*H
	1+2?yc5}
~0	+710010	UIL10U

StartCom Ltd.1+0)U"Secure Digital Certificate Signing1806U/StartCom Class 1 Primary Intermediate Client CA[0*H
	1010	UIL10U

StartCom Ltd.1+0)U"Secure Digital Certificate Signing1806U/StartCom Class 1 Primary Intermediate Client CA[0
	*H
)xu,i}}Ӌ
$F`CYMH0:L3+}oR
l[5>Ih`jhxz!!:zfjW+X4]ox3iaD-|8xU߳U*ZGbH
󶩞h%»䜂9Fs6Y'TK1)ޭl^b1	DSB$:̟/{>=N5{],G|q{p9Fri%K

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B0FBD68-B6F7-4C35-955F-EFCA88C3C6BA>