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

Behavior when no legal comment is included #3670

Closed
TomokiMiyauci opened this issue Feb 27, 2024 · 1 comment
Closed

Behavior when no legal comment is included #3670

TomokiMiyauci opened this issue Feb 27, 2024 · 1 comment
Labels
breaking

Comments

@TomokiMiyauci
Copy link

If legalComments is link or external and the file has no legal comments, an empty entry is output.

This may result in a large number of empty files in some cases.

Why not make it the standard behavior to output nothing if the file does not contain a legal comment?

I don't think we need a new option to control this, what do you think?
@evanw evanw added the breaking label Apr 27, 2024
@martinpitt
Copy link

Indeed in our cockpit project, esbuild generates 61 *.LEGAL.txt files, of which 40 are empty (zero bytes). This mostly applies to *.css files, but there are *.js as well.

martinpitt added a commit to martinpitt/cockpit that referenced this issue Oct 11, 2024
@martinpitt
pkg: Install *.LEGAL.txt as documentation
8e455cb 
These are not really part of the cockpit package data, but
documentation, so put them into /usr/share/doc/cockpit/legal/ instead.

Also, out of the 61 *.LEGAL.txt files that esbuild creates, 40 are empty
(0 bytes). Installing them is just noise, so weed them out. See
evanw/esbuild#3670
jelly pushed a commit to cockpit-project/cockpit that referenced this issue Oct 11, 2024
@martinpitt @jelly
pkg: Install *.LEGAL.txt as documentation
2735cf2 
These are not really part of the cockpit package data, but
documentation, so put them into /usr/share/doc/cockpit/legal/ instead.

Also, out of the 61 *.LEGAL.txt files that esbuild creates, 40 are empty
(0 bytes). Installing them is just noise, so weed them out. See
evanw/esbuild#3670
SludgeGirl pushed a commit to Nykseli/cockpit that referenced this issue Nov 12, 2024
@martinpitt @SludgeGirl
pkg: Install *.LEGAL.txt as documentation
353df74 
These are not really part of the cockpit package data, but
documentation, so put them into /usr/share/doc/cockpit/legal/ instead.

Also, out of the 61 *.LEGAL.txt files that esbuild creates, 40 are empty
(0 bytes). Installing them is just noise, so weed them out. See
evanw/esbuild#3670
@evanw evanw closed this as completed in e5d4303 Feb 2, 2025
@BrewTestBot BrewTestBot mentioned this issue Feb 8, 2025
Merged
@martinpitt martinpitt mentioned this issue Feb 10, 2025
Merged
martinpitt pushed a commit to cockpit-project/cockpit-ostree that referenced this issue Feb 10, 2025
@dependabot @martinpitt
build(deps-dev): bump the esbuild group with 2 updates
04946ff 
Bumps the esbuild group with 2 updates: [esbuild](https://github.com/evanw/esbuild) and [esbuild-wasm](https://github.com/evanw/esbuild).

Updates `esbuild` from 0.24.2 to 0.25.0
- [Release notes](https://github.com/evanw/esbuild/releases)
- [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md)
- [Commits](evanw/esbuild@v0.24.2...v0.25.0)

Updates `esbuild-wasm` from 0.24.2 to 0.25.0
- [Release notes](https://github.com/evanw/esbuild/releases)
- [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md)
- [Commits](evanw/esbuild@v0.24.2...v0.25.0)

---
updated-dependencies:
- dependency-name: esbuild
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: esbuild
- dependency-name: esbuild-wasm
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: esbuild
...

Signed-off-by: dependabot[bot] <[email protected]>

Updated by Martin Pitt <[email protected]>: Drop ostree.css.LEGAL.txt
from the spec. It was an empty file before, and esbuild 0.25.0 skips
these now (evanw/esbuild#3670)
allisonkarlitskaya pushed a commit to cockpit-project/cockpit-ostree that referenced this issue Feb 10, 2025
@dependabot @allisonkarlitskaya
build(deps-dev): bump the esbuild group with 2 updates
b99779b 
Bumps the esbuild group with 2 updates: [esbuild](https://github.com/evanw/esbuild) and [esbuild-wasm](https://github.com/evanw/esbuild).

Updates `esbuild` from 0.24.2 to 0.25.0
- [Release notes](https://github.com/evanw/esbuild/releases)
- [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md)
- [Commits](evanw/esbuild@v0.24.2...v0.25.0)

Updates `esbuild-wasm` from 0.24.2 to 0.25.0
- [Release notes](https://github.com/evanw/esbuild/releases)
- [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md)
- [Commits](evanw/esbuild@v0.24.2...v0.25.0)

---
updated-dependencies:
- dependency-name: esbuild
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: esbuild
- dependency-name: esbuild-wasm
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: esbuild
...

Signed-off-by: dependabot[bot] <[email protected]>

Updated by Martin Pitt <[email protected]>: Drop ostree.css.LEGAL.txt
from the spec. It was an empty file before, and esbuild 0.25.0 skips
these now (evanw/esbuild#3670)
@martinpitt martinpitt mentioned this issue Feb 11, 2025
Merged
@samayer12 samayer12 mentioned this issue Feb 11, 2025
Merged
5 tasks
github-merge-queue bot pushed a commit to defenseunicorns/pepr that referenced this issue Feb 11, 2025
@samayer12 @cmwylie19
chore: bump esbuild version (#1807)
6439684 
## Description

This PR mitigates
[GHSA-67mh-4wv8-2f99](https://vat.dso.mil/vat/finding?name=GHSA-67mh-4wv8-2f99).
We use `esbuild` to bundle the Pepr module, which is during the build
process in the CLI. This code is not accessible during runtime. There is
no attack vector in Pepr vulnerable to this.

Doing this upgrade will avoid us flagging on security scans so that we
do not need to justify a security finding that does not impact us.

This PR also uses parameterized tests to assert on the presence of
expected files after `pepr build --no-embed`.

### Update Consideration

`esbuild:0.25.0` omits legal comment output files when empty, see
[esbuild/#3670](evanw/esbuild#3670).

As a result, pepr now mirrors this behavior with output to `dist/`. It
is unlikely that this will cause issues but is something users should be
aware of, depending upon what they care about in build artifacts.

## Related Issue

Security hotfix.

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging
- [x] Unit,
[Journey](https://github.com/defenseunicorns/pepr/tree/main/journey),
[E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples),
[docs](https://github.com/defenseunicorns/pepr/tree/main/docs),
[adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or
updated as needed
- [x] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Co-authored-by: Case Wylie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@martinpitt @evanw @TomokiMiyauci