--index-url with local path is dropped silently if directory exists

[uranusjr]

Say I have this setup:

$ mkdir /some/path

Running pip install with it as an index URL gives me this:

$ pip install packaging --index-url=/some/path
Looking in indexes: /some/path
Collecting packages
  Url '/some/path/packaging/' is ignored. It is either a non-existing path or lacks a specific scheme.
  Could not find a version that satisfies the requirement packaging (from versions: )
No matching distribution found for packaging

But if I create an empty directory under it…

$ mkdir /some/path/packaging
$ pip install packaging --index-url=/some/path
Looking in indexes: /some/path
Collecting packaging
  Could not find a version that satisfies the requirement packaging (from versions: )
No matching distribution found for packaging

The warning message disappears.

The reason is probably this block of code:

pip/src/pip/_internal/index.py

Lines 202 to 233 in 4cc5f37

	for url in locations:
	is_local_path = os.path.exists(url)
	is_file_url = url.startswith('file:')
	if is_local_path or is_file_url:
	if is_local_path:
	path = url
	else:
	path = url_to_path(url)
	if os.path.isdir(path):
	if expand_dir:
	path = os.path.realpath(path)
	for item in os.listdir(path):
	sort_path(os.path.join(path, item))
	elif is_file_url:
	urls.append(url)
	elif os.path.isfile(path):
	sort_path(path)
	else:
	logger.warning(
	"Url '%s' is ignored: it is neither a file "
	"nor a directory.", url,
	)
	elif is_url(url):
	# Only add url with clear scheme
	urls.append(url)
	else:
	logger.warning(
	"Url '%s' is ignored. It is either a non-existing "
	"path or lacks a specific scheme.", url,
	)

A local path to an existing directory would go into the if branch at line 212, but would match either conditions inside (expand_dir is only true for --find-links, and this is a path, not URL), and is silently dropped.

The fix would be to add an extra else branch in the L212 if block, but I want to know what is the intended behaviour here. A file: URL would be allowed here (if the directory contains a valid index.html):

$ curl https://pypi.org/simple/packaging/ -o /some/path/packaging/index.html
...
$ pip install packaging --index-url file:///some/path
Looking in indexes: file:///some/path
Collecting packaging
  Using cached https://files.pythonhosted.org/packages/89/d1/92e6df2e503a69df9faab187c684585f0136662c12bb1f36901d426f3fab/packaging-18.0-py2.py3-none-any.whl
...

Should a path be allowed as well?

[cjerdonek]

At least for now, it seems like it can’t hurt to add a warning for the case you mention of a local path that’s a directory. Either way, it would be great to simplify that tree of if-elses. It seems like it should be able to be made much simpler while retaining the same behavior.

[cjerdonek]

Thinking more about the branching, I think it would be clearest if the first “if” determines whether you are in the path or URL case. This is partly because that’s the biggest determining factor in how the location should be handled.

I also think branching like this can be simplified a lot if a function is factored out that simply returns the paths or URL’s given a location, and then the caller is responsible for appending it, etc. This lets you use guard clauses in the factored out function to return early — eliminating the need for excessive branching and indentation, etc.

[uranusjr]

Cool, I’ll submit a patch adding the warning first.

This is the part I plan to tackle after I’m done with HTMLPage, and I’ll definitely try to simplify this. Here’s a table of logic in this function, in case you want to confirm it first:

	To file	To directory
Remote URL	Use as URL	N/A
file: URL	Use as URL	Use os.listdir() results as files if expand_dir is True Otherwise use as URL
Path [#]	Use as file	Use os.listdir() results as files if expand_dir is True Otherwise ignore [*]

[*]: Used to ignore silently, warning will be added.
[#]: If string does not look like a URL, and does not exist as a path, ignore with a warning.

[cjerdonek]

Were you saying some of the combinations involving expand_dir can’t occur? Can you note that too, if so?

[uranusjr]

expand_dir is only meaningful if the input points to a directory (either file: or a path). It is only set for find-links, but not for index-url (and not for dependency links but 🤷‍♂️). I think the flag is working as intended, it is just confusing since it only applies to a subset of inputs in specific situations. It is an area I wish to rewrite to make it more obvious.

[cjerdonek]

Oh, okay. Thanks. Originally I thought you were saying that expand_dir can't ever be true in the path case. But you were just referring to the case of the example you provided (that doesn't pass --find-links).

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.