Skip to content

created devclean.sh#415

Merged
natalie-perlin merged 2 commits into
ufs-community:developfrom
natalie-perlin:develop_1
Oct 28, 2022
Merged

created devclean.sh#415
natalie-perlin merged 2 commits into
ufs-community:developfrom
natalie-perlin:develop_1

Conversation

@natalie-perlin
Copy link
Copy Markdown
Collaborator

@natalie-perlin natalie-perlin commented Oct 13, 2022

a script to clean all the SRW App builds, including submodules, binaries directory (bin or exec), submodules themselves.

DESCRIPTION OF CHANGES:

A script that allows cleaning up the builds of the SRW App, including main build directory and any ./build/ subdirectories in the submodules, a binaries directory, and submodules themselves if requested.

./devclean.sh -h provides a summary of the options

-h, --help
show this help guide
-a, --all
removes "bin", "build" directories, and other build artifacts
--remove
removes the "build" directory, keeps the "bin", "lib" and other build artifacts intact
--clean
removes "bin", "build" directories, and other build artifacts (same as "-a", "--all")
--install-dir=INSTALL_DIR
installation directory name (${SRW_DIR} by default)
--build-dir=BUILD_DIR
main build directory, absolute path (${SRW_DIR}/build/ by default)
--bin-dir=BIN_DIR
binary directory name ("exec" by default); full path is ${INSTALL_DIR}/${BIN_DIR})
--sub-modules
remove sub-module directories. They will need to be checked out again by sourcing "${SRW_DIR}/manage_externals/checkout_externals" before attempting subsequent builds
-v, --verbose
provide more verbose output

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

TESTS CONDUCTED:

Post-build tests conducted to clean up the build and the submodules, in case of non-standard customized installation directory, build, and bin directories. The SRW app was built successfully after the cleanup. Additional cleanup was performed to remove the sub-modules; the submodules were checked out again (./manage_externals/checkout_externals) and the SRW App was built successfully after that.

Tested on the following platforms:
[x] Hera
[x] Gaea
[x] Jet
[x] Orion
[x] Cheyenne.

DOCUMENTATION:

[x] documentation update needed for this additional cleanup option

ISSUE:

This PR resolves the issue mentioned in [SRW-Issues/?]

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes do not require updates to the documentation (explain).
  • My changes generate no new warnings
  • [N/A] New and existing tests pass with my changes
  • Any dependent changes have been merged and published

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

  • enhancement

CONTRIBUTORS (optional):

@ Bruce Kropp - provided an initial script draft

a script to clean all the SRW builds in subdirectories, bin/exec directory
@MichaelLueken MichaelLueken added the run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests label Oct 13, 2022
Copy link
Copy Markdown
Collaborator

@danielabdi-noaa danielabdi-noaa left a comment

Choose a reason for hiding this comment

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

@natalie-perlin This looks good to me! I've left a few minor suggestions.

Comment thread devclean.sh Outdated
Comment thread devclean.sh Outdated
Comment on lines +122 to +124
rm -rf share
rm -rf include
rm -rf lib
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If an install location is specified as is the case for test/build.sh, this will not works since it assumes the install location is HOMEdir

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The updated script uses current directory as a SRW_DIR, where build directory is located by default; installation directory is SRW_DIR by default

Comment thread devclean.sh Outdated
Copy link
Copy Markdown
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@natalie-perlin I was able to clone your develop_1 branch on Hera, checkout the externals, and build the app (following modifications to the Hera modulefile due to updates made on HPC-stack this afternoon). I had to use chmod 755 on devclean.sh, but after that, I was able to run the new devclean script. The script performs well and does exactly as described.

After making modifications from @danielabdi-noaa, please use:

git add --chmod=+x devclean.sh

This command should add the file to be staged for commit and update the permissions on the file at the same time.

Also, I have executed the Jenkins workflow tests. The build on Hera will fail due to the update made to the HPC-stack on the machine. Your changes should be fine to merge in once the rest of the jobs run.

@MichaelLueken MichaelLueken added the release This PR/issue is related to a release branch label Oct 27, 2022
@natalie-perlin
Copy link
Copy Markdown
Collaborator Author

natalie-perlin commented Oct 28, 2022

@MichaelLueken - an updated devclean.sh has been uploaded with the correct permissions in place

(Thank you for the tip on using git add --chmod=+x <file> !)

@danielabdi-noaa
Copy link
Copy Markdown
Collaborator

@natalie-perlin Thanks for addressing my suggestion! It looks good to me so approving.

Copy link
Copy Markdown
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@natalie-perlin Thank you for updating the permissions for the devclean.sh script! I was able to test it and it works well. I did have a minor comment. If the line is not necessary, please remove it.

Comment thread devclean.sh
printf '%s\n' "Removing submodules ..."
declare -a submodules='()'
submodules=(${SRW_DIR}/sorc/*)
# echo " submodules are: ${submodules[@]} (total of ${#submodules[@]}) "
Copy link
Copy Markdown
Collaborator

@MichaelLueken MichaelLueken Oct 28, 2022

Choose a reason for hiding this comment

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

@natalie-perlin Is the commented out debug write on line 133 required? If it isn't, please remove it.

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

Labels

enhancement New feature or request release This PR/issue is related to a release branch run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants