R-lava: drop explicit test dependencies due to possible CI failures #20736
Conversation
macportsbot
added
type: update
maintainer
maintainer: open
barracuda156
force-pushed
the
r_updates
branch
from
8f9b0ce
to
0184716
Compare
|
@cjones051073 Hmm, we keep getting failures here for dependencies (unchanged ones, which were fine earlier). Not sure what goes wrong, since logs do not show what exactly fails. |
|
@cjones051073 So it appears that the whole thing is still pretty broken for whatever reason. I guess, let me try allowing gcc13 (without revbumps of ports). At least we may know whether it works or not. |
|
Everything looks fine to me locally. |
|
Isn't the problem here the number of R updates you are squeezing into one PR, and then the timeout is hit ? Please try reducing the number of updates to just one or two, and see what then happens. |
|
One thing you should bear in mind is the default variants on all R ports have now changed, from I think it would be useful to manually trigger rebuilds on the buildbots (I can do this without a rev bump) hjust to get all the ports built with the new variants. |
|
@cjones051073 Everything worked fine for me locally too, but… Anyway, I will try splitting this. Maybe heavier that usual dependencies. (Usually 27–28 commits per R-related PR are all fine.) |
I suspect the issue is all the dependencies currently need to be rebuilt on the ci, due to the default variant change, and this is just pushing things over the timeout limit. |
|
I've triggered a rebuild of all |
|
@cjones051073 Thank you very much! Yeah, I will wait for a while with pushing an update. |
There are quite a number of ports to build, 700 odd, but each seems reasonably fast so hopefully will not take too long, perhaps a day or two, for each builder to work through the list. |
|
https://build.macports.org/waterfall progress can be tracked there |
|
Interesting to note though we have ended up doing precisely what pegging the ports to gcc12 was supposed to prevent, having to rebuild them all via a rev-bump. At least, after this the R ecosystem will have control itself of which gcc/clang it wishes to use, and won't just automatically migrate when a new compiler is released. Side note, I have seen a few R ports (and R itself) checking for recent C++ standard support. c++20 and c++23 even. Clang 15, and gcc 12 not being the most recent releases, do not have great support for these standards so I do think you might want to as a second step do a mass migration to updated versions, clang-17 and gcc-13.. |
|
@cjones051073 For gcc13 all should be good, I believe: there were no drastic changes, AFAIK. With clang17 someone has to verify it works; I recall we have initially blacklisted clang16 because there were build failures with it on CI. |
According to https://trac.macports.org/ticket/67144 which is the ticket referenced where the clang blacklisting is done, the reason for it was not because the builds would fail with the newer compiler, but because of the issue of builds failing to find the right clang compiler, because of a mis-match between what R was built with and what the R-X port was trying to use. This therefore does not mean if you switch to a newer clang, and then rev-bump everything, R and all the R-X ports they wouldn't work just fine. |
|
@cjones051073 Great. Then we can move to a newer clang and newer gcc at once. @i0ntempest What do you think about switching to clang17? My concern is gcc, and it is desirable not to require to build an extra one (which is currently the case), but it makes sense to revbump ports once for both compilers together. (Not it this PR, of course.) |
barracuda156
force-pushed
the
r_updates
branch
from
0184716
to
03c9a39
Compare
|
Still same silly failures: notice, the message itself is off – dependencies do install (OK status for all), but then reported as failed: Gonna leave it until tomorrow. |
|
As I said before please do not run any more ci tests until the buildbots have finished the ongoing rebuilds… |
|
@cjones051073 Is there a way to know when they are done or some estimate? (Sorry, I just never dealt with this matter.) |
Just keep an eye on the link i sent before… |
barracuda156
force-pushed
the
r_updates
branch
from
03c9a39
to
4ecfe38
Compare
|
@cjones051073 I am afraid we are in trouble. Take a look: If the port has been installed previously, it fails to be updated normally. I did not notice that earlier, because used |
|
@cjones051073 (Also, while maybe this is just because of some delays in communication between buildbots and CI, but macos-13 but have finished rebuilding |
|
@cjones051073 BTW, we can revbump only those |
I have no clue why you see the errors, timeouts was just a suggestion. but, having all these updates in one PR makes understanding things hard. Please, if only as a test, update this to update a single port and lets see what that does. |
I believe so yes. If the above is causing what you see you will have to ask someone who knows their way around mpbb like @jmroot |
It is still though triggered by the attempt to build R-lava and we already know for a fact R-lava has a circular dependency (when you take test deps into account). |
|
@jmroot Could you please take a look? Could the errors we get be related your recent commits to |
@cjones051073 Yes, but unless logs lie (reported sequence is unrelated to actual), that happens prior to installing anything |
That I cannot explain, but then I don’t really know enough about what precisely the ci and/or mpbb scripts do to be able to. |
Doing this instead of using registry::run_target saves running another mportopen for each dependency, so should perform better. This wasn't originally possible due to the workaround for https://trac.macports.org/ticket/24857.
|
@cjones051073 @kencu If we are okay with committing c75100f I will drop the last commit (which was for testing). I have no objections in principle against a non-default tests variant as long as we do not do that for every |
check is not really a good name for this variant, test would be better. That aside I would prefer if you moved this into a subport rather than a variant, for the very reason the variant still has the circular dependency issue and when ports are built in the buildbots all variants are parsed, if not built, so potentially this issue will still be hit. |
|
@cjones051073 I do not find subports to be a neat solution. Also, removing subports is more problematic than dropping a variant (which hopefully can be done once the problem is actually fixed). I used check due to |
barracuda156
force-pushed
the
r_updates
branch
from
8c3cf0b
to
b295b87
Compare
Before you assume there is anything to fix we need feedback from those more familiar with the ci and buildbot scripts. Your workaround might have to be more long lived than you are assuming, and I will still worry about the same circular dependency issues hitting the buildbots even if you move it to a none default variant, as as I said all variants are tested, if not built. So putting your tastes aside you might have to move away from using a variant for this.
|
|
@cjones051073 Well, if now you consider CI unreliable (they all passed earlier), I can just remove test option here and make a note to be displayed to a user upon port install. Let us wait until CI results (for the record, so that we know if success is reproducible and not random), and then I can remove testing option from this port. |
Removing the test entirely also works for me, as that is also a way to completely remove the possibility of the circular dependency rearing its ugly head. If, later on it is concluded there is indeed something in ci/base/mpbb that needs fixing then you can reenable it once that update is available wherever it needs to be. |
|
@cjones051073 CI passed. Well, okay, I remove test option then, add a note for a user with a list of ports needed for tests, and let us merge this. |
barracuda156
force-pushed
the
r_updates
branch
from
b295b87
to
926e86e
Compare
barracuda156
changed the title
|
@cjones051073 CI passed (expectedly); if you are okay with this version, please help with merging this, so that I can deal with delayed updates to other |
|
@cjones051073 @kencu If anything still to be changed, please ping me here. |
|
@cjones051073 Thank you |
|
@barracuda156 with the resolution in https://trac.macports.org/ticket/68408 could you, if you have not already, open an MR that reverts the changes here, so remove the notes and put back the original test dependencies? Lets see if that fixes the issues you had here. |
|
Certainly nothing in the CI config or mpbb has ever looked at depends_test, except maybe the distfile mirroring. |
|
@cjones051073 @jmroot Thank you, sure. |
Description
Nothing will be added here.
(This PR does not depend on recommended packages update one. It can be merged once CI pass.)
Type(s)
Tested on
macOS 10.6
Xcode 3.2
Verification
Have you
port lint --nitpick?sudo port test?sudo port -vst install?