Skip to content

[Build] Support windows in resolve_buck.py - #4856

Merged
mergennachin merged 2 commits into
pytorch:mainfrom
python3kgae:win_bld_buck2
Jan 8, 2025
Merged

[Build] Support windows in resolve_buck.py#4856
mergennachin merged 2 commits into
pytorch:mainfrom
python3kgae:win_bld_buck2

Conversation

@python3kgae

@python3kgaepython3kgae commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Add Windows support to BUCK_PLATFORM_MAP.
Also use urllib.request.urlretrieve directly to work around PermissionError on Windows.

For #4661

@pytorch-bot

pytorch-botBot commented Aug 23, 2024•
edited
Loading

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4856

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit 86429ca with merge base 241cd0c (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 23, 2024
python3kgae
python3kgae commented Aug 23, 2024
View reviewed changes
Comment threadbuild/resolve_buck.py
try:
print(f"Downloading buck2 from {buck2_archive_url}...", file=sys.stderr)
urllib.request.urlretrieve(buck2_archive_url, archive_file.name)
archive_file, _ = urllib.request.urlretrieve(buck2_archive_url)

@python3kgaepython3kgaeAug 23, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This change is based on posit-dev/py-shiny#287

It works around PermissionError on Windows.
This happens because urlretrieve is trying to open an already-open temporary file by name, which isn't permitted on Windows. It's basically the same as

from tempfile import NamedTemporaryFile
with NamedTemporaryFile() as tmp:
open(tmp.name, "wb")

@digantdesaidigantdesai added module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Aug 23, 2024
@python3kgae

python3kgae commented Sep 5, 2024

Copy link
Copy Markdown
ContributorAuthor

Gentle ping.

@python3kgae

python3kgae commented Sep 12, 2024

Copy link
Copy Markdown
ContributorAuthor

Don't have access to add more reviewer myself :(
Hope someone could see this.

@kirklandsign
@dbort
@tarun292
@dvorjackz

@python3kgae

python3kgae commented Oct 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Rebased and gentle ping again.

@mergennachinmergennachin mentioned this pull request Jan 7, 2025
Closed
@mergennachin

mergennachin commented Jan 7, 2025

Copy link
Copy Markdown
Contributor

@python3kgae - sorry for the delayed response

we missed this PR.

Would you mind rebasing this and retest the flow? Happy to merge

@python3kgaeXiang Li
[Build] Support windows in resolve_buck.py
952624e
Add Windows support to BUCK_PLATFORM_MAP.
Also use urllib.request.urlretrieve directly to work around PermissionError on Windows.
For pytorch#4661
@python3kgae

python3kgae commented Jan 7, 2025

Copy link
Copy Markdown
ContributorAuthor

@python3kgae - sorry for the delayed response

we missed this PR.

Would you mind rebasing this and retest the flow? Happy to merge

Not a problem.
Rebased.

@mergennachin

mergennachin commented Jan 8, 2025

Copy link
Copy Markdown
Contributor

@python3kgae

@swolchok just landed this PR (#7448), which updates the buck version altogether.

would you mind rebasing the PR and finding the appropriate updated hash version for windows for 2024-12-16 version instead?

@python3kgae

python3kgae commented Jan 8, 2025

Copy link
Copy Markdown
ContributorAuthor

@python3kgae

@swolchok just landed this PR (#7448), which updates the buck version altogether.

would you mind rebasing the PR and finding the appropriate updated hash version for windows for 2024-12-16 version instead?

Done.

@mergennachinmergennachin added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Jan 8, 2025
@mergennachin
mergennachin merged commit 1bac885 into pytorch:mainJan 8, 2025
@GregoryComerGregoryComer mentioned this pull request Aug 28, 2025
Open
20 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mergennachinmergennachinmergennachin approved these changes
@GregoryComerGregoryComerAwaiting requested review from GregoryComer
@swolchokswolchokAwaiting requested review from swolchok
@tarun292tarun292Awaiting requested review from tarun292
@dbortdbortAwaiting requested review from dbort
@kirklandsignkirklandsignAwaiting requested review from kirklandsign

Assignees

No one assigned

Labels

ciflow/trunkCLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.module: build/installIssues related to the cmake and buck2 builds, and to installing ExecuTorchrelease notes: buildChanges related to build, including dependency upgrades, build flags, optimizations, etc.triagedThis issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@python3kgae@mergennachin@digantdesai@facebook-github-bot