Several Fixes for Protobuf Python Code Generation

[diguida]

• Use pathlib instead of glob
• Remove deprecated distutils package in favour of the setuptools API (see PEP-632)
• fix recursive lookup of proto files
• Increment version to 1.1.0.

[diguida]

@haberman can you see any issues in this PR? Let me know if you find problems in the proposed changes.

[haberman]

Is there a way to add unit tests for this?

[diguida]

I do not know if I can add a unit test here. Apart from that, are the changes ok? Do you want me to rebase the branch?

Thanks

[haberman]

The changes look fine. I am just concerned that recursive lookup of proto files was broken and we didn't know it. A unit test could help ensure that recursive lookup works.

How are distutils extensions usually tested?

[diguida]

The changes look fine. I am just concerned that recursive lookup of proto files was broken and we didn't know it. A unit test could help ensure that recursive lookup works.

How are distutils extensions usually tested?

I see your point, and agree. I can check if the python packaging authority has some guidance about such tests.

[diguida]

The changes look fine. I am just concerned that recursive lookup of proto files was broken and we didn't know it. A unit test could help ensure that recursive lookup works.

How are distutils extensions usually tested?

I see your point, and agree. I can check if the python packaging authority has some guidance about such tests.

BTW, maybe there could be two further improvements here:

• Use console scripts from setuptools to call generate_py_protobufs standalone as an executable
• Remove output folder if already exists: this guarantees that you always produce updated generated sources, and avoids the OSError if you forget to delete it.

Let me know if I can add at least the latter in this PR.

[haberman]

Remove output folder if already exists: this guarantees that you always produce updated generated sources, and avoids the OSError if you forget to delete it.

How is the output folder specified? Is there any chance that the user might accidentally set the output folder to one of their source folders, and this would end up deleting the user's source files? That would be my main concern, we don't want to accidentally delete the user's precious files.

[diguida]

Remove output folder if already exists: this guarantees that you always produce updated generated sources, and avoids the OSError if you forget to delete it.

How is the output folder specified? Is there any chance that the user might accidentally set the output folder to one of their source folders, and this would end up deleting the user's source files? That would be my main concern, we don't want to accidentally delete the user's precious files.

Good point. Basically, a tool should not remove files unless explicitly requested by the user. If instead the tool fails, the user realizes that something is wrong in the output folder, and can have a look.

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it.

This PR was closed and archived because there has been no new activity in the 14 days since the inactive label was added.