Fix passing constraints file path into `pipx install` operation via `--pip-args` #1390

guysalt · 2024-05-04T00:23:53Z

I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

Fix error with pipx install Command Using Relative Path for Pip Arguments

Test plan

Tested by running

pipx install --pip-args=--constraint=some_constraint_file.txt some_package_inside_constraint_file

chrysle

Thanks for catching this! Please also extend the docstring of the changed function.

src/pipx/package_specifier.py

changelog.d/1389.bugfix.md

src/pipx/package_specifier.py

guysalt · 2024-05-04T13:59:49Z

Thanks for catching this! Please also extend the docstring of the changed function.

I have considered this * Convert local paths to absolute paths as the same description, doesn't it?

src/pipx/package_specifier.py

chrysle · 2024-05-07T13:32:41Z

Great, let's get down to business once again, then. Don't fail to inform us about your XONSH compatibility research.
Could you add a functional test, please?

guysalt · 2024-05-07T16:30:02Z

Great, let's get down to business once again, then. Don't fail to inform us about your XONSH compatibility research. Could you add a functional test, please?

sure, hope to do so soon...

guysalt · 2024-05-07T23:12:12Z

Great, let's get down to business once again, then. Don't fail to inform us about your XONSH compatibility research. Could you add a functional test, please?

hey, so I wrote a test, but i will glad if you help me with a question...

the test right now is not passing and I dont understand why.
first I wrote a test just for checking the path is absolute and the command working (with empty constraint file).

then I want to add a check for installing the right constraint from the file.
when I put in the constraint file pycowsay==0.0.0.1 it does not installing the package (says something about conflict).

and if I run similar command on my terminal its working, so im trying to understand what is the different?
maybe you can help me understand its related to the tests architecture or something like this.

guysalt · 2024-05-08T01:53:24Z

Great, let's get down to business once again, then. Don't fail to inform us about your XONSH compatibility research. Could you add a functional test, please?

hey, so I wrote a test, but i will glad if you help me with a question...

the test right now is not passing and I dont understand why. first I wrote a test just for checking the path is absolute and the command working (with empty constraint file).

then I want to add a check for installing the right constraint from the file. when I put in the constraint file pycowsay==0.0.0.1 it does not installing the package (says something about conflict).

and if I run similar command on my terminal its working, so im trying to understand what is the different? maybe you can help me understand its related to the tests architecture or something like this.

understand the catch, add pycowsay==0.0.0.1 to the tests_packages files...
need to see all the tests pass

chrysle

Thanks for adding the tests.

tests/test_install.py

chrysle · 2024-05-08T04:48:38Z

tests/test_install.py

+
+    temp_file = current_directory / constraint_file_name
+    temp_file.touch()
+    temp_file.write_text(f"pycowsay=={pycowsay_version}")


We have a small database for our test packages, which we use in tests.

Suggested change

temp_file.write_text(f"pycowsay=={pycowsay_version}")

temp_file.write_text(PKG["ipython"]["spec"])

@guysalt I think you overlooked this comment?

Also here, if someone change to the newer version the test will not check if the older versions indeed installed.

Changed

testdata/tests_packages/unix-python3.10.txt

tests/test_install.py

Approved to early

chrysle · 2024-05-11T12:05:08Z

tests/test_install.py

+    subprocess_pycowsay_available_versions = subprocess.run(
+        [sys.executable, "-m", "pip", "index", "versions", "pycowsay"], capture_output=True, text=True, check=False
+    )
+    subprocess_pycowsay_available_versions_output = subprocess_pycowsay_available_versions.stdout
+    output_lines = subprocess_pycowsay_available_versions_output.split("\n")
+    available_versions_line = next(line for line in output_lines if "Available versions:" in line)
+    versions_string = available_versions_line.split(":")[1].strip()
+    available_versions = versions_string.split(", ")
+    assert pycowsay_version in available_versions


Why do we need this? Besides, pip index is still experimental.

For double check if the required package version is available and if not, will be more clearly to say why the test fails.

do you prefer to remove this block?

Hmm, if it wasn't available, there would be something wrong with our test packages setup. I'd tend not to over-complicate things.

Ok, just wanted to check the setup for the tests good, and we have two different versions available on the tests and see the constraint flag works and the older version is installed.

Now after I wrote this I also see I did not check there at lease newer version indeed...

Anyway, Im ok also with not checking this,

Removed.

Ok, just wanted to check the setup for the tests good, and we have two different versions available on the tests and see the constraint flag works and the older version is installed.

Although I think we can trust pip it handles the constraints files it is given correctly (if it does find them), creating a constraints file without a proper constraint doesn't seem good. Could you try specifying a version not equal (!=) to the spec in the constraints file? That way, we should be able to determine whether the mechanism works.

ok now I got you, to do so with the PKG[package_name]["spec"] or with const?

I prefer const as the PKG[package_name]["spec"] taking the older version, and with const I can put not equal != to the newer version.

I prefer const as the PKG[package_name]["spec"] taking the older version, and with const I can put not equal != to the newer version.

You could run a replace on that, but either way is fine.

Now I see the newer version isn't the same in all the tests_packages constraint files 😪 ...

How do I run a replace? Can I change it manually?

What do you think about put in the constraint file < from PKG[package_name]["spec"] and then check the version is indeed less then the PKG[package_name]["spec"].

For this I need that every test constraint file will contain at lease one version less the PKG[package_name]["spec"] version.

How do I run a replace? Can I change it manually?

I thought of just using the str types' replace() function?

As far as I'm aware, we don't even need to test whether the version after installing with constraints is less than the spec given; only if it is not equal? Sorry for the hassle here.

Hey im sorry for the delay, yea im sorry for the hassle too lets finish it.

I wanted to check if it less than the spec given, to validate for sure the constraint file is working. as I mention before, even if I using not equal sign and the constraint file somehow does not working, the test still can pass if the pypi versions matching the constraint.

Anyway, if its ok for you its ok for me, so now I believe I did it as you asked for.

thanks for your time!

tests/test_install.py

testdata/tests_packages/unix-python3.10.txt

src/pipx/package_specifier.py

tests/test_install.py

guysalt · 2024-05-19T23:24:50Z

Its seems that the tests fails exactly on what I was trying to tell you on the pip args split...

that the splitting does now working on the workflow, as it does now working on my terminal (xonsh).

do you have a clue why does it happening?

guysalt changed the title ~~Fix error with pipx install command using --pip-args (#1389)~~ Fix error with pipx install command using --pip-args May 4, 2024

guysalt force-pushed the main branch from 2de11f6 to b0ecfd8 Compare May 4, 2024 00:37

chrysle reviewed May 4, 2024

View reviewed changes

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

changelog.d/1389.bugfix.md Outdated Show resolved Hide resolved

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

guysalt force-pushed the main branch 2 times, most recently from 47f4b58 to 138fa87 Compare May 4, 2024 14:14

chrysle reviewed May 4, 2024

View reviewed changes

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

guysalt force-pushed the main branch 3 times, most recently from 6bd33c5 to d9d8663 Compare May 4, 2024 23:32

guysalt changed the title ~~Fix error with pipx install command using --pip-args~~ Fix passing constraints file path into pipx install operation via --pip-args May 4, 2024

chrysle reviewed May 5, 2024

View reviewed changes

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

guysalt force-pushed the main branch from d9d8663 to a591e9a Compare May 5, 2024 11:03

guysalt force-pushed the main branch 2 times, most recently from 677d47b to 919d7e9 Compare May 7, 2024 18:42

guysalt force-pushed the main branch from 919d7e9 to 7b7590f Compare May 8, 2024 01:51

chrysle previously approved these changes May 8, 2024

View reviewed changes

guysalt force-pushed the main branch from 7b7590f to be82042 Compare May 9, 2024 11:14

chrysle reviewed May 11, 2024

View reviewed changes

chrysle reviewed May 13, 2024

View reviewed changes

tests/test_install.py Outdated Show resolved Hide resolved

tests/test_install.py Outdated Show resolved Hide resolved

testdata/tests_packages/unix-python3.10.txt Outdated Show resolved Hide resolved

chrysle reviewed May 13, 2024

View reviewed changes

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

src/pipx/package_specifier.py Outdated Show resolved Hide resolved

guysalt force-pushed the main branch 4 times, most recently from ec8ed19 to d45a72e Compare May 14, 2024 16:35

guysalt force-pushed the main branch 2 times, most recently from c2764bb to d77d362 Compare May 15, 2024 09:55

chrysle reviewed May 15, 2024

View reviewed changes

tests/test_install.py Show resolved Hide resolved

tests/test_install.py Outdated Show resolved Hide resolved

guysalt force-pushed the main branch 3 times, most recently from dbe79eb to 45b925b Compare May 15, 2024 13:38

guysalt added 3 commits May 18, 2024 15:01

parse_specifier_for_install: using abs path for constraint option

8dd28d9

test_install: add test_pip_args_with_constraint_relative_path

43441c7

test_install: add test_pip_args_with_wrong_constraint_fail

cc6a26f

guysalt force-pushed the main branch from 45b925b to cc6a26f Compare May 18, 2024 12:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix passing constraints file path into `pipx install` operation via `--pip-args` #1390

Fix passing constraints file path into `pipx install` operation via `--pip-args` #1390

guysalt commented May 4, 2024 •

edited

chrysle left a comment

guysalt commented May 4, 2024

chrysle commented May 7, 2024

guysalt commented May 7, 2024

guysalt commented May 7, 2024

guysalt commented May 8, 2024

chrysle left a comment

chrysle May 8, 2024 •

edited

chrysle May 15, 2024

guysalt May 15, 2024

chrysle May 11, 2024

guysalt May 14, 2024

chrysle May 15, 2024

guysalt May 15, 2024

chrysle May 15, 2024 •

edited

guysalt May 15, 2024

chrysle May 17, 2024

guysalt May 17, 2024

chrysle May 17, 2024

guysalt May 18, 2024

guysalt commented May 19, 2024

	temp_file.write_text(f"pycowsay=={pycowsay_version}")
	temp_file.write_text(PKG["ipython"]["spec"])

Fix passing constraints file path into pipx install operation via --pip-args #1390

Are you sure you want to change the base?

Fix passing constraints file path into pipx install operation via --pip-args #1390

Conversation

guysalt commented May 4, 2024 • edited

Summary of changes

Test plan

chrysle left a comment

Choose a reason for hiding this comment

guysalt commented May 4, 2024

chrysle commented May 7, 2024

guysalt commented May 7, 2024

guysalt commented May 7, 2024

guysalt commented May 8, 2024

chrysle left a comment

Choose a reason for hiding this comment

chrysle May 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrysle May 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guysalt commented May 19, 2024

Fix passing constraints file path into `pipx install` operation via `--pip-args` #1390

Fix passing constraints file path into `pipx install` operation via `--pip-args` #1390

guysalt commented May 4, 2024 •

edited

chrysle May 8, 2024 •

edited

chrysle May 15, 2024 •

edited