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

build: reduce rebuilds, don't install dev tools #1090

Merged
merged 4 commits into from
Sep 30, 2023

Conversation

trws
Copy link
Member

@trws trws commented Sep 29, 2023

Fixes #1078
Fixes #1089
Avoids rebuilds of docs, and global rebuilds on tweaks to CMakeLists.txt files as discussed in #1082.

@trws trws force-pushed the cmake-reduce-rebuilds branch from 40d0048 to aaf5cb0 Compare September 29, 2023 18:10
@trws trws requested review from grondo and milroy September 29, 2023 18:11
@trws
Copy link
Member Author

trws commented Sep 29, 2023

Since everything passed (before the rebase), I'm happy with this pending an accept review from anyone. Please add MWP for me if it's good since I'm about to head to bed and it will add about a day otherwise.

@grondo grondo changed the title Reduce rebuilds, don't install dev tools build: reduce rebuilds, don't install dev tools Sep 29, 2023
Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

Tried this out and worked great for me!

@trws trws added the merge-when-passing mergify.io - merge PR automatically once CI passes label Sep 30, 2023
Problem: any time cmake was rerun, it would rebuild *EVERYTHING*

Solution: instead of unconditionally writing out the config.h every time
(what I get for using the auto-generated config for that) use the proper
`configure_file` command to generate it from a template.
Problem: docs were being rebuilt every build, regardless of changes

Solution: split add_custom_target into command and target to properly
track inputs and outputs
Removes the install of grug2dot and resource-query.  They are still
built, but no longer placed into an installation.
Ensure we only add REQUIRED to the check for sphinx when the ENABLE_DOCS
option is "On" rather than "Try".
@trws trws force-pushed the cmake-reduce-rebuilds branch from aaf5cb0 to 7ff8a93 Compare September 30, 2023 07:37
@codecov
Copy link

codecov bot commented Sep 30, 2023

Codecov Report

Merging #1090 (9581a83) into master (5209665) will not change coverage.
The diff coverage is n/a.

❗ Current head 9581a83 differs from pull request most recent head 7ff8a93. Consider uploading reports for the commit 7ff8a93 to get more accurate results

@@          Coverage Diff           @@
##           master   #1090   +/-   ##
======================================
  Coverage    71.7%   71.7%           
======================================
  Files          89      89           
  Lines       11665   11665           
======================================
  Hits         8371    8371           
  Misses       3294    3294           

@mergify mergify bot merged commit b29177b into flux-framework:master Sep 30, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-passing mergify.io - merge PR automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build: don't install grug2dot and resource-query cmake build fails on toss4 when missing sphinx
2 participants