Skip to content

Conversation

casteryh
Copy link
Contributor

Summary

  • Added a check for existing build/ directory before installing the forge package in .meta/mast/env_setup.sh
  • When detected, the script warns the user that the directory may contain artifacts from a previous pip installation that could interfere with the current installation
  • User is prompted to continue or cancel the installation
  • If they continue, they are warned that things might go wrong and given instructions on how to manually remove the directory

Test plan

  • Run the env_setup.sh script with and without an existing build/ directory to verify the warning and prompt work correctly
  • Verify that the user can cancel the installation if they choose
  • Verify that the user can continue with the installation if they choose

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 16, 2025
The env_setup.sh script now checks for an existing build/ directory
before installing the forge package. If found, it warns the user that
the directory may contain artifacts from a previous pip installation
that could interfere with the current installation.

The user is prompted to continue or cancel, with instructions on how
to manually remove the build/ directory if they encounter issues.
@casteryh casteryh force-pushed the warn-existing-build-dir branch from f1596eb to ed14fae Compare October 16, 2025 20:55
@casteryh casteryh requested a review from allenwang28 October 16, 2025 20:56
log_warn "Detected existing build/ directory at: $(pwd)/build"
log_warn "This directory may contain artifacts from a previous pip installation"
log_warn "that could interfere with the current installation."
log_warn "If you encounter issues, manually remove it with: rm -rf build"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the risk of always deleting it all? lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

idk lol

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.63%. Comparing base (633b219) to head (ed14fae).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #441      +/-   ##
==========================================
- Coverage   64.69%   64.63%   -0.06%     
==========================================
  Files          79       79              
  Lines        7775     7788      +13     
==========================================
+ Hits         5030     5034       +4     
- Misses       2745     2754       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@casteryh casteryh merged commit b89a4a0 into main Oct 16, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants