Skip to content

Conversation

GregoryComer
Copy link
Member

@GregoryComer GregoryComer commented Aug 25, 2025

CMake doesn't seem to like empty exclude patterns.

GregoryComer and others added 30 commits August 8, 2025 14:44
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@GregoryComer
Copy link
Member Author

GregoryComer commented Aug 25, 2025

Stack from ghstack (oldest at bottom):

[ghstack-poisoned]
[ghstack-poisoned]
Copy link

pytorch-bot bot commented Aug 25, 2025

🔗 Helpful Links

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

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

❌ 2 New Failures, 2 Unrelated Failures

As of commit 3a32779 with merge base 7dab7c1 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@meta-cla meta-cla 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 25, 2025
@GregoryComer GregoryComer added the release notes: none Do not include this in the release notes label Aug 26, 2025
CMakeLists.txt Outdated
PATTERN "*.h"
PATTERN ${data_loader_exclude_pattern} EXCLUDE
)
if (DEFINED data_loader_exclude_pattern)
Copy link
Member Author

@GregoryComer GregoryComer Aug 26, 2025

Choose a reason for hiding this comment

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

@swolchok It looks like CMake complains when there's an exclude with an empty pattern (see https://github.com/pytorch/executorch/actions/runs/17222786697/job/48861521892, line 818). This is a bit of a pain as there doesn't seem to be an elegant solution to this. Do you have any preferences on this approach?

I suppose we could default the exclude pattern to some string that matches nothing, though that seems a little hacky. It looks like we could also maybe build a list of install args, including the "magic" keywords, though I haven't tried it.

@GregoryComer GregoryComer requested a review from swolchok August 26, 2025 00:03
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
GregoryComer added a commit that referenced this pull request Aug 26, 2025
ghstack-source-id: 251096e
ghstack-comment-id: 3222086254
Pull-Request: #13672
@GregoryComer GregoryComer changed the title Fix data loader build on Windows, re-enable Windows build CI job Fix build on Windows, re-enable Windows build CI job Aug 26, 2025
@GregoryComer GregoryComer marked this pull request as draft August 26, 2025 05:35
GregoryComer added a commit that referenced this pull request Aug 26, 2025
The Windows CI build job was disabled in
#13669 due to a new Windows
build failure in #13485. Since
that diff was reverted in
#13685, we can re-enable the
job.

I started fixing the issue in
#13672, but dropped it due to
the revert. It's a simple change, so it can likely be bundled when the
affected PR is re-landed.

Test Plan:
The Build Preset / Windows job is passing on this PR.
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 Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: none Do not include this in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant