Page MenuHomeFreeBSD

sysutils/burp-devel: Update to 2.3.24
ClosedPublic

Authored by salvadore on Mar 17 2020, 9:54 PM.

Details

Summary
  • Protocol 2: warn and skip on verify/restore of unsupported file types.
  • Resurrect, improve and use sysutils/burp/files/burp.in instead of the rc script from distfile, both for sysutils/burp (master port) and sysutils/burp-devel (slave port): this is necessary because, while sysutils/burp's distfile still distributes the script, sysutils/burp-devel's distfile does not anymore starting with version 2.3.24. I also made a few modifications to the file so that it behaves well with non standard PREFIX values, licensing the resulting file (originally in public domain) under BSD2CLAUSE, under my copyright name and maintaining a reference to the original script.

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.

Event Timeline

There's two aspects I'd love some people with more expertise to chime in:

The rc script as such - it's an existing script coming from a new source plus some improvements, so unless someone else is able to review within, say, a week or ten days, I am fine.

The license change. The current license is fine for us, and I am not sure you can simply put a more restrictive license on something in the public domain (but in any case: what's the advantage)? If you want to proceed with that, we'd need to consult some legal experts.

The license change. The current license is fine for us, and I am not sure you can simply put a more restrictive license on something in the public domain (but in any case: what's the advantage)? If you want to proceed with that, we'd need to consult some legal experts.

Disclaimer: I am not a laywer.

The problem is that I edited the script and I think that as an Italian and French citizen I get intellectual property of my edits automatically, probably due do Berne convention. I am unsure I can simply renounce to that property, but if I can I should state it.
See for example article 6bis from Berne convention: https://www.law.cornell.edu/treaties/berne/6bis.html . Apparently authors always keep some rights ("the right to claim authorship of the work and to object to any distortion, mutilation or other modification of, or other derogatory action in relation to, the said work, which would be prejudicial to his honor or reputation"), so I am unsure I can release my edits under PD.

Moreover, at the moment the edits are very minimal, but in the future the changes might increase in number and I think a BSD2CLAUSE license is preferable for the same reason we prefer BSD2CLAUSE for FreeBSD rather than PD.

Legally, I think anything can be done on a PD work without restriction (that's how I interpret public domain), relicensing included. And I did maintain a reference to the original script in case someone prefers to use it rather than the edited relicensed version. If preferable, I can add an URL to that reference: https://github.com/grke/burp/blob/0ef17869257f81bbc2f32a032c0340b00a849e1f/freebsd/rc.d/burp .

Another strategy can be to deal with this problem later (if ever) in the hope that changes stay minimal: we can leave the script as is in the ports tree and patch it through Makefile at installation time.

I added portmgr as reviewers: maybe they can help on the licensing issue.

and IMHO I would let the license as it is

head/sysutils/burp/Makefile
47–50

you don't need this anymore when using USE_RC_SUBR

head/sysutils/burp/Makefile
47–50

I am not sure if this is necessary with USE_RC_SUBR.

head/sysutils/burp/files/burp.in
7

All rights reserved. is no longer necessary BTW.

See https://spdx.org/licenses/BSD-2-Clause-FreeBSD.html for example.

42

Is it necessary to export PATH here?

64

This is the default.

69

From what I understand, unless there are some special requirements like overriding command_args in precmd functions (e.g., burp_premonitor) then it should be fine to use just command_args instead.

At the end of the day, modifying command_args instead here would be less confusing to future readers.

70

In case it is useful, you may want to take a look at the variables like ${name}_env in rc.subr(8). Maybe it would be nice for this script to support our standard variables like ${name}_env to be passed not only when burp starts but also when the user runs monitor or summary. But this is really something we mostly likely don't need.

My understanding is that you need permission from the original author if you intend to change the license on work they contributed.

BTW, shouldn't all the configuration files in ETCDIR be prefixed with @sample in pkg-plist?

Address received comments

Thanks everyone for your comments!

I fixed USE_RC_SUBR and put the @sample macro for config files as requested.
For the license issue, I choosed to leave the file unmodified in the ports tree and to modify it at installation time through the post-install target: that way we do not distribute any modified file and we avoid the problem. For the same reason, I would prefer to refrain from addressing 0mp's comments about the rc script: while I recognize that they are valuable comments and we would get a better script by following them, nobody reported any issue with the original script ("if it isn't broken don't fix it") while I feel it is preferable to keep post-install action on it as simple as possible. That said, if there is the feeling that those modifications are important I can add them: avoiding them is only a preference, not a need.

When it comes to the script, none of my comments were essential to the proper functioning of the script. They were more about style and future maintenance.

sysutils/burp/Makefile
49 ↗(On Diff #69750)

It makes no sense. The resulting package is still going to contain the modified script. Also, this file is in the public domain so I guess that it can be just included in the ports tree and modified.

Nevertheless, if you are still unsure about the situation, maybe the best way is to contact the foundation and ask for their opinion.

sysutils/burp/pkg-plist
6 ↗(On Diff #69750)

Just to make sure: is it necessary to add "@sample" to %%ETCDIR%%/clientconfdir/incexc/example.sample and %%ETCDIR%%/clientconfdir/testclient.sample? They are examples anyway, right? So they are never picked up by the application anyway, I guess.

You're right 0mp, I forgot about the binary packages...
I asked for help to the FreeBSD Foundation as you suggested. I am still confused about this topic.

I am starting to consider the hypothesis to get rid of the rc script altogether and writing a new one, unrelated to the original that I can thus license freely... This would also allow to address your comments on the original script with more freedom.

By the way, I just realized that up to now we have indeed distributed through binary packages the modified rc script, without any declaration on copyrights and license for these modifications. And we regularly do that in the ports tree for any port that patches the files it fetches. I guess then that it would be fine to just continue doing that?

By the way, I just realized that up to now we have indeed distributed through binary packages the modified rc script, without any declaration on copyrights and license for these modifications. And we regularly do that in the ports tree for any port that patches the files it fetches. I guess then that it would be fine to just continue doing that?

That's pretty much what we do. I guess that this approach works for most pieces of software we have in the ports due to their open-source nature. There are some ports, however, which prohibit us from modifying their certain parts (e.g., https://www.freshports.org/biology/linux-foldingathome),

In case of this port, however, there shouldn't be any problems, I suppose.

By the way, I just realized that up to now we have indeed distributed through binary packages the modified rc script, without any declaration on copyrights and license for these modifications. And we regularly do that in the ports tree for any port that patches the files it fetches. I guess then that it would be fine to just continue doing that?

That has been my position, too, yes - just one I did not want to enforce, but recommend.

Patch updated!

  • For the licensing issue, I did what you all suggested: nothing. I only modified the script where needed, avoiding to touch anything about licenses or copyrights. (I had no answer from the Foundation, I might have written to the wrong address but never mind: let's not annoy them with that issue, you all convinced me.)
  • I removed the @sample for clientconfdir/incexc/example, as it is indeed an example but I kept it for clienconfdir/testclient: I don't think that the latter is an example.
  • I corrected the comment about PORTREVISION.

Can someone please approve this patch (at least one of my mentors) or suggest more improvements?
I do not want to hurry, but I am under the impression that this review has been forgotten (I am starting to forgetting it myself).

Thanks :-)

Sure, this looks good enough to me :)

This revision is now accepted and ready to land.Apr 1 2020, 5:00 PM
This revision was automatically updated to reflect the committed changes.