ci: add an hook to automatically add OET's SPDX header

[tgilon]

Closes # (if applicable).

Changes proposed in this Pull Request

This PR wants to introduce a new hook to ensure the repository remains compliant with REUSE (see #89). Currently, the hook edits the entire repository rather than just new files.

Hook configuration
Script

Checklist

• I tested my contribution locally and it works as intended.
• Code and workflow changes are sufficiently documented.
• Changed dependencies are added to envs/environment.yaml.
• Changes in configuration options are added in config/config.default.yaml.
• Changes in configuration options are documented in doc/configtables/*.csv.
• Changes in configuration options are added in config/test/*.yaml.
• OET license identifier is added to all edited or newly created code files.
• Sources of newly added data are documented in doc/data_sources.rst.
• A release note doc/release_notes.rst is added.
• Major features are listed in README and doc/index.rst.

[tgilon]

@coroa @euronion Do you have any advices here on how to make this work?

[euronion]

You could use reuse annotate --copyright="Contributors to Open-TYNDP <https://github.com/open-energy-transition/open-tyndp>" for this.

• If you run it on a file, that already has the correct header, it doesn't change anything
• If the file doesn't have the right header, it is added
• Add it to a new pre-commit rule in the .pre-commit-config.yaml
• Limitation: Not sure if this works with pre-commit.ci, because it requires reuse to be installed. Should work locally though. You might be able to convinve the pre-commit.ci to also install the hook properly with its dependencies (not sure how that works). See the original hook for linting here that you could expand on: https://github.com/fsfe/reuse-tool/blob/main/.pre-commit-hooks.yaml , where they provide the hook for lint.

[coroa]

Maybe try something like:

  # Check and auto-fix SPDX copyright headers in modified files
- repo: local
  hooks:
  - id: reuse-annotate-open-tyndp-contrib
    name: reuse annotate open-tyndp contrib
    entry: reuse
    args: ["annotate", "--copyright=Contributors to Open-TYNDP <https://github.com/open-energy-transition/open-tyndp>"]
    language: python
    description: "..."
    additional_dependencies: ["reuse"]
    exclude_types: [binary]
    exclude: ^LICENSES/

[tgilon]

Thank you for both suggestions! I considered using reuse directly in the hook. I expected this to annotate all the files, including those that haven't been modified by the project. Of course, it's not what we need. This is why I came up with the idea of a custom bash script that only executes for modified files. This works locally when committing. In this case, a header is only added to newly modified files. However, running this with pre-commit-ci doesn't work as the header is added to every single file here.

[coroa]

Thank you for both suggestions! I considered using reuse directly in the hook. I expected this to annotate all the files, including those that haven't been modified by the project. Of course, it's not what we need. This is why I came up with the idea of a custom bash script that only executes for modified files. This works locally when committing. In this case, a header is only added to newly modified files. However, running this with pre-commit-ci doesn't work as the header is added to every single file here.

Typically local pre-commit hooks only ever operate on changed files, so i don't think your premise is true. but i have not looked for documentation on that.

The main option you have for making it work with pre-commit.ci is to skip it: https://pre-commit.ci/#configuration-skip

[euronion]

I think that's a difference between local pre-commit hooks and the pre-commit.ci:
The CI doesn't know about staged changes and is always applied to all of the code.

@coroa Option to just not run it in the CI is a simple one.

If you still want to run it in the CI, I would advocate for a solution using reuse, as it saves you the trouble of a script that accounts for existing/non-existing headers and different file formats.
You'd need to change the target of that pre-commit rule to only apply to the files that have been updated in this particular branch after it has been branched of from the main or master.

- repo: local
  hooks:
  - id: reuse-annotate-branch-diff
    name: REUSE annotate on branch diff
    entry: bash -c 'reuse annotate --fallback-dot-license --copyright="<Placeholder>" $(git diff --name-only $(git merge-base HEAD master) HEAD)'
    language: system
    types: [file]

Not sure if this will work with the CI though. Did your bash file work with the CI?

[coroa]

Not sure if this will work with the CI though. Did your bash file work with the CI?

i think a64eb01 says yes :).

i am not sure how/if reuse is installed in your example, though. as a language: python hook, you get, additional_dependencies: [reuse]; maybe that could be combined.

[tgilon]

So I managed to use reuse properly. The only annoying thing is that smk is not supported by default. This requires a specific hook.

Locally, everything works as expected. Online, I ran this test: 2e81da9. pre-commit.ci partially fixed it: 878555f.

What do you think?

[euronion]

I think that's acceptable. This will cover most of the cases. You won't be able to cover all. I guess you noticed this when including the exclude glob for files. Some manual intervention and supervision will always be necessary.

[coroa]

For me that is fine. I think i would prefer to only run it locally, instead of using the git stuff to figure out what changed in a PR, since i expect this to produce false positives regularly, but we can try to run with this and then fine tune over time.

[coroa]

Note also that it is not difficult to make reuse-tool support the smk ending, basically the PR https://github.com/fsfe/reuse-tool/pull/1124/files with .smk.

[euronion]

Note also that it is not difficult to make reuse-tool support the smk ending, basically the PR https://github.com/fsfe/reuse-tool/pull/1124/files with .smk.

.smk files are simple, I think the Snakefile will be tricky ;)

[coroa]

Note also that it is not difficult to make reuse-tool support the smk ending, basically the PR https://github.com/fsfe/reuse-tool/pull/1124/files with .smk.

.smk files are simple, I think the Snakefile will be tricky ;)

True, but, i would count this as the remaining 1% manual adjustments :)

[coroa]

Ah, one thought.

What happens when you merge an updated master branch into an in-process PR? Which commit is the merge-base then and which files will the git diff then label as updated?

Only the PR updated ones or also of the new commits on master?

[euronion]

git diff to the master/main branch should only list the files that were changed in the branch or the files that were changed in the branch and the reference branch (which are also fine).
Non-overlapping files should just be replayed and not appear in git diff, right?

[coroa]

git diff to the master/main branch should only list the files that were changed in the branch or the files that were changed in the branch and the reference branch (which are also fine). Non-overlapping files should just be replayed and not appear in git diff, right?

I think you are right. git merge-base finds the main branch commit that was last merged in (with the changes that thus have made it into the PR already). And the diff then should only show the changes that the PR adds on-top of that.

[tgilon]

PR opened: fsfe/reuse-tool#1206

[coroa]

PR opened: fsfe/reuse-tool#1206

Thanks. Cool that the other files are easy too. You introduced a typo into Snakefile, though.

[tgilon]

Thanks. Cool that the other files are easy too. You introduced a typo into Snakefile, though.

Yes, I saw... It's already fixed

[tgilon]

Locally, everything works as expected. Online, I ran this test: 2e81da9. pre-commit.ci partially fixed it: 878555f.

There is still an issue with the current approach since this test partially fails.

[coroa]

Locally, everything works as expected. Online, I ran this test: 2e81da9. pre-commit.ci partially fixed it: 878555f.

There is still an issue with the current approach since this test partially fails.

Sorry, i did not understand the first time what you mean by the test partially fails and i still don't

[tgilon]

Sorry, i did not understand the first time what you mean by the test partially fails and i still don't

pre-commit-ci adds a header, but only to one of the two files, but not to both.

[tgilon]

Closed in favour of #93 and #94.