-
Notifications
You must be signed in to change notification settings - Fork 119
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
Add arguments to devbuild.sh script #155
Add arguments to devbuild.sh script #155
Conversation
b921240
to
0319ce3
Compare
@gsketefian I was looking at the ufs-weather-model CMakeList.txt and they use -DDEBUG to set CMAKE_BUILD_TYPE. Do you want me to replace --build-type=? with --debug? |
I like --build-type because --debug sounds to me like a true/false flag (but isn't). |
0319ce3
to
9247726
Compare
* Documented with --help * Automatically determines machine name * Automatically determines compiler based on machine name * Added options for app, ccpp suite, and components * Added option for build type DEBUG, RELEASE, RELWITHDEBINFO * Added option for build jobs, default 4 * Added verbosity option for make
9247726
to
e961aad
Compare
@danrosen25 This looks good now. I guess it should be tested on one more machine, e.g. Hera. I would volunteer but have some other things to finish. I can get to it say Thursday, but if someone else can test it beforehand, that would be great. |
@danrosen25 I can test this on Hera and Cheyenne. I am out Friday, and Cheyenne is down next week, so I will start with Hera. Let me know when it is ready for testing. |
@JeffBeck-NOAA @JulieSchramm @mkavulich @gsketefian |
When I run |
Oops, need the --platform. I'm not a big fan of the machine-setup.sh script. It figures out the machine name from the available disk names. This caused problems when "yellowstone" became "cheyenne" and the disk names changed, and this had to be fixed in the many instances of this script. Can the platform be a required argument, and then we wouldn't need machine-setup.sh? |
@JulieSchramm
|
@danrosen25 When I type ``./devbuild.sh hera``, I don't get an error
message and the build starts. The --build-dir and --install-dir work as
expected if I specify ``--platform=hera``.
…On Mon, Jul 26, 2021 at 3:53 PM Dan ***@***.***> wrote:
@JulieSchramm <https://github.com/JulieSchramm>
I removed the platform requirement. ./devbuild.sh hera will throw an
error because the argument is unknown. ./devbuild.sh will automatically
determine that you're running on hera or ./devbuild.sh --platform=hera
will manually specify hera.
--build-dir=build_intel will build into <CURRENT_DIRECTORY>/build_intel
and --install-dir=../bin_intel sets CMAKE_INSTALL_PREFIX=../bin_intel,
which will install binaries into
<CURRENT_DIRECTORY>/build_intel/../bin_intel/bin, libraries into
<CURRENT_DIRECTORY>/build_intel/../bin_intel/lib, share into
<CURRENT_DIRECTORY>/build_intel/../bin_intel/share, and include into
<CURRENT_DIRECTORY>/build_intel/../bin_intel/include
Is that what you want?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#155 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA3WNUY5AIMQLUMM4KA6N63TZXKNVANCNFSM5AUFVAJA>
.
|
@JulieSchramm |
@danrosen25 Making PLATFORM a required argument would be great. Thanks for
doing this.
…On Mon, Jul 26, 2021 at 6:43 PM Dan ***@***.***> wrote:
@JulieSchramm <https://github.com/JulieSchramm>
Oh, I see that I'm just ignoring arguments that don't start with -, so
yes you can type ./devbuild.sh hera but hera is ignored. Do you want me
to make PLATFORM a required argument since the machine_setup.sh script can
be buggy? I would change usage to Usage: .devbuild.sh PLATFORM
[OPTIONS]... and remove --platform=PLATFORM.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#155 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA3WNUZD5SANNYQAECMQ5QTTZX6KDANCNFSM5AUFVAJA>
.
|
* remove --components * add --enable-options * add --disable-options
@JulieSchramm I made |
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.
Looks good, my concerns have been addressed. Once @JulieSchramm is okay with the updates I believe this PR is good to go.
I will test on Cheyenne next week Aug 2 or 3 when the computer is back up. |
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.
Tested on Jet, Hera and Cheyenne.
DESCRIPTION OF CHANGES:
TESTS CONDUCTED:
Tested each devbuild.sh option in isolation. Tested on Jet (needs testing and confirmation of platform/compiler defaults on other machines)
DEPENDENCIES:
None
DOCUMENTATION:
CONTRIBUTORS:
@danrosen25