Do not use `<filesystem>` on Linux #134494

malfet · 2024-08-26T20:05:04Z

Because right now it leads to symbol conflict from binary builds.
Use of std::filesystem::file_exists was introduced by #126601 and in this PR it is replaced with a very straightforward implementation that calls stat on the given path, which is a classic C-way of checking for the file existence.

This PR should be reverted once one figures out how to keep std::filesystem methods linked into the binary private

Fixes symptoms of #133437

cc @XilunWu @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @c-p-i-o

Because right now it leads to symbol conflict from binary builds This PR should be reverted once one figures out how to keep `std::filesystem` methods linked into the binary private

pytorch-bot · 2024-08-26T20:05:08Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

❌ 1 New Failure, 16 Cancelled Jobs

As of commit 9ffe0f8 with merge base 0a3c064 ():

NEW FAILURE - The following job has failed:

linux-aarch64-binary-manywheel / manywheel-py3_12-cpu-aarch64-build / build (gh)

CANCELLED JOBS - The following jobs were cancelled. Please retry:

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

atalman

lgtm

d4l3k

LGTM

malfet · 2024-08-27T01:39:15Z

There is one more use case, in aoti_runner...

atalman · 2024-08-27T14:39:20Z

confirmed this is resolved. By installing pip install torch-2.5.0.dev20240827+cpu-cp311-cp311-linux_x86_64.whl
Then running:

nm -D libtorch_cpu.so | grep "recursive_directory_iterator"

atalman · 2024-08-27T14:42:04Z

@pytorchmergebot merge -f "Builds jobs are passing"

pytorchmergebot · 2024-08-27T14:43:46Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

malfet · 2024-08-27T16:09:10Z

@atalman thank you for the merge, but I've tried downloading other builds and still seeing some symbols there....

atalman · 2024-08-27T17:42:20Z

@malfet confirmed - it works with cuda binary as well. torch-2.5.0.dev20240827+cu121-cp311-cp311-linux_x86_64.whl

atalman · 2024-08-27T17:42:58Z

@pytorchbot cherry-pick --onto release/2.4 -c critical

pytorchbot · 2024-08-27T17:47:18Z

Cherry picking #134494

Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x 0fa0ac80e4cb2aa7eba5c5753be6c428b5eee45e returned non-zero exit code 1

Auto-merging torch/csrc/distributed/c10d/control_plane/WorkerServer.cpp
CONFLICT (content): Merge conflict in torch/csrc/distributed/c10d/control_plane/WorkerServer.cpp
Auto-merging torch/csrc/inductor/aoti_runner/model_container_runner.cpp
CONFLICT (content): Merge conflict in torch/csrc/inductor/aoti_runner/model_container_runner.cpp
error: could not apply 0fa0ac80e4... Do not use `<filesystem>` on Linux (#134494)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

Details for Dev Infra team

Raised by workflow job

Because right now it leads to symbol conflict from binary builds. Use of `std::filesystem::file_exists` was introduced by pytorch#126601 and in this PR it is replaced with a very straightforward implementation that calls `stat` on the given path, which is a classic C-way of checking for the file existence. This PR should be reverted once one figures out how to keep `std::filesystem` methods linked into the binary private Fixes symptoms of pytorch#133437 Pull Request resolved: pytorch#134494 Approved by: https://github.com/atalman, https://github.com/d4l3k

Because right now it leads to symbol conflict from binary builds. Use of `std::filesystem::file_exists` was introduced by #126601 and in this PR it is replaced with a very straightforward implementation that calls `stat` on the given path, which is a classic C-way of checking for the file existence. This PR should be reverted once one figures out how to keep `std::filesystem` methods linked into the binary private Fixes symptoms of #133437 Pull Request resolved: #134494 Approved by: https://github.com/atalman, https://github.com/d4l3k Co-authored-by: Nikita Shulga <nshulga@meta.com>

Because right now it leads to symbol conflict from binary builds. Use of `std::filesystem::file_exists` was introduced by pytorch#126601 and in this PR it is replaced with a very straightforward implementation that calls `stat` on the given path, which is a classic C-way of checking for the file existence. This PR should be reverted once one figures out how to keep `std::filesystem` methods linked into the binary private Fixes symptoms of pytorch#133437 Pull Request resolved: pytorch#134494 Approved by: https://github.com/atalman, https://github.com/d4l3k

Similar to: #134494 We are seeing come back of #133437 due to use of filesystem on Linux Pull Request resolved: #137209 Approved by: https://github.com/kit1980, https://github.com/malfet

* Apply changes from #135374 * Fix dependency on filesystem on Linux (#137209) Similar to: #134494 We are seeing come back of #133437 due to use of filesystem on Linux Pull Request resolved: #137209 Approved by: https://github.com/kit1980, https://github.com/malfet --------- Co-authored-by: atalman <atalman@fb.com>

Do not use <filesystem> on Linux

91c54ed

Because right now it leads to symbol conflict from binary builds This PR should be reverted once one figures out how to keep `std::filesystem` methods linked into the binary private

pytorch-bot bot added oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (c10d) release notes category labels Aug 26, 2024

malfet requested a review from d4l3k August 26, 2024 20:05

malfet added topic: not user facing topic category ciflow/trunk Trigger trunk jobs on your pull request labels Aug 26, 2024

atalman approved these changes Aug 26, 2024

View reviewed changes

d4l3k approved these changes Aug 26, 2024

View reviewed changes

And this one

9ffe0f8

pytorch-bot bot added the ciflow/inductor label Aug 27, 2024

atalman added the ciflow/binaries_wheel Trigger binary build and upload jobs for wheel on the PR label Aug 27, 2024

pytorchmergebot added the merging label Aug 27, 2024

pytorchmergebot added the Merged label Aug 27, 2024

pytorchmergebot closed this in 0fa0ac8 Aug 27, 2024

pytorchmergebot removed the merging label Aug 27, 2024

This was referenced Aug 27, 2024

Do not use <filesystem> on Linux (#134494) #134604

Merged

[v2.4.1] Release Tracker #132400

Closed

atalman mentioned this pull request Oct 2, 2024

Fix dependency on filesystem on Linux #137209

Closed

github-actions bot deleted the malfet/do-not-use-std-filesystem branch October 25, 2024 02:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Do not use `<filesystem>` on Linux #134494

Do not use `<filesystem>` on Linux #134494

Uh oh!

malfet commented Aug 26, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Aug 26, 2024 •

edited

Loading

Uh oh!

atalman left a comment

Uh oh!

d4l3k left a comment

Uh oh!

malfet commented Aug 27, 2024

Uh oh!

atalman commented Aug 27, 2024 •

edited

Loading

Uh oh!

atalman commented Aug 27, 2024

Uh oh!

pytorchmergebot commented Aug 27, 2024

Uh oh!

malfet commented Aug 27, 2024

Uh oh!

atalman commented Aug 27, 2024

Uh oh!

atalman commented Aug 27, 2024

Uh oh!

pytorchbot commented Aug 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Do not use <filesystem> on Linux #134494

Do not use <filesystem> on Linux #134494

Uh oh!

Conversation

malfet commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

❌ 1 New Failure, 16 Cancelled Jobs

Uh oh!

atalman left a comment

Choose a reason for hiding this comment

Uh oh!

d4l3k left a comment

Choose a reason for hiding this comment

Uh oh!

malfet commented Aug 27, 2024

Uh oh!

atalman commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atalman commented Aug 27, 2024

Uh oh!

pytorchmergebot commented Aug 27, 2024

Merge started

Uh oh!

malfet commented Aug 27, 2024

Uh oh!

atalman commented Aug 27, 2024

Uh oh!

atalman commented Aug 27, 2024

Uh oh!

pytorchbot commented Aug 27, 2024

Cherry picking #134494

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Do not use `<filesystem>` on Linux #134494

Do not use `<filesystem>` on Linux #134494

malfet commented Aug 26, 2024 •

edited

Loading

pytorch-bot bot commented Aug 26, 2024 •

edited

Loading

atalman commented Aug 27, 2024 •

edited

Loading