feat(developer): source files from remote URLs #12336
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
keymanapp-test-bot
bot
added
the
user-test-missing
User Test ResultsTest specification and instructions ERROR: user tests have not yet been defined Test Artifacts
|
github-actions
bot
added
developer/
common/
developer/compilers/
common/web/
feat
labels
|
@mhosken @DavidLRowe @LornaSIL feedback appreciated -- both in terms of FLO integration and in terms of referencing external fonts. This will feed into eventual deprecation of the majority of the shared fonts folder, hopefully. There will still always be maintenance in updating font versions in the repo, but there is potential for that to be more scriptable in future with this change. |
mcdurdin
commented
Oct 17, 2024
Comment on lines
4
to
5
| import followRedirects from 'follow-redirects'; | ||
| const { https } = followRedirects; |
There was a problem hiding this comment.
network access needs to be in callbacks
TEMP: this is very much not ready for production; opening as a draft for public comment. I will split the 'file.description' removal into a separate PR later. The concept here is that the 'Name' property for a file can now be a remote reference, rather than a local file. There are two supported formats in this commit: * GitHub: This is a cutdown version of a plain github.com URL, and must match this exact format: ``` github:<owner>/<repo>/raw/<hash>/<filepath/filename> ``` This format is mandated in order to ensure that we always have a hashed version of a file from the origin. This gives us reproducible builds, which avoids churn issues when font files change. Example: `github:silnrsi/fonts/raw/b88c7af5d16681bd137156929ff8baec82526560/fonts/sil/alkalami/Alkalami-Regular.ttf` gets https://github.com/silnrsi/fonts/raw/b88c7af5d16681bd137156929ff8baec82526560/fonts/sil/alkalami/Alkalami-Regular.ttf An alternative could be to just have `https://github.com/silnrsi/fonts/raw/b88c7af5d16681bd137156929ff8baec82526560/fonts/sil/alkalami/Alkalami-Regular.ttf` which could be matched with a regex in the same way as the `github` prefix, and would avoid the need to munge the input URL. **Discuss!** * fonts.languagetechnology.org: references just a font identifier. This is somewhat broken, because if the source file changes, we don't know about it and won't publish an updated version of the package. So this needs some more discussion (we could e.g. embed the version number in the request, e.g. `flo:[email protected]`). **Discuss!** ``` flo:<family> ``` e.g. `flo:andika` gets https://fonts.languagetechnology.org/fonts/sil/andika/Andika-Bold.ttf Future sources could be considered, e.g. noto. We don't want to allow arbitrary URLs, both for stability and for security reasons. This change is entirely compiler-side, so we don't need to make any changes to apps, and so packages will be backwardly compatible. A lot of work will need to be done with the Package Editor in TIKE to support this feature. Fixes: #11236
mcdurdin
force-pushed
the
feat/developer/kmc-package-source-files-from-remotes
branch
from
1a0161c
to
6119759
Compare
|
Replaced by #12667. |
mcdurdin
deleted the
feat/developer/kmc-package-source-files-from-remotes
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TEMP: this is very much not ready for production; opening as a draft for public comment.
I will split the 'file.description' removal into a separate PR later.
The concept here is that the 'Name' property for a file can now be a remote reference, rather than a local file. There are two supported formats in this commit:
GitHub: This is a cutdown version of a plain github.com URL, and must match this exact format:
This format is mandated in order to ensure that we always have a hashed version of a file from the origin. This gives us reproducible builds, which avoids churn issues when font files change.
Example:
github:silnrsi/fonts/raw/b88c7af5d16681bd137156929ff8baec82526560/fonts/sil/alkalami/Alkalami-Regular.ttfgets https://github.com/silnrsi/fonts/raw/b88c7af5d16681bd137156929ff8baec82526560/fonts/sil/alkalami/Alkalami-Regular.ttfAn alternative could be to just have
https://github.com/silnrsi/fonts/raw/b88c7af5d16681bd137156929ff8baec82526560/fonts/sil/alkalami/Alkalami-Regular.ttfwhich could be matched with a regex in the same way as thegithubprefix, and would avoid the need to munge the input URL. Discuss!fonts.languagetechnology.org: references just a font identifier. This is somewhat broken, because if the source file changes, we don't know about it and won't publish an updated version of the package. So this needs some more discussion (we could e.g. embed the version number in the request, e.g.
flo:[email protected]). Discuss!e.g.
flo:andikagets https://fonts.languagetechnology.org/fonts/sil/andika/Andika-Bold.ttfFuture sources could be considered, e.g. noto. We don't want to allow arbitrary URLs, both for stability and for security reasons.
This change is entirely compiler-side, so we don't need to make any changes to apps, and so packages will be backwardly compatible. A lot of work will need to be done with the Package Editor in TIKE to support this feature.
Fixes: #11236