Add support for disabling installation from pre-built wheels #956

zanieb · 2024-01-17T23:09:09Z

Adds support for disabling installation from pre-built wheels i.e. the package must be built from source locally.
We will still always use pre-built wheels for metadata during resolution.

Available via --no-binary and --no-binary-package <name> flags in pip install and pip sync. There is no flag for pip compile since no installation happens there.

--no-binary

    Don't install pre-built wheels.
    
    When enabled, all installed packages will be installed from a source distribution. 
    The resolver will still use pre-built wheels for metadata.


--no-binary-package <NO_BINARY_PACKAGE>

    Don't install pre-built wheels for a specific package.
    
    When enabled, the specified packages will be installed from a source distribution. 
    The resolver will still use pre-built wheels for metadata.

When packages are already installed, the --no-binary flag will have no affect without the --reinstall flag. In the future, I'd like to change this by tracking if a local distribution is from a pre-built wheel or a locally-built wheel. However, this is significantly more complex and different than pip's behavior so deferring for now.

For reference, pip's flag works as follows:

--no-binary <format_control>

    Do not use binary packages. Can be supplied multiple times, and each time adds to the
    existing value. Accepts either ":all:" to disable all binary packages, ":none:" to empty the
    set (notice the colons), or one or more package names with commas between them (no colons).
    Note that some packages are tricky to compile and may fail to install when this option is
    used on them.

Note we are not matching the exact pip interface here because it seems complicated to use. I think we may want to consider adjusting our interface for this behavior since we're not entirely compatible anyway e.g. I think --force-build and --force-build-package are clearer names. We could also consider matching the pip interface or only allowing --no-binary <package> for compatibility. We can of course do whatever we want in our own install interfaces later.

Additionally, we may want to further consider the semantics of --no-binary. For example, if I run pip install pydantic --no-binary I expect just Pydantic to be installed without binaries but by default we will build all of Pydantic's dependencies too.

This work was prompted by #895, as it is much easier to measure performance gains from building source distributions if we have a flag to ensure we actually build source distributions. Additionally, this is a flag I have used frequently in production to debug packages that ship Cythonized wheels.

zanieb · 2024-01-17T23:28:49Z

crates/puffin-traits/src/lib.rs

@@ -155,3 +159,33 @@ impl Display for BuildKind {
        }
    }
 }
+
+#[derive(Debug)]
+pub enum NoBinary {


Please do correct me if there's somewhere better for this.

I tried to put it alongside Reinstall but it's needed in the resolver as well so I put it in this shared crate.

It's still re-exported in the installer.

Seems okay -- we don't have a much better / obvious answer.

zanieb · 2024-01-17T23:31:24Z

crates/puffin-cli/tests/pip_install.rs

+
+/// Install a package without using pre-built wheels.
+#[test]
+fn install_no_binary() -> Result<()> {


These tests are slow (~5s). Maybe I should use a scenario that builds faster?

It'd also be ideal to have coverage for e.g. installing a wheel via direct URL with --no-binary (which should fail)

zanieb · 2024-01-17T23:36:10Z

crates/puffin-resolver/src/finder.rs

-            // Find the most-compatible wheel
-            for (wheel, file) in files.wheels {


The following made conditional and indented without change.

charliermarsh · 2024-01-18T14:47:02Z

Will review today!

charliermarsh · 2024-01-18T21:35:58Z

crates/puffin-installer/src/lib.rs

@@ -2,9 +2,10 @@ pub use downloader::{Downloader, Reporter as DownloadReporter};
 pub use editable::{BuiltEditable, ResolvedEditable};
 pub use installer::{Installer, Reporter as InstallReporter};
 pub use plan::{Plan, Planner, Reinstall};
+// TODO(zanieb): Just import this properly everywhere else
+pub use puffin_traits::NoBinary;


Is this stale / can it be removed? It looks like you are importing from puffin_traits in the various files.

Yeah commentary at #956 (comment)

We should probably remove it and fix the imports that use this crate.

charliermarsh

This looks good to me, nice work. My only concern is that we now have --no-binary and --no-build which does the opposite. Should those somehow be a single flag...? Should they be marked as conflicting in some sense, since providing both excludes all distributions?

zanieb · 2024-01-18T22:25:16Z

My only concern is that we now have --no-binary and --no-build which does the opposite. Should those somehow be a single flag...?

Well, pip has --no-binary <package,:all:,:none:> and --only-binary <package,:all:,:none:>. Perhaps we should change ours to match this interface?

I don't think we should explore some sort of "unified" flag in the pip-compatible interface. I think we can consider it in our own interface later.

Should they be marked as conflicting in some sense, since providing both excludes all distributions?

Yes I think we should mark them as conflicting.

zanieb · 2024-01-18T23:03:47Z

Opening #977 for the first point.

zanieb force-pushed the zb/no-binary branch from b1714ca to c806ec3 Compare January 17, 2024 23:10

zanieb changed the title ~~Add --no-binary and --no-binary-package <name>~~ Add support for disabling installation from pre-built wheels Jan 17, 2024

zanieb added the enhancement New feature or improvement to existing functionality label Jan 17, 2024

zanieb commented Jan 17, 2024

View reviewed changes

zanieb requested a review from charliermarsh January 18, 2024 14:43

zanieb mentioned this pull request Jan 18, 2024

Use daemon to run Python build hooks #895

Closed

charliermarsh reviewed Jan 18, 2024

View reviewed changes

charliermarsh approved these changes Jan 19, 2024

View reviewed changes

zanieb added 3 commits January 19, 2024 10:01

Add --no-binary and --no-binary-package <name>

f96c16e

Ignore "too many argument" lints

bdde04f

Remove unused NoBinary::is_none

89bc1cb

zanieb force-pushed the zb/no-binary branch from ab5cf38 to 89bc1cb Compare January 19, 2024 16:01

zanieb merged commit 33b35f7 into main Jan 19, 2024
3 checks passed

zanieb deleted the zb/no-binary branch January 19, 2024 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for disabling installation from pre-built wheels #956

Add support for disabling installation from pre-built wheels #956

zanieb commented Jan 17, 2024 •

edited

Loading

zanieb Jan 17, 2024 •

edited

Loading

charliermarsh Jan 18, 2024

zanieb Jan 17, 2024

zanieb Jan 17, 2024 •

edited

Loading

zanieb Jan 17, 2024 •

edited

Loading

charliermarsh commented Jan 18, 2024

charliermarsh Jan 18, 2024

zanieb Jan 18, 2024 •

edited

Loading

charliermarsh left a comment

zanieb commented Jan 18, 2024

zanieb commented Jan 18, 2024

		// Find the most-compatible wheel
		for (wheel, file) in files.wheels {

Add support for disabling installation from pre-built wheels #956

Add support for disabling installation from pre-built wheels #956

Conversation

zanieb commented Jan 17, 2024 • edited Loading

zanieb Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

charliermarsh Jan 18, 2024

Choose a reason for hiding this comment

zanieb Jan 17, 2024

Choose a reason for hiding this comment

zanieb Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

zanieb Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

charliermarsh commented Jan 18, 2024

charliermarsh Jan 18, 2024

Choose a reason for hiding this comment

zanieb Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

charliermarsh left a comment

Choose a reason for hiding this comment

zanieb commented Jan 18, 2024

zanieb commented Jan 18, 2024

zanieb commented Jan 17, 2024 •

edited

Loading

zanieb Jan 17, 2024 •

edited

Loading

zanieb Jan 17, 2024 •

edited

Loading

zanieb Jan 17, 2024 •

edited

Loading

zanieb Jan 18, 2024 •

edited

Loading