Skip to content

autogenerate NOTICE.BINARY from NOTICE and licenses.yaml#8306

Merged
clintropolis merged 10 commits intoapache:masterfrom
clintropolis:generated-notice.binary
Aug 21, 2019
Merged

autogenerate NOTICE.BINARY from NOTICE and licenses.yaml#8306
clintropolis merged 10 commits intoapache:masterfrom
clintropolis:generated-notice.binary

Conversation

@clintropolis
Copy link
Copy Markdown
Member

@clintropolis clintropolis commented Aug 15, 2019

Description

This PR migrates binary NOTICE entries to live in licenses.yaml, and adds a new script, docs/_bin/generate-notice-binary.py that uses licenses.yaml and NOTICE to generate NOTICE.BINARY at distribution time. Adapted from generate-license.py.

$ distribution/bin/generate-notice-binary.py NOTICE licenses.yaml NOTICE.BINARY

This should make maintaining the notice file a bit easier, but this PR doesn't include anything to automatically check that everything that needs a notice has one in here, so we will still need to manually make sure the notice or notices are set for a dependency for now.

Additionally, all scripts related to performing releases have been moved into distribution/bin from docs/_bin.

…yaml and NOTICE to generate NOTICE.BINARY at distribution time
@clintropolis clintropolis added Apache Items related to being a part of the ASF Area - Dependencies WIP labels Aug 15, 2019
@clintropolis
Copy link
Copy Markdown
Member Author

#8309 and #8310 should potentially be merged and this PR updated prior to getting merged, since those PRs relate to some dependencies that were previously listed in the NOTICE.BINARY that upon looking closer do not appear to be necessary dependencies.

Comment thread docs/_bin/generate-notice-binary.py Outdated
import yaml


outfile = sys.stdout
Copy link
Copy Markdown
Contributor

@ccaominh ccaominh Aug 15, 2019

Choose a reason for hiding this comment

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

When is the value sys.stdout used? out_path is a required positional command-line arg, so this variable should always get initialized to that value. If sys.stdout is not used there, then perhaps print_error should be printing to stdout instead?

Comment thread docs/_bin/generate-notice-binary.py Outdated
outfile = sys.stdout


moduleHeaderLine = "############"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI, you can do something like: moduleHeaderLine = "#" * 12

Comment thread docs/_bin/generate-notice-binary.py Outdated
print(string, file=sys.stderr)

def print_jar(name, version, notice):
print_outfile("{} {}-{}.jar {}".format(dependencyHeaderLine, name, version, dependencyHeaderLine))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider using an f-string instead of string.format(): https://www.python.org/dev/peps/pep-0498/#code-equivalence

print_outfile(f"{dependencyHeaderLine} {name}-{version}.jar {dependencyHeaderLine}")

Comment thread docs/_bin/generate-notice-binary.py Outdated
# Print Apache license first.
print_outfile(source_notice)
with open(dependences_yaml) as registry_file:
dependencies_list = list(yaml.load_all(registry_file))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it make sense to use a set instead of a list? Using a set may also be beneficial for the membership test below (O(1) versus O(n)).

Comment thread docs/_bin/generate-notice-binary.py Outdated
modules_map = {}
for dependency in dependencies_list:
if 'notice' in dependency or 'notices' in dependency:
if dependency['module'] not in modules_map:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider using defaultdict for this pattern: https://docs.python.org/3.7/library/collections.html#defaultdict-examples

from collections import defaultdict
modules_map = defaultdict(list)
for dependency in dependencies_list:
    if 'notice' in dependency or 'notices' in dependency:
        modules_map[dependency['module']].append(dependency)

Comment thread docs/_bin/generate-notice-binary.py Outdated
generate_notice(source_notice, dependencies_yaml)

except KeyboardInterrupt:
print('Interrupted, closing.') No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm guessing the weird character showing up here is because of a missing newline at the end of the file?

source_notice = apache_notice_file.read()
dependencies_yaml = args.license_yaml

with open(args.out_path, "w") as outfile:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI, if it's desired for this to fail if the file already exists, then the "x" mode can be used.

Also, this line has the side-effect of changing the behavior of print_outfile() which is a bit hidden as it goes through the outfile global variable.

Comment thread docs/_bin/generate-notice-binary.py Outdated


def generate_notice(source_notice, dependences_yaml):
# Generate NOTICE.BINARY file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment is redundant with the message string on the next line.

Comment thread docs/_bin/generate-notice-binary.py Outdated
def print_outfile(string):
print(string, file=outfile)

def print_error(string):
Copy link
Copy Markdown
Contributor

@ccaominh ccaominh Aug 15, 2019

Choose a reason for hiding this comment

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

This name is a bit misleading as it's used to print informational messages instead of error messages

@clintropolis clintropolis added this to the 0.16.0 milestone Aug 15, 2019
@clintropolis clintropolis removed the WIP label Aug 17, 2019
Copy link
Copy Markdown
Contributor

@ccaominh ccaominh left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@clintropolis clintropolis removed the WIP label Aug 21, 2019
@clintropolis clintropolis merged commit 010f70b into apache:master Aug 21, 2019
@clintropolis clintropolis deleted the generated-notice.binary branch August 21, 2019 19:46
@clintropolis clintropolis mentioned this pull request Aug 22, 2019
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Apache Items related to being a part of the ASF Area - Dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants