Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add licensecheck target, copied from macports-infrastructure #211

Closed
wants to merge 1 commit into from

Conversation

danchr
Copy link
Member

@danchr danchr commented Oct 14, 2020

This essentially copies over port_binary_distributable.tcl and distributable_lib.tcl from the infrastructure project, but with the database support removed as it seemed a poor fit for the general structure of MacPorts. My main motivation is that this is rather nice information to have, and that it would help answer the question why a given port doesn't have binaries. As is, you have to either clone the infrastructure project, or check the build logs for that.

@jmroot
Copy link
Member

jmroot commented Oct 14, 2020

I'm not a fan of the "kitchen sink" approach of adding every feature to the port command itself. But more importantly, this will get out of sync with what is actually being used to decide whether to distribute binaries. The license db is important for buildbot performance, and the library/front-end split is needed for other scripts to use the functionality, so buildbot can't switch to using this port action.

@danchr
Copy link
Member Author

danchr commented Oct 15, 2020

I'm not a fan of the "kitchen sink" approach of adding every feature to the port command itself.

Normally, I'd agree, but I really don't see another location where we could put this to make it useful and discoverable for porters.

But more importantly, this will get out of sync with what is actually being used to decide whether to distribute binaries. The license db is important for buildbot performance, and the library/front-end split is needed for other scripts to use the functionality, so buildbot can't switch to using this port action.

Yeah, I had a feeling this was incomplete; I mostly wrote it on a whim after finding the port_binary_distributable.tcl command, and thinking that it was a bit obscure and hidden. The use of the library is interesting though; searching GitHub, I found one other use. Is that it, or do you know others?

The reason I tore it out was that I really couldn't find a suitable location for the database, and having a database argument to a port command seemed rather odd. Would the change be acceptable if I, say, moved the database into ${prefix}, used it by default and kept the library?

Personally, I find the ability to check why there are no archives for a given port quite useful; archives are great 🙂

(One slightly unrelated thought I had was that the proper name for this command would actually be distcheck — but then I noticed that port already had such a command. I suppose one option would be to reinstate the library, and add this to distcheck, but that does feel a bit iffy.)

@ryandesign
Copy link
Contributor

Indeed the ability for users to check if a port is distributable is useful. I use a shell script wrapper around the port command in which I have mapped this functionality to port distributable. I am not certain whether "distributable" or "licensecheck" is a better name for this command.

port distcheck "Check[s] if a port can be fetched from all of its mirrors", so that's not related to and should not be combined with your new license checking feature.

If the problems are resolved and this PR is accepted, it would close https://trac.macports.org/ticket/60315.

The existing lists of distributable licenses and license conflicts, as well as a new list of known nondistributable licenses (to implement https://trac.macports.org/ticket/60316), should be abstracted out into data files that get stored in _resources/port1.0/ in the ports tree so that they can be updated without requiring a new release of MacPorts base.

@kurthindenburg
Copy link
Contributor

As Ryan mentioned in https://trac.macports.org/ticket/60315, having the license data in the ports tree under _resources would be better as it could be updated w/o new macport release.

@mascguy
Copy link
Member

mascguy commented May 15, 2021

I'm not a fan of the "kitchen sink" approach of adding every feature to the port command itself. But more importantly, this will get out of sync with what is actually being used to decide whether to distribute binaries. The license db is important for buildbot performance, and the library/front-end split is needed for other scripts to use the functionality, so buildbot can't switch to using this port action.

@jmr Josh, if the license data itself is moved to _resources, would you be willing to reconsider?

After trying in vain to figure out the license chain for ffmpeg, and diagnose why binaries weren't publishable for MacOS 10.6, I finally rebuilt MacPorts with Dan's licensecheck addition. And sure enough, it provided us the answers we needed.

See: https://trac.macports.org/ticket/54633#comment:14

Your folks' thoughts?

@mascguy
Copy link
Member

mascguy commented May 15, 2021

FYI, I've updated Dan's work to source license data from _resources. All of my changes reside in branches named mascguy-licensecheck, across both repos.

Base repo:
macports/macports-base@master...mascguy:mascguy-licensecheck

Ports repo, including updated BASH completions:
macports/macports-ports@master...mascguy:mascguy-licensecheck

@danchr
Copy link
Member Author

danchr commented May 16, 2021

Thanks for picking this up!

@danchr danchr closed this May 16, 2021
@danchr danchr deleted the licensecheck branch May 16, 2021 13:39
@mascguy
Copy link
Member

mascguy commented May 16, 2021

Thanks for picking this up!

No problem, thanks for all of your great work on this Dan!

@jmroot
Copy link
Member

jmroot commented May 16, 2021

@jmr Josh, if the license data itself is moved to _resources, would you be willing to reconsider?

That would partially solve one of the three issues I raised.

  • There needs to be a single source of truth for distributability checking, both data and logic. It's pointless having port tell you something if it might be different to what the buildbot thinks.
  • The core functionality needs to remain a library that can be used by other scripts.
  • The license_db stuff needs to continue to exist for performance.

After trying in vain to figure out the license chain for ffmpeg, and diagnose why binaries weren't publishable for MacOS 10.6, I finally rebuilt MacPorts with Dan's licensecheck addition. And sure enough, it provided us the answers we needed.

Better human-readable reporting of license conflict reasons is a separate issue (and a separate PR IIRC).

@mascguy
Copy link
Member

mascguy commented May 17, 2021

In terms of sharing the data and logic, can the MacPorts infrastructure scripts utilize MacPorts base? (For example, simplifying distributable_lib.tcl to delegate the license check to port licensecheck.)

That would alleviate all of your concerns, correct?

@mascguy
Copy link
Member

mascguy commented May 18, 2021

And if we don't want the infrastructure scripts to depend on MacPorts base, where would the shared functionality be sourced from?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants