Skip to content

Commit

Permalink
osbuild: drop libdir from download() methods
Browse files Browse the repository at this point in the history
The libdir is passed down for sources but it is never used in
any of our sources. As this is confusing and we want to eventually
support multiple libdirs remove this code.

It looks like the libdir for soruces was added a long time ago in 8423da3
but there is no indication if/how it is/was supposed to get used and
AFACT from going over the git history it was very used.

SourceService:dispatch() never sends "libdir" to the actual sources,
so it is not an even technically an API break.
  • Loading branch information
mvo5 committed Aug 26, 2024
1 parent a2fed3e commit 3379e05
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 12 deletions.
2 changes: 1 addition & 1 deletion osbuild/main_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def osbuild_cli() -> int:
monitor = osbuild.monitor.make(monitor_name, args.monitor_fd, total_steps)
monitor.log(f"starting {args.manifest_path}", origin="osbuild.main_cli")

manifest.download(object_store, monitor, args.libdir)
manifest.download(object_store, monitor)

r = manifest.build(
object_store,
Expand Down
4 changes: 2 additions & 2 deletions osbuild/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,13 @@ def add_source(self, info, items: List, options: Dict) -> Source:
self.sources.append(source)
return source

def download(self, store, monitor, libdir):
def download(self, store, monitor):
with host.ServiceManager(monitor=monitor) as mgr:
for source in self.sources:
# Workaround for lack of progress from sources, this
# will need to be reworked later.
monitor.begin(source)
source.download(mgr, store, libdir)
source.download(mgr, store)
monitor.finish({"name": source.info.name})

def depsolve(self, store: ObjectStore, targets: Iterable[str]) -> List[str]:
Expand Down
4 changes: 1 addition & 3 deletions osbuild/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

from . import host
from .objectstore import ObjectStore
from .util.types import PathLike


class Source:
Expand All @@ -25,7 +24,7 @@ def __init__(self, info, items, options) -> None:
self.runner = None
self.source_epoch = None

def download(self, mgr: host.ServiceManager, store: ObjectStore, libdir: PathLike):
def download(self, mgr: host.ServiceManager, store: ObjectStore):
source = self.info.name
cache = os.path.join(store.store, "sources")

Expand All @@ -34,7 +33,6 @@ def download(self, mgr: host.ServiceManager, store: ObjectStore, libdir: PathLik
"cache": cache,
"output": None,
"checksums": [],
"libdir": os.fspath(libdir)
}

client = mgr.start(f"source/{source}", self.info.path)
Expand Down
12 changes: 6 additions & 6 deletions test/run/test_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ def make_test_cases():
yield source, case


def check_case(source, case, store, libdir):
def check_case(source, case_options, store):
with host.ServiceManager() as mgr:
expects = case["expects"]
expects = case_options["expects"]
if expects == "error":
with pytest.raises(host.RemoteError):
source.download(mgr, store, libdir)
source.download(mgr, store)
elif expects == "success":
source.download(mgr, store, libdir)
source.download(mgr, store)
else:
raise ValueError(f"invalid expectation: {expects}")

Expand All @@ -131,5 +131,5 @@ def test_sources(source, case, tmp_path):

with osbuild.objectstore.ObjectStore(tmp_path) as store, \
fileServer(test.TestBase.locate_test_data()):
check_case(src, case_options, store, index.path)
check_case(src, case_options, store, index.path)
check_case(src, case_options, store)
check_case(src, case_options, store)

0 comments on commit 3379e05

Please sign in to comment.