Skip to content

Allow requirements_in to be generated - #829

Merged
f0rmiga merged 6 commits into
bazel-contrib:mainfrom
philsc:fix-generated-requirements-in
Oct 5, 2022
Merged

Allow requirements_in to be generated#829
f0rmiga merged 6 commits into
bazel-contrib:mainfrom
philsc:fix-generated-requirements-in

Conversation

@philsc

@philsc philsc commented Sep 15, 2022

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

We generate the requirements_in file from various input files roughly like so:

genrule(
    name = "generate_3.7_x86_requirements",
    srcs = [
        "requirements_base.in.txt",
        "requirements_extra_37.in.txt",
    ],
    outs = ["requirements_3.7_x86.txt"],
    cmd = "cat $(SRCS) > $(OUTS)",
)

compile_pip_requirements(
    name = "compile_requirements_3.7_x86",
    requirements_in = ":requirements_3.7_x86.txt",
    requirements_txt = "requirements_3.7_x86.lock.txt",
)

The current code errors out with a message like this:

Updating common/python/requirements_3.7_x86.lock.txt
Usage: pip_compile.py [OPTIONS] [SRC_FILES]...
Try 'pip_compile.py -h' for help.

Error: Invalid value for '[SRC_FILES]...': Path 'common/python/requirements_3.7_x86.txt' does not exist.

What is the new behavior?

This patch here fixes the issue by resolving the requirements_in path
before the tool cds into the workspace directory.

The downside is that the output of pip still contains the fully resolved
paths on the terminal output. You'll see them in the following example.

$ cd examples/pip_parse
$ bazel run //:requirements.update

The console output will contain the full paths, but the saved
requirements_lock.txt file will not. I could not think of a good way to
also fix up pip's console output.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

We generate the `requirements_in` file from various input files
roughly like so:

    genrule(
        name = "generate_3.7_x86_requirements",
        srcs = [
            "requirements_base.in.txt",
            "requirements_extra_37.in.txt",
        ],
        outs = ["requirements_3.7_x86.txt"],
        cmd = "cat $(SRCS) > $(OUTS)",
    )

    compile_pip_requirements(
        name = "compile_requirements_3.7_x86",
        requirements_in = ":requirements_3.7_x86.txt",
        requirements_txt = "requirements_3.7_x86.lock.txt",
    )

The current code errors out with a message like this:

    Updating common/python/requirements_3.7_x86.lock.txt
    Usage: pip_compile.py [OPTIONS] [SRC_FILES]...
    Try 'pip_compile.py -h' for help.

    Error: Invalid value for '[SRC_FILES]...': Path 'common/python/requirements_3.7_x86.txt' does not exist.

This patch here fixes the issue by resolving the `requirements_in`
path before the tool `cd`s into the workspace directory.
@philsc
philsc marked this pull request as ready for review September 24, 2022 19:29
@philsc
philsc marked this pull request as draft September 24, 2022 19:34
@philsc
philsc marked this pull request as ready for review September 24, 2022 19:54
@philsc

philsc commented Sep 30, 2022

Copy link
Copy Markdown
Contributor Author

@brandjon , @lberki , @thundergolfer , any thoughts?

@f0rmiga
f0rmiga merged commit b15e15f into bazel-contrib:main Oct 5, 2022
@philsc
philsc deleted the fix-generated-requirements-in branch December 3, 2023 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants