pip_compile: remove external/workspace_name prefix from generated requirements.txt

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.

[BoleynSu]

Fixes#689

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?

Issue Number: #689

What is the new behavior?

In the generated file via -r filename will be replaced with via -r -.

Does this PR introduce a breaking change?

• Yes
• No

The existing generated tests will fail and a regeneration is required.

Other information

[BoleynSu]

The test is not added yet and I am expecting to break some existing ones.
I will fix them and add a new one once the PR is SGTMed.

[BoleynSu]

Friendly ping.

[alexeagle]

I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel.

Can we just fixup the external pathing before writing the file instead?

[BoleynSu]

Sure. That makes sense. Just that's more work to do and I want to make sure the upstream maintainer would think the issue is acceptable to be fixed (which is not always the case) in the first place before I spend more time on it. Sent from https://boleyn.su/phone

…

On Fri, Apr 22, 2022, 21:14 Alex Eagle ***@***.***> wrote: I don't think this is really an improvement - these from lines are useful documentation to say where a requirement came from, and the user doesn't understand what stdin is in this case, it's hidden by Bazel. Can we just fixup the external pathing before writing the file instead? — Reply to this email directly, view it on GitHub <#690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXVSKBYKEZLSBJASDHHLQ3VGKQZVANCNFSM5TSQKVLQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BoleynSu]

As a byproduct, this also allows |compile_pip_requirements| to comsume a generated requirements.in.
@alexeagle PTAL. I can add some tests after you think the PR is OK.

[f0rmiga]

@alexeagle Seems valid to me with proper tests. Anything else you want @BoleynSu to change?

[alexeagle]

Yup, seems like a targeted fix. Even a simple test by checking in a requirements file that currently has an external/ segment would help.

[BoleynSu]

Cool. I will add some tests later. I have implemented something simliar to @unpinned_maven//:pin which depend on this fix. Maybe I can use that as a test here.

[BoleynSu]

Hi @alexeagle and @f0rmiga, I have added the tests PTAL. Also I implemented something simliar to @unpinned_maven//:pin in the test. Would you mind also check if that can be part of the contributed repository rules?

[alexeagle]

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[BoleynSu]

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

[alexeagle]

why doesn't it work on Windows?

[BoleynSu]

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

[alexeagle]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[BoleynSu]

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

[alexeagle]

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

[BoleynSu]

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

[BoleynSu]

Hi @alexeagle, I will remove pip_deps for now and add the test to existing an e2e test. I will propose another PR regarding pip_deps if you think it is worth the effort. If so, maybe we can first decide a better name for it.

[BoleynSu]

Hi @alexeagle, I implemented another pip_deps at https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/deps/pip_deps.bzl which addressed this issue you pointed out. Thanks!

Use site:
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_2.bzl#L21
https://github.com/BoleynSu-Org/monorepo/blob/e819723afa334f5220097291969cd7c7bb419716/configs/build/workspace_3.bzl#L7

Please let me know if you think this can be upstreamed.

[alexeagle]

Thanks! sorry this got stalled for so long.

[alexeagle]

I'll get this in so that #689 can be fixed.