Skip to content

[build] Fix flatc #8816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 1, 2025
Merged

Conversation

larryliu0820
Copy link
Contributor

@larryliu0820 larryliu0820 commented Feb 28, 2025

Stack from ghstack (oldest at bottom):

Fixes #8784

  1. We need to install build/pip_data_bin_init.py.in into <executorch root>/data/bin/__init__.py. This PR rewrite the logic into a
    BuiltFile so that it works well in editable mode.

  2. Since BuiltFile by default looks into cmake cache directory, this PR adds a placeholder %CMAKE_CACHE_DIR% for those are actually built by CMake and for build/pip_data_bin_init.py.in we don't add this placeholder.

  3. Since editable mode doesn't support creating a directory in build command and install it as a new module, I need to create executorch/data/bin/ and add it to the pyproject.toml file, so that executorch.data.bin can be installed by editable mode.

Test:

python -c "from executorch.data.bin import flatc"

Added unittest-editable for Linux and Mac

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Feb 28, 2025

🔗 Helpful Links

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

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

⏳ No Failures, 1 Pending

As of commit 226bc15 with merge base 76fb844 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot facebook-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 Feb 28, 2025
@larryliu0820 larryliu0820 added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Feb 28, 2025
@larryliu0820 larryliu0820 requested a review from dbort February 28, 2025 05:21

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]
larryliu0820 added a commit that referenced this pull request Feb 28, 2025
We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

ghstack-source-id: 433f50c
Pull Request resolved: #8816
@mergennachin mergennachin self-requested a review February 28, 2025 15:42

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]
larryliu0820 added a commit that referenced this pull request Feb 28, 2025
We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

ghstack-source-id: 015cf34
Pull Request resolved: #8816
@larryliu0820 larryliu0820 merged commit d0b27b5 into gh/larryliu0820/61/base Mar 1, 2025
120 checks passed
@larryliu0820 larryliu0820 deleted the gh/larryliu0820/61/head branch March 1, 2025 00:54
@larryliu0820
Copy link
Contributor Author

@pytorchbot -h

Copy link

pytorch-bot bot commented Mar 1, 2025

PyTorchBot Help

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci,cherry-pick,close} ...

In order to invoke the bot on your PR, include a line that starts with
@pytorchbot anywhere in a comment. That line will form the command; no
multi-line commands are allowed. Some commands may be used on issues as specified below.

Example:
    Some extra context, blah blah, wow this PR looks awesome

    @pytorchbot merge

optional arguments:
  -h, --help            Show this help message and exit.

command:
  {merge,revert,rebase,label,drci,cherry-pick,close}
    merge               Merge a PR
    revert              Revert a PR
    rebase              Rebase a PR
    label               Add label to a PR
    drci                Update Dr. CI
    cherry-pick         Cherry pick a PR onto a release branch
    close               Close a PR

Merge

usage: @pytorchbot merge [-f MESSAGE | -i] [-ic] [-r [{viable/strict,main}]]

Merge an accepted PR, subject to the rules in .github/merge_rules.json.
By default, this will wait for all required checks (lint, pull) to succeed before merging.

optional arguments:
  -f MESSAGE, --force MESSAGE
                        Merge without checking anything. This requires a reason for auditting purpose, for example:
                        @pytorchbot merge -f 'Minor update to fix lint. Expecting all PR tests to pass'
                        
                        Please use `-f` as last resort, prefer `--ignore-current` to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.
  -i, --ignore-current  Merge while ignoring the currently failing jobs.  Behaves like -f if there are no pending jobs.
  -ic                   Old flag for --ignore-current. Deprecated in favor of -i.
  -r [{viable/strict,main}], --rebase [{viable/strict,main}]
                        Rebase the PR to re run checks before merging.  Accepts viable/strict or main as branch options and will default to viable/strict if not specified.

Revert

usage: @pytorchbot revert -m MESSAGE -c
                          {nosignal,ignoredsignal,landrace,weird,ghfirst}

Revert a merged PR. This requires that you are a Meta employee.

Example:
  @pytorchbot revert -m="This is breaking tests on trunk. hud.pytorch.org/" -c=nosignal

optional arguments:
  -m MESSAGE, --message MESSAGE
                        The reason you are reverting, will be put in the commit message. Must be longer than 3 words.
  -c {nosignal,ignoredsignal,landrace,weird,ghfirst}, --classification {nosignal,ignoredsignal,landrace,weird,ghfirst}
                        A machine-friendly classification of the revert reason.

Rebase

usage: @pytorchbot rebase [-s | -b BRANCH]

Rebase a PR. Rebasing defaults to the stable viable/strict branch of pytorch.
Repeat contributor may use this command to rebase their PR.

optional arguments:
  -s, --stable          [DEPRECATED] Rebase onto viable/strict
  -b BRANCH, --branch BRANCH
                        Branch you would like to rebase to

Label

usage: @pytorchbot label labels [labels ...]

Adds label to a PR or Issue [Can be used on Issues]

positional arguments:
  labels  Labels to add to given Pull Request or Issue [Can be used on Issues]

Dr CI

usage: @pytorchbot drci 

Update Dr. CI. Updates the Dr. CI comment on the PR in case it's gotten out of sync with actual CI results.

cherry-pick

usage: @pytorchbot cherry-pick --onto ONTO [--fixes FIXES] -c
                               {regression,critical,fixnewfeature,docs,release}

Cherry pick a pull request onto a release branch for inclusion in a release

optional arguments:
  --onto ONTO           Branch you would like to cherry pick onto (Example: release/2.1)
  --fixes FIXES         Link to the issue that your PR fixes (Example: https://github.com/pytorch/pytorch/issues/110666)
  -c {regression,critical,fixnewfeature,docs,release}, --classification {regression,critical,fixnewfeature,docs,release}
                        A machine-friendly classification of the cherry-pick reason.

Close

usage: @pytorchbot close

Close a PR [Can be used on issues]

@larryliu0820
Copy link
Contributor Author

@pytorchbot cherry-pick --onto main

Copy link

pytorch-bot bot commented Mar 1, 2025

❌ 🤖 pytorchbot command failed:

@pytorchbot cherry-pick: error: the following arguments are required: -c/--classification

usage: @pytorchbot cherry-pick --onto ONTO [--fixes FIXES] -c
                               {regression,critical,fixnewfeature,docs,release}

Try @pytorchbot --help for more info.

@larryliu0820
Copy link
Contributor Author

@pytorchbot cherry-pick --onto main -c fixnewfeature

pytorchbot pushed a commit that referenced this pull request Mar 1, 2025
* [build] Fix flatc

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

* Update on "[build] Fix flatc"

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

* Update on "[build] Fix flatc"

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

* Update on "[build] Fix flatc"

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

(cherry picked from commit d0b27b5)
@pytorchbot
Copy link
Collaborator

Cherry picking #8816

The cherry pick PR is at #8858 and it is recommended to link a fixnewfeature cherry pick PR with an issue.

Details for Dev Infra team Raised by workflow job

@pytorchbot pytorchbot mentioned this pull request Mar 1, 2025
larryliu0820 added a commit that referenced this pull request Mar 1, 2025
[build] Fix flatc (#8816)

* [build] Fix flatc

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

* Update on "[build] Fix flatc"

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

* Update on "[build] Fix flatc"

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

* Update on "[build] Fix flatc"

Fixes #8784

We need to install `build/pip_data_bin_init.py.in` into `<executorch
root>/data/bin/__init__.py`. This PR rewrite the logic into a
`BuiltFile` so that it works well in editable mode.

Since `BuiltFile` by default looks into cmake cache directory, this PR adds a placeholder `%CMAKE_CACHE_DIR%` for those are actually built by CMake and for `build/pip_data_bin_init.py.in` we don't add this placeholder.

Test:

```
python -c "from executorch.data.bin import flatc"
```
Will add unit test in next PR.

[ghstack-poisoned]

(cherry picked from commit d0b27b5)

Co-authored-by: Mengwei Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants