Fix distribution embeds and update docs

[nmay231]

Closes #79

I updated the docs with what would have (hopefully) prevented any of my confusion.

[nmay231]

Hold on, this is broken. I thought I tested this change, but I must have been using the wrong one when doing so. The binary compiles fine with this change but doesn't run.

Error: download failed: ../dist/python.tar.gz

Caused by:
    0: builder error: relative URL without a base
    1: relative URL without a base

[nmay231]

I'm not exactly certain what the issue was before, but I haven't gotten it since I started deleting ~/.local/share/pyapp and ~/.cache/pyapp before each test install.

Changing the line to local_path does fix the main issue, except I also had to update PYAPP_DISTRIBUTION_PYTHON_PATH from bin/python3 to python/bin/python3 since that's how the standalone archives are arranged. Docs have been updated to reinforce that point.

[ofek]

I don't have time to review this today but can you please make sure the change is correct? I'm almost positive that the empty string is required to indicate that there is no source at runtime.

[nmay231]

Certainly!

During runtime, there is no check at all; it blindly tries downloading the source if no embed exists. During build-time, the behavior is identical to providing PYAPP_DISTRIBUTION_SOURCE and PYAPP_DISTRIBUTION_EMBED except that PYAPP_DISTRIBUTION_PIP_AVAILABLE gets set to true; that's a simple fix though. There is no other .is_empty() check that changes behavior.

To clarify what I should do, I need to know how you intend PYAPP_DISTRIBUTION_PATH to be used. Do you intend *_PATH to only be one of the standalone (or pypy) builds, or do you expect someone to zip their local python builds and package that? My personal usecase was to cache the download of the build(s) to allow faster iteration.

In any case, I think we should deprecate *_PATH and allow *_SOURCE to be a file. Then any branching on embed vs download would check if *_SOURCE is a url.

Sidenote: I remember why I got the error I was getting earlier.

Error: download failed: ../dist/python.tar.gz

I was curling the tar file without -L and getting an empty file because of url redirects. distribution.rs:materialize then assumed since there was no embedding (empty file) that it has to download from a url.

[nmay231]

I updated to fix the issue with PYAPP_DISTRIBUTION_PIP_AVAILABLE.

The only issues that remain are:

• Do you want to deprecate PYAPP_DISTRIBUTION_PATH? (the reason why is explained above)
• How would we automatically test this and future changes?

You have the test.yml workflow that runs cowsay on a matrix of OSs and architectures, but that's obvisouly not enough on its own since this bug went unnoticed.

I could probably add tests, but I'm unfortunately no longer using pyapp for the project I'm working on right now so I'm less motivated to work on this contribution. However, I'll try not to leave this issue unresolved.

[ofek]

Thanks for your patience here! I'm about to push a change and merge this.

• Yes the option is intended for any prebuilt distribution as long as it's compressed with a known format.
• Currently, I don't want to deprecate the option because I like the explicitness of options for URLs vs paths. I'm open to changing my mind however.
• Testing is a good question! I honestly haven't had time to write proper tests so no worries, my bad.
• I'm going to remove the pip available change because it's possible that custom distributions that folks provide do not have pip already installed. I will update the documentation to make it more explicit that it would be a performance benefit in that case.

[ofek]

FYI I see what was happening and the pip available conditional you had there was indeed necessary so I added that back. I need more sleep... and to write tests lol

[nmay231]

Lol, we all could probably use a bit more sleep :)