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.

Sign up for GitHub

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

Jump to bottom

port_binary_distributable: more verbose description of failures #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

port_binary_distributable: more verbose description of failures #14

wants to merge 1 commit into from
+26 −10

Conversation

danchr
Copy link
Member

@danchr danchr commented Aug 18, 2020 •
edited
Loading

Please note that this only describes the first such instance of any failure, but that's better than not describing it at all, I'd say. An example:

"emacs-app" is not distributable because its license "gpl" conflicts with license "OpenSSL": librsvg -> cairo -> xorg-libXext -> xorg-libX11 -> xorg-libxcb -> python38 -> openssl

Fixes: https://trac.macports.org/ticket/60995

UPDATED

Now describes all failures, for example:

$ ./port_binary_distributable.tcl -v ffmpeg +nonfree
"ffmpeg" is not distributable because its license "Restrictive" is not known to be distributable
"ffmpeg" is not distributable because its dependency "libfdk-aac" has license "Restrictive" which is not known to be distributable: libfdk-aac
$ ./port_binary_distributable.tcl -v mplayer +glx +faac
"mplayer" is not distributable because its dependency "faac" has license "Restrictive" which is not known to be distributable: faac
"mplayer" is not distributable because its license "GPL" conflicts with license "OpenSSL": mesa -> xorg-libxcb -> python38 -> openssl
"mplayer" is not distributable because its license "GPL" conflicts with license "SSLeay": mesa -> xorg-libxcb -> python38 -> openssl

@danchr
port_binary_distributable: more verbose description of failures
df1d5d6 
I've changed this to describe all reasons why a port isn't
distributable, and properly capitalise license names. An
example:

$ ./port_binary_distributable.tcl -v mplayer +glx +faac
"mplayer" is not distributable because its dependency "faac" has license "Restrictive" which is not known to be distributable: faac
"mplayer" is not distributable because its license "GPL" conflicts with license "OpenSSL": mesa -> xorg-libxcb -> python38 -> openssl
"mplayer" is not distributable because its license "GPL" conflicts with license "SSLeay": mesa -> xorg-libxcb -> python38 -> openssl

Fixes: https://trac.macports.org/ticket/60995
@danchr danchr requested a review from ryandesign August 21, 2020 11:02
@ryandesign
Copy link
Contributor

@jmroot Do you have any concerns about this change? I'd like to have this more verbose output available, but I haven't analyzed the code to determine if this introduces any regressions.

@jmroot
Copy link
Member

jmroot commented Sep 14, 2020

I haven't had a chance to look at it. No idea if it's correct or not at this point, sorry.

@danchr
Copy link
Member Author

danchr commented Sep 14, 2020

FWIW I made a go at moving this to a command in port — it appears to work reasonably well. The main difference between that and the command here is that I ripped out the database support, as it didn't seem a good fit for port, and probably doesn't make that much of a difference in practice, considering you have to build ports anyway.

jmroot
jmroot requested changes Oct 19, 2020
View reviewed changes
Copy link
Member

@jmroot jmroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an option to turn on this new mode of operation, which we might call something like "explain mode", and don't do all the extra work if it is not turned on.

jobs/distributable_lib.tcl
@@ -203,6 +203,7 @@ proc remove_version {license} {

proc check_licenses {portName variantInfo} {
array set portSeen {}
set failures {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use [list] so the variable is a pure list.

jobs/distributable_lib.tcl
}
}

# start with deps of top-level port
set portPaths [dict create [lindex $top_info 0] [list]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really need a dict rather than just an array? And what's the purpose of adding an entry for the whole list of dependencies?

jobs/distributable_lib.tcl
# mark as seen and remove from the list
set portSeen($aPort) 1
set portList [lreplace $portList 0 0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this makes the comment above incorrect.

jobs/distributable_lib.tcl
while {[llength $portList] > 0} {
set aPort [lindex $portList 0]
set portList [lreplace $portList 0 0]
if {[info exists portSeen($aPort)] && $portSeen($aPort) eq 1} {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eq is for string comparisons and strings should always be quoted in expressions. However there's really no need to check the value at all.

@mascguy
Copy link
Member

mascguy commented May 11, 2021

This would be great to finish and merge.

Dan, are you still interested in working on this, or would you prefer that someone take this over?

@danchr
Copy link
Member Author

danchr commented May 16, 2021

Dan, are you still interested in working on this, or would you prefer that someone take this over?

Please do pick this up!

@mascguy
Copy link
Member

mascguy commented May 16, 2021

Dan, are you still interested in working on this, or would you prefer that someone take this over?

Please do pick this up!

Sounds good. And thanks again for your hard work, it'll be great to see it used!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmroot jmroot jmroot requested changes

@ryandesign ryandesign Awaiting requested review from ryandesign

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danchr @ryandesign @jmroot @mascguy