Skip to content
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.

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

Fixup relative and absolute path handling #1329

Closed
wants to merge 34 commits into from

Conversation

AndydeCleyre
Copy link
Contributor

@AndydeCleyre AndydeCleyre commented Feb 22, 2021

Initial Summary (Outdated)

Rewrite input file paths as relative to the output file, or as absolutes if using stdout:

- Change click arg output_file from click.File to click.Path,
  so we can get its absolute path
- Change to output file's parent folder before opening it,
  helping upstream pip code to use correct relative paths
- Return to the starting dir after compilation,
  for test or other calls from code which don't expect dir changes
- Rewrite src_files as absolute paths as soon as we have them,
  to resolve relative paths properly (CLI args are relative to CWD)
- When deriving the output path from a single input path, stay safer and more predictable,
  particularly if the basename has no dot, and the path does (see example in #1067)
- Rewrite src_files as relative to the output path once that's known,
  unless output is stdout
- Don't overwrite an input file ending in '.txt' when deriving the output file path
- Add tests:
    - test_annotation_relative_paths
    - test_input_file_with_txt_extension
    - test_input_file_without_extension_and_dotted_path

Fixes #1107

Contributes to #1067

Minor tag-alongs:
  - fix some comment typos
  - git-ignore sublime text project and workspace files

QUESTIONS:
1. If output is stdout, should paths (in annotations) be absolute,
  or relative to current working folder? 
  With this PR, they're absolute, and I think that is appropriate.
2. With this PR, `output_file` changes its type, early on. 
  Does that bother anyone? It starts as a `click.Path`, then 
  after paths are resolved by our logic, it's replaced by a `click.File`. 
  It's a small window of `click.Path`-ness. An example consequence
  is seen in this PR's change to `test_writer.py`.
3. What is the best result of using `name.txt` as an input file, without specifying the output file?
  With this PR, it outputs to `name.txt.txt`, which is the best I can think of.
4. What additional tests would be good to have, if any? 
  More annotation variants, like with `-c`? More complicated relative paths?

**Changelog-friendly one-liners**: 
- Rewrite input file paths as relative to the output file, or as absolutes if using stdout
- Don't overwrite an input file ending in '.txt' when deriving the output file path
- Don't confuse dots in folder names with file extensions when deriving the output file path

Summary, Take 2 (also outdated)

Fixup relative and absolute path handling:

These changes have been made with the general guideline of
storing paths as absolute as soon as we can, and rendering
them as relative or absolute as needed.

Path Initial Interpretation Output Format (file) Output Format (stdout)
source file relative to invocation dir (annotation) relative to output file absolute
ireq from source file relative to its source file relative to output file, unless initially absolute absolute
ireq from --upgrade-package relative to invocation dir relative to output file I think: relative to output file if passed as relative, absolute if passed as absolute, pathless if passed as pathless absolute
git+file: ireq from source file relative to its source file absolute (pip doesn't support relative paths in that form) absolute
Itemized Changes by File

utils.py

  • Changed:
    • format_requirement:
      • Add optional str kwarg from_dir:
        • If used, it'll rewrite local paths as relative (to from_dir).
      • Replace alternative path separators in relpaths with forward slashes.
      • Use pip's path_to_url for abs paths.
      • Ensure fragment is attached if present originally.
  • Added:
    • Function abs_ireq:

      def abs_ireq(ireq: InstallRequirement, from_dir: str) -> InstallRequirement:
          """
          Return the given InstallRequirement if its source isn't a relative path;
          Otherwise, return a new one with the relative path rewritten as absolute.
      
          In this case, an extra attribute is added: _was_relative,
          which is always True when present at all.
          """
    • Context manager working_dir:

      @contextmanager
      def working_dir(folder: Optional[str]) -> Iterator[None]:
          """Change the current directory within the context, then change it back."""
    • Function fragment_string:

      def fragment_string(ireq: InstallRequirement) -> str:
          """
          Return a string like "#egg=pkgname&subdirectory=folder", or "".
          """

pip_compat.py

  • Changed:
    • parse_requirements:
      • Add optional str kwarg from_dir to parse_requirements.
        If left to its default, None, the parent of the source file is used.
        Either way it's passed to abs_ireq, so any yielded local ireqs
        have absolute .links, and some have ._was_relative.
      • Ensure pip's install_req_from_parsed_requirement is called from a sensible folder,
        to better resolve relative paths; and try to detect if each ireq was initially relative,
        to "manually" mark the resulting (absolute) ireq with _was_relative.

compile.py

  • Change Click argument type for the output file from File to Path.
    When Click's File object is initialized with the absolute path,
    that full path is preserved as the .name attribute. So we now instantiate
    the output File ourselves after resolving its absolute path.
  • Resolve src_files to their absolute paths.
  • When deriving the output path from a single input path,
    ensure it's properly adjacent to the input,
    and stay safer and more predictable when the basename has no dot and the path does,
    or the input file ends in .txt (see Choose an approach for coherent path handling #1067, pip-compile annotations should be relative to file they are in #1107, and tests below).
  • Use abs_ireq when collecting upgrade_install_reqs (--upgrade-package),
    passing the invocation dir as from_dir.
  • No support for relative paths is introduced for setup.py install_requires,
    given the discussion @ https://discuss.python.org/t/what-is-the-correct-interpretation-of-path-based-pep-508-uri-reference/2815/18
  • Ensure a suitable from_dir is passed to parse_requirements
    when parsing from setup.py or stdin, which really parses a temp file.
    This means setup.py's parent folder, or the invocation dir if the source is stdin.

writer.py

  • Added:
    • comes_from_line_project_re pattern for parsing and rewriting
      comes_from strs that point to setup.pys and pyproject.tomls.
  • Changed:
    • strip_comes_from_line_re:
      • Extend/replace the pattern as comes_from_line_re,
        with named groups for opts (-r/-c), path, and line_num.
    • _comes_from_as_string:
      • Add optional str kwarg from_dir.
        If the ireq.comes_from is already a str and from_dir is passed,
        in addition to stripping the line number as before, rewrite the path as relative.
      • Add handling for comes_from_line_project_re matches.
    • _format_requirement:
      • If the ireq has ._was_relative and the output is a file,
        pass the output file's parent as from_dir to format_requirement,
        ensuring the written path for the ireq is relative in that case.
      • Pass the parent of the output file, if any, as from_dir to _comes_from_as_string.

test_cli_compile.py

  • Added:
    • test_relative_local_package
      • Relative paths are properly resolved between input, output, and local packages.
      • Input file paths/URIs can be relative, as long as they start with file: or ..
    • test_input_file_with_txt_extension
      • Compile an input file ending in .txt to a separate output file (*.txt.txt),
        without overwriting the input file.
    • test_input_file_without_extension_and_dotted_path
      • Compile a file without an extension, in a subdir with a dot,
        into an input-adjacent file with .txt as the extension.
    • test_annotation_relative_paths
      • Annotations referencing reqs.in files use paths relative to the reqs.txt.
    • test_local_vcs_package
      • git+file urls are rewritten to use absolute paths, and otherwise remain intact.
  • Changed:
    • test_duplicate_reqs_combined
      • Use pip's path_to_url to detect the normalized package path in URL form.

test_writer.py

  • Changed:
    • writer fixture:
      • Open an output file object to pass to OutputWriter,
        rather than passing the click ctx entry (now just a Path).
    • test_write_header:
      • Access the user-supplied output file path via
        writer.click_ctx.params["output_file"] (now just a Path), rather than
        checking that for a .name.
    • test_iter_lines__hash_missing:
      • Use regex match to recognize Windows drive names.
    • test_iter_lines__no_warn_if_only_unhashable_packages:
      • Use regex match to recognize Windows drive names.
  • Added:
    • test_format_requirement_annotation_source_ireqs

test_utils.py

  • Changed:
    • test_format_requirement_editable_local_path
      • Use regex match to recognize Windows drive names.
  • Added:
    • test_working_dir
    • test_local_abs_ireq_preserves_source_ireqs

Changelog-friendly one-liners:

  • Support relative paths in input files, as long as they lead with file:, <vcs>+file:, or ..
  • If a local requirement path is relative in the input, interpret it as relative to that input file, and write it as relative to the output file, if any. Otherwise, write the absolute path.
  • Rewrite input file paths (in annotations) as relative to the output file, or as absolute if using stdout.
  • Don't overwrite an input file ending in '.txt' when deriving the output file path.
  • Don't confuse dots in folder names with file extensions when deriving the output file path.
  • Write requirement paths using forward slashes rather than backslashes, on Windows.

Changelog-friendly one-liners:

  • Support relative paths in input files, as long as they lead with file:, <vcs>+file:, or ..
  • Relative paths in input files become relative paths in output files.
  • pip-compile will interpret relative paths in an input file as relative to that input file, rather than the current folder, if --read-relative-to-input is passed.
  • pip-compile will reconstruct relative req paths as relative to the output file, rather than the current folder, if --write-relative-to-output is passed.
  • pip-sync will interpret relative paths in an input file as relative to that input file, rather than the current folder, if --read-relative-to-input is passed.
  • Annotation paths are now relative to the output file.
  • Don't overwrite an input file ending in '.txt' when deriving the output file path.
  • Don't confuse dots in folder names with file extensions when deriving the output file path.
  • Include extras more reliably in output lines, like pkg[extra1,extra2].

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@codecov

This comment has been minimized.

@AndydeCleyre

This comment has been minimized.

@AndydeCleyre AndydeCleyre force-pushed the bugfix/1107 branch 2 times, most recently from dde20b5 to ab47233 Compare February 24, 2021 21:40
@AndydeCleyre AndydeCleyre changed the title [WIP] Rewrite input and output file paths as either absolute or relative to the output [WIP] Rewrite input file paths as relative to the output file, or as absolutes if using stdout; and extra path fixes Feb 24, 2021
@AndydeCleyre

This comment has been minimized.

@AndydeCleyre AndydeCleyre changed the title [WIP] Rewrite input file paths as relative to the output file, or as absolutes if using stdout; and extra path fixes Rewrite input file paths as relative to the output file, or as absolutes if using stdout; and extra path fixes Feb 24, 2021
@AndydeCleyre AndydeCleyre marked this pull request as ready for review February 24, 2021 22:05
@AndydeCleyre

This comment has been minimized.

@AndydeCleyre AndydeCleyre changed the title Rewrite input file paths as relative to the output file, or as absolutes if using stdout; and extra path fixes [WIP] Rewrite input file paths as relative to the output file, or as absolutes if using stdout; and extra path fixes Feb 25, 2021
@AndydeCleyre AndydeCleyre marked this pull request as draft February 25, 2021 07:06
@AndydeCleyre

This comment has been minimized.

@AndydeCleyre

This comment has been minimized.

@atugushev

This comment has been minimized.

@AndydeCleyre

This comment has been minimized.

@AndydeCleyre

This comment has been minimized.

@AndydeCleyre AndydeCleyre changed the title [WIP] Rewrite input file paths as relative to the output file, or as absolutes if using stdout; and extra path fixes Fixup relative and absolute path handling: Mar 9, 2021
@AndydeCleyre AndydeCleyre changed the title Fixup relative and absolute path handling: Fixup relative and absolute path handling Mar 9, 2021
@AndydeCleyre

This comment has been minimized.

@AndydeCleyre

This comment has been minimized.

@AndydeCleyre

This comment has been minimized.

AndydeCleyre and others added 20 commits June 16, 2022 13:20
This fixes false-positive _was_relative assignments
for some direct references like:

ptrender[yaml] @ file:///home/andy/Code/ptrender
As we always want to set that True
Preserve ireq.constraint and _source_ireqs
At this code point we currently expect the original ireq.extras to
be an empty set every time, so this does not change any current behavior.

But if in the future ireqs with already parsed extras are passed,
it should now preserve those.
@AndydeCleyre
Copy link
Contributor Author

Closing in favor of #1650

AndydeCleyre added a commit to AndydeCleyre/pip-tools that referenced this pull request Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality
Projects
None yet
9 participants