Skip to content

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Jun 6, 2022

Needed for matrix-org/synapse#12968.

Since we now use poetry-core to build distributions for Synapse, the locate_file(".") trick no longer seems to work to find Synapse's source directory.

In principle we could do something like

  • have the get-a-version function accept a module object
  • look up the distribution name given the module's __name__ in importlib.metadata.packages_distributions(), and use that to get a version as today
  • get cwd from the module's __file__ attribute and use that for the git invocations

Unfortunately the second step is brittle. When I tried to test this on matrix.org's configuration, packages_distributions()["synapse"] gave me a KeyError for my troubles.

Instead, keep things simple and have the caller give us an optional cwd.

If accepted, I will do a minor release of matrix-python-common and change Synapse to depend on it in matrix-org/synapse#12973 (which is presently a WIP).

to workaround psf/black#2964
@DMRobertson DMRobertson force-pushed the dmr/versionstring-explicit-cwd branch from d4504da to 1b58254 Compare June 6, 2022 18:16
@DMRobertson DMRobertson marked this pull request as ready for review June 6, 2022 18:18
@DMRobertson DMRobertson requested a review from a team as a code owner June 6, 2022 18:18
@DMRobertson DMRobertson marked this pull request as draft June 6, 2022 18:40
DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request Jun 6, 2022
Feels a bit icky, but this is hopefully good enough.
@DMRobertson DMRobertson force-pushed the dmr/versionstring-explicit-cwd branch from aaa6c39 to c27e6d8 Compare June 6, 2022 19:14
@DMRobertson DMRobertson marked this pull request as ready for review June 6, 2022 19:15
Copy link
Contributor

@squahtx squahtx left a comment

Choose a reason for hiding this comment

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

lgtm!

@DMRobertson DMRobertson merged commit 4b0a3e7 into main Jun 7, 2022
@DMRobertson DMRobertson deleted the dmr/versionstring-explicit-cwd branch June 7, 2022 09:42
Comment on lines +50 to +51
cwd: if provided, the directory to run all git commands in. `cwd` may also be
the path to a file, in which case `os.path.dirname(cwd)` is used instead.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely had code which did this. Must have lost it when force pushing. Will fix on main branch and do another release.

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