Bug 215761 - refactor do-patch
Summary: refactor do-patch
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Mathieu Arnold
URL: https://reviews.freebsd.org/D9029
Depends on:
Blocks: 215726
  Show dependency treegraph
Reported: 2017-01-04 11:39 UTC by Mathieu Arnold
Modified: 2017-01-16 16:51 UTC (History)
3 users (show)

See Also:
mat: exp-run?

v1 (7.32 KB, patch)
2017-01-04 11:39 UTC, Mathieu Arnold
no flags Details | Diff
v2 (9.14 KB, patch)
2017-01-10 12:07 UTC, Mathieu Arnold
no flags Details | Diff
v3 (9.64 KB, patch)
2017-01-11 14:22 UTC, Mathieu Arnold
no flags Details | Diff
v4 (20.42 KB, patch)
2017-01-11 15:00 UTC, Mathieu Arnold
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Arnold freebsd_committer 2017-01-04 11:39:45 UTC
Created attachment 178501 [details]

I tested all the edge cases I could think of, and find, I don't expect much failures.
Comment 1 Antoine Brodin freebsd_committer 2017-01-09 12:36:00 UTC
There is a problem with the handling of strip flags

emulators/xen-kernel and sysutils/xen-tools fail to patch for instance.
Comment 2 Mathieu Arnold freebsd_committer 2017-01-09 14:14:56 UTC
there must have been a bug in the old code, with some variables being left over from the previous patch, fixing these two.
Comment 3 Mathieu Arnold freebsd_committer 2017-01-09 15:04:36 UTC
Yeah, the strip instruction was not wiped between patches, so the -p of one watch would be applied to the next if it did not have one.
Comment 4 Mathieu Arnold freebsd_committer 2017-01-10 12:07:16 UTC
Created attachment 178702 [details]
Comment 6 Mathieu Arnold freebsd_committer 2017-01-11 14:22:50 UTC
Created attachment 178747 [details]

Spaces in a WRKSRC directory where not an edge case I found, fixed. (along with a couple of other issues)
Comment 7 Antoine Brodin freebsd_committer 2017-01-11 14:37:30 UTC
There are some ports that use or set PATCH_ARGS and PATCH_DIST_ARGS directly
Comment 8 Mathieu Arnold freebsd_committer 2017-01-11 14:41:30 UTC
Ah, yes, I had it on my todo list and forgot about those.
Comment 9 Adam Weinberger freebsd_committer 2017-01-11 14:44:00 UTC
(In reply to Antoine Brodin from comment #7)

Is it worth also checking for Makefile.local in the external tree? It makes sense to want those in VCS too and it'd allow for altering PATCH_ARGS at the same time.
Comment 10 Mathieu Arnold freebsd_committer 2017-01-11 15:00:14 UTC
Created attachment 178749 [details]
Comment 11 Mathieu Arnold freebsd_committer 2017-01-11 15:02:30 UTC
I don't think it is, and in any way, it would be a different changeset.
Comment 12 Mathieu Arnold freebsd_committer 2017-01-11 15:05:26 UTC
Also, I don't remember anyone asking for that.
In a general case, we try to not add functionalities nobody asks or want, because once we have them, we have to maintain them :-)
Comment 13 Antoine Brodin freebsd_committer 2017-01-14 19:49:47 UTC
Exp-run looks fine.
Comment 14 commit-hook freebsd_committer 2017-01-16 16:48:16 UTC
A commit references this bug:

Author: mat
Date: Mon Jan 16 16:47:02 UTC 2017
New revision: 431680
URL: https://svnweb.freebsd.org/changeset/ports/431680

  Extract do-patch into a separate script.

  PR:		215761
  Submitted by:	mat
  Exp-run by:	antoine
  Sponsored by:	Absolight
  Differential Revision:	https://reviews.freebsd.org/D9029