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

Add git commit to version command #239

Merged
merged 12 commits into from
Apr 2, 2024
Merged

Conversation

Marshall-Hallenbeck
Copy link
Collaborator

  • add git commit to version command so we know exactly what code is being ran during issue triage

@Marshall-Hallenbeck Marshall-Hallenbeck added the enhancement New feature or request label Mar 30, 2024
@Marshall-Hallenbeck Marshall-Hallenbeck self-assigned this Mar 30, 2024
mpgn
mpgn previously approved these changes Mar 30, 2024
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

Crashing on my end
image

@NeffIsBack
Copy link
Contributor

NeffIsBack commented Mar 31, 2024

That's what importlib.metadata.version("netexec") returns for me
image

@Marshall-Hallenbeck
Copy link
Collaborator Author

You need to install the poetry_dynamic_versioning dependency.

@NeffIsBack
Copy link
Contributor

Hmm I will try again, but I just did poetry install and then poetry run

@NeffIsBack
Copy link
Contributor

@Marshall-Hallenbeck fresh installation:
image

@NeffIsBack
Copy link
Contributor

Huh, pipx works fine

image

@Marshall-Hallenbeck Marshall-Hallenbeck force-pushed the marshall-version-setting branch from 74844a9 to 4f24b11 Compare April 1, 2024 16:22
@Marshall-Hallenbeck
Copy link
Collaborator Author

Marshall-Hallenbeck commented Apr 1, 2024

When I pull down a fresh repo and run it via poetry, this all works for me.

@zblurx @XiaoliChan could you guys test this as well?

pull down this branch/a new repo, then run:

poetry lock --no-update
poetry install
poetry version
poetry run nxc --version

@NeffIsBack
Copy link
Contributor

Did you try to reinstall poetry? My guess is still that this is set in poetry

@Marshall-Hallenbeck
Copy link
Collaborator Author

Did you try to reinstall poetry? My guess is still that this is set in poetry

No, I'll try it on another host, too.

@Marshall-Hallenbeck
Copy link
Collaborator Author

Hmm so yeah it looks like this doesn't work for normal poetry install, unless we distribute the package built with poetry build.

I'll have to figure out how to hook it into that process if I can...

@Marshall-Hallenbeck
Copy link
Collaborator Author

this works for pip/pipx since its respects the build-system, but poetry unfortunately doesn't (at least that's what it appears like: python-poetry/poetry#6154)

@Marshall-Hallenbeck
Copy link
Collaborator Author

@NeffIsBack so right now, pip/pipx will properly use the versioning, but poetry doesn't respect the build-system section, which is annoying.

Luckily most people just install via pipx, and we really care about this for them when users report issues, so I think it's in a current state to help us out in that sense.

I reverted to define the version # in the pyproject, so we will continue to update that with the latest tag, but we also get the bonus of getting the actual commit when someone uses pip/pipx.

@Marshall-Hallenbeck
Copy link
Collaborator Author

Should the COMMIT just be a blank string, or should we put like "None". This is really only applicable when someone uses poetry to install (developing), and they should be able to easily tell what the latest commit is anyway. It just sorta looks meh on output:
image

@Marshall-Hallenbeck Marshall-Hallenbeck force-pushed the marshall-version-setting branch from c03a00e to ecf5772 Compare April 1, 2024 18:23
@NeffIsBack
Copy link
Contributor

Imo "" is fine, although its bothering if you know, most of the people will probably never notice it :D
None could be confusing

@NeffIsBack
Copy link
Contributor

I also opened an issue on the plugin repository: mtkennerly/poetry-dynamic-versioning#176
Maybe they can help

@NeffIsBack
Copy link
Contributor

@Marshall-Hallenbeck how about we add it also to the debug output, so we know which commit people are running when they open up issues?
image

@Marshall-Hallenbeck
Copy link
Collaborator Author

@NeffIsBack I've updated how we parse the args a bit, to parse debug and verbose early, so we can now properly debug log in the parsing of other CLI args. I also functionized/moved some stuff to logging to better fit our needs.

Now we can output info like this:

image

or even just if someone wants to debug the -h option, etc:

image

Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

LGTM
image

@NeffIsBack NeffIsBack merged commit 5c249d5 into main Apr 2, 2024
6 checks passed
@NeffIsBack NeffIsBack deleted the marshall-version-setting branch April 2, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants