<div dir="auto"><div><div data-smartmail="gmail_signature">On Aug 26, 2017 5:13 PM, "Nathaniel Smith" <<a href="mailto:njs@pobox.com">njs@pobox.com</a>> wrote:</div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">On Sat, Aug 26, 2017 at 1:47 PM, C Anthony Risinger<br>
<<a href="mailto:c@anthonyrisinger.com">c@anthonyrisinger.com</a>> wrote:<br>
> On Aug 26, 2017 2:17 PM, "Nathaniel Smith" <<a href="mailto:njs@pobox.com">njs@pobox.com</a>> wrote:<br>
>><br>
>> [removed Guido from CC]<br>
>><br>
>> On Aug 26, 2017 02:29, "Paul Moore" <<a href="mailto:p.f.moore@gmail.com">p.f.moore@gmail.com</a>> wrote:<br>
>><br>
>> On 26 August 2017 at 03:17, Guido van Rossum <<a href="mailto:guido@python.org">guido@python.org</a>> wrote:<br>
>> > In pretty much any other context, if you have an operation that returns<br>
>> > an<br>
>> > regular value or an error value, the error value should be None.<br>
>> > (Exceptions<br>
>> > include e.g. returning a non-negative int or -1 for errors, or True for<br>
>> > success and False for errors.)<br>
>><br>
>> So, given that build_sdist returns the path of the newly built sdist,<br>
>> the correct way to signal "I didn't manage to build a sdist" would be<br>
>> to return None.<br>
>><br>
>> Now that it's put this way, it seems glaringly obvious to me that this<br>
>> is the correct thing to do.<br>
>><br>
>><br>
>> Eh... I would really prefer something that's (a) more explicit about what<br>
>> specifically went wrong, and (b) harder to return by accident. It's not at<br>
>> all obvious that if the list of requirements is 'None' that means 'this<br>
>> build supports making sdists in general but cannot make them from this<br>
>> source tree but might still be able to make a wheel'. And if you forget to<br>
>> put in a return statement, then python returns None for you, which seems<br>
>> like it could lead to some super confusing error modes.<br>
><br>
><br>
> Why does the frontend need to know why an sdist was not created?<br>
<br>
</div>This whole discussion is about handling a specific case: suppose you<br>
have a frontend like pip that when given a source directory and asked<br>
to build a wheel, wants to implement that as:<br>
- build sdist<br>
- unpack sdist<br>
- build wheel from unpacked sdist<br>
<br>
And suppose you have a backend like flit, that can build sdists from<br>
some source directories (e.g. VCS checkouts) but not others (e.g.<br>
unpacked sdists). We need some way for pip and flit to negotiate that<br>
even though pip *normally* would implement its build-a-wheel operation<br>
by first building an sdist, in this case it's ok to silently fall back<br>
to some other strategy (like building the wheel directly in the source<br>
tree, or manually copying the source tree somewhere else and then<br>
building a wheel in it).<br>
<br>
But, we don't want this fallback behavior to hide real bugs. So if the<br>
backend says "look, I just can't do sdists here, and that's an<br>
expected thing, it's not something where the user needs to take any<br>
particular action like filing a bug report or fixing their system or<br>
anything like that, so if you have an alternative way to accomplish<br>
what you're trying to do then you should just silently discard this<br>
error and try that", ...cool. But if it doesn't explicitly say that,<br>
then we don't want to silently discard the error and do something<br>
else.<br>
<br>
It's taken a *lot* of back and forth to reach consensus that all we<br>
need here is some special error signal from the *_sdist operations.<br>
Let's focus on resolving that :-)</blockquote></div></div></div><div dir="auto"><div style="" dir="auto"><div style="margin:16px 0px"><div style=""><div dir="auto" style=""><div style=""><div style=""><div dir="auto" style="">Sure sure, I understand all that, and why we think we need some special error signal from `build_sdist`, as currently written.</div><div dir="auto" style=""><br></div><div dir="auto" style="">What I'm suggesting, is maybe calling `build_sdist` without knowing if it can succeed is already a mistake.</div><div dir="auto" style=""><br></div><div dir="auto" style="">Consider instead, if we make the following small changes:</div><div dir="auto" style=""><br></div><div dir="auto" style="">1. `get_requires_for_build_*` is passed the sdist and wheel directories, just like `build_*`, giving them the chance to actually look at tree before deciding what other reqs might be necessary.</div><div dir="auto" style=""><br></div><div dir="auto" style="">2. `get_requires_for_build_*` returns None to signal `build_*` is unsupported (superceded by static reqs defined in TOML) and [...] to signal support (can be empty).</div><div dir="auto" style=""><br></div><div dir="auto" style="">3. `get_requires_for_build_*` assumed to return None if missing (so optional and implies no support).</div><div dir="auto" style=""><br></div><div dir="auto" style="">4. sdist reqs = `get_requires_for_build_sdist` (dynamic) + ??? (static)<br></div><div dir="auto" style=""><br></div><div dir="auto" style="">5. wheel reqs = `get_requires_for_build_wheel` (dynamic) + `build-system.requires` (static)</div><div dir="auto" style=""><br></div><div dir="auto" style="">6. If no reqs are found for sdist (no declared reqs in TOML and `get_requires_for_build_sdist` is missing or returns None), then `build_sdist` is unsupported.</div><div dir="auto" style=""><br></div><div dir="auto" style=""><div dir="auto">7. If no reqs are found for wheel (no declared reqs in TOML and `get_requires_for_build_wheel` is missing or returns None), then `build_wheel` is unsupported. This one is a spec violation because at least one req is expected here (the backed itself).</div><br></div><div dir="auto" style="">This arrangement allows pip to know ahead-of-time if `build_sdist` is appropriate. If no sdist reqs are explicitly acknowledged, then no sdist can be created. Full stop.</div><div dir="auto" style=""><br></div><div dir="auto" style="">Example usages:</div><div dir="auto" style=""><br></div><div dir="auto" style="">* Backeds that only support wheel creation do not implement `get_requires_for_build_sdist` at all.</div><div dir="auto" style=""><br>* Backeds that conditionally support sdist creation implement `get_requires_for_build_sdist` and return None if unsupported. This is where flit signals "impossible" from an unpacked sdist and [...] from VCS.</div><div dir="auto" style=""><br>* Backeds that always support sdist creation (setuptools) implement `get_requires_for_build_sdist` and return [...] unconditionally.<br></div><div dir="auto" style=""><br></div><div dir="auto" style="">So for pip's sdist -> unpack -> wheel path, it knows right away if it should build an sdist or skip right to building a wheel because the backend will inform via `get_requires_for_build_sdist` instead of overloading `build_sdist`.</div><div dir="auto" style=""><br></div><div dir="auto" style="">I also think this achieves nice parity across sdist and wheel operations, keeps the same hooks optional, adds no special cases, and maybe even less overhead.</div><div dir="auto" style=""><br></div><div dir="auto" style="">What say you?</div><div dir="auto" style=""><br></div><div dir="auto" style="">-- </div><div dir="auto" style=""><br></div><div dir="auto" style="">C Anthony</div></div></div></div></div></div></div></div></div>