-
-
Notifications
You must be signed in to change notification settings - Fork 503
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
fortran: update compiler version #1359
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
note migration.yml similar to https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/837/files may be required. |
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.
This change will need to be discussed with @conda-forge/core and probably requires a migration.
it's been pending for a while now, and wasn't done. I thought I'd shake things loose. |
Yes, I don't think there's anything to discuss, but we do need a migration yaml. |
Is this okay? |
@conda-forge/core Do we want to make an announcement? Should we ship both versions for a while? |
Friendly nudge @conda-forge/core (since this came in on the weekend people may have missed it) |
cc @h-vetinari |
We still need to add an option to the bot to send this PRs to only packages which use flang on windows. I think right now the bot will send a PR to packages that use fortran on unix, but use cc @conda-forge/bot |
Would be awesome if this can be picked up again. 🙃 |
@isuruf, any hint on how to do that? I might give it a shot. |
How many windows flang packages are there? Maybe we rebuild a few extra ones if it is small? |
How many Fortran packages are there? And how many of those support Windows? Guessing this is probably quite small, but don't really know. |
@beckermr, @jakirkham, are you saying that we don't need to modify the bot per @isuruf's suggestion, just because even if this would happen (i.e. some packages get superfluous PRs), those are likely so few that we can deal with them just by manually closing the PRs? Hence, this migrator can be started as is? |
That is my suggestion yes. We'll need everyone to agree before we go that way. I also could be wrong. |
No. There's 300+ fortran feedstocks and only about 2 dozen feedstocks using flang. I wouldn't characterize that as |
Ok. So then we’ll need a pr to the bot to avoid rebuilding about 30 feedstocks out of 300. I don’t think I’ll have time to do this but I’m happy to review! |
No, it's the other way around. We have to avoid rebuilding 270 out of 300. |
Gotcha. Do I understand correctly that this needs a PR to change @isuruf, did you have something general in mind, like an addition to the yaml format for the migrators that allows filtering on the graph, or rather something specific, more like a custom migrator? |
Sorry, @h-vetinari, I have been taken up by other things. I am not sure when I will manage to pick this up in earnest. |
With llvm-flang hopefully not toooooo far out, I gave this a shot in regro/cf-scripts#1655. Feedback & advice appreciated! |
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.
@xoviat, not sure if you're still around? 😅
If yes, could you try to add the following (based on regro/cf-scripts#1655)? Since we couldn't write tests I'm a bit apprehensive if it'll work right away, but we can always pause & debug the migration.
PS. Not sure if relevant; clang 11.0 had an ABI break and 11.1 was released to fix it. I had an open PR to rebuild flang for that, but this ran into some weird build tooling errors I wasn't able to debug at the time.
@isuruf, are your concerns about the migration now addressed? I believe the very old default flang version is the reason why the compilers package is having resolution problems: conda-forge/compilers-feedstock#58 |
As an update to anyone subscribed here - we discussed this in a core meeting a few months ago, and the status was that the "updated" flang 11 builds based on classic-flang have no support (we also haven't heard from @xoviat for a long time), and so we decided to wait for llvm-flang to materialise. We now have flang 17 live in conda-forge, but it's still somewhat experimental, so the earliest to retry this realistically is for flang 18 early next year. |
Another update: flang 18 didn't work for compiling OpenBLAS on windows, but with the in-progress flang 19 (ETA ~September), things are looking much better. |
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
This is still a draft, but it's looking like flang 19 will finally be ready on windows to compile most of our feedstocks. Already tested successfully: OpenBLAS (which still failed with v18), SciPy and a few others in progress. |
The requested preparation and discussion has happened
This was discussed several times in core over the last few years (most recently, I had proposed to do this as soon as LLVM 19 goes GA -- there was no opposition to doing this). As we now have all the builds in place, I'm planning to merge this in 48h unless there are other comments. |
Finally the rename from flang-new to flang landed: llvm/llvm-project@06eb10d 🥳 I've tried backporting the patch to LLVM 19, and aside from some trivial conflicts, it looks like we could do it, though I think I'd prefer to leave that for LLVM 20. Let me know if someone thinks differently... |
Alright, let's get this party started! |
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 I totally missed this, but the migration files need to end in .yaml
. We should add a test and/or hook in pre-commit for this.
No description provided.