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

Support alternative pip-compile setup #1475

Open
tatablack opened this issue Oct 28, 2019 · 17 comments
Open

Support alternative pip-compile setup #1475

tatablack opened this issue Oct 28, 2019 · 17 comments
Labels
F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. L:python:pip-compile Python packages via pip-compile T: feature-request Requests for new features

Comments

@tatablack
Copy link

Hi! We standardised our Python projects to define dependencies using a setup.py file, and lock them to a requirements.txt using pip-compile.

The documentation for pip-compile says that using either a requirements.in or a setup.py as input is supported but, from what I can see in dependabot-core, pip-compile will be used to regenerate the lockfile ONLY when files ending in .in are present (I'm simplifying a bit).

Is there a reason why having a setup.py as input for pip-compile is not supported? Have I overlooked something?

We're currently using a fork of dependabot-core, so we can try to make it support our use case, but before contributing back we wanted to make sure the current behaviour wasn't, in fact, expected.

@feelepxyz feelepxyz added the T: feature-request Requests for new features label Oct 29, 2019
@rebelagentm rebelagentm added the L: python:pip Python packages via pip label Oct 30, 2019
@rebelagentm
Copy link
Contributor

👋 I THINK this may have to do with the way we parse dependencies, but I'm not 100% sure on that. @feelepxyz, thoughts?

@feelepxyz
Copy link
Contributor

@tatablack good question, we definitely have some support for parsing setup.py files but not sure what should be happening in your case. Do you have an example setup where this isn't working as expected? Would be super useful to test out and see what Dependabot currently does as I'm not entirely sure what is expected.

@infin8x
Copy link
Contributor

infin8x commented Jul 20, 2020

Closing this due to age and low demand. If this scenario is critical to your workflow please feel free to comment.

@infin8x infin8x closed this as completed Jul 20, 2020
@debo
Copy link
Contributor

debo commented Jul 20, 2020

Hi @feelepxyz and @infin8x,

sorry for the very late reply on this but we weren't aware of it (read we forgot) until I started looking for open issues months down the line when we faced problems.

So our current setup is fairly straight forward, we define all our dependencies constraints in the setup.py and then via a Makefile we handle operations like install, install-dev, freeze and freeze-upgrade, this translates to the following:

install:
	PIP_CONFIG_FILE=pip.conf pip install -r requirements.txt

install-dev: install
	PIP_CONFIG_FILE=pip.conf pip install -e ".[dev]"

freeze:
	CUSTOM_COMPILE_COMMAND="make freeze" pip-compile --no-index --output-file requirements.txt setup.py

freeze-upgrade:
	CUSTOM_COMPILE_COMMAND="make freeze" pip-compile --no-index --upgrade --output-file requirements.txt setup.py

That way, as part or our CI/CD workflow, we can use the locked requirements.txt as standard. We did this mainly because it's supported by pip-tools and because it fits well our environment where we have both services (using requirements.txt) and libraries (which rely more on setup.py).

However, whilst the title of this announcement says you support pip-compile, you do only seems to support it if based on a requirements.in file or more in general *.in files.

We tried to improve our fork so that wherever pip-compile was detected as resolver type it would happen also when there was a setup.py + a autogenerated requirements.txt but without success because it turns out is probably not that simple. Even if we trick dependabot to think it's a pip-compile based project even if there is no requirements.in manifest then the code doesn't know how to properly update the setup.py.

I hope that clarifies the scenario and the need. I'm sure we are not alone in the industry having this setup, otherwise I don't see why pip-tools would support it in the first place, so I now we are wondering why dependabot doesn't? Was it something overlooked? Was it intended like this by design because too difficult to implement? Is it worth trying to collaborate
to add support for setup.py or do you propose/suggest we change our workflow because it's better to have a requirement.in instead?

Thanks for your time, help and consideration.

We look forward to hearing from you.

@infin8x
Copy link
Contributor

infin8x commented Jul 20, 2020

Thanks for the write-up. Reopening.

@infin8x infin8x reopened this Jul 20, 2020
@infin8x infin8x added the F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. label Jul 20, 2020
@debo
Copy link
Contributor

debo commented Jul 20, 2020

@infin8x no problem, thanks for reopening it and for any help/support you could provide. Also, if something is not clear and you require any further clarification do not hesitate to ask.

@luislew
Copy link

luislew commented Apr 19, 2021

👋🏻 We'd love to use Dependabot too, and our current workflow is similar to what's described here. Applications define their primary dependencies in setup.py, and then we use pip-compile to generate a requirements.txt lock file. We're likely to move our dependencies from setup.py to setup.cfg, but pip-compile works the same either way.

I also saw this PR just get merged to add support for parsing dependencies from setup.cfg: #3423

So, I'm hoping that if you add support for setup.py based pip-compile, you'll think about supporting setup.cfg as well, since it's preferred to setup.py these days.

@honnix
Copy link
Contributor

honnix commented Apr 23, 2021

If I understood correctly, I guess the problem here is, dependabot doesn't know whether a dependency in requirements.txt is pulled in from setup.py/cfg. Technically it could look at the comment, for example # via foo (setup.cfg), and decide whether to include setup.cfg as input, but that doesn't feel reliable. If it unconditionally includes setup.cfg, it might generate unintended content of requirements.txt.

@honnix
Copy link
Contributor

honnix commented Apr 23, 2021

A few things I don't understand yet:

  1. def write_updated_dependency_files
    dependency_files.each do |file|
    path = file.name
    FileUtils.mkdir_p(Pathname.new(path).dirname)
    File.write(path, freeze_dependency_requirement(file))
    end
    # Overwrite the .python-version with updated content
    File.write(".python-version", python_version)
    setup_files.each do |file|
    path = file.name
    FileUtils.mkdir_p(Pathname.new(path).dirname)
    File.write(path, sanitized_setup_file_content(file))
    end
    setup_cfg_files.each do |file|
    path = file.name
    FileUtils.mkdir_p(Pathname.new(path).dirname)
    File.write(path, "[metadata]\nname = sanitized-package\n")
    end
    end
    , why do we need to write setup.py and setup.cfg before executing pip-compile?
  2. In the use case described by this thread, will it end up with two separated PRs for the same dependency? One for setup.cfg/py and one for requirements.txt, with the latter one being a broken one because it won't read setup.cfg/py when executing pip-compile. Also it might generate wrong comment?

@debo
Copy link
Contributor

debo commented Apr 23, 2021

Not quite, the problem is that at the moment dependabot does this to detect whether pip-compile is used or not. So if you don't have a *.in dependabot will treat the project as if it's using other dependency management workflow. You can try to be "smart" as we did and say, check this file and check if there is a comment on the top of the requirements.txt but at least for us, without complete knowledge of what is happening in the code and admittedly with little ruby knowledge as well, that didn't quite work as seamlessly as we expected. We still ended up with unexpected behaviour and outcomes.

@debo
Copy link
Contributor

debo commented Apr 23, 2021

@honnix if I understood correctly the before/after is used to keep track of actual upgradability, it's actually quite useful for logging and such, again, it's been a little while ago now so my memories are a bit blurry. About our use case the problem is that PR changes only one file and not the other. We experimented using a requirements.in file instead in that case it would generate a single PR with changes in both files.

@honnix
Copy link
Contributor

honnix commented Apr 23, 2021

@debo Thanks for explaining in details. I assumed there was a *.in file.

Do you mean if you have both setup.py/cfg and requirements.in containing the same set of dependencies, a single PR will change both setup.py/cfg and requirements.in, as well as regenerate requirements.txt file?

@honnix
Copy link
Contributor

honnix commented Apr 23, 2021

I think I misunderstood this thread a bit. I thought it was about when running pip-compile in dependabot, setup.py/cfg was not fed in as input, shown by this line:

version_part = "#{dependency.version} #{filename}"
where it only takes in *.in file if I read it correctly.

@luislew
Copy link

luislew commented Apr 23, 2021

I think I misunderstood this thread a bit. I thought it was about when running pip-compile in dependabot, setup.py/cfg was not fed in as input, shown by this line:

version_part = "#{dependency.version} #{filename}"
where it only takes in *.in file if I read it correctly.

That is the issue, at least for us. We use pip-compile without requirements.in. Currently we store top level dependencies in setup.py but we plan on migrating to setup.cfg.

Ideally, Dependabot would allow us to specify the input file to pip-compile, bump versions in that file, and then regenerate requirements.txt.

@debo
Copy link
Contributor

debo commented Apr 23, 2021

No, the problem is the following. We have our dependencies in the setup.py and we use pip-compile to generate the requirements.txt, unfortunately dependabot because we don't have a requirements.in, see the code line I linked earlier, doesn't recognise the project as a pip-compile based one and because of that it only bumps stuff in the requirements.txt and not the setup.py.

We modified the code to force pip-compile as resolver_type here but because the overall logic is probably couple to the existence of *.in files the all process ended up misbehaving as well. I hope that clarifies.

@honnix
Copy link
Contributor

honnix commented Apr 23, 2021

Our use case is slightly different and is harder for me to understand the actual behaviour of dependabot.

We use both requirements.in and setup.cfg. We declare all dependencies in setup.cfg, and use requirements.in to workaround same limitations of pip-comiple, as well as to define -c constraints.txt. With this setup, I have a feeling dependabot might create two PRs for us, because one dependency is found in setup.cfg and other one is found in requirements.txt and they might be treated as two different ones since dependabot tracks which file the dependency is defined in.

@debo
Copy link
Contributor

debo commented May 4, 2021

I think that whichever is the case Dependabot should support the most combination of this workflow rather than be overly opinionated and support only one, but this is for their development team to take a final decision on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. L:python:pip-compile Python packages via pip-compile T: feature-request Requests for new features
Projects
None yet
Development

No branches or pull requests

8 participants