Skip to content

Conversation

@appolloford
Copy link
Contributor

(created using eb --new-pr)

@Micket
Copy link
Contributor

Micket commented Feb 21, 2025

Not much description here. I'm guessing this is to make it consistent with the meaning of "debug" in toolchain options? I.e. debug is supposed to mean only debug symbols, not a debug build.

Ideally, the easyblock should respect lowopt/noopt options that should reflect a debug type build scenario, though i suppoe the old code in the easyblock didn't do that either?

@Micket Micket added the bug fix label Feb 21, 2025
self.build_type = 'Debug'
else:
self.build_type = 'Opt'
# Always set build type to Opt, let easybuild decide CFLAGS and CXXFLAGS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't see a need for this, especially in EasyBuild 5.0 where we have a global setting to determine the value of the debug toolchain option (which is disabled by default).

What's the motivation for this exactly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

        if self.toolchain.options['debug']:
            self.build_type = 'Debug'

is just wrong logic. self.toolchain.options['debug'] means "enable_debug_symbols"

        if self.toolchain.options['noopt']:
            self.build_type = 'Debug'

would be appropriate.

@boegel boegel added this to the 4.x milestone Feb 26, 2025
@appolloford
Copy link
Contributor Author

Since EB5 uses debug symbol in toolchains by default. The original OpenFOAM easyblock captures the value and sets the build type to 'debug' and use the build-in debug flags such as the c++Debug file:

c++DBUG     = -ggdb3 -DFULLDEBUG
c++OPT      = -O0 -fdefault-inline

I think here are two problems:

  1. debug symbol should not cause OpenFOAM built in -O0 and have additional debug messages from -DFULLDEBUG
  2. EB always sets build FLAGS in environment variables so easyblock should always use the FLAGS set by EB. There should be no need to use the FLAGS come from OpenFOAM itself.
    If there is a need to have -DFULLDEBUG, I think this should be passed as a buildopt

To summary, the PR would like to force EB always use Opt case as this is the only one which passes the EB environment variables into their wmake in current openfoam easyblock

@boegel boegel changed the base branch from 5.0.x to develop March 19, 2025 11:09
@boegel boegel modified the milestones: 4.x, 5.x Mar 19, 2025
@boegel
Copy link
Member

boegel commented Mar 19, 2025

@appolloford I changed to target branch in this PR from 5.0.x to develop, you should synchronize your PR branch with current develop branch (which has received a massive update after the release of EasyBuild v5.0.0, see #3670)

@appolloford
Copy link
Contributor Author

I don't think anything in this PR is affected by the synchronization.

@boegel boegel modified the milestones: 5.x.x, 5.x Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants