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

tools for analyzing, updating and adding copyright headers in source files #8674

Merged
merged 1 commit into from Nov 2, 2016

Conversation

Projects
None yet
6 participants
@isle2983
Copy link
Contributor

commented Sep 7, 2016

Three scripts in contrib/devtools/ are introduced:

copyright_header_report.py

  • outputs a report of copyright claims in the source files
  • helps to identify files that are missing copyright
  • helps to identify files that were added that have a new copyright holder beyond the current known set
  • helps to find typos in the copyright header

copyright_header_update.py

  • replaces fix-copyright-headers.py
  • does file editing in native python rather than an external call to perl

copyright_header_insert.py

  • inserts a properly-formatted header with proper dates into a given source file

Altogether, I hope they make the task of managing the copyright headers a bit
easier.

@fanquake fanquake added the Docs label Sep 7, 2016

@fanquake

This comment has been minimized.

Copy link
Member

commented Sep 7, 2016

I'm not sure that maintaining copyright headers is such an issue that it warrants splitting one tool into three. Interested in other opinions.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Sep 7, 2016

Agree with @fanquake: I'd prefer to have the functionality (report and update) in a single script.

Also, I don't think copyright_header_insert.py is required. (If someone forgets to add the header, they will forget to run the script as well)

@isle2983 isle2983 force-pushed the isle2983:copyright-scripts branch to 74f2e24 Sep 8, 2016

@isle2983

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2016

I have condensed the three files into one and made them subcommands:

$ ./copyright_header_report.py <base_directory> [verbose]
$ ./copyright_header_update.py <base_directory>
$ ./copyright_header_report.py <file>

becomes:

$ ./copyright_header.py report <base_directory> [verbose]
$ ./copyright_header.py update <base_directory>
$ ./copyright_header.py insert <file>

running with no subcommand to copyright_header.py lists a usage string with the subcommands. Running a subcommand without arguments displays the subcommands usage string.

If we are against the 'insert' functionality, I will remove it, however I think
it is useful for a few things not mentioned:

  1. It defines the exact format of a proper header
  2. It gets the date right
  3. It can be a building block of a strict CI system that rejects bad/missing headers and directs you to "please run./copyright_header.py insert <your file>" to fix the problem.
@fanquake

This comment has been minimized.

Copy link
Member

commented Sep 9, 2016

You'll also need to update the contrib docs here.

[devtools] script support for managing source file copyright headers
Three subcommands to this script:

1) ./copyright_header.py report

Examines git-tracked files with extensions that match:

INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.py']

Helps to:

-> Identify source files without copyright
-> Identify source files added with something other than "The Bitcoin Core
developers" holder so we can be sure it is appropriate
-> Identify unintentional typos in the copyright line

2) ./copyright_header.py update

Replaces fix-copyright-headers.py. It does file editing in native python
rather than subprocessing out to perl as was the case with
fix-copyright-headers.py. It also shares code with the 'report' functions.

3) ./copyright_header.py insert

Inserts a copyright header into a source file with the proper format and
dates.

@isle2983 isle2983 force-pushed the isle2983:copyright-scripts branch from 74f2e24 to 159597a Sep 10, 2016

@isle2983

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2016

Thanks @fanquake. 159597a includes an update to README.md to document the new script and subcommands. Also, I tweaked some grammar and fixed some spelling in the usage strings inside copyright_header.py for clarity and made sure they matched README.md in content.

Rendered README.md

@isle2983 isle2983 changed the title tools for analyzing, updating and adding copyright headers in souce files tools for analyzing, updating and adding copyright headers in source files Sep 10, 2016

@laanwj

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

Concept ACK 159597a

]
EXCLUDE_COMPILED = re.compile('|'.join([fnmatch.translate(m) for m in EXCLUDE]))

INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.py']

This comment has been minimized.

Copy link
@luke-jr

luke-jr Sep 21, 2016

Member

Probably should include *.sh, *.am, *.m4, and *.include

Might make more sense as an exclusion list...

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Sep 21, 2016

Member

We don't use cc

This comment has been minimized.

Copy link
@laanwj

laanwj Sep 22, 2016

Member

Leveldb does, though, but yes it makes no sense to include that here.

@laanwj laanwj merged commit 159597a into bitcoin:master Nov 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Nov 2, 2016

Merge #8674: tools for analyzing, updating and adding copyright heade…
…rs in source files

159597a [devtools] script support for managing source file copyright headers (isle2983)
@rebroad

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2016

Who should run this and when?

@isle2983

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2016

Good question.

I have a working branch that hooks it into TravisCI to fail branches that are missing copyright headers - forcing the submitter to either add them or to manually exclude the file from the check. I plan on submitting the PR when I get the chance to polish it a bit more

If the overall feeling is that this kind of thing is helpful and not a pain for day-to-day work, there might be utility in doing similar scripts for TravisCI to gradually ratchet in enforcement of basic whitespace rules and/or eventually clang-format and/or pylint style checks.

Also, the years in the headers need to be upgraded every so often to make sure they are current. December 31st might be a good day to run the script and post the PR.

@laanwj

This comment has been minimized.

Copy link
Member

commented Dec 21, 2016

Who should run this and when?

  • The best time to update a copyright header is when you touch the file
  • The second-best is a single mass update at the end of the year of all the files that were touched in that year (that's what the scripting is for).

sickpig referenced this pull request in sickpig/BitcoinUnlimited Jan 10, 2018

[devtools] script support for managing source file copyright headers
This is a port of Core #8674

Three subcommands to this script:

1) ./copyright_header.py report

Examines git-tracked files with extensions that match:

INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.py']

Helps to:

-> Identify source files without copyright
-> Identify source files added with something other than "The Bitcoin Core
developers" holder so we can be sure it is appropriate
-> Identify unintentional typos in the copyright line

2) ./copyright_header.py update

Replaces fix-copyright-headers.py. It does file editing in native python
rather than subprocessing out to perl as was the case with
fix-copyright-headers.py. It also shares code with the 'report' functions.

3) ./copyright_header.py insert

Inserts a copyright header into a source file with the proper format and
dates.

gandrewstone referenced this pull request in BitcoinUnlimited/BitcoinUnlimited Jan 11, 2018

Port copyright tool from Core repo. (#895)
* [devtools] script support for managing source file copyright headers

This is a port of Core #8674

Three subcommands to this script:

1) ./copyright_header.py report

Examines git-tracked files with extensions that match:

INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.py']

Helps to:

-> Identify source files without copyright
-> Identify source files added with something other than "The Bitcoin Core
developers" holder so we can be sure it is appropriate
-> Identify unintentional typos in the copyright line

2) ./copyright_header.py update

Replaces fix-copyright-headers.py. It does file editing in native python
rather than subprocessing out to perl as was the case with
fix-copyright-headers.py. It also shares code with the 'report' functions.

3) ./copyright_header.py insert

Inserts a copyright header into a source file with the proper format and
dates.

* [scripts] Add missing univalue file to copyright_header.py

* Adapt copyright_header.py to the BU repo

- add a new set of expected copyright holders
- define "The Bitcoin Unlimited developers" as the default copyright
holder

codablock added a commit to codablock/dash that referenced this pull request Jan 13, 2018

Merge bitcoin#8674: tools for analyzing, updating and adding copyrigh…
…t headers in source files

159597a [devtools] script support for managing source file copyright headers (isle2983)

andvgal added a commit to energicryptocurrency/energi that referenced this pull request Jan 6, 2019

Merge bitcoin#8674: tools for analyzing, updating and adding copyrigh…
…t headers in source files

159597a [devtools] script support for managing source file copyright headers (isle2983)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.