Page MenuHomeFreeBSD

Update libproxy to 0.4.15
ClosedPublic

Authored by tcberner on Jun 3 2018, 7:41 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 8, 6:47 AM
Unknown Object (File)
Sun, Dec 8, 4:13 AM
Unknown Object (File)
Sat, Nov 30, 2:08 PM
Unknown Object (File)
Sat, Nov 30, 7:45 AM
Unknown Object (File)
Mon, Nov 25, 9:19 PM
Unknown Object (File)
Mon, Nov 25, 9:58 AM
Unknown Object (File)
Sun, Nov 24, 6:46 PM
Unknown Object (File)
Sun, Nov 24, 6:50 AM
Subscribers

Details

Reviewers
kwm
Group Reviewers
gnome
portmgr
Commits
rP473062: Update net/libproxy to 0.4.15
Summary
  • Update
  • Clean up the master portfile
  • Add support for
    • python3
    • webkit2-gtk3
  • Move libproxy-gnome to libproxy-gnome2
  • I would like some input on how to do the PYTHON part properly, as it looks rather hacky at the moment.
  • libproxy-mozjs is broken on 10.4
Test Plan
  • exp-run

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 17010
Build 16878: arc lint + arc unit

Event Timeline

@mat, do you have an idea how to do the python stuff nicer?

@mat, do you have an idea how to do the python stuff nicer?

Can you elaborate, what part do you want to make nicer?

net/libproxy/Makefile
69

@mat line 75--90

net/libproxy-python/Makefile
11

Python?

net/libproxy/Makefile
28

This is a bit wrong, you define options, and then you say all options are excluded. The current implementation handles this by enforcing the _SLAVE before enforcing the _EXCLUDE but this could change. This should probably not exclude the current set option.

net/libproxy/Makefile
72

This should probably go in the Python slave port.

74

Having two plists is probably not required, USE_PYTHON=py3kplist should handle it.

  • Cleanup python part
  • Don't exclude the current option.

Side note, I don't really like abusing OPTIONS with a vengeance like this, I think it was better before with the LIBPROXY_SLAVE variable.

In D15655#330950, @mat wrote:

Side note, I don't really like abusing OPTIONS with a vengeance like this, I think it was better before with the LIBPROXY_SLAVE variable.

Isn't that exactly what OPTIONS_EXLUDE is for?

In D15655#330950, @mat wrote:

Side note, I don't really like abusing OPTIONS with a vengeance like this, I think it was better before with the LIBPROXY_SLAVE variable.

Isn't that exactly what OPTIONS_EXLUDE is for?

No, OPTIONS_EXCLUDE/SLAVE is for when you have "some condition" when a port only supports a subset the the master port options.
For example, most of OPTIONS_EXCLUDE_<arch>=DTRACE, or the remaining ports with a -nox11 slave port where the master port has options.

Here, in the end, none of the ports have options. Thus what I said about really abusing the options framework to gain a few helpers that are not really needed.

net/libproxy/Makefile
38–39

Both these should be WITH_${OPTIONS_SLAVE}.

net/libproxy/Makefile
38–39

Mmmm, no, only the first, but the way this is done is so convoluted that it should probably not be done this way.

tcberner added inline comments.
net/libproxy/Makefile
38–39

I see, your not a fan of my cleverness ... I'll revert it to the old way

Macro failboat:

:)

Light visual inspection only, seems mat@ already pointed out all the framework issues.

Macro lgtm:

Also feel free to axe the "$MCom:" lines, those where from the old GNOME svn development repo.

net/libproxy-kde/Makefile
4

Don't forget to reset this one.

Without fancy option magic... :)
... and with working mozjs on 10.4

Fix CMAKE_ARGS for libproxy-webkit3.

  • Now they all should build fine on 10.4@32 and 12.0@64.

In all the slave ports, the LIBPROXY_SLAVE definition is in a bad place, it should go along the MASTERDIR definition.

Also, I think all the ${.CURDIR:H:H}/net/ might be replaced by ${.CURDIR}/../ as they are all in the net category, I find it more readable.

net/libproxy-python/Makefile
23–25

This should happen before the target.

Move variables around, use CURDIR/..

There are still two .CURDIR:H:H occurrences in the master port. :-)

net/libproxy/Makefile
38–39

Oh, no, I love clever people, but you have to remember that you are not the only one who will work on any given port. If it takes more a minute or two to understand what is happening, it could probably be done in an easier way.

In this case, now that I have thought more about these 2 lines (so more like 30 minutes of thinking) looking at it, I think using options, only the _BOOL was needed, because when the options was excluded, it would be "off" and the WITH_FOO=OFF would be added. (I may be wrong, though.)

So being clever is nice, but the more clever you think you are, the more you can create bugs that will take longer to figure out, and the more you, yourself will wonder wtf you wanted to do when you did this change 18 months ago.

Change to .CURDIR in masterport and fix the others.

Mmmm, I had not seen that in the master port, the CURDIR:H were before any include, and thus PORTSDIR did not exist, sorry, you should change back the CURDIR/.. with CURDIR:H :(

Other than that, looks good.

CURDIR:H in the masterport again.

I was looking at the LIBPROXY_SLAVES variable, and there is two places where you use it directly in its uppercased form, and three dozen where you use the lowercased version, it could probably be simplified by making its content lowercase and use :tu in the two places where an uppercase version is actually needed.

Use lowercase names for LIBPROXY_SLAVES

my guess is your nexst suggestion is to get rid of LIBPROXY_SLAVE and see if PKGNAMESUFFIX is set? :D

mat edited reviewers, added: portmgr; removed: mat.

my guess is your nexst suggestion is to get rid of LIBPROXY_SLAVE and see if PKGNAMESUFFIX is set? :D

Not sure, I would probably go the other way and set PKGNAMESUFFIX in the master Makefile.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 22 2018, 7:59 PM
This revision was automatically updated to reflect the committed changes.