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

[DO NOT MERGE] 4133: Complete hatchling #12

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

btlogy
Copy link

@btlogy btlogy commented Dec 11, 2024

Complete hatchling, at least for nix (and maybe to make it work for debian-12 and ubuntu-24.04 too if it make sense)?

ticket:4133

WARN: this is mostly a playground to learn, not to be merged.

@btlogy btlogy force-pushed the 4133.complete-hatchling branch from 75a301d to e4af300 Compare December 11, 2024 15:46
@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

In this sandbox branch on top of the changes that were fixing nix build before the merge of hatch:

  • the nix pkgs are now correctly build using hatch
  • but the allmydata.test.test_client.Basic.test_versions is failing because version is unknown (under nix only).

WiP:

  1. Figure what this test is actually doing here:

    https://github.com/tahoe-lafs/tahoe-lafs/blob/dc6ea19d9697a4e95cf8cad999205ec7d0ac6843/src/allmydata/test/test_client.py#L587-L608

  2. Identify what portion of the "client" code would responsible to return this unknown version, presumably here (called "server"):

    https://github.com/tahoe-lafs/tahoe-lafs/blob/dc6ea19d9697a4e95cf8cad999205ec7d0ac6843/src/allmydata/storage/http_server.py#L702-L724

  3. Understand why this version differs on nix and how it was previously handled (and working) before we switched to hatch.
    ...

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

3. Understand why this version differs on nix and how it was previously handled (and working) before we switched to hatch.
...

The application-version comes from allmydata.__full_version__, which is declared in __init__.py.
But our _version.py postPatch is overriding __version__ and full_version, but not __fullversion__.
Adding this one does not seem to help...

I need to be sure that:

  1. the postPatch is generating the correct _version.py file,
  2. the _version.py file is actually consumed in the hatch build process.

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

The application-version comes from allmydata.__full_version__, which is declared in __init__.py.
But our _version.py postPatch is overriding __version__ and full_version, but not __fullversion__.

Wrong: we only need __version__ to be overwritten, because __init__.py says:

    from allmydata._version import __version__
...
__full_version__ = __appname__ + '/' + str(__version__)

So if the call to /protocols/storage/v1 returns application-version = xxx/unknown, it must be because the _version.py is not patched in the right place.

And indeed, in the outPath of the derivation, I do not see our patched file, but this (similarly to what I see in the src dir on other linux builder):

# file generated by setuptools_scm
# don't change, don't track in version control
TYPE_CHECKING = False
if TYPE_CHECKING:
    from typing import Tuple, Union
    VERSION_TUPLE = Tuple[Union[int, str], ...]
else:
    VERSION_TUPLE = object

version: str
__version__: str
__version_tuple__: VERSION_TUPLE
version_tuple: VERSION_TUPLE

__version__ = version = '1.19.0.post1'
__version_tuple__ = version_tuple = (1, 19, 0)

The version is correct there, and BTW: calling tahoe-lafs --version does return '1.19.0.post1'!
So why is this different when running the test!?

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

Commit 3e3ce7a should fix the unknown version in the test.
But this has already been fixed in tahoe-lafs#1403 in a similar way by @hacklschorsch.

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

The version is correct there, and BTW: calling tahoe-lafs --version does return '1.19.0.post1'!
So why is this different when running the test!?

Thanks to @hacklschorsch:

  • the package is build using hatchling version tool,
  • but the test are run from the source tree, not the package,
  • so we also have to "patch" the source tree to pass the version to the test node too.

@meejah
Copy link

meejah commented Dec 12, 2024

Why can't you let hatch "patch" the source tree?

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

Why can't you let hatch "patch" the source tree?

It's the way it's done.
But maybe your ask is: can you just patch it once and be done?
Then I suppose the answer is because nix build patches a copy of the source somewhere else (pure) and patch that copy (using hatchling implicitley) to create a package, but the unpatched sources are then re-used to run the test and fails.
That is why we need to explicitely patch those sources again w/ hatchling.

We were thinking w/ @hacklschorsch that we may try the test using the package directly, but we are not sure we are packaging the test suite with it...

Does that answer your question?

@meejah
Copy link

meejah commented Dec 12, 2024

That is why we need to explicitely patch those sources again w/ hatchling.

Except you're not patching them again with hatch, you're patching them with your own Nix template (or whatever that is called). That's what I meant: can you not use hatch there as well?

@meejah
Copy link

meejah commented Dec 12, 2024

We were thinking w/ @hacklschorsch that we may try the test using the package directly, but we are not sure we are packaging the test suite with it...

Are you asking, "should we be shipping the test suite in source tarballs and/or wheels"?

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

Are you asking, "should we be shipping the test suite in source tarballs and/or wheels"?

No. This sounds like a bad idea.
I'm not sure what we can do - but that would be only for nix, maybe 2 packages: one with the test suite, so we can just use that one to test and the version should be correct already w/o having to call hatching. Something like that. No sure it's feasible.

@meejah
Copy link

meejah commented Dec 12, 2024

Are you asking, "should we be shipping the test suite in source tarballs and/or wheels"?

No. This sounds like a bad idea.

What are you asking then?
In any case, we do ship the unit-test suite in the source tarball, at least in the last release.

@btlogy
Copy link
Author

btlogy commented Dec 12, 2024

Are you asking, "should we be shipping the test suite in source tarballs and/or wheels"?

No. This sounds like a bad idea.

What are you asking then?

Sorry if I wasn't clear: I'm merely suggesting that if we could package the test suite as a nix package, maybe we woud not have to call hatchling explicitely

Nothing to change with the wheels we build for other platforms. Sorry for the confusion.

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.

3 participants