Code review comment for ~smoser/curtin:feature/bug-1746348-fsimage-support

Revision history for this message
Ryan Harper (raharper) wrote :

The extract logic currently is like:

dd-
  continue

cp://
  copy_to_target

isfile:
  extract_root_tgz_file

file://
  extract_root_tgz_file

http[s]://
  extract_root_tgz_url

Can we just remove _tgz_ from the method name,
and add the squashfs handling in there? Something like

  extract_root_from_file
  extract_root_from_url

Then we could push some of the logic for source and format hanlding into those two functions.
Something like this:

    if source['type'].startswith('dd-'):
        continue
    if source['uri'].startswith("cp://"):
        copy_to_target(source['uri'], target)
    elif os.path.isfile(source['uri']) or source['uri'].startswith("file://"):
        extract_root_from_file(source['uri'], fmt=source['type'],
                                target=target)
    elif (source['uri'].startswith("http://") or
          source['uri'].startswith("https://")):
        extract_root_from_url(source['uri'], fmt=source['type'],
                              target=target)
    else:
        raise TypeError("do not know how to extract '%s'" % source['uri'])

def extract_root_from_file(source, fmt=None, target=None):
    if source.startswith('file://'):
        source = source[len('file://'):]
    if fmt == "fsimage":
        return extract_root_fsimage(source, target)
    else:
        curtin.util.subp(args=['tar', '-C', target] +
                         tar_xattr_opts() +
                         ['-Sxpzf', source, '--numeric-owner'])

def extract_root_from_url(source, fmt=None, target=None):
    if fmt == "fsimage":
        return extract_root_fsimage_url(source, target)
    else:
        curtin.util.subp(args=['sh', '-cf',
                           ('wget "$1" --progress=dot:mega -O - |'
                            'smtar -C "$2" ' + ' '.join(tar_xattr_opts()) +
                            ' ' + '-Sxpf - --numeric-owner'),
                           '--', source, target])

« Back to merge proposal