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

Update the pipeline to generate addon build versions that are compatible with MV3 #215

Merged
merged 5 commits into from
Jun 13, 2024
Merged
Changes from 3 commits
Commits
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
90 changes: 72 additions & 18 deletions taskcluster/docker/node/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,32 @@ def write_package_info(package_info):

def get_buildid():
now = datetime.utcnow()
return now.strftime("%Y%m%d.%H%M%S")
# note the `-` (hyphen) in `%-H`
# this removes the leading zero
# from the hour (leading zeros are not allowed)
return now.strftime("%Y%m%d.%-H%M%S")


def get_buildid_version(version):
"""Version schema check+append a `buildid{buildid}` to ensure unique version."""
# XXX is there a more precise schema we should verify ourselves against?
if len(version.split(".")) not in (1, 2, 3):
raise Exception("{version} has too many version parts!")
if "buildid" in version:
raise Exception("{version} already has a buildid specified!")
buildid_version = f"{version}buildid{get_buildid()}"
"""Check the version's formating and append `date.time` as a buildid to ensure a unique version.
The addon's in-tree `package.json` file specifies `major.minor.0`.
The format of the resulting version field is:
<number>.<number>.%Y%m%d.%-H%M%S
"""
parts = version.split(".")
if len(parts) not in (1, 2, 3):
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be (1,2,3,4)? From the docs:

The version string consists of 1 to 4 numbers separated by dots, for example, 1.2.3.4. Non-zero numbers must not include a leading zero. For example, 2.01 is not allowed; however, 0.2, 2.0.1, and 2.10 are allowed.

Copy link
Member Author

@gabrielBusta gabrielBusta Jun 13, 2024

Choose a reason for hiding this comment

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

Element 3 is over-ridden by the pipeline to append a timestamp in such a way that the generated versions are compatible with MV3 (the timestamp is part 3 and 4). Ideally it would be (1, 2) but two part versions break a library called node-semver. Two part versions are technically valid in the semver spec, but the version in package.json must be parasable by node-semver to be a valid semantic version as far as npm is concerned. This library thinks a version must have 3 parts to be a valid semantic version (and tooling relying on this library breaks when package.json has a version that this library can't parse 🫠)

Copy link
Member Author

Choose a reason for hiding this comment

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

So allowing a third part (that is over-ridden) is a escape hatch for the addons relying on such tooling.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, taking a look at the spec again it sounds like it requires a 3 part version.

raise Exception(
f"{version} has an invalid number of version parts! The pipeline supports a `major.minor.0` version format in the extension's manifest"
)
# Append (or override) the last two parts of the version with a timestamp to ensure a unique version that is compliant with MV3.
if len(parts) == 3:
Copy link
Contributor

Choose a reason for hiding this comment

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

...and this should be 4, and further down we should take the first 3 parts?

# Print a noisy warning if we override an element in the extension's version field.
if parts[2] != "0":
msg = f"! WARNING: THE 3RD ELEMENT IN THE VERSION {version} IS {parts[2]} NOT 0. THIS VALUE WILL BE OVERRIDEN BY THE PIPELINE !"
print(f"{'!' * len(msg)}\n\n{msg}\n\n{'!' * len(msg)}")
parts = parts[:2]
version = ".".join(parts)
buildid_version = f"{version}.{get_buildid()}"
print(f"Buildid version is {buildid_version}")
return buildid_version

Expand Down Expand Up @@ -128,11 +143,35 @@ def mkdir(path):
def get_hash(path, hash_alg="sha256"):
h = hashlib.new(hash_alg)
with open(path, "rb") as fh:
for chunk in iter(functools.partial(fh.read, 4096), b''):
for chunk in iter(functools.partial(fh.read, 4096), b""):
h.update(chunk)
return h.hexdigest()


def is_version_mv3_compliant(version):
# Split the version string by dots
parts = version.split(".")

# Check if there are 1 to 4 parts
if len(parts) < 1 or len(parts) > 4:
return False

for part in parts:
# Check if the part is a number
if not part.isdigit():
return False
# Check that the part doesn't have leading zeros (unless it's "0")
if part[0] == "0" and part != "0":
return False
# Convert part to integer to check the digit count
num = int(part)
# Check if the integer has more than 9 digits
if num > 999999999:
return False

return True


def check_manifest(path, buildid_version):
xpi = ZipFile(path, "r")
manifest = {}
Expand All @@ -150,20 +189,35 @@ def check_manifest(path, buildid_version):
continue
_found = True
if not _id.endswith(ID_ALLOWLIST):
raise Exception(f"{_key}.gecko.id {_id} must end with one of the following suffixes!\n{ID_ALLOWLIST}")
raise Exception(
f"{_key}.gecko.id {_id} must end with one of the following suffixes!\n{ID_ALLOWLIST}"
)
else:
print(f"Add-on id {_id} matches the allowlist.")
if manifest["version"] != buildid_version:
raise Exception(f"{manifest['version']} doesn't match buildid version {buildid_version}!")
raise Exception(
f"{manifest['version']} doesn't match buildid version {buildid_version}!"
)
if manifest["manifest_version"] == 3 and not is_version_mv3_compliant(manifest["version"]):
Copy link
Member

Choose a reason for hiding this comment

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

Seeing the check against the buildid_version right before, this new check would mean we somehow failed to make a compliant version string, right?

Copy link
Member Author

@gabrielBusta gabrielBusta Jun 10, 2024

Choose a reason for hiding this comment

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

Yeah, that's right. This is checking the generated versions in the manifest(s) to make sure they are compliant. It is most likely that this check will fail because of the manually crafted part of the string. But the in-container docker script/executable is not unit tested. So I added this as a sanity check/sign-post.

raise Exception(
(
f"The version in {manifest_name} is {manifest['version']}, which is not MV3 compliant. "
"The value must be a string with 1 to 4 numbers separated by dots (e.g., 1.2.3.4). "
"Each number can have up to 9 digits and leading zeros before another digit are not allowed "
"(e.g., 2.01 is forbidden, but 0.2, 2.0.1, and 2.1 are allowed)."
)
)
if not _found:
raise Exception("Can't find addon ID in manifest.json!")


def main():
test_var_set([
"ARTIFACT_PREFIX",
"XPI_NAME",
])
test_var_set(
[
"ARTIFACT_PREFIX",
"XPI_NAME",
]
)

artifact_prefix = os.environ["ARTIFACT_PREFIX"]
xpi_name = os.environ["XPI_NAME"]
Expand Down Expand Up @@ -204,10 +258,10 @@ def main():
run_command(["npm", "install"])
run_command(["npm", "run", "build"])

if 'XPI_ARTIFACTS' in os.environ:
if "XPI_ARTIFACTS" in os.environ:
xpi_artifacts = os.environ["XPI_ARTIFACTS"].split(";")
else:
xpi_artifacts = glob.glob('*.xpi') + glob.glob("**/*.xpi")
xpi_artifacts = glob.glob("*.xpi") + glob.glob("**/*.xpi")

all_paths = []
for artifact in xpi_artifacts:
Expand All @@ -232,4 +286,4 @@ def main():
fh.write(json.dumps(build_manifest, indent=2, sort_keys=True))


__name__ == '__main__' and main()
__name__ == "__main__" and main()