Code review comment for lp:~nskaggs/juju-release-tools/add-daily-build

Revision history for this message
Curtis Hovey (sinzui) wrote :

Your change looks good, but I believe something is incomplete. Looking at
    http://juju-ci.vapour.ws:8080/job/release-juju-create-source-packages-daily/
I see a orig.tar.gz that is identical to what we have uploaded before:
    juju-core_2.0-beta6.orig.tar.gz

The orig.tar.gz name is identical to the official package we released at
     http://juju-ci.vapour.ws:8080/job/release-juju-create-source-packages/

Lp never forgets a tarfile. A new orig file was not uploaded. Instead, Lp accepted the tarfile because its hash is identical to the one previously uploaded. Anything tarfile that claims to have the same name, btu a different hash will be rejected because it could be a man in the middle attack. When release-juju-create-source-packages-daily creates juju-core_2.0-beta6.orig.tar.gz, the next version (juju-core_2.0-beta7.orig.tar.gz) we will never be able to release 2.0-beta7.

I think a single rule in build_source() will suffice to avoid this problem. Before we create the SourceFile, we get the version from the tarfile. We can rename the tarfile. Maybe we can change these lines

    tarfile_name = os.path.basename(tarfile_path)
    version = tarfile_name.split('_')[-1].replace('.tar.gz', '')

to something like:

    tarfile_name = os.path.basename(tarfile_path)
    version = tarfile_name.split('_')[-1].replace('.tar.gz', '')
    if revid:
        daily_version = {}~{}'.format(version, revid)
        daily_tarfile_name = tarfile_name.replace(version, daily_version)
        tarfile_dir = os.path.dirname(tarfile_path)
        daily_tarfile_path = os.path.join(tarfile_dir, daily_tarfile_name)
        os.rename(tarfile_path, daily_tarfile_path)
        tarfile_path = daily_tarfile_path

^ I typed that really fast. It probably has errors. Maybe we want the orig to have the date and build id too. Maybe this should be extracted to a helper function that is easy to test.

    tarfile_name = os.path.basename(tarfile_path)
    version = tarfile_name.split('_')[-1].replace('.tar.gz', '')
    if revid:
        tarfile_path = rename_daily_tarfile(tarfile_path, version, revid)

review: Needs Fixing (code)

« Back to merge proposal