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: Allow options to wget curl cmake during setup #12201

Conversation

anandamideShakyan
Copy link
Contributor

Description

The current setup scripts generate excessive output, the goal is to reduce the log output by implementing the following changes:

  1. Avoid printing the files extracted by tar.
  2. Silence the download progress from curl by using an appropriate curl option.
  3. Suppress unnecessary CMake messages, logging only warnings instead of info or status messages.
  4. Introduce a new environment variable to enable the extensive logging when needed.
  5. These changes will help minimize log size while maintaining flexibility for detailed logging when required.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 29, 2025
Copy link

netlify bot commented Jan 29, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit b184a92
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67a37f9271d4cf0008c1c12e

@majetideepak majetideepak changed the title build: reduce excessive logging during setup and build process build: Reduce excess logging during setup and build process Jan 29, 2025
@majetideepak majetideepak changed the title build: Reduce excess logging during setup and build process build: Reduce excess logging during setup Jan 29, 2025
@@ -29,6 +29,12 @@ if [[ "$OSTYPE" == darwin* ]]; then
export INSTALL_PREFIX=${INSTALL_PREFIX:-"$(pwd)/deps-install"}
fi

if [[ "${VERBOSE_LOGGING}" == "true" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's define WGET_OPTIONS and let the user specify the options. We can remove VERBOSE_LOGGING var.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Basically like this

WGET_OPTIONS=${WGET_OPTIONS:-""}

@@ -20,6 +20,14 @@ DEPENDENCY_DIR=${DEPENDENCY_DIR:-$(pwd)/deps-download}
OS_CXXFLAGS=""
NPROC=${BUILD_THREADS:-$(getconf _NPROCESSORS_ONLN)}

if [[ "${VERBOSE_LOGGING}" == "true" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here. Let's define CURL_OPTIONS and CMAKE_OPTIONS and leave the value to the user to set.

@majetideepak majetideepak changed the title build: Reduce excess logging during setup build: Allow options to wget curl cmake during setup Jan 31, 2025
@anandamideShakyan anandamideShakyan force-pushed the shakyan-reduce-log branch 2 times, most recently from 0d76f64 to 7d4b28d Compare January 31, 2025 15:24
@majetideepak majetideepak added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Feb 4, 2025
@facebook-github-bot
Copy link
Contributor

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kgpai merged this pull request in 179cef7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants