[Refractor:System] Optimize install script: skip unneeded builds by jndlansh · Pull Request #12751 · Submitty/Submitty

jndlansh · 2026-04-05T16:58:34Z

Why is this Change Important & Necessary?

The submitty_install_site build script was extremely slow due to several inefficiencies:

Running full frontend builds (npm/esbuild) even when frontend files didn't change
Vendor directory permissions using find -exec chmod per-file (28,018 seconds on shared folders)
Using rsync compression flag -z which adds unnecessary CPU overhead
Pre-deleting entire ts/ and vue/ directories on every run

This optimization allows full reinstalls to complete in 96.6% less time when frontend files haven't changed (477s → 16s).

What is the New Behavior?

Key optimizations implemented:

Frontend Change Detection: Parse rsync itemized output to detect actual changes in site/ts/, site/vue/, site/package*.json/site/vite.config.* ,site/.build.js and site/tsconfig.json files. Only run npm build if changes detected.
Vendor Permissions Parallelization: Replace find ... -exec chmod with chmod -R + xargs batch operations (28,000x speedup on shared folders)
Removed Pre-deletion: Eliminated pre-deletion of ts/ and vue/ directories; rsync now handles incremental updates correctly
Rsync Optimization: Remove -z compression flag (5s+ savings)
Correct Deletion Handling : Use rsync --delete to remove stale frontend source files from installation.

Behavior changes:

When no frontend changes detected: Entire npm install and npm run build phases are skipped (logs: "Skipping esbuild (no frontend changes detected)") and when no composer changes detected, vendor permissions limited to vendor/composer only
When frontend changes present: Full build runs as before
All other functionality remains identical

What steps should a reviewer take to reproduce or test the bug or new feature?

Test 1: Verify skip behavior (no frontend changes)

time /usr/local/submitty/GIT_CHECKOUT/Submitty/.setup/install_submitty/install_site.sh config=/usr/local/submitty/config
# Should see: "Skipping esbuild (no frontend changes detected)"
# Expected: 15-20 seconds total
# Make a frontend change
echo "// test" >> /usr/local/submitty/site/vue/src/components/NotificationsDisplay.vue

time /usr/local/submitty/GIT_CHECKOUT/Submitty/.setup/install_submitty/install_site.sh config=/usr/local/submitty/config
# Should see: npm run build executing (300-400 seconds)
# Revert the change
git -C /usr/local/submitty/GIT_CHECKOUT/Submitty checkout site/vue/src/components/NotificationsDisplay.vue

time /usr/local/submitty/GIT_CHECKOUT/Submitty/.setup/install_submitty/install_site.sh config=/usr/local/submitty/config
# Should see: "Skipping esbuild (no frontend changes detected)" again

# Run install when composer unchanged - should complete faster
time /usr/local/submitty/GIT_CHECKOUT/Submitty/.setup/install_submitty/install_site.sh config=/usr/local/submitty/config
# Second run should be ~40% faster than first due to vendor/composer-only permissions

Automated Testing & Documentation

Manual validation completed: Tested in Vagrant environment with multiple scenarios (no changes, with changes, after reverting)
No unit tests applicable: This is build optimization for installation script
No documentation updates needed: Script behavior is backward-compatible
Performance benchmarks verified: 96.6% improvement on no-change installs in real Vagrant environment

Other information

Not a breaking change: All functionality preserved; only performance improved
No migrations required: Pure optimization of existing script

…arallelize vendor permissions

GarvitKhandelwal31 · 2026-04-05T19:35:17Z

This PR removes ts/vue directory cleanup and also does not add rsync deletion mode, it then conditionally skips frontend build based on rsync output.
That combination can leave stale source files/artifacts in place and skip rebuild when deletions are the relevant change. This could be a problem.

jndlansh · 2026-04-06T05:37:39Z

thanks for pointing this out, I have addressed this issue

GarvitKhandelwal31 · 2026-04-06T06:08:03Z

I think your change for addressing the deleted fronted-file issue is solid.
One remaining concern that I have-
This PR appears to reintroduce broad vendor-wide permission work, which can undo part of the optimization merged in #12692.

In #12692, the no-composer-change path narrowed permission updates to vendor/composer only to reduce repeated heavy traversal. In this PR, I see unconditional vendor-wide chmod again after the composer block.
Could we preserve the #12692 behavior or maybe prove equivalent or better performance in this respect.
And also it would be great if you could also update the description of the PR in correspondence with the recent changes.

jndlansh · 2026-04-06T07:02:00Z

Now in sync with PR #12692, Vendor permissions now only traverse the full vendor directory when composer.json or composer.lock actually changed. Otherwise only vendor/composer is updated, preserving the lighter performance path.

GarvitKhandelwal31 · 2026-04-06T11:35:09Z

The frontend change detector seems incomplete. It is only checking site/ts/* , site/vue/* , site/package.json and site/vite.config , but the actual build also depends on vue-tsc and node.build.js from package.json . That would mean changes to tsconfig.json or build.js could still incorrectly skip the frontend is build.

Rest I have also checked your fix for my previous concern and that looks solid to me.

GarvitKhandelwal31 · 2026-04-06T18:28:53Z

Great working with you on this. I don't find any other issue with this just the fact with this is maintainability. Bcoz maybe be in future if the frontend build starts depending on more files later, the trigger list will need updating.

jndlansh · 2026-04-06T18:33:17Z

Thanks @GarvitKhandelwal31, you pointed some quality concerns and issues, hope the maintainers give it a review!

dagemcn · 2026-04-07T16:43:27Z

Please resolve conflicts before we review this PR.

jndlansh · 2026-04-07T17:24:06Z

Hi @dagemcn
I have resolved conflicts and updated the PR description.

codecov · 2026-04-07T19:51:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.64%. Comparing base (b2db10a) to head (32f2998).

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #12751   +/-   ##
=========================================
  Coverage     21.64%   21.64%           
  Complexity     9654     9654           
=========================================
  Files           268      268           
  Lines         36244    36244           
  Branches        487      487           
=========================================
  Hits           7845     7845           
  Misses        27916    27916           
  Partials        483      483

Flag	Coverage Δ
autograder	`21.32% <ø> (ø)`
js	`2.04% <ø> (ø)`
migrator	`100.00% <ø> (ø)`
php	`20.66% <ø> (ø)`
python_submitty_utils	`80.08% <ø> (ø)`
submitty_daemon_jobs	`91.13% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

GarvitKhandelwal31 · 2026-04-08T04:51:39Z

Great working with you on this. I don't find any other issue with this just the fact with this is maintainability. Bcoz maybe be in future if the frontend build starts depending on more files later, the trigger list will need updating.

I want to further iterate on this. The frontend build inputs are defined in one place but the rebuild trigger list is maintained in a different place, so every future build-input change requires manual sync in two locations.
What I mean to say is-
Build definition is in package.json
Trigger logic is in install_site.sh (the list that you have)
Its clearly a technical debt.

Fix that can be done-

Create one single source of truth for trigger paths.
Use only that block in detection logic.
Add on CI check that fails if know build-critical files are missing from the block.

…dd CI check to enforce required build-critical paths remain in sync

jndlansh · 2026-04-08T18:58:08Z

It makes sense. Now trigger paths are declared once and consumed by detection logic. Added respective CI check to It verify that the known build-critical paths are present in the trigger block.

williamjallen · 2026-04-11T00:47:34Z

Unfortunately we're already working on an alternative solution for this and are not able to consider other contributions towards this goal at this time.

jndlansh · 2026-04-11T05:24:01Z

Unfortunately we're already working on an alternative solution for this and are not able to consider other contributions towards this goal at this time.

Okay @williamjallen, that's alright!
I shall work on migrating the UI from twig/jQuery -> Vue.js

Optimize submitty_install_site: skip frontend builds on no changes, p…

0d182ec

…arallelize vendor permissions

github-project-automation Bot added this to Submitty Development Apr 5, 2026

github-project-automation Bot moved this to Seeking Reviewer in Submitty Development Apr 5, 2026

jndlansh mentioned this pull request Apr 5, 2026

[Refactor:System] Only chmod on vendor directory changes #12692

Merged

Fix frontend build trigger for deleted files

59ba5ad

Preserve Submitty#12692 vendor permission optimization

f456e81

Add .build.js and tsconfig.json to frontend change patterns

e0433a5

Merge branch 'main' into system/build-script

32f2998

jndlansh changed the title ~~[System:Refractor] Optimize install script: skip unneeded builds~~ [Refractor:System] Optimize install script: skip unneeded builds Apr 7, 2026

Centralize frontend build trigger paths to single source of truth & a…

0d33d06

…dd CI check to enforce required build-critical paths remain in sync

williamjallen closed this Apr 11, 2026

github-project-automation Bot moved this from Seeking Reviewer to Done in Submitty Development Apr 11, 2026

Uh oh!

Conversation

jndlansh commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why is this Change Important & Necessary?

What is the New Behavior?

What steps should a reviewer take to reproduce or test the bug or new feature?

Automated Testing & Documentation

Other information

Uh oh!

GarvitKhandelwal31 commented Apr 5, 2026

Uh oh!

jndlansh commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GarvitKhandelwal31 commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jndlansh commented Apr 6, 2026

Uh oh!

GarvitKhandelwal31 commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GarvitKhandelwal31 commented Apr 6, 2026

Uh oh!

jndlansh commented Apr 6, 2026

Uh oh!

dagemcn commented Apr 7, 2026

Uh oh!

jndlansh commented Apr 7, 2026

Uh oh!

codecov Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GarvitKhandelwal31 commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jndlansh commented Apr 8, 2026

Uh oh!

williamjallen commented Apr 11, 2026

Uh oh!

jndlansh commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jndlansh commented Apr 5, 2026 •

edited

Loading

jndlansh commented Apr 6, 2026 •

edited

Loading

GarvitKhandelwal31 commented Apr 6, 2026 •

edited

Loading

GarvitKhandelwal31 commented Apr 6, 2026 •

edited

Loading

codecov Bot commented Apr 7, 2026 •

edited

Loading

GarvitKhandelwal31 commented Apr 8, 2026 •

edited

Loading