Skip to content
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

[mypyc] Introduce subcommand to mypyc command line interface #9030

Closed
wants to merge 6 commits into from
Closed

[mypyc] Introduce subcommand to mypyc command line interface #9030

wants to merge 6 commits into from

Conversation

TH3CHARLie
Copy link
Collaborator

related mypyc/mypyc#739

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! The new command line will be a big usability improvement. I left a bunch of ideas about how to improve this further.

scripts/mypyc Outdated Show resolved Hide resolved
scripts/mypyc Show resolved Hide resolved
scripts/mypyc Outdated Show resolved Hide resolved
except OSError as e:
print('Error "{}" occurred when removing {}'.format(e.strerror, file))
for directory in directories:
shutil.rmtree(directory)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also delete the build directory, if it's empty now? Also, we should delete the shared libraries outside the build directory (but somehow only those what have been compiled with mypyc using this build directory).

directories = []
# TODO: this simply hardcodes what generates now
# a better solution is to use some configuration files
# maybe use a `.mypyc_cache` folder to store them
Copy link
Collaborator

Choose a reason for hiding this comment

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

Brainstorming: maybe the build directory should be renamed to something with mypyc in the name, perhaps __mypyc__ or something, and then we can assume that everything in the directory are owned by us can be deleted.

Copy link
Collaborator Author

@TH3CHARLie TH3CHARLie Jun 23, 2020

Choose a reason for hiding this comment

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

__mypyc__ would be a decent choice, but I am worried about the generated shared object files. Currently, we place them under cwd. To clean them, we probably need to search all *.so in cwd and delete them, which IMO is not a good approach.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Deleting the .so files would make the command much more useful, I think.

We should be able to maintain the paths to all generated .so files in a file under build/__mypyc__ and delete only the files we have generated. If we are still worried that we may deleting the wrong files, we can also record the mtime and size of the files, and only delete files if the mtime and size match what we have recorded.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This approach looks way better. Let's try go with __mypyc__ to place all the *.c, *.h, ops.txt, setup.py(or any other intermediate scripts) and __mypyc__/.mypyc_meta(the name can be tweak) to record such file information.

@JukkaL JukkaL mentioned this pull request Nov 14, 2020
11 tasks
TH3CHARLie added a commit to TH3CHARLie/mypy that referenced this pull request May 1, 2021
@TH3CHARLie
Copy link
Collaborator Author

I've lost the branch for this both locally and on Github, so I am creating a new PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants