Add new argument "-c", "--custom_tag" to allow custom tag

[MarcChen46]

Add new argument "-c", "--custom_tag" to allow custom tag format(ex: UEFI BIOS version could be WW.XX.YY.ZZ), the default set to False to maintain the backward compatible

[MarcChen46]

@apop5 @cfernald , please help review and provide feedback, thanks.

[makubacki]

Since you're using a f-string:

Suggested change

	logging.debug(f"custom_tag set to: " + str(is_custom_tag))
	logging.debug(f"custom_tag set to: {str(is_custom_tag)}")

[makubacki]

One of the original reasons for defining tags was consistency (and to some extent controlled/consistent auto versioning updates). Why is a custom tag needed?

• Can the original version be enhanced to cover the need?
• Does the custom tag being used include useful component versioning information (the ability to indicate breaking vs non-breaking changes, etc.)?

[MarcChen46]

The original tag format is like [Major].[Minor].[Patch], but we would like to use UEFI FW release version format as the tag for product development, so since product is primary focus on the PlatformPkg development, which has no one depends on it, so ideally it will not have breaking change case.

From product development perspective, it may have UEFI FW release version like [ProductName].[FirmwareComponent].[MajorVersion][MinorVersion].[Sku].[Patch], and different products from different projects may have different requirement and format, that's the reason I want to introduce custom_tag concept in the mu_devops, but if you think it would be better to keep the tag format consistent, I'm also ok to drop this PR and create a customized version of the TagGenerator.py in our private repo, the primary part I would like to re-use is the TagGenerator.py.

Let me know your thought, thanks. :)

[makubacki]

I think part of my apprehension is that this bolts on an ambiguous extension to the code.

Concern

1. Existing format: Is specific leading to the code's ability to derive more information using a well-known format.
2. New Change: Is ambiguous meaning common code logic cannot consistently derive similar information based on the version format used.

New Format vs Existing Format

The new format makes different assumptions about the organization of code than the existing format. For example, different components and products in the existing format would be stored in different repos / branches meaning that information does not need to reside directly in the tag but can be associated with the tag when it is fetched from the corresponding repo / branch. The new format also does not follow semantic versioning which was a goal in the existing format for compatibility with other tools.

I'm not opposed to the new format; I think it makes sense within a given structure of code.

Proposal

I'd prefer to continue to capture the full context of the version in this script similar to the existing format. Ideally, the formats would be abstracted behind a class. The class would provide information needed to make common decisions irrespective of the underlying format details. Then we don't need half-baked information here connected with other scripts elsewhere, all of the version information can be maintained behind a single abstracted version API. This also gives the benefit that other Python code using the class wouldn't need to change significantly if the underlying version format changed in the future.

Module level functions like is_breaking_change(), is_security_change(), is_new_feature(), generate_notes(), etc. would become class methods and return consistent information regardless of how they return the format for the underlying version.

In the end, this would mean:

• That the existing format ("semantic versioning format") and new format ("Product Component format") would have their underlying format details readily visible to developers that look at the scripts in mu_devops.
  • Other projects can easily choose the appropriate format without reinventing the wheel.
• Other Python code can use the class API to get various version information regardless of the underlying format details.
• The script can easily and obviously be extended in the future to support new formats without modifying various lines of code here and there in the procedural logic which is error prone and messy.

---

I don't think this would require significant additional effort, mostly pulling together pre-existing code and moving it behind instances of the version class API and some minor refactor. Is this something you or someone else could look into?

[MarcChen46]

Thanks @makubacki for your comment, it is a good feedback, since the suggestion might be a breaking change that would impact any repo that consume it, I would like to let MU team to assess the right and expected refactoring and make the code change, then I will consume it after that. do you think it is ok?

[makubacki]

Sure. It doesn't really matter who does it as long as it goes through code review in the end. If you don't have time for it, we can see if someone else does.

• Do the scripts for this specific custom format already exist for reference?
• When is this needed by?

[MarcChen46]

Our current format is like "[ProductName].[VendorNumber].[FirmwareComponent].[MajorVersion][MinorVersion].[Sku].[Patch]", but I would recommend to just let user to parse a string(ex: XXXXX.0.1A01.XX.1) as custom tag since the format may get changed in the future, so let user provide a string as custom tag may have better flexibility, the custom tag format detail can be handled by user it self separately.

Since I can use my current python script to cover our current requirement, so it is not urgent to us, but once the refactoring is done and PR is created, I can just verify and consume the new version from mu_devops.

[makubacki]

@cfernald to see if you're interested since you added the original script.

[cfernald]

You have the default as a boolean, but then use the paramater as a string. Could we make this a bit more consistent? I wonder if using the custom tag directly as the tag value if not None or "" makes more sense then the boolean to indicate to use the major version. This would also avoid the oddity of trying to use a flag in the pipeline file.

[makubacki]

I agree the parameter type should at least be consistent.

[makubacki]

Closing since a large rework is described in #293 (comment). A future PR can be submitted for #294.