The offensive USE flag seems like a pointless diversion from upstream. Users are not presented with any offensive material unless they specifically fetch it using a relevant URL. This is in contrast to packages like fortune and sudo, where the "offensive" data is installed and may displayed accidentally. As well, the sed hackery seems somewhat fragile. I humbly suggest dropping this USE flag from the ebuild.
(In reply to Mike Gilbert from comment #0) > Users are not presented with any offensive material unless they specifically > fetch it using a relevant URL. Or when they have FEATURES=test. > This is in contrast to packages like fortune and sudo, where the "offensive" > data is installed and may displayed accidentally. In contrast to other packages that have IUSE=offensive, youtube-dl's internal test suite accesses "offensive" sites. > As well, the sed hackery seems somewhat fragile. Patch welcome. > I humbly suggest dropping this USE flag from the ebuild. We would simultaneously need to RESTRICT=test or people who use FEATURES=test + USE=offensive on the job could get fired. We could also set IUSE=+offensive then but that isn't normally done for good reasons.
If the test suite accesses the network, it should be restricted anyway, right?
(In reply to Michael Orlitzky from comment #2) > If the test suite accesses the network, it should be restricted anyway, > right? Is that a thing now?
(In reply to Jeroen Roovers from comment #3) > > Is that a thing now? I don't think it's written down anywhere, but that's roughly the consensus. I can make up a couple of reasons: * When installing a package, you aren't required to have network access. * Tests that require network access can fail due to transient network issues, making the tests (and therefore package installation) unreliable. In particular, youtube-dl supports so many sites that one of them is always broken due to a change in a third-party website. * We have FEATURES=network-sandbox in portage, and it will eventually be enabled by default. There are a large number of bugs open (search: network test) for packages that fail their test suites due to network issues.
I find it strange that you would maintain a USE flag for the small (possible zero) number of people who are installing youtube-dl, have FEATURES=test, and their organization's network policy allows downloading videos in general, just not porn videos. To echo mjo's point, it is generally accepted that ebuilds should not access network resources, with the exception of live ebuilds fetching source code in src_unpack. I do not have any written policy to which to refer, but I did ask around and several of my peers agree with this. I think RESTRICT=test is the way to go here.
The devmanual does have some text advising against network access during ebuild phases. https://devmanual.gentoo.org/ebuild-writing/functions/ If you want to run the tests before committing new versions, a simple workaround would be to comment-out RESTRICT="test", run the tests, and then re-comment it.
Ironically, the test suite works fine with FEATURES=network-sandbox and doesn't access any websites in that case (obviously: it's sandboxed). It simply skips any tests that fail due to network errors. You might want to leave a comment next to the RESTRICT=test to the effect that, if FEATURES=network-sandbox ever becomes default, the test suite can be re-enabled.
(In reply to Michael Orlitzky from comment #7) > You might want to leave a comment next to the RESTRICT=test to the effect > that, if FEATURES=network-sandbox ever becomes default, the test suite can > be re-enabled. Assuming that everyone uses portage with default settings is not a good idea.
(In reply to Mike Gilbert from comment #8) > (In reply to Michael Orlitzky from comment #7) > > You might want to leave a comment next to the RESTRICT=test to the effect > > that, if FEATURES=network-sandbox ever becomes default, the test suite can > > be re-enabled. > > Assuming that everyone uses portage with default settings is not a good idea. I had this thought a moment too late. Even portage users won't necessarily have the network sandbox available, since it requires kernel support. I was able to fake a sandbox by editing test/parameters.json and setting the proxy URL to something bogus on localhost. However, the test suite doesn't respond gracefully to every failure that results from that approach.
(In reply to Mike Gilbert from comment #5) > I find it strange that you would maintain a USE flag for the small (possible > zero) number of people who are installing youtube-dl, have FEATURES=test, > and their organization's network policy allows downloading videos in > general, just not porn videos. Yes, well, if you put it like that, it does look strange indeed. Perhaps you shouldn't put it like that. > I think RESTRICT=test is the way to go here. So now you want to block the useful test suite because you want to remove IUSE=offensive because you think it's "pointless" (despite users apparently using it and then reporting to me when something has changed in the Matrix) or prone to error (which, again, users notice and report because they apparently *do* appreciate the USE flag, and which then gets fixed). You propose its removal without anyone asking you to invest your time in maintaining IUSE=offensive and with ample evidence of users filing bug reports, and at times providing very useful improvements to the `! use offensive` code, and myself fixing the bug reports and thanking users who filed them and who provided patches. youtube-dl is prone to frequent updates due to the upstream sites changing their web APIs on a regular basis, and due to something inexplicably intrinsic to python, none of the mangling it does in various ebuild phases makes it bail out when an "import" fails, which necessitates installing and running the executable to find out whether it broke again, which is time consuming for me and makes me despair at how python works. But you didn't hear me complaining, right? I do regular, nearly weekly updates and try to glean from the upstream change log what new features to check for or sites add or remove from the xxx list. So no.
(In reply to Jeroen Roovers from comment #10) I am suggesting to improve the quality of code in this package by simplifying the ebuild and removing a mis-feature that does not need to be there. It seems like most of the bug reports on this are due to the ebuild breaking with USE="-offensive". https://bugs.gentoo.org/buglist.cgi?quicksearch=ALL%20youtube-dl%20offensive Most users have USE="offensive" disabled because it is disabled by default. That does not indicate a conscious decision on their part. The fact that they report bugs when it breaks does not mean that they want to keep the USE flag there, or even care about it at all. Save yourself and users some time and energy and remove this USE flag!
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=122ab201e342645a4d3aac722fdae303230812c9 commit 122ab201e342645a4d3aac722fdae303230812c9 Author: Jeroen Roovers <jer@gentoo.org> AuthorDate: 2019-01-28 18:26:07 +0000 Commit: Jeroen Roovers <jer@gentoo.org> CommitDate: 2019-01-28 18:30:00 +0000 net-misc/youtube-dl: Remove USE=offensive, add RESTRICT=test Bug: https://bugs.gentoo.org/show_bug.cgi?id=599928 Closes: https://bugs.gentoo.org/show_bug.cgi?id=605010 Closes: https://bugs.gentoo.org/show_bug.cgi?id=635996 Closes: https://bugs.gentoo.org/show_bug.cgi?id=643200 Closes: https://bugs.gentoo.org/show_bug.cgi?id=676532 Package-Manager: Portage-2.3.58, Repoman-2.3.12 Signed-off-by: Jeroen Roovers <jer@gentoo.org> net-misc/youtube-dl/youtube-dl-2019.01.27.ebuild | 59 ++--------------------- net-misc/youtube-dl/youtube-dl-99999999.ebuild | 60 ++---------------------- 2 files changed, 7 insertions(+), 112 deletions(-)
