Skip to content
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

Makefile fix for external FP submodule inclusion #28609

Merged
merged 4 commits into from
Sep 27, 2024

Conversation

GiudGiud
Copy link
Contributor

refs #27027

@GiudGiud GiudGiud self-assigned this Sep 12, 2024
@loganharbour
Copy link
Member

Added install targets.

@GiudGiud
Copy link
Contributor Author

ugh commit 2 fixes something but breaks another.
I ll leave only commit 1 for now

@moosebuild
Copy link
Contributor

moosebuild commented Sep 12, 2024

Job Documentation on cff8a9c wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Sep 12, 2024

Job Coverage on cff8a9c wanted to post the following:

Framework coverage

Coverage did not change

Modules coverage

Fluid properties

b9e8bd #28609 cff8a9
Total Total +/- New
Rate 85.19% 85.18% -0.01% -
Hits 7770 7769 -1 0
Misses 1351 1352 +1 0

Diff coverage report

Full coverage report

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud GiudGiud marked this pull request as ready for review September 12, 2024 21:30
@GiudGiud GiudGiud changed the title Makefile fixes for external FP submodule inclusion Makefile fix for external FP submodule inclusion Sep 12, 2024
@loganharbour
Copy link
Member

I think we probably need a sanity check from a few for this one

@GiudGiud
Copy link
Contributor Author

This does not solve the header symlink warnings btw (which are caused by SodiumRevision.h being created in every module when building combined, for some reason)

But it should solve the missing "SinglePhaseFluidProperties.h" include when building the modules inside combined apps we have seen in the apptainer recipes

@loganharbour
Copy link
Member

Looks like this breaks things: https://civet.inl.gov/job/2442762/

@GiudGiud
Copy link
Contributor Author

Seems to be tied to the revision, the other problem. I invalidated to see if it gets it past

In file included from /data/civet1/build/blue_crab/build/unity_src/base_Unity.C:2:
/data/civet1/build/blue_crab/src/base/BlueCrabApp.C: In member function 'virtual std::string BlueCrabApp::header() const':
/data/civet1/build/blue_crab/src/base/BlueCrabApp.C:253:50: error: 'SODIUM_REVISION' was not declared in this scope; did you mean 'HELIUM_REVISION'?
  253 |   os << std::setw(25) << "  Sodium version: " << SODIUM_REVISION << "\n";
      |                                                  ^~~~~~~~~~~~~~~
      |                                                  HELIUM_REVISION
Compiling C++ (in opt mode) /data/civet1/build/blue_crab/build/unity_src/timesteppers_Unity.C...
Compiling C++ (in opt mode) /data/civet1/build/blue_crab/build/unity_src/postprocessors_Unity.C...
make: *** [/data/civet1/build/moose/framework/build.mk:150: /data/civet1/build/blue_crab/build/unity_src/base_Unity.x86_64-pc-linux-gnu.opt.lo] Error 1
make: *** Waiting for unfinished jobs....

ERROR: exiting with code 2

@GiudGiud
Copy link
Contributor Author

Trying to fix the other issue too again

@GiudGiud
Copy link
Contributor Author

Ok should be good.

The only fail is Okami, so I'll look to patch Okami most likely

@GiudGiud
Copy link
Contributor Author

Okami patch is up https://hpcgitlab.hpc.inl.gov/idaholab/okami/-/merge_requests/42

this is good to go

@loganharbour
Copy link
Member

moosebuild.hpc.inl.gov has been off for a while; need to get that back going

@milljm
Copy link
Member

milljm commented Sep 17, 2024

Its crazy how often I fix this, and then it breaks the following day

@milljm
Copy link
Member

milljm commented Sep 17, 2024

https://moosebuild.hpc.inl.gov is back online

@GiudGiud
Copy link
Contributor Author

let's merge? stuff is failing downstream because this is not in
https://civet.inl.gov/job/2462068/

@GiudGiud
Copy link
Contributor Author

Unrelated failure

@GiudGiud GiudGiud merged commit c889ff8 into idaholab:next Sep 27, 2024
56 of 57 checks passed
@GiudGiud GiudGiud deleted the PR_fp_followup branch September 27, 2024 15:42
@loganharbour
Copy link
Member

Failure in https://civet.inl.gov/job/2445689/ was not unrelated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants