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

Hatchbuild retry #379

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

fleetingbytes
Copy link
Contributor

Retry of PR #377.

I don't know did not find a way how to fix the links coerce.py and semverwithvprefix.py. Maybe because I develop under Windows which does not support symlinks. I had to copy the code from their targets into them, but then I had an import conflict, as pytest tried to import both the "links" and their targets under the same name. So I additionally had to exclude the tests dir from testpaths.
Also pytest was complaining that it cannot find the ndiff folder, so I had to exclude it from addopts as well.
And I had to change the matrix from 37 to 310 because I currently only have python 3.10. Then the tests ran! 🥲 . But I undid these changes for this PR. However, I would be happy to solve these errors in the future, as I don't want to do this switcheroo every time I'd contribute some test code.

I also had to change the wheel build target (see packages and forced inclusion), lest python would not find the semver package after installing the wheel, or mypy would not find its py.types file, respectively.

I also did not test your docs scripts (I have PowerShell, where it's a hassle to run .bat scripts), but I do think that they need to have the invocation docs/make.bat to be conveniently called from the project root. Btw, I commented out the sphinx dependency in .envs.docs, because it comes with either of the other two dependencies.

I also was too lazy to find out how to rebase and update the previous PR (maybe another time I'll learn how to do this), so I closed it and here comes a new one.

@tomschr
Copy link
Member

tomschr commented Nov 27, 2022

Many thanks for all your efforts! 👍

I also was too lazy to find out how to rebase and update the previous PR (maybe another time I'll learn how to do this), so I closed it and here comes a new one.

That's fine for the time being. Sometimes I do that too when I did something so horribly wrong that I can't find an escape. 😂

However, proliferating PRs where the same topic is opened and closed again and again should be avoided. After some time, nobody really knows anymore and it gets harder to look through the history of changes if you have a pile of PRs. 😉

The steps for a git rebase (click me)

Once you get used to it, it's not really that hard. Before you do it, I would recommend to make a copy/backup of your local directory. Just in case.

The simplest case is fairly easy. This is all you need:

$ git checkout <YOUR_BRANCH>
$ git rebase master

If you get conflicts after you have rebased, you need a bit more work:

# 1. Get a list of files which has conflicts:
$ git status

# 2. Fix any conflicts in the files.
# Watch for conflict markers, search for "<<<", "===", or ">>>.
# Look which parts of the conflict you can remove. When done, use:
$ git add <CONFLICTED_FILE>

# 3. Proceed with the next conflicted file.

# 4. When you are done with conflict resolution, use:
$ git rebase --continue

# 5. Finally:
$ git push --force-with-lease

If you want a more detailed explanation (with nice images!) I can recommend https://www.geeksforgeeks.org/rebasing-of-branches-in-git/ and for conflict resolution look at https://codeinthehole.com/guides/resolving-conflicts-during-a-git-rebase/

I don't know did not find a way how to fix the links coerce.py and semverwithvprefix.py. Maybe because I develop under Windows which does not support symlinks.

No worries, I can do that for you. However, even Windows supports links. If I'm not mistaken, Git should do most of the things for you.

Also pytest was complaining that it cannot find the ndiff folder, so I had to exclude it from addopts as well.

That sounds a bit strange as there is no ndiff folder, but ndiff is a doctest option.

I also did not test your docs scripts (I have PowerShell, where it's a hassle to run .bat scripts), but I do think that they need to have the invocation docs/make.bat to be conveniently called from the project root.

That's fine, I think you can overwrite the command(s) for Windows. I have seen that Hatch offers some means to do this.


I haven't look into the details of the PR yet, so the above reply is just the things that I wanted to share with you. I'll keep you updated!

@fleetingbytes
Copy link
Contributor Author

fleetingbytes commented Nov 27, 2022

I wonder if I did not forget the most important thing in my PR. setup.py is now no longer needed, is it? So we should remove it. Unless it's somehow still required by the older Pythons.

However, even Windows supports links.

True, but I only have a Windows-based dev environment set up on my company's computers where the Windows policies forbid users to create symlinks, or hard links. I don't think I will ever bother with setting up Windows for development on my private machines. I'm way behind in schedule to get my Linux machines managed by dotfiles and into shape for doing some development work at home.

@tomschr
Copy link
Member

tomschr commented Nov 27, 2022

I wonder if I did not forget the most important thing in my PR. setup.py is now no longer needed, is it?

Probably it's not needed anymore. However, that would already anticipate the decision from the discussion about removing Python 3.6, wouldn't it?

I have no problem of removing the setup.py script, but I'm wondering if it's too early to close the discussion. Currently we have 3 votes for removing it which is not much. It seems, we don't get more votes as people probably don't care or don't know it. 🤷‍♂️

[...] where the Windows policies forbid users to create symlinks, or hard links.

Ohh... that's really unfortunate. Does your policy allow to work on WSL or in a virtual machine?

I also did not test your docs scripts (I have PowerShell, where it's a hassle to run .bat scripts), but I do think that they need to have the invocation docs/make.bat to be conveniently called from the project root.

I've tried to look into overwriting commands/scripts and it seems I need a [tool.hatch.envs.docs.scripts.overrides] section. However, I don't know what to add or how to overwrite. 🤔

By the way, this line looks suspicious to me:

docs/make.bat -C docs <SUBCOMMAND>

Does the make.bat command takes into account where it is stored? In that case, the -C option would be useless. Unfortunately, I can't test it (I don't have a Windows machine).

@tomschr
Copy link
Member

tomschr commented Nov 27, 2022

I've played around a bit and was able to create only one test environment which is able to run a specific Python version.

This is the patch:

Patch for consolidating test environment
commit 
Author: Tom Schraitle <tomschr@users.noreply.github.com>
Date:   Sun Nov 27 21:38:12 2022 +0100

    Consolidate test environment
    
    Use _all_ supported Python environments. Add a default matrix.
    Run a specific test with "hatch run +py=37 test"
    
    TODO:
    * Show errors
    * Can the supported Python matrix be written only once?

diff --git a/pyproject.toml b/pyproject.toml
index 4f2099d..5d93f10 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -88,6 +88,10 @@ dependencies = [
     "wheel",
 ]
 
+[[tool.hatch.envs.default.matrix]]
+python = ["37", "38", "39", "310", "311",]
+
+
 [tool.hatch.envs.style]
 dependencies = [
     "black",
@@ -123,23 +127,13 @@ dependencies = [
 ]
 
 [tool.hatch.envs.test.scripts]
-python = ["37", ]
+python = ["37", "38", "39", "310", "311", ]
+
 
 [tool.hatch.envs.test.scripts]
-cov = "pytest -vx"
+cov = "pytest"
 no-cov = "cov --no-cov"

 -[tool.hatch.envs.testall]
-dependencies = [
-    "coverage[toml]",
-    "pytest-cov",
-]
-[[tool.hatch.envs.testall.matrix]]
-python = ["37", "38", "39", "310", "311", ]
-
-[tool.hatch.envs.testall.scripts]
-cov = "pytest -vx"
-no-cov = "cov --no-cov"
-
-[tool.hatch.envs.testall.scripts]
-cov = "pytest -vx"
-no-cov = "cov --no-cov"
 
 [tool.hatch.envs.tox]
 dependencies = [
@@ -149,6 +143,9 @@ dependencies = [
 [tool.hatch.envs.tox.scripts]
 test = "tox"
 
+
+## --------------------------------------------------------
+## Other tools
 [tool.coverage.run]
 source = [
     "semver",
@@ -207,12 +204,13 @@ filterwarnings = [
     'ignore:Function semver.*:DeprecationWarning',
 ]
 addopts = [
+    "--verbose",
     "--no-cov-on-fail",
     "--cov=semver",
     "--cov-report=term-missing",
     "--doctest-glob='*.rst'",
     "--doctest-modules",
-    "--doctest-report ndiff",
+    "--doctest-report=ndiff",
 ]
 
 # flake8 does not support configuration in pyproject.toml

After the patch, you have only this environments:

$ hatch env show
                        Standalone                        
┏━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Name  ┃ Type    ┃ Dependencies             ┃ Scripts   ┃
┡━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ style │ virtual │ black                    │ fmt       │
│       │         │ flake8                   │ lint      │
│       │         │ pycodestyle              │           │
├───────┼─────────┼──────────────────────────┼───────────┤
│ docs  │ virtual │ sphinx-argparse          │ build     │
│       │         │ sphinx-autodoc-typehints │ linkcheck │
│       │         │                          │ serve     │
├───────┼─────────┼──────────────────────────┼───────────┤
│ tox   │ virtual │ tox                      │ test      │
└───────┴─────────┴──────────────────────────┴───────────┘
                          Matrices                           
┏━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━━━━┓
┃ Name    ┃ Type    ┃ Envs       ┃ Dependencies   ┃ Scripts ┃
┡━━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━╇━━━━━━━━━┩
│ default │ virtual │ py37       │ towncrier      │         │
│         │         │ py38       │ wheel          │         │
│         │         │ py39       │                │         │
│         │         │ py310      │                │         │
│         │         │ py311      │                │         │
├─────────┼─────────┼────────────┼────────────────┼─────────┤
│ test    │ virtual │ test.py37  │ coverage[toml] │ cov     │
│         │         │ test.py38  │ pytest-cov     │ no-cov  │
│         │         │ test.py39  │                │         │
│         │         │ test.py310 │                │         │
│         │         │ test.py311 │                │         │
└─────────┴─────────┴────────────┴────────────────┴─────────┘

To run only, let's say, Python 3.7, you run:

$ hatch run +py=37 test

However, it gives me a return code 1 😢 and it shows me nothing, although pytest was given a --verbose option.
How can I direct pytest and pass additional options? For example, tox allows tox -e py37 -- tests/test_bump.py to only select tests from the tests/test_bump.py file. Would that be possible with Hatch too?

@ofek Could you shed some light on our endeavor? I have basically these questions:

  • Is it possible to replicate tox behaviour and pass additional arguments to the hatch environment (as shown before with the double dashes)?
  • How can I change (override?) the commands for building documentation for Windows/MacOS?
  • It seems, when I need the list of all supported Python versions I need to replicate it. The same code is in [[tool.hatch.envs.default.matrix]] and [[tool.hatch.envs.test.matrix]]. Double code is a code smell and is bad. Can we have the list only at one place and maybe refer to it?

Thanks!

@ofek
Copy link
Contributor

ofek commented Nov 27, 2022

Is it possible to replicate tox behaviour and pass additional arguments

args https://hatch.pypa.io/latest/config/environment/advanced/

How can I change (override?) the commands for building documentation for Windows/MacOS

https://hatch.pypa.io/latest/config/environment/advanced/#platform-overrides

Non-platform example https://github.com/mkdocs/mkdocs/blob/56b235a8ad43f2300d17f87e6fa4de7a3d764397/pyproject.toml#L114-L127

when I need the list of all supported Python versions I need to replicate it [...] Can we have the list only at one place and maybe refer to it

No https://hatch.pypa.io/latest/config/environment/overview/#inheritance

@tomschr
Copy link
Member

tomschr commented Nov 28, 2022

Thanks @ofek for the great tips! 👍

How can I change (override?) the commands for building documentation for Windows/MacOS

https://hatch.pypa.io/latest/config/environment/advanced/#platform-overrides

Non-platform example https://github.com/mkdocs/mkdocs/blob/56b235a8ad43f2300d17f87e6fa4de7a3d764397/pyproject.toml#L114-L127

Thanks. I looked at the documentation. I don't get it. Really sorry, but this part was incomprehensible to me and confused me (that's why I opened the doc issue in your repo). 😉

After I've looked at your example, it seems we're one step further. I did this:

[tool.hatch.envs.docs.scripts]
# The default command are on Linux:
build = "make -C docs html"
linkcheck = "make -C docs linkcheck"
serve = "python3 -m webbrowser -t docs/_build/html/index.html"

[[tool.hatch.envs.docs.matrix]]
platform = ["windows", "macos",]

[tool.hatch.envs.docs.overrides]
matrix.platform.scripts = [
    {
      run = "docs/make.bat -C docs html",
      value = "build",
      if = ["windows"]
    },
    {
      run = "docs/make.bat -C docs linkcheck",
      value = "linkcheck",
      if = ["windows"]
    },
]

However, when I run hatch env show, I get this error message:

TOMLDecodeError: Invalid initial character for a key part (at line 128, column 6)

The line 128 points to the curly open brace. 🤔
Furthermore, the syntax highlighting for matrix.platform.scripts in VSCode complains about

Expected "=", [ \t] or [A-Za-z0-9_\-] but "." found.Toml Parser

That is a minor issue, but still it's a bit confusing.

when I need the list of all supported Python versions I need to replicate it [...] Can we have the list only at one place and maybe refer to it

No https://hatch.pypa.io/latest/config/environment/overview/#inheritance

I was afraid of that. That's really unfortunate. Is it planned to maybe have some sort of "global" variables in the config that can be referenced and used in different parts of the config?

I'm sorry, but I don't want to have the same information again in different parts. That only creates inconsistencies. Would it be worth to open a feature request?

Many thanks for your help!

@tomschr
Copy link
Member

tomschr commented Nov 28, 2022

Ok, I think, I "fixed" it. It were some TOML problems.
The strange thing is I had to quote matrix.platform.scripts as "matrix.platform.scripts" and remove the linebreaks:

[tool.hatch.envs.docs.scripts]
build = "make -C docs html"
linkcheck = "make -C docs linkcheck"
serve = "python3 -m webbrowser -t docs/_build/html/index.html"
_build_win = "docs/make.bat -C docs html"

[tool.hatch.envs.docs.overrides]
"matrix.platform.scripts" = [
    { key="build", value = "build", if = ["linux"] },
    { run="_build_win", value = "build", if = ["windows"] },
    { run="docs/make.bat -C docs linkcheck", value = "linkcheck", if = "windows" },
]

Does that make sense?

At least it gives me no errors:

Output of hatch env show
$ hatch env show
                 Standalone                 
┏━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━━┓
┃ Name  ┃ Type    ┃ Dependencies ┃ Scripts ┃
┡━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━━┩
│ style │ virtual │ black        │ fmt     │
│       │         │ flake8       │ lint    │
│       │         │ pycodestyle  │         │
├───────┼─────────┼──────────────┼─────────┤
│ tox   │ virtual │ tox          │ test    │
└───────┴─────────┴──────────────┴─────────┘
                                 Matrices                                  
┏━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Name    ┃ Type    ┃ Envs         ┃ Dependencies             ┃ Scripts   ┃
┡━━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ default │ virtual │ py37         │ towncrier                │           │
│         │         │ py38         │ wheel                    │           │
│         │         │ py39         │                          │           │
│         │         │ py310        │                          │           │
│         │         │ py311        │                          │           │
├─────────┼─────────┼──────────────┼──────────────────────────┼───────────┤
│ docs    │ virtual │ docs.linux   │ sphinx-argparse          │ build     │
│         │         │ docs.windows │ sphinx-autodoc-typehints │ linkcheck │
│         │         │ docs.macos   │                          │ serve     │
├─────────┼─────────┼──────────────┼──────────────────────────┼───────────┤
│ test    │ virtual │ test.py37    │ coverage[toml]           │ cov       │
│         │         │ test.py38    │ pytest-cov               │ no-cov    │
│         │         │ test.py39    │                          │           │
│         │         │ test.py310   │                          │           │
│         │         │ test.py311   │                          │           │
└─────────┴─────────┴──────────────┴──────────────────────────┴───────────┘

I can run it with:

$ hatch run +platform=linux docs:build

However, if I leave out the +platform=linux it runs all platforms. Can I set a default? Can I configure it in a way that only one of the matrix runs depending on the OS?

@fleetingbytes
Copy link
Contributor Author

However, if I leave out the +platform=linux it runs all platforms. Can I set a default? Can I configure it in a way that only one of the matrix runs depending on the OS?

I don't know these things. Just thinking out loud, I would solve it by having a little utility script around in the repository (excluded from the package build) and there I would program the CLI with the defaults and all the logic. My next step would be to generalise it and make this a utility, since it will be likely needed in other Hatch projects. Or maybe Hatch has already thought of this and it would be possible to write it as a hatch plugin. Maybe it would be possible to write an environment plugin which would dynamically generate a build environment based on some parameters (such environment would have some dynamically generated scripts), and then you could run those scripts.

References:

@tomschr
Copy link
Member

tomschr commented Dec 2, 2022

I don't know these things. Just thinking out loud, I would solve it by having a little utility script around in the

I had a similar thought; maybe a wrapper around +platform=linux and +platform=window for the respective OS. 🤷‍♂️ 🤔
On the other side, it should be already integrated in Hatch. There should be a way to do it, we just haven't discovered it yet.

Could you ask on the Hatch discussion forum? Maybe the community could help us.

Or maybe Hatch has already thought of this and it would be possible to write it as a hatch plugin.

That would be probably the more general and platform neutral way.

However, keep in mind, it's about semver, not hatch. 😉 Hatch is just the means to build and maintain semver. I don't have the time nor the capacity to maintain plugins. Whatever approach we choose, it should be stable, simple, and easy to maintain.

@fleetingbytes
Copy link
Contributor Author

fleetingbytes commented Dec 2, 2022

Could you ask on the Hatch discussion forum? Maybe the community could help us.

To ask for a solution in a forum I need to understand the problem. I don't understand why we need to have a matrix of documentation envs and why we occasionally need to run a script which would only do just one piece of the matrix. I'm missing the whole picture here.

In your former post I don't get where this "matrix.platform.scripts" you had to enclose in double-quotes is, how does it look like, how does it relate to the quoted [tool.hatch.envs.docs.scripts], and [tool.hatch.envs.docs.overrides]. Where can I see the current code of pyproject.toml? Because it seems you made some improvements to it but I can only see what I commited.

Let me lay out my understanding (and the lack thereof) of what we actually want from hatch. Our ultimate goal here is to have a pyproject.toml which could be utilised by hatch to:

  • run the test code in several different python versions and generate the respective reports and coverage reports
    • do you want to do all this locally by running some hatch scripts?
    • Is this all not already being done in the Github CI?
    • should this be done in the Github CI by invoking some hatch scripts?
  • generate the API documentation
    • why do we need a platform matrix for documentation?
    • are there platform-dependent versions of API-documentation?
    • Even if there were, when we build a new version of python-semver documentation, do we not need to build it for all supported platforms anyway?
    • why would we need a script with some extra CLI parameters to build just one particular version of the documentation?
  • lint and format the code
    • although, again, this is something which could be done in the CI (in the CI I would suggest to run the real thing. Invoke the linters and formatters directly, rather than through some hatch script.)
  • build wheel and src packages of python-semver
    • Github CI should probably do this as well.
  • bump the project version
  • generate the changelog (towncrier config, scripts)
  • publish the build artifacts in TestPyPI, PyPI
    • Github CI could probably do this as well.

I have the feeling I misunderstand here a lot. Maybe you don't want different versions of API-documentations, maybe it's just the sphinx invocation and the whole doc-building script that must be different on Windows than it is on linux and for this we'd need one linux script, and one for script for Windows (.bat or .ps1, or both). (Well then let's have them, no?) Or let's just let the API documentation be built in the Github CI which is always whatever you set it, let's say linux, and then we would only need just one kind of script to invoke, no?

It's like ... I kind of see (or can guess) what you want, but I don't understand why you want to solve it this particular way. From my point of view there are better ways to do this kind of things than to try to do everything manually by invoking hatch scripts.

@tomschr
Copy link
Member

tomschr commented Dec 2, 2022

To do that I'm missing the whole picture here.

First thanks for your great post and really sorry for not be clear enough with my ideas.

[...]
Where can I see the current code of pyproject.toml?

It's not in your branch (yet) as you need to allow me to push. I hope I got it right, but you need to allow changes to a pull request branch created from a fork. It's should be only a checkmark, If you are okay with that, I can push my changes for your review.

Let me lay out my understanding (and the lack thereof) of what we actually want from hatch.

You raised a lot of good questions. In the following list I try to answer them as good as I can. Hope that make things a bit clearer. Again, many thanks for making this list! ❤️

run the test code in several different python versions and generate the respective reports and coverage reports
[...]

Yes. If we switch to hatch, the commands should run locally and on our GitHub Action workflow (=CI). Otherwise it becomes inconsistent.

Currently, in the CI this is done by tox.

generate the API documentation

why do we need a platform matrix for documentation?

To my understanding, the matrix is needed to provide different commands for Windows and Linux. I may be wrong, I'm not a Hatch expert. This was the part that I was trying to solve in my previous comments.

are there platform-dependent versions of API-documentation?
[...] do we not need to build it for all supported platforms anyway?

Oh, you mean dependent on the Python version? No, the API documentation is for all Python versions, there is nothing specific. The build part is done by Sphinx.

lint and format the code

could be done in the CI

I would advise against this. Technically it could be done. However, it doesn't mean we should. In my experience, this opens all sort of problems. Additionally, it would mean we need to add more GitHub Action power.

That's why we have only a "check" GH Action job for linting and formatting. It doesn't commit automatically, but will report back to the developer (as you can see in this PR).
Automation is good, but I would like to be in control of some things.

build wheel and src packages of python-semver

Yes, that would be something we could (and should) automate. That would relate to issue #205 and possibly is a task for a different pull request as it's out of scope for this PR (also it needs some thoughts what the workflow should look like).

bump the project version
generate the changelog (towncrier config, scripts)

As described above, I'm hesitant to do automatic commits etc. For my taste, that would be too much automation. From my experiences, changelogs are better done manually.

publish the build artifacts in TestPyPI, PyPI

Github CI could probably do this as well.

Probably yes. Also related to issue #205.

I have the feeling I misunderstand here a lot.

Not really. :-) You did already a great job. Thanks for asking and sorry for making your PR harder than it should be. Wasn't my intention.

It's like ... I kind of see (or can guess) what you want, but I don't understand why you want to solve it this particular way

Well, there are different reasons. 🙂 Some goes mostly into "you can't teach an old dog a lot of new tricks" territory. 😂

At the moment, tox is used to maintain this project. If you run tox -av you can see a list of all the targets/tasks that the tox.ini file provides. That file exists for several years now. Currently, I do most of the work so I tried to make it fit my workflow. I get used to, it's stable, I like it, and most of the time I understand it. 😉

That doesn't mean it's perfect nor it can't be improved or replaced with something better. But if we want to replace this, it should have some benefits. "Modern" is not always equivalent to "better". If we can replicate somehow the tox targets I think it would fit nicely into my workflow.


TL;DR: From my point of view, we could use hatch to:

  • run the test suite against different Python versions
  • check code against style/formatting and docstrings
  • run typing checks
  • create changelog updates
  • create a source distribution
  • prepare/create archives for TestPyPI and PyPI

A lot of of these targets overlap with tox (which is intended). Perhaps we can replace tox completely with hatch. I don't know yet if it's possible or desirable. There should be only one tool which does it all.

How we really design the targets is probably a matter of taste and how granular we want to have them. Certainly it also depends on what hatch supports. However, I can't do that alone as I need support.

A second goal is to adapt the GitHub Action workflow. This would be out of scope for this PR, but should be done soon after we merge your branch.

Does that help?

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.

None yet

3 participants