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.

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

Bug 1882852 - remove vendored createprecomplete in iscript and signin… #955

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Bug 1882852 - remove vendored createprecomplete in iscript and signin… #955

Show file tree
Hide file tree
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
33 changes: 21 additions & 12 deletions iscript/src/iscript/autograph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from mozpack import mozjar
from requests_hawk import HawkAuth

from iscript.createprecomplete import generate_precomplete
from iscript.exceptions import IScriptError
from scriptworker_client.aio import raise_future_exceptions, retry_async
from scriptworker_client.utils import makedirs, rm
Expand Down Expand Up @@ -101,9 +100,9 @@ async def sign_widevine_dir(config, sign_config, app_dir):
all_files.append(to)
await raise_future_exceptions(tasks)
remove_extra_files(app_dir, all_files)
# Regenerate the `precomplete` file, which is used for cleanup before
# Update the `precomplete` file, which is used for cleanup before
# applying a complete mar.
_run_generate_precomplete(config, app_dir)
_update_precomplete(config, app_dir)
return app_dir


Expand Down Expand Up @@ -143,16 +142,26 @@ def _get_widevine_signing_files(file_list):
return files


# _run_generate_precomplete {{{1
def _run_generate_precomplete(config, app_dir):
# _update_precomplete {{{1
def _update_precomplete(config, app_dir):
"""Regenerate `precomplete` file with widevine sig paths for complete mar."""
log.info("Generating `precomplete` file...")
path = _ensure_one_precomplete(app_dir, "before")
with open(path, "r") as fh:
precomplete = _ensure_one_precomplete(app_dir)
with open(precomplete, "r") as fh:
before = fh.readlines()
generate_precomplete(os.path.dirname(path))
path = _ensure_one_precomplete(app_dir, "after")
with open(path, "r") as fh:
with open(precomplete, "w") as fh:
for line in before:
fh.write(line)
instr, path = line.strip().split(None, 1)
if instr != "remove":
continue
if not path or path[0] != '"' or path[-1] != '"':
continue
file = path[1:-1]
if _get_widevine_signing_files([file]):
sigfile = _get_mac_sigpath(file)
fh.write('remove "{}"\n'.format(sigfile))
with open(precomplete, "r") as fh:
after = fh.readlines()
# Create diff file
makedirs(os.path.join(config["artifact_dir"], "public", "logs"))
Expand All @@ -163,13 +172,13 @@ def _run_generate_precomplete(config, app_dir):


# _ensure_one_precomplete {{{1
def _ensure_one_precomplete(tmp_dir, adj):
def _ensure_one_precomplete(tmp_dir):
"""Ensure we only have one `precomplete` file in `tmp_dir`."""
precompletes = glob.glob(os.path.join(tmp_dir, "**", "precomplete"), recursive=True)
if len(precompletes) < 1:
raise IScriptError('No `precomplete` file found in "%s"', tmp_dir)
if len(precompletes) > 1:
raise IScriptError('More than one `precomplete` file %s in "%s"', adj, tmp_dir)
raise IScriptError('More than one `precomplete` file in "%s"', tmp_dir)
return precompletes[0]


Expand Down
76 changes: 0 additions & 76 deletions iscript/src/iscript/createprecomplete.py
View file
Open in desktop

This file was deleted.

14 changes: 6 additions & 8 deletions iscript/tests/test_autograph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ def fake_isfile(path):

mocker.patch.object(autograph, "sign_widevine_with_autograph", new=noop_async)
mocker.patch.object(autograph, "makedirs", new=noop_sync)
mocker.patch.object(autograph, "generate_precomplete", new=noop_sync)
mocker.patch.object(autograph, "_run_generate_precomplete", new=noop_sync)
mocker.patch.object(autograph, "_update_precomplete", new=noop_sync)
mocker.patch.object(os.path, "isfile", new=fake_isfile)
mocker.patch.object(os, "walk", new=fake_walk)

Expand Down Expand Up @@ -172,22 +171,21 @@ def test_get_widevine_signing_files(filenames, expected):
assert autograph._get_widevine_signing_files(filenames) == expected


# _run_generate_precomplete {{{1
# _update_precomplete {{{1
@pytest.mark.parametrize("num_precomplete,raises", ((1, False), (0, True), (2, True)))
def test_run_generate_precomplete(tmp_path, num_precomplete, raises, mocker):
mocker.patch.object(autograph, "generate_precomplete", new=noop_sync)
def test_update_precomplete(tmp_path, num_precomplete, raises, mocker):
work_dir = tmp_path / "work"
config = {"artifact_dir": tmp_path / "artifacts"}
for i in range(0, num_precomplete):
path = os.path.join(work_dir, "foo", str(i))
makedirs(path)
with open(os.path.join(path, "precomplete"), "w") as fh:
fh.write("blah")
fh.write('remove "blah"\n')
if raises:
with pytest.raises(IScriptError):
autograph._run_generate_precomplete(config, work_dir)
autograph._update_precomplete(config, work_dir)
else:
autograph._run_generate_precomplete(config, work_dir)
autograph._update_precomplete(config, work_dir)


# remove_extra_files {{{1
Expand Down
2 changes: 0 additions & 2 deletions iscript/tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,3 @@ addopts = -vv -s --color=yes

[coverage:run]
branch = true
omit =
src/iscript/createprecomplete.py
76 changes: 0 additions & 76 deletions signingscript/src/signingscript/createprecomplete.py
View file
Open in desktop

This file was deleted.

37 changes: 23 additions & 14 deletions signingscript/src/signingscript/sign.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
from winsign.crypto import load_pem_certs

from signingscript import task, utils
from signingscript.createprecomplete import generate_precomplete
from signingscript.exceptions import SigningScriptError
from signingscript.rcodesign import RCodesignError, rcodesign_notarize, rcodesign_notary_wait, rcodesign_staple

Expand Down Expand Up @@ -297,9 +296,9 @@ async def sign_widevine_zip(context, orig_path, fmt):
all_files.append(to)
await raise_future_exceptions(tasks)
remove_extra_files(tmp_dir, all_files)
# Regenerate the `precomplete` file, which is used for cleanup before
# Update the `precomplete` file, which is used for cleanup before
# applying a complete mar.
_run_generate_precomplete(context, tmp_dir)
_update_precomplete(context, tmp_dir)
await _create_zipfile(context, orig_path, all_files, mode="w", tmp_dir=tmp_dir)
return orig_path

Expand Down Expand Up @@ -354,9 +353,9 @@ async def sign_widevine_tar(context, orig_path, fmt):
all_files.append(to)
await raise_future_exceptions(tasks)
remove_extra_files(tmp_dir, all_files)
# Regenerate the `precomplete` file, which is used for cleanup before
# Update the `precomplete` file, which is used for cleanup before
# applying a complete mar.
_run_generate_precomplete(context, tmp_dir)
_update_precomplete(context, tmp_dir)
await _create_tarfile(context, orig_path, all_files, compression, tmp_dir=tmp_dir)
return orig_path

Expand Down Expand Up @@ -558,16 +557,26 @@ def _get_omnija_signing_files(file_list):
return files


# _run_generate_precomplete {{{1
def _run_generate_precomplete(context, tmp_dir):
# _update_precomplete {{{1
def _update_precomplete(context, tmp_dir):
"""Regenerate `precomplete` file with widevine sig paths for complete mar."""
log.info("Generating `precomplete` file...")
path = _ensure_one_precomplete(tmp_dir, "before")
with open(path, "r") as fh:
precomplete = _ensure_one_precomplete(tmp_dir)
with open(precomplete, "r") as fh:
before = fh.readlines()
generate_precomplete(os.path.dirname(path))
path = _ensure_one_precomplete(tmp_dir, "after")
with open(path, "r") as fh:
with open(precomplete, "w") as fh:
for line in before:
fh.write(line)
instr, path = line.strip().split(None, 1)
if instr != "remove":
continue
if not path or path[0] != '"' or path[-1] != '"':
continue
file = path[1:-1]
if _get_widevine_signing_files([file]):
sigfile = _get_mac_sigpath(file)
fh.write('remove "{}"\n'.format(sigfile))
Copy link
Contributor Author

@jcristau jcristau May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm now worrying that this could add a duplicate remove instruction if the file was already signed in the input archive.

[EDIT: s/not/now/ sigh]

with open(precomplete, "r") as fh:
after = fh.readlines()
# Create diff file
diff_path = os.path.join(context.config["work_dir"], "precomplete.diff")
Expand All @@ -578,14 +587,14 @@ def _run_generate_precomplete(context, tmp_dir):


# _ensure_one_precomplete {{{1
def _ensure_one_precomplete(tmp_dir, adj):
def _ensure_one_precomplete(tmp_dir):
"""Ensure we only have one `precomplete` file in `tmp_dir`."""
return get_single_item_from_sequence(
glob.glob(os.path.join(tmp_dir, "**", "precomplete"), recursive=True),
condition=lambda _: True,
ErrorClass=SigningScriptError,
no_item_error_message='No `precomplete` file found in "{}"'.format(tmp_dir),
too_many_item_error_message='More than one `precomplete` file {} in "{}"'.format(adj, tmp_dir),
too_many_item_error_message='More than one `precomplete` file in "{}"'.format(tmp_dir),
)


Expand Down
14 changes: 6 additions & 8 deletions signingscript/tests/test_sign.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,9 @@ def fake_isfile(path):
mocker.patch.object(sign, "sign_file", new=noop_async)
mocker.patch.object(sign, "sign_widevine_with_autograph", new=noop_async)
mocker.patch.object(sign, "makedirs", new=noop_sync)
mocker.patch.object(sign, "generate_precomplete", new=noop_sync)
mocker.patch.object(sign, "_update_precomplete", new=noop_sync)
mocker.patch.object(sign, "_create_tarfile", new=noop_async)
mocker.patch.object(sign, "_create_zipfile", new=noop_async)
mocker.patch.object(sign, "_run_generate_precomplete", new=noop_sync)
mocker.patch.object(os.path, "isfile", new=fake_isfile)

if raises:
Expand Down Expand Up @@ -576,21 +575,20 @@ def test_get_widevine_signing_files(filenames, expected):
assert sign._get_widevine_signing_files(filenames) == expected


# _run_generate_precomplete {{{1
# _update_precomplete {{{1
@pytest.mark.parametrize("num_precomplete,raises", ((1, False), (0, True), (2, True)))
def test_run_generate_precomplete(context, num_precomplete, raises, mocker):
mocker.patch.object(sign, "generate_precomplete", new=noop_sync)
def test_update_precomplete(context, num_precomplete, raises, mocker):
work_dir = context.config["work_dir"]
for i in range(0, num_precomplete):
path = os.path.join(work_dir, "foo", str(i))
makedirs(path)
with open(os.path.join(path, "precomplete"), "w") as fh:
fh.write("blah")
fh.write('remove "blah"\n')
if raises:
with pytest.raises(SigningScriptError):
sign._run_generate_precomplete(context, work_dir)
sign._update_precomplete(context, work_dir)
else:
sign._run_generate_precomplete(context, work_dir)
sign._update_precomplete(context, work_dir)


# remove_extra_files {{{1
Expand Down
2 changes: 1 addition & 1 deletion signingscript/tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ addopts = -vv --color=yes

[coverage:run]
branch = True
omit = tests/*,src/signingscript/createprecomplete.py,src/signingscript/vendored/*
omit = tests/*,src/signingscript/vendored/*