Differential D5988 Diff 22783 swh/loader/cvs/loader.py

Changeset View

Standalone View

View Options

swh/loader/cvs/loader.py

This file was added.

# See the AUTHORS file at the top-level directory of this distribution

# License: GNU General Public License version 3, or any later version

# See top-level LICENSE file for more information

"""Loader in charge of injecting either new or existing cvs repositories to

swh-storage.

"""

from datetime import datetime

from mmap import ACCESS_WRITE, mmap

import os

import pty

import re

import shutil

from subprocess import Popen

import tempfile

from typing import Dict, Iterator, List, Optional, Tuple

from swh.loader.core.loader import BaseLoader

from swh.loader.core.utils import clean_dangling_folders

from swh.loader.exception import NotFound

from swh.loader.cvs.cvs import CvsRepo

from swh.model import from_disk, hashutil

from swh.model.model import (

Content,

Directory,

Origin,

Revision,

SkippedContent,

Snapshot,

SnapshotBranch,

TargetType,

)

from swh.storage.algos.snapshot import snapshot_get_latest

from swh.storage.interface import StorageInterface

DEFAULT_BRANCH = b"HEAD"

TEMPORARY_DIR_PREFIX_PATTERN = "swh.loader.cvs."

class CvsLoader(BaseLoader):

"""Swh cvs loader.

The repository is local. The loader deals with

update on an already previously loaded repository.

"""

visit_type = "cvs"

def __init__(

self,

storage: StorageInterface,

url: str,

origin_url: Optional[str] = None,

visit_date: Optional[datetime] = None,

destination_path: Optional[str] = None,

swh_revision: Optional[str] = None,

start_from_scratch: bool = False,

temp_directory: str = "/tmp",

debug: bool = False,

check_revision: int = 0,

max_content_size: Optional[int] = None,

super().__init__(

storage=storage,

logging_class="swh.loader.cvs.CvsLoader",

max_content_size=max_content_size,

)

self.cvsroot_url = url

# origin url as unique identifier for origin in swh archive

self.origin_url = origin_url if origin_url else self.cvsroot_url

self.debug = debug

self.temp_directory = temp_directory

self.done = False

self.cvsrepo = None

# Revision check is configurable

self.check_revision = check_revision

# internal state used to store swh objects

self._contents: List[Content] = []

self._skipped_contents: List[SkippedContent] = []

self._directories: List[Directory] = []

self._revisions: List[Revision] = []

self._snapshot: Optional[Snapshot] = None

# internal state, current visit

self._last_revision = None

self._visit_status = "full"

self.visit_date = visit_date

self.destination_path = destination_path

self.start_from_scratch = start_from_scratch

self.snapshot = None

# state from previous visit

self.latest_snapshot = None

self.latest_revision = None

def load_status(self):

return {

"status": self._load_status,

}

def visit_status(self):

return self._visit_status

vlorentzUnsubmitted

Done

if self._last_revision:

- parents = tuple([bytes(self._last_revision.id)])

+ parents = (self._last_revision.id,)

else:

no need to convert to bytes, it's already of that type. and (x,) is more idiomatic than tuple([x])

vlorentz: no need to convert to bytes, it's already of that type. and `(x,)` is more idiomatic than…

vlorentzUnsubmitted

Not Done

Could you inline this in compute_swh_revision? It's only used there AFAICT

vlorentz: Could you inline this in `compute_swh_revision`? It's only used there AFAICT

vlorentzUnsubmitted

Done

name and docstring doesn't seem to match the actual code (it yields Revision objects too)

vlorentz: name and docstring doesn't seem to match the actual code (it yields Revision objects too)

vlorentzUnsubmitted

Done

dirname = os.path.dirname(wtpath)

- try:

- os.makedirs(dirname)

- except FileExistsError:

- pass

+ os.makedirs(dirname, exist_ok=True)

self.log.debug("checkout to %s\n" % wtpath)

vlorentz:

vlorentzUnsubmitted

Not Done

Can you do this directly in swh_hash_data_per_cvs_changeset? This way swh_hash_data_per_cvs_changeset returns nice structured types, instead of dicts; and we can remove one function

vlorentz: Can you do this directly in `swh_hash_data_per_cvs_changeset`? This way…

vlorentzUnsubmitted

Not Done

ditto

vlorentz: ditto

vlorentzUnsubmitted

Not Done

if f[-2:] == ",v":

- try:

- rcsfile = rcsparse.rcsfile(filepath) # noqa: F841

- except (Exception):

- raise

- else:

- self.log.debug(

- "Looks like we have data to convert; "

- "found a valid RCS file at %s" % filepath

- )

- have_rcsfile = True

- break

+ rcsfile = rcsparse.rcsfile(filepath) # noqa: F841

+ self.log.debug(

+ "Looks like we have data to convert; "

+ "found a valid RCS file at %s" % filepath

+ )

+ have_rcsfile = True

+ break

if have_rcsfile:

vlorentz:

vlorentzUnsubmitted

Not Done

you missed this comment

vlorentz: you missed this comment

vlorentzUnsubmitted

Not Done

nothing to do for the other schemes?

vlorentz: nothing to do for the other schemes?

vlorentzUnsubmitted

Not Done

I think you can remove this part of the docstring; it's redundant with the type annotation

vlorentz: I think you can remove this part of the docstring; it's redundant with the type annotation

vlorentzUnsubmitted

Not Done

can you use longer variable names?

vlorentz: can you use longer variable names?

vlorentzUnsubmitted

Not Done

wtpath = os.path.join(self.worktree_path, path)

- self.log.info("rev %s of file %s" % (f.rev, f.path))

+ self.log.debug("rev %s of file %s" % (f.rev, f.path))

if not logmsg:

this is going to be noisy otherwise

vlorentz: this is going to be noisy otherwise

vlorentzUnsubmitted

Not Done

contents = rcs.expand_keyword(f.path, rcsfile, f.rev)

- try:

- outfile = open(wtpath, mode="wb")

- except FileNotFoundError:

- os.makedirs(os.path.dirname(wtpath))

- outfile = open(wtpath, mode="wb")

+ os.makedirs(os.path.dirname(wtpath), exist_ok=True)

+ outfile = open(wtpath, mode="wb")

outfile.write(contents)

this should work too

vlorentz: this should work too

vlorentzUnsubmitted

Not Done

outfile.close()

- (_revision, swh_dir) = self.compute_swh_revision(k, logmsg)

- (_contents, _skipped_contents, _directories) = from_disk.iter_directory(

+ (revision_, swh_dir) = self.compute_swh_revision(k, logmsg)

+ (contents_, skipped_contents_, directories_) = from_disk.iter_directory(

swh_dir

)

- yield _contents, _skipped_contents, _directories, _revision

+ yield contents_, skipped_contents_, directories_, revision_

def process_cvs_rlog_changesets(

PEP8 recommends a trailing underscore: https://www.python.org/dev/peps/pep-0008/#descriptive-naming-styles

vlorentz: PEP8 recommends a trailing underscore: https://www.python.org/dev/peps/pep-0008/#descriptive…

vlorentzUnsubmitted

Not Done

Are all names guaranteed to be ascii?

vlorentz: Are all names guaranteed to be ascii?

vlorentzUnsubmitted

Not Done

except Exception as e:

- self.log.exception(e)

+ self.log.exception("Error while fetching data")

return False # Stopping iteration

the exception and its traceback are automatically added by self.log.exception to the message

vlorentz: the exception and its traceback are automatically added by `self.log.exception` to the message

vlorentzUnsubmitted

Not Done

if not have_cvsroot:

self.log.warn(

"The CVS repository at '%s' lacks a CVSROOT directory; "

- "we might be ingesting an incomplete copy of the repository"

- % self.cvsroot_path

+ "we might be ingesting an incomplete copy of the repository",

+ self.cvsroot_path

)

# Unfortunately, there is no way to convert CVS history in an

vlorentz: