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

Convert all make rules to be overridable, clean up typos #12867

Conversation

andrewimeson
Copy link
Contributor

Various guides on the internet suggest editing rules/config to enable features. This slightly complicates maintaining a build, because you either have to git stash (and hope that when you re-apply a change to the file hasn't caused a conflict) before pulling, or commit your changes and merge or rebase upstream changes. Prior to this these were the only configurable values in the file:

  • SONIC_DPKG_CACHE_METHOD
  • SONIC_DPKG_CACHE_SOURCE
  • INCLUDE_KUBERNETES_MASTER
  • SONIC_ENABLE_IMAGE_SIGNATURE
  • SONIC_ENABLE_SECUREBOOT_SIGNATURE
  • PACKAGE_URL_PREFIX
  • SONIC_VERSION_CONTROL_COMPONENTS
  • REGISTRY_PORT
  • REGISTRY_SERVER
  • ENABLE_ASAN
  • DEFAULT_CONTAINER_REGISTRY
  • ENABLE_FIPS_FEATURE
  • ENABLE_FIPS
  • SONIC_SLAVE_DOCKER_DRIVER

Why I did it

Since these are build flags, it shouldn't require modifying source-controlled files to build.

How I did it

Switching from variable assignment to optional variable assignment if unset.

How to verify it

$ make init
$ make configure PLATFORM=broadcom
$ make ENABLE_SFLOW_DROPMON=y SONIC_BUILD_JOBS=4 target/sonic-broadcom.bin

SONiC Build System

Build Configuration
"CONFIGURED_PLATFORM"             : "broadcom"
SONiC Build System

Build Configuration


"ENABLE_SFLOW_DROPMON"            : "y"

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

Allow overriding rules/config values with make

A picture of a cute animal (not mandatory but encouraged)

Bartalina the house sheep @ 1-2 days old
image

Signed-off-by: Andrew Imeson <andrew@andrewimeson.com>
@andrewimeson
Copy link
Contributor Author

I learned on the mailing list that I can create rules/config.user which will get read in after rules/config.

I think it may make sense to support both? It's a little confusing to have only some flags configureable via make FLAG_NAME=y ….

@andrewimeson
Copy link
Contributor Author

example of an issue due to this confusion
#12362


# SONIC_CONFIG_ENABLE_COLORS - enable colored output in build system.
# Comment next line to disable:
# SONIC_CONFIG_ENABLE_COLORS = y
SONIC_CONFIG_ENABLE_COLORS ?= y
Copy link
Contributor

Choose a reason for hiding this comment

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

Contrary to the original description, this might actually be disabled by default. It's also not used anywhere (besides defining some log_* color functions), as far as I can tell, but it's probably best to default this to n instead.

@saiarcot895
Copy link
Contributor

example of an issue due to this confusion #12362

This is actually due to a different reason, I think related to Makefile.work not propagating this variable into the slave docker container. However, I think it turns out that the intended value for that variable does get applied due to it getting stored in src/sonic-build-hooks/buildinfo/config/buildinfo.config. Regardless, this should probably get fixed to at least show the correct value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants