gentoo
Bugfix/bug 864259
AFAICS, the failures have nothing to do with this PR, am I wrong?
I'm not sure what gives you that impression; it looks like this PR does cause the failure. It will need to be investigated before this can be merged.
From the log:
2022-09-09T19:01:27.9583005Z ======================================================================
2022-09-09T19:01:27.9585459Z FAIL: testSimple (portage.tests.emerge.test_simple.SimpleEmergeTestCase)
2022-09-09T19:01:27.9586076Z ----------------------------------------------------------------------
2022-09-09T19:01:27.9589072Z Traceback (most recent call last):
2022-09-09T19:01:27.9601848Z File "/home/runner/work/portage/portage/build/lib/portage/tests/__init__.py", line 273, in run
2022-09-09T19:01:27.9602233Z testMethod()
2022-09-09T19:01:27.9602618Z File "/home/runner/work/portage/portage/build/lib/portage/tests/emerge/test_simple.py", line 243, in testSimple
2022-09-09T19:01:27.9602942Z loop.run_until_complete(
2022-09-09T19:01:27.9603331Z File "/home/runner/work/portage/portage/build/lib/portage/util/_eventloop/asyncio_event_loop.py", line 129, in _run_until_complete
2022-09-09T19:01:27.9603684Z return self._loop.run_until_complete(future)
2022-09-09T19:01:27.9604070Z File "/opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
2022-09-09T19:01:27.9604382Z return future.result()
2022-09-09T19:01:27.9604728Z File "/home/runner/work/portage/portage/build/lib/portage/tests/emerge/test_simple.py", line 722, in _async_test_simple
2022-09-09T19:01:27.9605062Z self.assertEqual(
2022-09-09T19:01:27.9605780Z AssertionError: 0 != 1 : emerge failed with args ('/home/runner/work/portage/portage/.tox/py310-pylint/bin/python', '-b', '-Wd', '/home/runner/work/portage/portage/build/bin/emerge', '-e', '--getbinpkgonly', 'dev-libs/A')
In other words, it expects emerge -e --getbinpkgonly dev-libs/A to exit with status 0, but it is getting status 1 instead.
I fixed the problem following a different approach (since the other was broken). As I'm quite new to portage I would appreciate a critical view.
Among all possible solutions that I saw, I tried the one that is more testable and less invasive. I hope I did it well...
Sorry, but I have to ask you to clean up the commit history a bit.
- Rebase your changes on current master (no merge commits!).
- Some of the commits undo changes made in prior commits. Please squash these into a single commit.
- In cases where you are making large amounts of whitespace changes (de-indenting blocks), please move the whitespace changes into a separate commit. We can then add that commit to .git-blame-ignore-revs.
Thank you for the feedback. I hope to do the git surgery by Friday. But I cannot promise: these days I'm quite busy.
Sorry, but I have to ask you to clean up the commit history a bit.
* Rebase your changes on current master (no merge commits!). * Some of the commits undo changes made in prior commits. Please squash these into a single commit. * In cases where you are making large amounts of whitespace changes (de-indenting blocks), please move the whitespace changes into a separate commit. We can then add that commit to .git-blame-ignore-revs.
Done it. Please let me know if I missed something, or there is another problem.
Hmmm. Before cleaning up the history that was passing. During the git surgery I didn't touch anything related to the (what I see is) the fail. Any thoughts?
Hmmm. Before cleaning up the history that was passing. During the git surgery I didn't touch anything related to the (what I see is) the fail. Any thoughts?
Don't worry about it: there's a flakey test that passed after I restarted it. See bug 850127.
I would say set getbinpkg_refresh=False at https://github.com/gentoo/portage/blob/06e43403a8d198cbf67c4dab81c799351f44f02f/lib/_emerge/actions.py#L171-L174
should be enough.
There are only 3 places in portage will call populate() with binpkg index.
The db init at https://github.com/gentoo/portage/blob/06e43403a8d198cbf67c4dab81c799351f44f02f/lib/_emerge/actions.py#L3512-L3514
this issue, which will always called after db init https://github.com/gentoo/portage/blob/06e43403a8d198cbf67c4dab81c799351f44f02f/lib/_emerge/actions.py#L172-L174
and followed https://github.com/gentoo/portage/blob/06e43403a8d198cbf67c4dab81c799351f44f02f/lib/_emerge/actions.py#L429-L431 This one already disabled refresh binpkg index.
I was thought action_build() could be called by others but did not found one.
If external program call action_build(), maybe
getbinpkg_refresh=(not emerge_config.target_config.trees["bintree"].populated)
I don't have a strong opinion on changing the default versus changing the single call site as @syu-nya suggests.
@syu-nya, the advantage of your proposal is obviously that it requires to change less code.
However, the idea behind my proposal is as follows. The bug fix must ensure that the remote package indices are transferred only once. If that must be the case, I thought it would be better to be explicit about it: only one call to binarytree.populate is explicitly done with getbinpkg_refresh=True in the PR, as it is now.
Of course I'm more inclined towards my proposal, but yours is also good. I can change to yours, if necessary.
In any case, all this is telling me that, in my opinion, a deeper change is needed in the interface of the binarytree class, particularly the way the binarytree.populate method works and must be used. Somehow is not clean. That is clear to me. But, honestly, I don't have a clear proposal to do yet... Still I thought that changing the default behavior to make more explicit what is going on is an improvement.
@palao I don't have strong prefer, but explicitly to refresh index is a good reason. Indeed a lot of old logic could be improved, but it would be very time consuming.