Skip to content

build: Add commandline versioning#7930

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.

Sign up for GitHub

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

Closed
dabekjakub wants to merge 1 commit into from
Closed

build: Add commandline versioning #7930

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion scripts/xtensa-build-zephyr.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -62,6 +62,7 @@
west_top = pathlib.Path(SOF_TOP, "..").resolve()

sof_fw_version = None
sof_build_version = None

if py_platform.system() == "Windows":
xtensa_tools_version_postfix = "-win32"
Expand DownExpand Up@@ -212,6 +213,11 @@ def parse_args():
default=[], help="Paths to overlays")
parser.add_argument("-p", "--pristine", required=False, action="store_true",
help="Perform pristine build removing build directory.")
parser.add_argument("-b", "--build_version", required=False,
help="Overrides cmake provided build version")
parser.add_argument("-f", "--fw_version", required=False,
help="Overrides cmake provided fw version")

parser.add_argument("-u", "--update", required=False, action="store_true",
help="""Runs west update command - clones SOF dependencies. Downloads next to this sof clone a new Zephyr
project with its required dependencies. Creates a modules/audio/sof symbolic link pointing
Expand DownExpand Up@@ -256,6 +262,15 @@ def parse_args():
if args.all:
args.platforms = platform_names

global sof_fw_version
global sof_build_version

@marc-hbmarc-hbJul 12, 2023

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.

Please don't define new globals, just use args.sof_fw_version directly in get_sof_version() and keep all the sof_fw_version related code in a single place.

We already use args as a global all over the place, as long as it's keep immutable it's fine.

@dabekjakubdabekjakubJul 12, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

There is a difference between sof_fw_version and sof_build_version when passing it to rimage so i would have to do string operations to get out the build number. I think global is less bothersome.

Or did I misunderstand what is your intent here ?

@dabekjakubdabekjakubJul 12, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

nvm got it


if args.fw_version is not None:
sof_fw_version = args.fw_version

if args.build_version is not None:
sof_build_version = args.build_version

# print help message if no arguments provided
if len(sys.argv) == 1:
parser.print_help()
Expand DownExpand Up@@ -624,7 +639,12 @@ def rimage_options(platform_dict):
# FIXME: drop this line once the following test is fixed
# tests/avs/fw_00_basic/test_01_load_fw_extended.py::TestLoadFwExtended::()::
# test_00_01_load_fw_and_check_version
opts.append(("-b", "1"))

global sof_build_version

@marc-hbmarc-hbJul 12, 2023

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.

Same thing as above, just use args.sof_build_version directly for consistency with the rest of the code, no need for a new global. One args global is enough.

if sof_build_version is not None:

@marc-hbmarc-hbJul 12, 2023

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.

Avoid double negations

ifsof_build_versionisNone:
opts.append(("-b", "1"))
else:
...

opts.append(("-b", sof_build_version))
else:
opts.append(("-b", "1"))

if args.ipc == "IPC4":
rimage_desc = platform_dict["IPC4_RIMAGE_DESC"]
Expand Down