Skip to content

[Test PR] See if fuzzing is working again #8

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

Closed
wants to merge 1 commit into from
Closed

Conversation

EliahKagan
Copy link
Owner

@EliahKagan EliahKagan commented Jan 18, 2025

a.k.a. Revert "deactivate fuzzing as it's not building anymore for unrelated reasons"

Like #1, this is a fork-internal PR for testing fuzzing.

There, it was necessary to open a PR on the upstream repo, in order to test whether a change to the code here would help, since the job won't actually check out and use code from a PR whose base branch (target) is in a fork. But so far, here, I'm mainly testing whether the problem has gone away by itself due to a new nightly Rust toolchain, so it might not end up being necessary to open an investigatory PR upstream before knowing if things are likely to work.

This reverts commit a661a8a.

By the way, the automated CodeQL review comments are due to another thing I'm testing out in this fork: experimental CodeQL scanning for GitHub Actions workflows. There are tradeoffs associated with pinning all actions at specific commit hashes, and I am unsure if that is really a better approach overall for GitoxideLabs/gitoxide. But that's what the comments are recommending. Those comments are conceptually unrelated to what I am trying to test here.

steps:
- name: Build Fuzzers
id: build
uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'CIFuzz' step
Uses Step: build
uses 'google/oss-fuzz/infra/cifuzz/actions/build_fuzzers' with ref 'master', not a pinned commit hash
language: rust

- name: Run Fuzzers
uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'CIFuzz' step
Uses Step
uses 'google/oss-fuzz/infra/cifuzz/actions/run_fuzzers' with ref 'master', not a pinned commit hash
Copy link
Owner Author

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

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

This seems to work, completing three times reporting success and with nothing particularly alarming jumping out in the logs. But I think it's not quite equivalent to how it works when it's a PR on the upstream repository. I'll close this and open a PR there, and see how the CI run goes.

@EliahKagan EliahKagan closed this Jan 18, 2025
@EliahKagan EliahKagan deleted the fuzz-next branch January 18, 2025 18:54
EliahKagan added a commit that referenced this pull request Apr 10, 2025
This uses the standard Apache-2.0 license file, available for
download from https://www.apache.org/licenses/LICENSE-2.0.txt, as
`LICENSE-APACHE`. The license text itself is unchanged, but this
fixes the broken license appendix. The appendix is expressly not
part of the licese terms, so probably nothing very bad would happen
due to it, but it is better either to have the appendix, or to omit
it entirely, than to have only some fragments of it.

At the very beginning, gitoxide was licensed under only the MIT
license. Early on, all contributors agreed to dual-license gitoxide
under the MIT license and the Apache-2.0 license (#8), and an
Apache-2.0 license file was added (ea353eb).

The standard Apache-2.0 license file ends in an appendix that
describes the usual way of explicitly applying it to code, which
contains placeholders which are meant to be substituted if the
boilerplate code is copied elsewhere, but not in the license file
appendix itself. However, in ea353eb, the placeholders were
substituted in the appendix in the license file itself, and the
more instructional portion of the appendix was removed.

This modification to the appendix, which created an unusual license
file, may have been done in order to put the copyright notice
somewhere where it would be specifically associated with the
Apache-2.0 license option. After all, the boilerplate text wasn't
(and intentionally continues not to be) used as a header in source
code files as the appendix suggests. But this carried two problems:

1. It was potentially confusing with respect to the significance of
   that text, since it was not present anywhere a copyright notice
   would be expected, and came after "END OF TERMS AND CONDITIONS".

2. It had the potential to confuse tooling that processed licenses.

One specific case of (2) is known, described in 76ae5d6 (GitoxideLabs#1232)
where the license file was changed to remove it. (The MIT license
file was also changed to remove the copyright notice, but it is
unlikely that the MIT license file contributed to tooling problems,
since the copyright line is expected in an MIT license file and is
typically present.)

That left fragments of the appendix in the Apache-2.0 license file,
which no longer even attempts to give any information more specific
than that which is present in the licese terms themselves.

There are two good ways to fix the problem. One of them is to use
the standard version of the Apache-2.0 license file, with the full
original appendix with instructions and unsubstituted placeholders
intact. That approach is followed here, since adding the missing
pieces of the standard appendix makes clear what the current
nonstandard fragment is from.

(The other approach is to remove the appendix altogether, which is
fine to do since it is expressly not part of the license terms, and
which seems to be fairly popularly done among Rust projects. We may
end up going with that, but for clarity it's not done just yet.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant