Skip to content

do_cmake.sh: print progress while updating submodules#65303

Closed
rishabh-d-dave wants to merge 1 commit intoceph:mainfrom
rishabh-d-dave:submod-update-progress
Closed

do_cmake.sh: print progress while updating submodules#65303
rishabh-d-dave wants to merge 1 commit intoceph:mainfrom
rishabh-d-dave:submod-update-progress

Conversation

@rishabh-d-dave
Copy link
Contributor

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

jenkins test api

@rishabh-d-dave
Copy link
Contributor Author

jenkins test api

2 similar comments
@rishabh-d-dave
Copy link
Contributor Author

jenkins test api

@batrick
Copy link
Member

batrick commented Sep 2, 2025

jenkins test api

@rishabh-d-dave
Copy link
Contributor Author

jenkins test api

2 similar comments
@batrick
Copy link
Member

batrick commented Sep 3, 2025

jenkins test api

@rishabh-d-dave
Copy link
Contributor Author

jenkins test api

@cbodley
Copy link
Contributor

cbodley commented Sep 3, 2025

cc @dmick who removed this in #63171

@ljflores ljflores requested review from dmick and zmc September 3, 2025 21:06
@dmick
Copy link
Member

dmick commented Sep 3, 2025

why?

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Sep 4, 2025

why?

so that we can see progress when submodules are being cloned/updates. this is especially useful when submodule is huge or when network is a bit flaky.

Looking at 63171, perhaps we can add an extra flag that allows us to choose whether we want to see progress.


if [ -d .git ]; then
git submodule update --init --recursive --recommend-shallow
git submodule update --init --recursive --recommend-shallow --progress
Copy link
Member

@batrick batrick Sep 4, 2025

Choose a reason for hiding this comment

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

Suggested change
git submodule update --init --recursive --recommend-shallow --progress
EXTRA=()
if [ -t 1 ] ; then
EXTRA+=('--progress')
fi
git submodule update --init --recursive --recommend-shallow "${EXTRA[@]}"

would probably address @dmick 's concerns.

Copy link
Member

Choose a reason for hiding this comment

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

would probably address @dmick 's concerns.

I don't even wanna see that in an interactive run personally, but if others really do, we could add a -v switch

@cbodley
Copy link
Contributor

cbodley commented Sep 4, 2025

just my 2 cents, though not especially constructive:

this script has been serving two different use cases for quite a while: developers use it for local build setup, and ci uses it for make check. this pr is a great example of how those use cases differ

i would recommend that developers just use the git/cmake tools directly instead of relying on a helper script. you know these tools and can be opinionated about their options/configuration. Sage created this script at a time when the cmake port was relatively new and the team was still unfamiliar with it, but surely that has changed in the decade since

the problem with serving multiple use cases is that we have to make the script more complicated than either actually warrants

@dmick
Copy link
Member

dmick commented Sep 4, 2025

I would, and have with my removal, argue that "seeing progress" is not useful in any context. If you so strongly disagree then I'd support an extra argument to do_cmake (or just keep a personal patch around for the one time a year it gets changed and you have to update it). Blathering percentages to the screen helps no one.

@zmc
Copy link
Member

zmc commented Sep 18, 2025

Personally I am looking into removing this from make-dist since it has about fifteen thousand lines to log output, so I'd be opposed to more verbosity as default behavior

@dmick
Copy link
Member

dmick commented Sep 18, 2025

closed as rejected.

@dmick dmick closed this Sep 18, 2025
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.

6 participants