build: allow symlinks to be used in place of static asset artifacts #34894
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
kdmccormick
force-pushed
the
kdmccormick/assets-split
branch
2 times, most recently
from
8688fd7
to
2f4bcf0
Compare
kdmccormick
changed the title
kdmccormick
force-pushed
the
kdmccormick/assets-split
branch
from
2f4bcf0
to
3b98eb1
Compare
The git-ignored target directory for LMS Sass compilation is:
lms/static/css
Unfortunately, that directory contains git-controlled directory of
vendored-in static assets:
lms/static/css/vendor
This is a problem for a couple reasons:
1. In Tutor, we would like to make lms/static/css a symlink to an
external location for the sake of build efficiency. This is
impossible to do without clobbering lms/static/css/vendor and
dirtying the git state.
2. More generally, when optimizing (or just understanding) a build
system, it adds complexity when git-controlled source directories are
mixed up inside git-ignored target directories.
The solution is to simply merge these vendored-in assets to another
existing git-controlled vendor directory:
common/static/css/vendor
LMS already reads assets from this folder, so no further changes need to
be made. common/static/css is fully git-controlled, so we avoid the
complexity described above.
Remove the trailing slashes from the .gitignore entries for static asset build artifacts. With these slashes, only directory contents are ignored. Without these slashes, the artifact is ignored whether it is a directory *or* an actual file (particularly, in our case: a symlink). This allows us to build edx-platform static assets into a separate directory, and then symlink to them from edx-platform. Also: We remove the duplicate cms/static/css/ gitignore entry.
kdmccormick
force-pushed
the
kdmccormick/assets-split
branch
from
3b98eb1
to
c161fd1
Compare
feanil
approved these changes
Jun 4, 2024
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
This was referenced Jun 11, 2024
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.
Description
This PR supports an experimental optimization to tutor dev mode. Specifically, it will allow us to build static assets outside of edx-platform, and symlink to those externally-built assets from within edx-platform. For example, today, CSS compiled to:
With this change, we could have:
In Tutor, this would allow us to build static assets once, and then re-use them whether the developer wants to run a pre-built edx-platform clone or a bind-mounted local repository (currently, to use a bind-mounted local repo, the developer must re-build the assets locally, wasting time and bandwidth).
Regardless of Tutor, I believe the changes are also generally good for edx-platform's build story.
The changes are broken into two commits:
build: lms/static/css/vendor/* -> common/static/css/vendor
The git-ignored target directory for LMS Sass compilation is:
Unfortunately, that directory contains git-controlled directory of
vendored-in static assets:
This is a problem for a couple reasons:
In Tutor, we would like to make lms/static/css a symlink to an
external location for the sake of build efficiency. This is
impossible to do without clobbering lms/static/css/vendor and
dirtying the git state.
More generally, when optimizing (or just understanding) a build
system, it adds complexity when git-controlled source directories are
mixed up inside git-ignored target directories.
The solution is to simply merge these vendored-in assets to another
existing git-controlled vendor directory:
LMS already reads assets from this folder, so no further changes need to
be made. common/static/css is fully git-controlled, so we avoid the
complexity described above.
build: git-ignore static asset artifacts whether they are links or dirs
Remove the trailing slashes from the .gitignore entries for static asset
build artifacts. With these slashes, only directory contents are
ignored. Without these slashes, the artifact is ignored whether it is a
directory or an actual file (particularly, in our case: a symlink).
This allows us to build edx-platform static assets into a separate
directory, and then symlink to them from edx-platform.
Also: We remove the duplicate cms/static/css/ gitignore entry.
Testing
I've smoke-tested the build and basic legacy LMS/Studio frontend functionality with this branch.
Merge considerations
I'd like to merge this by Jun 6 so that I can backport it to Redwood. That way, we can utilize it as an optimization in minor Tutor v18 (Redwood-based) release.
This should be fully backwards-compatible. This will have zero effect on folks who don't use symlinks for static assets.
Even if the downstream experimental Tutor change does not work out, I think this change will be good to have in edx-platform.