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

Feature/dynamic lockfiles #6457

Merged
merged 36 commits into from Mar 9, 2020

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Feb 3, 2020

Changelog: Feature: Allow building dependency graphs when using lockfiles even if some requirements are not in the lockfiles. This can happen for example when test_package/conanfile.py has other requirements, as they will not be part of the lockfile.

Docs: conan-io/docs#1585

Close #6067
Close #6581

@memsharded
Copy link
Member Author

memsharded commented Mar 1, 2020

Hi @sztomi @tru @theodelrieu

This is the branch I would love if you could test with your projects. It is implementing:

  • Lockfiles are still "static", they do not change their structure, just able to update the build reerences
  • So the recommended way is still to capture the lock with the build requirements, conan graph lock ... --build, so all the information is locked
  • But if something is not captured in the lockfile, Conan is still able to construct the graph and keep going. Some warnings should be seen in the terminal.

This is what you both could benefit for ConanDays, so if you could test it fast, maybe we could try to release it in Conan 1.23. Many thanks!

@memsharded memsharded added this to the 1.23 milestone Mar 1, 2020
@memsharded memsharded requested a review from jgsogo Mar 1, 2020
@sztomi
Copy link
Contributor

sztomi commented Mar 2, 2020

@memsharded this is looking much better. However, I got a forbidden overwrite for some packages which may or may not be the fault of this branch. I'm clearing out the repo and rebuilding everything. I'll report how that goes.

@sztomi
Copy link
Contributor

sztomi commented Mar 2, 2020

@memsharded spoke too soon - it appears that I'm getting the following error for almost all tests:

 ERROR: Couldn't find 'breakpad/1.0-9d61e90-19' in graph-lock

even in the simplest test_package conanfiles (which are the majority) where there is no requires line at all.

I'm attaching the logs from one build node. You should still have access to our repo if you want to see the individual conanfiles, e.g.: https://github.com/plexinc/plex-conan/blob/master/packages/breakpad/test_package/conanfile.py

Some of them are not failing and I'm not sure why. Maybe it's related to where they are in the graph.

build_output.log.zip

jgsogo and others added 7 commits Mar 5, 2020
…6602)

* run build and related hooks in one single point

* duplicate arg

* prefer calling

* need to change the tests, message changes

* rename to 'run_build_method'

* build_folder is already assigned to the conanfile
…an-io#6607)

* set conaninfo environment variables

* add test

* fix indentation

* refactor and filter deps

* fix variable name

* fix format
Copy link
Contributor

@jgsogo jgsogo left a comment

This introduces a new paradigm for the graphlocks: they are no longer a lock, now there will be warnings that nobody reads. I was more comfortable with the errors.

I agree there is a problem to solve: we need to inject a new require (or we need to create and run the test_package) and new nodes will appear and some will disappear.

In this PR we are proposing this workflow:

  • create the graphlock,
  • modify the package,
  • run and realize about the warnings,
  • recreate the graphlock (the original graphlock would be needed as input, to ensure that only what is going to be added/removed will be modified)
  • run again?

A different approach could have been:

  • create the graphlock
  • modify the package
  • run and get an error (as it is now)
  • recreate the graphlock (using the original graphlock, as said before)
  • run again

I'm really afraid that this dynamic lockfile feature is against the lockfile feature itself. I need to know what others think about it cc/ @conan-io/barbarians

jgsogo
jgsogo approved these changes Mar 9, 2020
Copy link
Contributor

@jgsogo jgsogo left a comment

Ok with the opt-in, although this should appear in the docs as highly experimental, this PR might be reverted in the future.

@memsharded memsharded merged commit e29a29b into conan-io:develop Mar 9, 2020
@memsharded memsharded deleted the feature/dynamic_lockfiles branch Mar 9, 2020
@jgsogo jgsogo mentioned this pull request Mar 11, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants