-
Notifications
You must be signed in to change notification settings - Fork 19
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 for calculating dependencies of products that are implicit dependencies #43
base: master
Are you sure you want to change the base?
Conversation
…ducts Table.dependencies is iterating over Actions, which may include products that were loaded implicitly as dependencies of the default product. However, if addDefaultProduct is False, we explicitly do not want to include these.
python/eups/Eups.py
Outdated
# Check to see whether this product is in the list of default products | ||
defaultProductName = hooks.config.Eups.defaultProduct["name"] | ||
defaultProductList = [p for p, _, _ in dependencies if p.name == defaultProductName] | ||
if len(defaultProductList) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the > 0 needed here? It is already going to be testing truth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
7ce47ef
to
ebdf253
Compare
productDictionary=productDictionary, | ||
requiredVersions=requiredVersions): | ||
productDict = {} | ||
dependencies = prodtbl.dependencies(self, recursive=True, recursionDepth=1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling dependencies()
three times seems pretty wasteful. The defaultDeps
shouldn't be changing all the time, should they? Can't they be cached or otherwise stored, enabling a reduction to just one dependencies()
call (with addDefaultProduct
set to the proper value)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we need to get the dependencies of the default product to see if the top-level product is in it. In order to know which version of the default product, we need to get the dependencies of the top-level product. And then if (and only if) the top-level product is in the default product dependency list, we need a new dependency list for the top-level product. I don't see a way around this, though @RobertLuptonTheGood may know one.
In the usual case, dependencies()
is called only twice, and one of those is on the default product, which should be fairly quick (and I think the table files and all are already read in, so it's "just" CPU), so I don't think this is a terrible performance degradation.
ebdf253
to
2698a1c
Compare
…e defaults If the product is one of the default products, we don't want to add the default products to the list of dependencies because that leads to a situation where we can have cycles: we have to build X in order to build Y in order to build X.
We want to clone the versions on the server, which means also installing the implicit product defined there.
This PR is 4 years old now but it doesn't look like we have converged. Should it remain open? |
Should we close this PR without merging? |
Mineo-san noticed that in the recent HSC release candidate (hscPipe 3.8.0), python was a dependency of gcc. This is because both gcc and python are dependencies of the "toolchain" product, which are implicitly added to all table files. This PR contains fixes to this functionality, so that products which are part of the toolchain are not dependencies of each other. With this fix, gcc no longer depends on python.