archive: Add Info() method #69
Conversation
|
This PR depends on #80 |
woky
force-pushed
the
pub/archive-package-info
branch
from
95d7a17
to
107c2b7
Compare
woky
force-pushed
the
pub/archive-package-info
branch
2 times, most recently
from
4c66398
to
cbeac7f
Compare
woky
force-pushed
the
pub/archive-package-info
branch
2 times, most recently
from
e1c21ac
to
ddd61ef
Compare
woky
force-pushed
the
pub/archive-package-info
branch
8 times, most recently
from
da096d9
to
5bb81b4
Compare
|
This PR can should be reviewed only after #80 is deemed OK. |
Currently we only test with the embedded base-files files package. This quite limits what we can test. But since commit a3315e3 ("testutil/pkgdata: Create deb programatically") we have the ability to define our own custom package content. Add support for testing with custom packages. These can be defined in each test case per archive name. The content can be defined directly in test case definition with the help of testutil.MustMakeDeb(). The package content is wrapped in the testPackage structure. This introduces one level of nesting in each test case package definition. In future, we will add package metadata to the package definition, namely version. So wrapping the package content in the structure now and later just adding a new field to it the structure later will make the future diff much smaller.
At some point we will need to get information about (deb) packages from (Go) packages other than archive, namely from the slicer package. For instance: - Package entries in Chisel DB will include version, digest and architecture. - Multi-archive package selection will need to know versions of a package in all configured archives. Add Info() method to the Archive interface that returns a new PackageInfo interface that contains accessors for the underlying control.Section of the package. Currently these accessors are Name(), Version(), Arch() and SHA256() methods. The reason to introduce PackageInfo interface instead of returing control.Section directly is keep the archive abstraction and not leak implementation details.
woky
force-pushed
the
pub/archive-package-info
branch
from
5bb81b4
to
19c9af6
Compare
|
#80 is okay, FWIW.. just one trivial to solve there. |
| func (info pkgInfo) SHA256() string { return info.Get("SHA256") } | ||
|
|
||
| func (a *ubuntuArchive) Info(pkg string) PackageInfo { | ||
| if section, _, _ := a.selectPackage(pkg); section != nil { |
There was a problem hiding this comment.
Why drop errors on the floor?
There was a problem hiding this comment.
I just felt that the Info() method (and selectPackage() too) resembles a map read and since it's guaranteed to not hold nil values and will not fail in any expected way (as selectPackage only returns error when it doesn't find an entry in the map), it can just return nil on missing entry.
I understand that this is an interface and so it should be designed for unknown future changes to the implementation, but I assumed that Archive instances are expected to be fully loaded with archive metadata after creation. If e.g. we introduced some lazy loading in future, then even Exists() would have to return error in case lazy loading fails.
Anyway, after #80 gets merged, I'm going to update to return err from selectPackage. Python's dictionaries also fail on KeyError 🤷
|
Per our offline discussion, I think our best way forward is to close this PR and discuss with @rebornplusplus to see if it is a priority right now, and how to amend the code here to make it compatible with the latest changes. We will take that discussion offline and we can always re-open the PRs or create new ones as we see fit. |
At some point we will need to get information about (deb) packages from
(Go) packages other than archive, namely from the slicer package.
For instance:
architecture.
package in all configured archives.
Add Info() method to the Archive interface that returns a new
PackageInfo interface that contains accessors for the underlying
control.Section of the package. Currently these accessors are Name(),
Version(), Arch() and SHA256() methods.
The reason to introduce PackageInfo interface instead of returing
control.Section directly is keep the archive abstraction and not leak
implementation details.