net-p2p/qbittorrent: Updated to 4.0.4; net-p2p/qbittorrent-nox: Deprecated, converted into a FLAVOR=nogui in net-p2p/qbittorrent
ClosedPublic

Authored by yuri on Dec 7 2017, 9:38 PM.

Details

Summary

Deprecated net-p2p/qbittorrent-nox in favor of the flavor in net-p2p/qbittorrent

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
yuri created this revision.Dec 7 2017, 9:38 PM
yuri added a comment.Dec 7 2017, 9:40 PM

arc doesn't support deleted directories, otherwise, net-p2p/qbittorrent-nox is deleted.

mat added a comment.Dec 7 2017, 11:34 PM

The previous port was -nox11, not -nox, the flavor should probably be nox11, not nox

In D13416#280279, @yuri wrote:

arc doesn't support deleted directories, otherwise, net-p2p/qbittorrent-nox is deleted.

If you are using Subversion directly, you need to svn rm the directory for it to show as deleted.

net-p2p/qbittorrent/Makefile
30 ↗(On Diff #36367)

why += ?

32 ↗(On Diff #36367)
39 ↗(On Diff #36367)

Why move the DOCS here ? Does the "nox" flavor not have DOCS ?

45 ↗(On Diff #36367)

This was called -nox11 before, it should stay that way.

46 ↗(On Diff #36367)

why := ?

net-p2p/qbittorrent/pkg-plist
31 ↗(On Diff #36367)

empty line

yuri added a comment.Dec 8 2017, 12:19 AM
In D13416#280303, @mat wrote:

The previous port was -nox11, not -nox, the flavor should probably be nox11, not nox

In D13416#280279, @yuri wrote:

arc doesn't support deleted directories, otherwise, net-p2p/qbittorrent-nox is deleted.

If you are using Subversion directly, you need to svn rm the directory for it to show as deleted.

That's what I did, and I added the deleted directory to the command line, and it still doesn't show in phabricator.

yuri updated this revision to Diff 36375.Dec 8 2017, 4:55 AM
yuri marked 6 inline comments as done.

.

yuri added inline comments.Dec 8 2017, 4:55 AM
net-p2p/qbittorrent/Makefile
39 ↗(On Diff #36367)

It has been this way for -nox11 port. Fixed.

46 ↗(On Diff #36367)

It has been this way. Fixed.

net-p2p/qbittorrent/pkg-plist
31 ↗(On Diff #36367)
yuri marked 3 inline comments as done.Dec 8 2017, 4:56 AM

Why did you rename the flavor to nogui?

yuri added a comment.EditedDec 8 2017, 6:35 AM

Why did you rename the flavor to nogui?

nogui reflects the meaning better. Their option is called --disable-gui. I also suggested to upstream to change suffix to -nogui.
They are developing something called Wayland which is an alternative to Xorg. nogui will cover all GUI cases.

yuri retitled this revision from net-p2p/qbittorrent: Updated to 4.0.2; net-p2p/qbittorrent-nox: Removed/deprecated, converted into a FLAVOR=nox in net-p2p/qbittorrent to net-p2p/qbittorrent: Updated to 4.0.2; net-p2p/qbittorrent-nox: Deprecated, converted into a FLAVOR=nogui in net-p2p/qbittorrent.Dec 8 2017, 7:53 AM
mat added inline comments.Dec 8 2017, 9:17 AM
net-p2p/qbittorrent/Makefile
20–35 ↗(On Diff #36380)

I would use the flavor helpers for LIB_DEPENDS, PKGNAMESUFFIX and PLIST and keep them here, and move the rest after the USES block and before options. As Chapter 14. Order of Variables in Port Makefiles says.

yuri marked an inline comment as done.Dec 8 2017, 10:35 AM
yuri added inline comments.
net-p2p/qbittorrent/Makefile
20–35 ↗(On Diff #36380)

If nogui_LIB_DEPENDS, nogui_PKGNAMESUFFIX and nogui_PLIST are kept here, but the rest with ifs is moved after the USES block, wouldn't this be a violation of Chapter 14. Order of Variables in Port Makefiles? It says that flavors should be before the USES block.

Besides, shouldn't CONFIGURE_ARGS and PLIST_FILES also have corresponding flavor helpers?

yuri marked 2 inline comments as done.Dec 8 2017, 10:35 AM
mat added inline comments.Dec 8 2017, 10:45 AM
net-p2p/qbittorrent/Makefile
20–35 ↗(On Diff #36380)

I am quite sure this chapter does not say what you are saying. Mostly because I am the one who wrote it.

I know that there are two different paragraphs in 15.7 Flavors but the second paragraph is only one sentence long and is sort of answering exactly what you are asking.

yuri updated this revision to Diff 39435.Feb 17 2018, 7:58 PM
  • Update to 4.0.4
  • Improved flacors section as suggested by Mat.
yuri marked an inline comment as done.Feb 17 2018, 7:58 PM
yuri retitled this revision from net-p2p/qbittorrent: Updated to 4.0.2; net-p2p/qbittorrent-nox: Deprecated, converted into a FLAVOR=nogui in net-p2p/qbittorrent to net-p2p/qbittorrent: Updated to 4.0.4; net-p2p/qbittorrent-nox: Deprecated, converted into a FLAVOR=nogui in net-p2p/qbittorrent.

net-p2p/qbittorrent-nox11 is not removed.

tcberner added inline comments.Feb 21 2018, 6:54 AM
net-p2p/qbittorrent/Makefile
10 ↗(On Diff #39555)

Qt5 only

tcberner accepted this revision.Feb 21 2018, 7:20 AM

I find it a bit confusing that you chose to have a different flavour name than pkgnamesuffix...

This revision is now accepted and ready to land.Feb 21 2018, 7:20 AM
yuri marked an inline comment as done.Feb 21 2018, 8:09 AM
yuri added a comment.EditedFeb 21 2018, 9:19 AM

@mat Could you please unblock this? Subversion says that portmgr needs to unblock due to the FLAVORS use.

mat added a comment.Feb 21 2018, 9:27 AM

I find it a bit confusing that you chose to have a different flavour name than pkgnamesuffix...

So do I. I think it would be much better to not have three different strings, nogui, nox11 and nox. Pick one, and use it everywhere.

yuri updated this revision to Diff 39559.Feb 21 2018, 9:37 AM

I choose nogui because it reflects the meaning best (GUI doesn't have to be X11).

This revision now requires review to proceed.Feb 21 2018, 9:37 AM
yuri added a comment.Feb 22 2018, 6:41 PM

Subversion says that FLAVORS can't committed without portmgr approval.

yuri added a comment.Feb 25 2018, 6:48 PM

ping (subversion blocks the commit due to flavors)

mat added a comment.Feb 27 2018, 3:31 PM

Have you forgotten to update this review with the changes I requested ?

yuri added a comment.Apr 8 2018, 6:36 AM

I believe this is the latest version.

mat added inline comments.Apr 11 2018, 8:36 AM
net-p2p/qbittorrent/Makefile
19 ↗(On Diff #39559)

+=?

26 ↗(On Diff #39559)

+=?

48 ↗(On Diff #39559)

not +=?

mat added inline comments.Mon, Jun 18, 10:35 AM
net-p2p/qbittorrent/Makefile
39–51 ↗(On Diff #43933)

This should happen earlier.

mat accepted this revision.Mon, Jun 18, 4:01 PM
This revision is now accepted and ready to land.Mon, Jun 18, 4:01 PM
yuri added a comment.Mon, Jun 18, 7:09 PM

Commit still fails:

Committing transaction...
svn: E165001: Commit failed (details follow):
svn: E165001: Commit blocked by pre-commit hook (exit code 1) with output:
Do not commit a port with FLAVORS without first
getting approval from portmgr.
mat added a comment.Wed, Jun 20, 8:45 AM
In D13416#336036, @yuri wrote:

Commit still fails:

Committing transaction...
svn: E165001: Commit failed (details follow):
svn: E165001: Commit blocked by pre-commit hook (exit code 1) with output:
Do not commit a port with FLAVORS without first
getting approval from portmgr.

You got approval from portmgr. Did you say so in your commit message? If you did not, the script cannot even begin to know it.

yuri added a comment.Wed, Jun 20, 8:52 AM

Thanks.
I didn't know that it works through the Approved By value.

This revision was automatically updated to reflect the committed changes.