Skip to content

python.pipInstallHook: avoid producing wrong direct_url.json file #232414

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.

Sign up for GitHub

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

Closed
yajo wants to merge 4 commits into from
Closed

python.pipInstallHook: avoid producing wrong direct_url.json file #232414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cbc06ff
python.pipInstallHook: avoid producing wrong direct_url.json file
yajo May 2, 2023
9bd6625
python310Packages.lz4: fix build
yajo May 17, 2023
6a4bec7
python: extend audit tmpdir for direct_url.json files
yajo Jul 19, 2023
0c2bbc4
python.pipInstallHook: support wheelName
yajo Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions pkgs/development/interpreters/python/hooks/pip-install-hook.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ pipInstallPhase() {

mkdir -p "$out/@pythonSitePackages@"
export PYTHONPATH="$out/@pythonSitePackages@:$PYTHONPATH"
local _wheelPname="${wheelPname-$pname}"

pushd dist || return 1
@pythonInterpreter@ -m pip install ./*.whl --no-index --no-warn-script-location --prefix="$out" --no-cache $pipInstallFlags
popd || return 1
if ! @pythonInterpreter@ -m pip install "${_wheelPname}" --find-links dist --no-index --no-warn-script-location --prefix="$out" --no-cache $pipInstallFlags; then
echo "Pip install failed, probably because no wheel was found. Change pname or add wheelPname."
echo "Current value: $_wheelPname. Available wheels: $(ls dist)"
exit 1
fi

runHook postInstall
echo "Finished executing pipInstallPhase"
Expand All @@ -22,3 +25,20 @@ if [ -z "${dontUsePipInstall-}" ] && [ -z "${installPhase-}" ]; then
echo "Using pipInstallPhase"
installPhase=pipInstallPhase
fi


pipAuditTmpdir() {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a tool called pip-audit which makes this a bit confusing

Copy link
Contributor Author

@yajo yajo Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to follow the terminology from here because it's mostly extending that safety net:

nixpkgs/pkgs/build-support/setup-hooks/audit-tmpdir.sh

Line 12 in 3fa4913

auditTmpdir() {

But just tell me what name you prefer, I can change it, no problem.

local dir="$out/@pythonSitePackages@"

echo "checking for references to $TMPDIR/ in $dir..."

# OK if no files are found, or they don't have forbidden references
if grep "$TMPDIR/" "$dir"/*/direct_url.json; then
echo "direct_url.json contains a forbidden reference to $TMPDIR/"
exit 1
fi
}

if [[ -z "${noAuditTmpdir-}" ]]; then
fixupOutputHooks+=(pipAuditTmpdir)
fi
3 changes: 2 additions & 1 deletion pkgs/development/python-modules/bootstrapped-pip/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ stdenv.mkDerivation rec {
echo "Building pip wheel..."
pushd pip
rm pyproject.toml
${python.pythonForBuild.interpreter} -m pip install --no-build-isolation --no-index --prefix=$out --ignore-installed --no-dependencies --no-cache .
${python.pythonForBuild.interpreter} -m pip wheel --wheel-dir dist --no-build-isolation --no-index --no-dependencies --no-cache .
${python.pythonForBuild.interpreter} -m pip install pip --find-links dist --no-build-isolation --no-index --prefix=$out --ignore-installed --no-dependencies --no-cache
popd
'';

Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/python-modules/lz4/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
}:

buildPythonPackage rec {
pname = "python-lz4";
pname = "lz4";
version = "4.3.2";
format = "setuptools";

Expand Down
3 changes: 2 additions & 1 deletion pkgs/development/python-modules/twisted/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ buildPythonPackage rec {
pname = "twisted";
version = "22.10.0";
format = "setuptools";
wheelPname = "Twisted";

disabled = pythonOlder "3.6";

src = fetchPypi {
pname = "Twisted";
pname = wheelPname;
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pname = wheelPname;
pname = wheelName;

not sure if that is better or if we should rename the attr in fetchPypi

Copy link
Contributor Author

@yajo yajo Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think wheelPname is better because it is more intuitive that you need to put the value that you would be putting in pname and not in name. The wheel files that are generated include the pname and version (aka name), so here we really care about the pname part.

Copy link
Contributor Author

@yajo yajo Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if that is better or if we should rename the attr in fetchPypi

Most times the value will be the same. I can add that attribute there, and assert you're setting only one of both. Since those derivations are FODs, changes there shouldn't produce mass rebuilds.

inherit version;
extension = "tar.gz";
hash = "sha256-Mqy9QKlPX0bntCwQm/riswIlCUVWF4Oot6BZBI8tTTE=";
Expand Down