Add containers with older flex versions for older revisions of doxygen #661
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## vara-dev #661 +/- ##
============================================
- Coverage 67.54% 67.54% -0.01%
============================================
Files 322 322
Lines 24590 24593 +3
============================================
+ Hits 16610 16612 +2
- Misses 7980 7981 +1
☔ View full report in Codecov by Sentry. |
| GoodBadSubgraph([ | ||
| "a6238a4898e20422fe6ef03fce4891c5749b1553" | ||
| ], ["cf936efb8ae99dd297b6afb9c6a06beb81f5b0fb"], | ||
| "Needs flex <= 2.5.4 and >= 2.5.33"), | ||
| get_base_image(ImageBase.DEBIAN_10).run( | ||
| 'apt', 'install', '-y', 'cmake', 'flex-old', 'bison', | ||
| 'qt5-default' | ||
| ) | ||
| ), | ||
| ( | ||
| GoodBadSubgraph([ | ||
| "093381b3fc6cc1e97f0e737feca04ebd0cfe538d" | ||
| ], ["cf936efb8ae99dd297b6afb9c6a06beb81f5b0fb"], | ||
| "Needs flex <= 2.5.4 and >= 2.5.33"), | ||
| get_base_image(ImageBase.DEBIAN_10).run( | ||
| 'apt', 'install', '-y', 'cmake', 'flex-old', 'bison', | ||
| 'qt5-default' | ||
| ) | ||
| )] |
There was a problem hiding this comment.
Nice, that you found this.
Just one question: Do these ranges not overlap? That looks strange that they would have a gap where flex-old was no longer required.
There was a problem hiding this comment.
The starting commits are on two different branches the ending commit is after they have been merged. I'm not sure why I put it in two SubGraphs. I think it should work with one.
There was a problem hiding this comment.
kk, if we get this cleaned up, we'll can merge this fix :)
| @@ -260,8 +260,7 @@ def __init__( | |||
|
|
|||
| def versions(self) -> tp.List[bb.source.base.Variant]: | |||
| cache_path = self.fetch() | |||
| git_rev_list = git['rev-list', '--abbrev-commit', '--abbrev=10', | |||
| '--all'] | |||
| git_rev_list = git['rev-list', '--all'] | |||
There was a problem hiding this comment.
Why did you change this here? We, similar to benchbuild, us 10 chars long short commit hashes as the default.
There was a problem hiding this comment.
Because they don't use short hashes for their revision range list and this version string is checked against the revision range when looking for the correct container to use. As seen in the mentioned PR. This caused no container to be found. So either the benchbuild revision range needs to use the short version or we need a more verbose system to check for eqaulity similar to ours.
There was a problem hiding this comment.
A simpler way of checking equality would be nice but we definitively need a benchbuild patch here as changing the length on our side will not work and break a lot of our use cases.
There was a problem hiding this comment.
Okay then I will add that to the PR on benchbuild and revert this
|
Does this PR fix an issues? If yes, please not the issue number in the top comment of the PR |
| CONTAINER = [( | ||
| RevisionRange("cf936efb8ae99dd297b6afb9c6a06beb81f5b0fb", "HEAD"), | ||
| get_base_image( | ||
| ImageBase.DEBIAN_10 | ||
| ).run('apt', 'install', '-y', 'cmake', 'flex', 'bison', 'qt5-default') | ||
| ), | ||
| ( | ||
| GoodBadSubgraph([ | ||
| "a6238a4898e20422fe6ef03fce4891c5749b1553", | ||
| "093381b3fc6cc1e97f0e737feca04ebd0cfe538d" | ||
| ], ["cf936efb8ae99dd297b6afb9c6a06beb81f5b0fb"], | ||
| "Needs flex <= 2.5.4 and >= 2.5.33"), | ||
| get_base_image(ImageBase.DEBIAN_10).run( | ||
| 'apt', 'install', '-y', 'cmake', 'flex-old', 'bison', | ||
| 'qt5-default' | ||
| ) | ||
| )] |
There was a problem hiding this comment.
For some reason, github did not include that in my review:
The indentation here is very misleading. If yapf cannot be convinced to layout this better, we should do it by hand and disable yapf for this block.
depends on PolyJIT/benchbuild#554
resolves se-sic/VaRA#536