compilers: unify fortran sanity check with its parent Clike handling #10835
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is more correct to join commands with a command joiner than a whitespace joiner.
Codecov Report
@@ Coverage Diff @@
## master #10835 +/- ##
===========================================
- Coverage 68.99% 50.49% -18.50%
===========================================
Files 406 203 -203
Lines 88650 44306 -44344
Branches 19665 9818 -9847
===========================================
- Hits 61164 22373 -38791
+ Misses 22887 19925 -2962
+ Partials 4599 2008 -2591
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This code dates back to 2012 and probably has no specific reason...
We *mostly* just need to do the same thing. Plug in one utility method to make sanity_check_impl find the right compile args, and plug in DEVNULL to the test run. It's that simple. This solves a few inconsistencies. The main one is that fortran never logged the sanity checks to the Meson debug log, making it hard to debug. There's also some interesting quirks we built up in the dedicated fortran handling. For example: - in commit 5b109c9 we added cwd to building the fortran executable, with a wordy comment about how the compiler has defects. But the clike base has always done that on general principle anyway, so we would never have had that bug in the first place. - in commit d6be782 we added special deletion of an old "bad existing exe file" just for fortran. Looking at the PR discussion for this odd requirement, it turns out that the real problem is mixing WSL and native Windows without deleting the build directory. This is apparently fortran specific simply because "contemporary Windows 10 Fortran users" switch between the two? The actual problem is that this never used .exe as the output name, so Windows thinks you want to run something other than the thing you asked to run, because it's not even a Window executable. But... the common clike handling could have fixed that without needing special cases.
eli-schwartz
force-pushed
the
fortran-sanity-log
branch
from
2e0ca0d to
be10cff
Compare
3 tasks
Member
|
Looks pretty good to me :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We mostly just need to do the same thing. Plug in one utility method to make sanity_check_impl find the right compile args, and plug in DEVNULL to the test run. It's that simple.
This solves a few inconsistencies. The main one is that fortran never logged the sanity checks to the Meson debug log, making it hard to debug.
There's also some interesting quirks we built up in the dedicated fortran handling. For example:
The actual problem is that this never used .exe as the output name, so Windows thinks you want to run something other than the thing you asked to run, because it's not even a Window executable. But... the common clike handling could have fixed that without needing special cases.