Skip to content

Commit

Permalink
Take compatibility tags into account when comparing packages to upload (
Browse files Browse the repository at this point in the history
#17047)

* Fix upload decision logic to also take compatibility tags into account

* Make build numbers (timestamps) be always integers
  • Loading branch information
alopezz authored Mar 5, 2024
1 parent 29a81bc commit c07d085
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 17 deletions.
36 changes: 25 additions & 11 deletions .builders/tests/test_upload.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pathlib import Path
from unittest import mock
from zipfile import ZipFile
import fnmatch

import pytest

Expand All @@ -26,8 +27,13 @@ def fake_upload(wheel_name):
blob.metadata = files.get(name, None)
return blob

def list_blobs(*, prefix=''):
return [make_blob(f) for f in files if f.startswith(prefix)]
def list_blobs(*, prefix='', match_glob='*'):
# Note that this is a limited emulation of match_glob, as fnmatch doesn't treat '/' specially,
# but it should be good enough for what we use it for.
return (
make_blob(f) for f in files
if f.startswith(prefix) and fnmatch.fnmatch(Path(f).name, match_glob)
)

bucket = mock.Mock()
bucket.list_blobs.side_effect = list_blobs
Expand Down Expand Up @@ -85,7 +91,7 @@ def fake_hash(path: Path):

@pytest.fixture
def frozen_timestamp(monkeypatch):
timestamp = '20241327_17021709050439'
timestamp = 20241327_090504
monkeypatch.setattr(upload, 'timestamp_build_number', mock.Mock(return_value=timestamp))
return timestamp

Expand Down Expand Up @@ -207,7 +213,8 @@ def test_upload_built_existing_sha_match_does_not_upload_multiple_existing_build
setup_fake_bucket,
setup_fake_hash,
):
whl_hash = 'some-hash'
matching_hash = 'some-hash'
non_matching_hash = 'xxxx'

wheels = {
'built': [
Expand All @@ -217,15 +224,22 @@ def test_upload_built_existing_sha_match_does_not_upload_multiple_existing_build
targets_dir = setup_targets_dir(wheels)

bucket_files = {
'built/existing/existing-1.1.1-20241326_00000000000000-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': 'b'},
'built/existing/existing-1.1.1-20241327_00000000000000-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': whl_hash},
'built/existing/existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': non_matching_hash},
'built/existing/existing-1.1.1-20241326000000-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': non_matching_hash},
'built/existing/existing-1.1.1-20241327000000-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': matching_hash},
# The following two builds are for different platforms and should therefore not count
'built/existing/existing-1.1.1-2024132700001-cp311-cp311-manylinux2010_aarch64.whl':
{'requires-python': '', 'sha256': non_matching_hash},
'built/existing/existing-1.1.1-2024132700002-py27-py27mu-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': non_matching_hash},
}
bucket, uploads = setup_fake_bucket(bucket_files)

setup_fake_hash({
'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': whl_hash,
'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': matching_hash,
})

upload.upload(targets_dir)
Expand All @@ -250,9 +264,9 @@ def test_upload_built_existing_different_sha_does_upload_multiple_existing_build
targets_dir = setup_targets_dir(wheels)

bucket_files = {
'built/existing/existing-1.1.1-20241326_00000000000000-cp311-cp311-manylinux2010_x86_64.whl':
'built/existing/existing-1.1.1-2024132600000-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': 'b'},
'built/existing/existing-1.1.1-20241327_00000000000000-cp311-cp311-manylinux2010_x86_64.whl':
'built/existing/existing-1.1.1-2024132700000-cp311-cp311-manylinux2010_x86_64.whl':
{'requires-python': '', 'sha256': original_hash},
}
bucket, uploads = setup_fake_bucket(bucket_files)
Expand Down
22 changes: 16 additions & 6 deletions .builders/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import re
import time
from hashlib import sha256
from pathlib import Path
from pathlib import Path, PurePosixPath
from typing import TYPE_CHECKING, Iterator
from zipfile import ZipFile

Expand Down Expand Up @@ -58,9 +58,9 @@ def display_message_block(message: str) -> None:
print(divider)


def timestamp_build_number() -> str:
"""Produce a formatted timestamp to use as build numbers"""
return time.strftime('%Y%M%d_%H%m%s')
def timestamp_build_number() -> int:
"""Produce a numeric timestamp to use as build numbers"""
return int(time.strftime('%Y%M%d%H%m%s'))


def hash_file(path: Path) -> str:
Expand Down Expand Up @@ -88,6 +88,13 @@ def iter_wheel_dirs(targets_dir: str) -> Iterator[Path]:
yield entry


def _build_number_of_wheel_blob(wheel_path: Blob) -> int:
"""Extract the build number from a blob object representing a wheel."""
wheel_name = PurePosixPath(wheel_path.name).stem
_name, _version, *build_number, _python_tag, _abi_tag, _platform_tag = wheel_name.split('-')
return int(build_number[0]) if build_number else -1


def upload(targets_dir):
client = storage.Client()
bucket = client.bucket(BUCKET_NAME)
Expand Down Expand Up @@ -129,9 +136,12 @@ def upload(targets_dir):
else:
# https://packaging.python.org/en/latest/specifications/binary-distribution-format/#file-name-convention
name, version, python_tag, abi_tag, platform_tag = wheel.stem.split('-')
existing_wheels = list(bucket.list_blobs(prefix=f'{artifact_type}/{project_name}/{name}-{version}-'))
existing_wheels = list(bucket.list_blobs(
prefix=f'{artifact_type}/{project_name}/',
match_glob=f'{name}-{version}-*-{abi_tag}-{platform_tag}.whl',
))
if existing_wheels:
most_recent_wheel = max(existing_wheels, key=lambda b: b.name)
most_recent_wheel = max(existing_wheels, key=_build_number_of_wheel_blob)
# Don't upload if it's the same file
if most_recent_wheel.metadata['sha256'] == sha256_digest:
continue
Expand Down

0 comments on commit c07d085

Please sign in to comment.