Suggestion about a correction in Porter' HandBook, Chapter 10

[Enrico Maria Crisostomo]

Hi Mathieu,

> On 30 Oct 2014, at 17:30, Mathieu Arnold <mat at FreeBSD.org> wrote:
> 
> 
> 
> +--On 30 octobre 2014 12:50:18 +0100 Enrico Maria Crisostomo
> <enrico.m.crisostomo at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4142 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-doc/attachments/20141030/aff2b47c/attachment.bin>