• [gentoo-dev] [PATCH 1/2] distutils-r1.eclass: teach setuptools to respe

    From Eli Schwartz@21:1/5 to All on Tue Sep 12 21:20:01 2023
    Previously, setup.py was handled by:

    - manually passing makejobs, with a heuristic to guess whether it was a
    time saver to do so.
    - rm -rf'ing the build directory in between python versions to prevent
    cross-version contamination

    This is because in PEP 517 mode, it doesn't accept build options
    specific to a setuptools phase. So a crude hack is to just build_ext
    twice, once explicitly and once internally as part of bdist_wheel, and
    pray that in the latter case it detects that there's nothing to do. Unfortunately, sometimes build_ext does NOT detect that there is nothing
    to do -- e.g. for codegen tools such as mypyc, that produce *.c files
    which are different every time you try building. As for build
    directories, those were given up on as hopeless.

    There's a better hack which is to set a magic environment variable for a setup.cfg file which is parsed additionally to the one provided by the
    project. It can contain additional settings, such as the build-base and parallelism, which means that bdist_wheel intrinsically builds
    extensions in parallel the only time it is called. And we can set the
    output directory for all build artifacts to outside of the source tree,
    so it is no longer necessary to delete them (which among other things,
    makes debugging difficult).

    This is similar to .pydistutils.cfg, but is processed later and can be
    in arbitrary locations. Since we store it in the per-impl build
    directory we don't need to wipe it after using it to avoid leakage.

    Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
    ---
    eclass/distutils-r1.eclass | 48 ++++++++------------------------------
    1 file changed, 10 insertions(+), 38 deletions(-)

    diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
    index 91de144e1110..dd197a5f0693 100644
    --- a/eclass/distutils-r1.eclass
    +++ b/eclass/distutils-r1.eclass
    @@ -1461,12 +1461,6 @@ distutils_pep517_install() {
    [[ -n ${wheel} ]] || die "No wheel name returned"

    distutils_wheel_install "${root}" "${WHEEL_BUILD_DIR}/${wheel}"
    -
    - # clean the build tree; otherwise we may end up with PyPy3
    - # extensions duplicated into CPython dists
    - if [[ ${DISTUTILS_USE_PEP517:-setuptools} == setuptools ]]; then
    - rm -rf build || die
    - fi
    }

    # @FUNCTION: distutils-r1_python_compile
    @@ -1478,9 +1472,6 @@ distutils_pep517_install() {
    #
    # If DISTUTILS_USE_PEP517 is set to any other value, builds a wheel
    # using the PEP517 backend and installs it into ${BUILD_DIR}/install.
    -# May additionally call build_ext prior to that when using setuptools
    -# and the
  • From Ulrich Mueller@21:1/5 to All on Tue Sep 12 22:00:01 2023
    On Tue, 12 Sep 2023, Eli Schwartz wrote:

    + mkdir -p "${BUILD_DIR}" || die
    + local -x DIST_EXTRA_CONFIG="${BUILD_DIR}/extra-setup.cfg"
    + cat > "${DIST_EXTRA_CONFIG}" <<-EOF
    + [build]
    + build_base = ${BUILD_DIR}/build
    +
    + [build_ext]
    + parallel = ${jobs}
    + EOF

    "|| die" should also be added for the cat command.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Eli Schwartz@21:1/5 to Ulrich Mueller on Wed Sep 13 04:10:01 2023
    On 9/12/23 3:56 PM, Ulrich Mueller wrote:
    On Tue, 12 Sep 2023, Eli Schwartz wrote:

    + mkdir -p "${BUILD_DIR}" || die
    + local -x DIST_EXTRA_CONFIG="${BUILD_DIR}/extra-setup.cfg"
    + cat > "${DIST_EXTRA_CONFIG}" <<-EOF
    + [build]
    + build_base = ${BUILD_DIR}/build
    +
    + [build_ext]
    + parallel = ${jobs}
    + EOF

    "|| die" should also be added for the cat command.


    Redirecting output to a file in a directory you have just guaranteed to
    exist cannot fail. (mkdir -p will return nonzero and die, if it exits
    without having guaranteed its target is a directory on disk. There is
    one loophole, and that is if its target already existed beforehand, but
    the mkdir -p process did not have write permissions for it; mkdir will
    not check permissions if it detects it doesn't have to do work. This
    should not be the case inside the workdir, or something has gone
    significantly wrong beforehand.)

    That being said, as a style guide concern I can see why being cautious
    is generally desirable, as it's much easier to review code that uses it
    when it isn't strictly necessary than code that doesn't use it when it
    turns out to be necessary.

    I'm not very used to this yet :) so I blindly assumed while writing it
    that it wasn't necessary to do some additional typing and add this for
    call sites that shouldn't be able to fail. I will add this in the next revision.



    --
    Eli Schwartz

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From =?UTF-8?Q?Micha=C5=82_G=C3=B3rny?=@21:1/5 to Eli Schwartz on Wed Sep 13 04:30:01 2023
    On Tue, 2023-09-12 at 22:07 -0400, Eli Schwartz wrote:
    On 9/12/23 3:56 PM, Ulrich Mueller wrote:
    On Tue, 12 Sep 2023, Eli Schwartz wrote:

    + mkdir -p "${BUILD_DIR}" || die
    + local -x DIST_EXTRA_CONFIG="${BUILD_DIR}/extra-setup.cfg"
    + cat > "${DIST_EXTRA_CONFIG}" <<-EOF
    + [build]
    + build_base = ${BUILD_DIR}/build
    +
    + [build_ext]
    + parallel = ${jobs}
    + EOF

    "|| die" should also be added for the cat command.


    Redirecting output to a file in a directory you have just guaranteed to
    exist cannot fail.

    Eh, you make me prove you wrong:

    # cat > dupa <<-EOF
    blahblah
    EOF
    cat: write error: No space left on device

    --
    Best regards,
    Michał Górny

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Eli Schwartz@21:1/5 to All on Wed Sep 13 05:00:01 2023
    On 9/12/23 10:26 PM, Michał Górny wrote:
    On Tue, 2023-09-12 at 22:07 -0400, Eli Schwartz wrote:
    On 9/12/23 3:56 PM, Ulrich Mueller wrote:
    On Tue, 12 Sep 2023, Eli Schwartz wrote:

    + mkdir -p "${BUILD_DIR}" || die
    + local -x DIST_EXTRA_CONFIG="${BUILD_DIR}/extra-setup.cfg"
    + cat > "${DIST_EXTRA_CONFIG}" <<-EOF
    + [build]
    + build_base = ${BUILD_DIR}/build
    +
    + [build_ext]
    + parallel = ${jobs}
    + EOF

    "|| die" should also be added for the cat command.


    Redirecting output to a file in a directory you have just guaranteed to
    exist cannot fail.

    Eh, you make me prove you wrong:

    # cat > dupa <<-EOF
    blahblah
    EOF
    cat: write error: No space left on device


    ಠ_ಠ

    Is portage generally expected to successfully complete (including
    internal metadata write stages) when its workdir drive runs out of space partway through?


    --
    Eli Schwartz

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Sam James@21:1/5 to Eli Schwartz on Wed Sep 13 05:20:02 2023
    Eli Schwartz <eschwartz93@gmail.com> writes:

    On 9/12/23 10:26 PM, Michał Górny wrote:
    On Tue, 2023-09-12 at 22:07 -0400, Eli Schwartz wrote:
    On 9/12/23 3:56 PM, Ulrich Mueller wrote:
    On Tue, 12 Sep 2023, Eli Schwartz wrote:

    + mkdir -p "${BUILD_DIR}" || die
    + local -x DIST_EXTRA_CONFIG="${BUILD_DIR}/extra-setup.cfg"
    + cat > "${DIST_EXTRA_CONFIG}" <<-EOF
    + [build]
    + build_base = ${BUILD_DIR}/build
    +
    + [build_ext]
    + parallel = ${jobs}
    + EOF

    "|| die" should also be added for the cat command.


    Redirecting output to a file in a directory you have just guaranteed to
    exist cannot fail.

    Eh, you make me prove you wrong:

    # cat > dupa <<-EOF
    blahblah
    EOF
    cat: write error: No space left on device


    ಠ_ಠ

    Is portage generally expected to successfully complete (including
    internal metadata write stages) when its workdir drive runs out of space partway through?

    oh you

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Alexey Sokolov@21:1/5 to All on Wed Sep 13 10:00:01 2023
    13.09.2023 03:52, Eli Schwartz пишет:
    On 9/12/23 10:26 PM, Michał Górny wrote:
    On Tue, 2023-09-12 at 22:07 -0400, Eli Schwartz wrote:
    On 9/12/23 3:56 PM, Ulrich Mueller wrote:
    On Tue, 12 Sep 2023, Eli Schwartz wrote:

    + mkdir -p "${BUILD_DIR}" || die
    + local -x DIST_EXTRA_CONFIG="${BUILD_DIR}/extra-setup.cfg"
    + cat > "${DIST_EXTRA_CONFIG}" <<-EOF
    + [build]
    + build_base = ${BUILD_DIR}/build
    +
    + [build_ext]
    + parallel = ${jobs}
    + EOF

    "|| die" should also be added for the cat command.


    Redirecting output to a file in a directory you have just guaranteed to
    exist cannot fail.

    Eh, you make me prove you wrong:

    # cat > dupa <<-EOF
    blahblah
    EOF
    cat: write error: No space left on device


    ಠ_ಠ

    Is portage generally expected to successfully complete (including
    internal metadata write stages) when its workdir drive runs out of space partway through?



    This is a race with the user - the user could delete some files from
    disk just in time for metadata to successfully be written

    --
    Best regards,
    Alexey "DarthGandalf" Sokolov

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Ulrich Mueller@21:1/5 to All on Wed Sep 13 11:30:01 2023
    On Wed, 13 Sep 2023, Eli Schwartz wrote:

    "|| die" should also be added for the cat command.

    Redirecting output to a file in a directory you have just guaranteed
    to exist cannot fail.

    That's a rather bold statement. I can imagine a number of possible
    failures, e.g. no space left on device, quota exceeded, or a low-level
    I/O error of the filesystem. Also fork or exec of the cat command could
    fail (e.g. out of memory).

    While either of these may be unlikely here, best practice is to check
    for errors of _all_ external commands.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Orlitzky@21:1/5 to Eli Schwartz on Wed Sep 13 15:20:02 2023
    On Tue, 2023-09-12 at 22:52 -0400, Eli Schwartz wrote:

    Is portage generally expected to successfully complete (including
    internal metadata write stages) when its workdir drive runs out of space partway through?


    No, but I think what everyone else is forgetting to mention is that if
    it's going to fail, we want it to fail as soon as possible, i.e. as
    close to the thing that actually went wrong. If we proceed past ENOSPC
    or whatever, we could get five or six lines further in the ebuild, and
    then some other command will fail but possibly with a crazy unrelated
    error message.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Eli Schwartz@21:1/5 to Michael Orlitzky on Wed Sep 13 23:10:01 2023
    On 9/13/23 9:10 AM, Michael Orlitzky wrote:
    On Tue, 2023-09-12 at 22:52 -0400, Eli Schwartz wrote:

    Is portage generally expected to successfully complete (including
    internal metadata write stages) when its workdir drive runs out of space
    partway through?


    No, but I think what everyone else is forgetting to mention is that if
    it's going to fail, we want it to fail as soon as possible, i.e. as
    close to the thing that actually went wrong. If we proceed past ENOSPC
    or whatever, we could get five or six lines further in the ebuild, and
    then some other command will fail but possibly with a crazy unrelated
    error message.


    The consequences of this particular command failing are that:

    - setuptools will still build, but without spawning a ThreadPoolExecutor
    to distribute jobs; it builds single threaded

    - setuptools will still build, but using its default build directory
    instead of an alternative one; setuptools itself *cannot* fail as a
    result, but other external commands hardcoding the location of
    setuptools internal files could fail to find those files


    (It will still fail when setuptools reports its own ENOSPC, but this is
    not a crazy unrelated error message.)

    (Unless portage expects to successfully complete where possible if the
    workdir drive runs out of space partway through, then quickly has more
    space freed up for it as soon as monitoring warnings are triggered and
    no actually-fatal ebuild errors occurred during the window of ENOSPC.)




    --
    Eli Schwartz

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Eli Schwartz@21:1/5 to Ulrich Mueller on Wed Sep 13 23:10:01 2023
    On 9/13/23 5:27 AM, Ulrich Mueller wrote:
    On Wed, 13 Sep 2023, Eli Schwartz wrote:

    "|| die" should also be added for the cat command.

    Redirecting output to a file in a directory you have just guaranteed
    to exist cannot fail.

    That's a rather bold statement. I can imagine a number of possible
    failures, e.g. no space left on device, quota exceeded, or a low-level
    I/O error of the filesystem. Also fork or exec of the cat command could
    fail (e.g. out of memory).

    While either of these may be unlikely here, best practice is to check
    for errors of _all_ external commands.


    The implementation of `die` performs a fork+exec of the sed command,
    invokes eerror (many times!) which forks to pipe, invokes $() which
    forks, and could behave abnormally due to out of memory. It is not safe
    to treat `|| die` as error recovery for an out of memory condition.

    The implementation of `die` performs multiple writes to files inside of ${PORTAGE_BUILDDIR}, and could behave abnormally due to write failures.
    It is not safe to treat `|| die` as error recovery for a write failure
    of the ${PORTAGE_BUILDDIR} drive (whatever the reason for that write
    failure).

    Regarding:

    - no space left on device
    - quota exceeded
    - low-level I/O error of the filesystem

    this isn't "a number of possible failures" :) it is the same failure but elicited by different prompts. Regarding this, I have already commented...

    (And the `|| die` is added to the next revision of this patch either way.)


    --
    Eli Schwartz

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Ulrich Mueller@21:1/5 to All on Thu Sep 14 10:30:02 2023
    On Wed, 13 Sep 2023, Eli Schwartz wrote:

    That's a rather bold statement. I can imagine a number of possible
    failures, e.g. no space left on device, quota exceeded, or a low-level
    I/O error of the filesystem. Also fork or exec of the cat command could
    fail (e.g. out of memory).

    While either of these may be unlikely here, best practice is to check
    for errors of _all_ external commands.

    The implementation of `die` performs a fork+exec of the sed command,
    invokes eerror (many times!) which forks to pipe, invokes $() which
    forks, and could behave abnormally due to out of memory. It is not safe
    to treat `|| die` as error recovery for an out of memory condition.

    The implementation of `die` performs multiple writes to files inside of ${PORTAGE_BUILDDIR}, and could behave abnormally due to write failures.
    It is not safe to treat `|| die` as error recovery for a write failure
    of the ${PORTAGE_BUILDDIR} drive (whatever the reason for that write failure).

    An eclass must not rely on implementation details of any specific
    package manager.

    "die [...] aborts the build process." https://projects.gentoo.org/pms/8/pms.html#x1-12600012.3.6

    So please report a Portage (or other package manager) bug if you can
    reproduce a condition where die doesn't abort the build process.

    (And the `|| die` is added to the next revision of this patch either way.)

    Thanks.

    Ulrich

    --=-=-Content-Type: application/pgp-signature; name="signature.asc"

    -----BEGIN PGP SIGNATURE-----

    iQFDBAEBCAAtFiEEtDnZ1O9xIP68rzDbUYgzUIhBXi4FAmUCwy4PHHVsbUBnZW50 b28ub3JnAAoJEFGIM1CIQV4uUYIIAJ+i9BCgWEgQLPH8yisHDHx93jo7aZ+5wLDW /oCaiAxmmXiOFOfWM3pX3mFr9doa292kOMuSKbUXzNUNZ/Up4brIyH2887M2kXOJ 3CX9Rh/Rtbcy1zmIxT1JS0UNrTPZ5ZqpKbknLqV10JsOBGcZcVuVkrESeViRx7oi AO9ggyYCza8+qT2LgUnnynfb+u5LVoTafF+IPD44+24TwidZiHp5jkZqdBF6gNxe Sv2t3JDTDXMqBciKvnOH2MAUyAjvO7Hca94OZcjwWnpbU0LW9sCVYZm447yfDBxW yph3Qmrjh1HbkC4afWf8HfoCwLLQPzAVrkIcWK2VD410dlojY+k=Bs+C
    -----END PGP SIGNATURE-----

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)