Skip to content

Conversation

AlekseiNikiforovIBM
Copy link
Collaborator

@AlekseiNikiforovIBM AlekseiNikiforovIBM commented Jun 19, 2024

It is assumed that they are no longer needed.
And keeping their installation as is breaks
"python setup.py develop --user" workflow
when non-root user is used.

This change is follow up for 3d61733

Copy link

pytorch-bot bot commented Jun 19, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 2a6e8ae with merge base 6e43897 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jun 19, 2024
@AlekseiNikiforovIBM
Copy link
Collaborator Author

Without this change "python setup.py develop --user" fails when running from non-root user.

@cyyever
Copy link
Collaborator

cyyever commented Jun 19, 2024

@AlekseiNikiforovIBM It is used to install caffe2 py files. Now there are only three such files

caffe2/core/__init__.py
caffe2/perfkernels/__init__.py
caffe2/perfkernels/hp_emblookup_codegen.py

I suspect that they shouldn't be installed and that the entire section of CMake code should be deleted as well.

@AlekseiNikiforovIBM
Copy link
Collaborator Author

I suspect that they shouldn't be installed and that the entire section of CMake code should be deleted as well.

Sounds good to me.

@cyyever
Copy link
Collaborator

cyyever commented Jun 19, 2024

@AlekseiNikiforovIBM Would you mind change this PR to remove them?

@AlekseiNikiforovIBM AlekseiNikiforovIBM force-pushed the fix_develop_user branch 2 times, most recently from 5bf6ebf to 436add0 Compare June 19, 2024 14:38
@AlekseiNikiforovIBM AlekseiNikiforovIBM changed the title Fix python setup.py develop --user workflow Don't install remaining caffe2 python files Jun 19, 2024
@cyyever
Copy link
Collaborator

cyyever commented Jun 19, 2024

With this change it should work on your host.

@cyyever
Copy link
Collaborator

cyyever commented Jun 19, 2024

@AlekseiNikiforovIBM Please remove caffe2/core/init.py and caffe2/perfkernels/init.py here.

It is assumed that they are no longer needed.
And keeping their installation as is breaks
"python setup.py develop --user" workflow
when non-root user is used.

This change is follow up for 3d61733
@AlekseiNikiforovIBM
Copy link
Collaborator Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 25, 2024
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: Approvers from one of the following sets are needed:

  • superuser (pytorch/metamates)
  • Core Reviewers (mruberry, lezcano, Skylion007, ngimel, peterbell10)
  • Core Maintainers (soumith, gchanan, ezyang, dzhulgakov, malfet)
Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@kiszk
Copy link
Contributor

kiszk commented Jun 26, 2024

@malfet Could you please approve this if it is good?

@r-barnes r-barnes added caffe2 topic: deprecation topic category topic: improvements topic category topic: build python Pull requests that update Python code release notes: build release notes category labels Jun 27, 2024
@r-barnes
Copy link
Contributor

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caffe2 ciflow/trunk Trigger trunk jobs on your pull request Merged open source python Pull requests that update Python code release notes: build release notes category topic: build topic: deprecation topic category topic: improvements topic category topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants