Skip to content

fix(license): restore the MIT disclaimer, credit third-party components, ship the file - #1437

Merged
msluszniak merged 1 commit into
mainfrom
@ms/license-third-party
Sep 8, 2026
Merged

fix(license): restore the MIT disclaimer, credit third-party components, ship the file#1437
msluszniak merged 1 commit into
mainfrom
@ms/license-third-party

Conversation

@msluszniak

Copy link
Copy Markdown
Member

Description

Yes, it needed updating, on three counts.

1. The MIT text was incomplete. The final paragraph was missing, so the grant carried no warranty disclaimer and no limitation of liability. The all-caps block further down belongs to ExecuTorch's BSD notice and is scoped to ExecuTorch. Restored the standard paragraph.

2. The third-party list predates the rewrite. It has not been touched since #506. Only ExecuTorch is credited, but shipped binaries also link:

Component License Where
pffft / FFTPACK BSD 3-Clause legacy/cpp/pfft, compiled on both platforms
OpenCV 4.11 Apache-2.0 vision pipelines, multimodal LLMs
KleidiCV Apache-2.0 OpenCV Arm HAL, Android
ada MIT legacy/cpp/ada
phonemis MIT text-to-speech frontend
MLX MIT MLX backend, iOS

pffft and the Apache-2.0 components require the notice to be reproduced in binary redistributions, which is what an app built on this library is. Every license identifier and copyright line above was taken from the upstream source, not inferred.

3. No published package shipped a license at all. npm only packs a LICENSE that sits in the package directory, and ours lives at the repo root, so every tarball went out with "license": "MIT" in the manifest and no license text. Verified with npm pack --dry-run: 0 matches before, 21.8kB LICENSE after. create-package.sh now copies it in for the pack and removes it again; the three satellite packages get their own plain MIT copy.

Left the copyright year at 2024, which is the year of first publication and does not need bumping.

The third-party section is a judgment call about what has to be credited, so it is worth a second opinion before merging.

Introduces a breaking change?

  • Yes
  • No

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Other (chores, tests, code style improvements etc.)

Tested on

  • iOS
  • Android

Testing instructions

cd packages/react-native-executorch
cp ../../LICENSE LICENSE && npm pack --dry-run --ignore-scripts | grep LICENSE && rm LICENSE

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

…ts, ship the file

The MIT text was missing its final paragraph, so the grant carried no
warranty disclaimer and no limitation of liability. Restored it verbatim.

LICENSE also still described the dependency set from before the rewrite:
only ExecuTorch was credited, while shipped binaries also link pffft,
OpenCV, KleidiCV, ada, phonemis and MLX. Added each with its license.

Finally, npm only packs a LICENSE that sits in the package directory and
ours lives at the repo root, so every published tarball went out with no
license text at all despite declaring "license": "MIT". create-package.sh
now copies it in for the pack, and the three satellite packages carry
their own plain MIT copy.
@msluszniak
msluszniak merged commit e2fc074 into main Sep 8, 2026
4 checks passed
@msluszniak
msluszniak deleted the @ms/license-third-party branch September 8, 2026 07:35
msluszniak added a commit that referenced this pull request Sep 8, 2026
…g published (#1440)

> [!WARNING]
> **Do not merge until 0.10.0 is released.** This touches the publish
path, and there is no reason to carry that risk the day before shipping.

## Description

The publish job ran `yarn install` at the repo root, whose workspaces
glob is `packages/*` + `apps/*` + `apps/legacy/*`. Building one package
pulled dependencies for eight React Native example apps that are never
built.

From the 0.10.0 dry run:

| step | time |
| --- | --- |
| Install monorepo dependencies | 164s |
| Build package (bob + tsc) | 19s |
| everything else | 14s |

`yarn workspaces focus` installs only the package's own tree. It is
built into Yarn 4, the same command family as the `workspaces foreach`
already used in CI, so no plugin is needed.

| | packages | download |
| --- | --- | --- |
| `yarn install` (root) | 1659 | 597.4 MiB |
| `yarn workspaces focus` | 709 | 71.29 MiB |

The satellite workflow had the same problem: it passed `yarn install
--immutable && yarn workspace react-native-executorch prepare` as its
install command, so each of the three runs paid for the whole monorepo
too. Each satellite declares `react-native-executorch` as a
`workspace:*` devDependency, so focusing on the satellite pulls the core
workspace in with it and the core `prepare` still runs.

Two smaller fixes alongside it:

- `RNET_SKIP_DOWNLOAD=1`. The job runs bob and tsc, and the `files` list
excludes `third-party/include` from the tarball, so the postinstall
native download bought nothing and printed two "coreml is iOS-only"
warnings on a Linux runner.
- The install ran twice: once in the workflow, then again in
`create-package.sh`. The script now skips it when `RNE_SKIP_INSTALL` is
set, so a local run is unchanged.

### Also: the satellite tarballs shipped a build cache

Found while reading the 0.10.0 bare-fetcher dry run. All three
satellites pointed `tsBuildInfoFile` at
`./lib/typescript/tsconfig.tsbuildinfo`, inside `outDir`, and their
`files` list ships all of `lib/`. So every published tarball carried
TypeScript's incremental build cache: 1 of 18 files and roughly a third
of the packed bytes, useless to consumers.

Moved it to `.cache/`, which is outside `outDir` and already gitignored.
Rebuilt and packed locally: 18 files becomes 17, nothing else changes.
The core package was never affected, it sets no `tsBuildInfoFile`.

### Also: the dist-tag guard let through every tag it documented

The `dist-tag` override validated with `^[a-zA-Z][a-zA-Z0-9._-]*$`, on
the theory that requiring a leading letter ruled out semver. npm's
actual rule is that a dist-tag must not parse as a semver *range*, and
the input's own documented example fails it:

| input | old guard | npm |
| --- | --- | --- |
| `v0.8` (the documented example) | passes | rejects, parses as `>=0.8.0
<0.9.0-0` |
| `v0.10.0` | passes | rejects, parses as `0.10.0` |
| `latest`, `legacy`, `v0-8` | passes | fine |

The 0.10.0 dry run hit this, failing at npm ten steps in, after the
build and pack. Now it asks `semver` and names a form that works.
Verified against 11 inputs: `v0.10.0`, `v0.10`, `v0.8`, `0.8`, `1.x` and
`*` are rejected with the parsed range in the message; `latest`,
`legacy`, `executorch-nightly`, `v0-8` and `next` pass. `semver`
resolves from the repo root under the focused install this branch
introduces, and the check runs after that step.

The input description also said "e.g. v0.8" and never said the field is
the npm dist-tag rather than the git tag to publish from. Both fixed.

### Trade-off

`focus` does not accept `--immutable`, so this job stops asserting that
the lockfile is in sync. CI already asserts it on every pull request,
and a release publishes from a branch that passed. Verified separately
that `focus` leaves `yarn.lock` untouched, so nothing is silently
rewritten.

### Introduces a breaking change?

- [ ] Yes
- [x] No

### Type of change

- [ ] Bug fix (change which fixes an issue)
- [ ] New feature (change which adds functionality)
- [ ] Documentation update (improves or adds clarity to existing
documentation)
- [x] Other (chores, tests, code style improvements etc.)

### Tested on

- [ ] iOS
- [ ] Android

### Testing instructions

Locally, a focused install plus `yarn prepare` produces a `lib/`
identical to the one in the published 0.10.0 tarball: 496 files, same
paths. The full install measured locally at 1660 packages / 597.08 MiB /
1.6 GB on disk, matching what CI reports. For the satellites, each was
verified from a clean `node_modules`: the core `prepare` succeeds and
every satellite's `tsc` exits 0. Every static import in `src/` and
`legacy/src/` resolves from the package's own dependencies
(`@kesha-antonov/react-native-background-downloader` is a runtime
`require()` in a try/catch, so tsc never needs it).

The full pack could not be run locally because `create-package.sh`
checks out the phonemis submodule, which needs git-lfs. **Before
merging, run the core publish workflow from this branch with
`release-type: latest` and `dry-run: true`, and confirm the tarball is
still 1020 files / 1.5 MB** (1021 with `LICENSE` once #1437 lands).

### Checklist

- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [x] My changes generate no new warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore PRs that are chores

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants