Skip to content

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

Merged
bmcutler merged 1 commit into
mainfrom
submitty-install-site-chmod
Apr 1, 2026
Merged

[Refactor:System] Only chmod on vendor directory changes#12692
bmcutler merged 1 commit into
mainfrom
submitty-install-site-chmod

Conversation

@williamjallen

@williamjallen williamjallen commented Mar 29, 2026

Copy link
Copy Markdown
Member

Why is this Change Important & Necessary?

The submitty_install_site script currently performs a recursive chmod on the entire vendor directory every time. Developers using Windows have reported slowness at this point in the script.

What is the New Behavior?

The chmod is only applied to the vendor directory if it has been updated.

@github-project-automation github-project-automation Bot moved this to Seeking Reviewer in Submitty Development Mar 29, 2026
@williamjallen williamjallen changed the title Only chmod on vendor directory changes [Refactor:System] Only chmod on vendor directory changes Mar 29, 2026
@codecov

codecov Bot commented Mar 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.64%. Comparing base (02638f6) to head (ce975cf).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12692   +/-   ##
=========================================
  Coverage     21.64%   21.64%           
  Complexity     9654     9654           
=========================================
  Files           268      268           
  Lines         36245    36245           
  Branches        487      487           
=========================================
  Hits           7846     7846           
  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.

IDzyre
IDzyre approved these changes Mar 31, 2026
View reviewed changes

@IDzyre IDzyre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my machine (windows 11), it takes submitty_install_site from ~30s per run (up to 50 if installing) down to ~18s per run.

@github-project-automation github-project-automation Bot moved this from Seeking Reviewer to Awaiting Maintainer Review in Submitty Development Mar 31, 2026
@IDzyre IDzyre mentioned this pull request Mar 31, 2026
Closed
Rkoester47
Rkoester47 approved these changes Mar 31, 2026
View reviewed changes

@Rkoester47 Rkoester47 left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this branch, my submitty_install_site took 52 seconds (down from 1 minute and 20 seconds on main) and submitty_install took 6 minutes and 10 seconds (down from 7 minutes and 54 seconds on main).

Farhanxikram
Farhanxikram approved these changes Mar 31, 2026
View reviewed changes

@Farhanxikram Farhanxikram left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speed up submitty_install_site for me by a huge margin (from 5 minutes to 20 seconds), Submitty_install is still fluctuating, but i noticed quicker "fastest" times for submitty Install. It's fastest time for me was about 1 minute on this branch, but it averaged about 5 minutes .
Overall this would make Reviewing Pr'/ overall development s much more manageable, as submitty_install_site tends to be plenty for most submitty development :).

@williamjallen williamjallen moved this from Awaiting Maintainer Review to Ready to Merge in Submitty Development Mar 31, 2026
@williamjallen

williamjallen commented Mar 31, 2026

Copy link
Copy Markdown
Member Author

Looking for a final approval from @bmcutler before this gets merged to verify that not changing the permissions every time is acceptable.

@bmcutler
bmcutler merged commit d7b82e3 into main Apr 1, 2026
26 of 28 checks passed
@bmcutler
bmcutler deleted the submitty-install-site-chmod branch April 1, 2026 15:16
@jndlansh

jndlansh commented Apr 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey @williamjallen!
#12751
I have tried to optimise the submitty_install_site script further by overlooking the frontend builds, in case there is no frontend change, plus further optimisations, would you kindly take a look!

@GarvitKhandelwal31 GarvitKhandelwal31 mentioned this pull request Apr 6, 2026
Closed
jndlansh added a commit to jndlansh/Submitty that referenced this pull request Apr 6, 2026
@jndlansh
Preserve Submitty#12692 vendor permission optimization
f456e81
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IDzyre IDzyre IDzyre approved these changes
@Farhanxikram Farhanxikram Farhanxikram approved these changes
@Rkoester47 Rkoester47 Rkoester47 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Submitty Development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@williamjallen @jndlansh @IDzyre @Farhanxikram @Rkoester47 @bmcutler