Closed bencord0 closed 4 years ago
Should lal update
just fetch from the current environment, which is the behaviour when you do specify the version?
What else explodes if you don't have components for all supported environments?
The panic is definitely bad error handling. It should probably bail with an error message if the component does not exist in the environment you are in.
We only loosely assume that you have components built in the same environment. You're not meant to mix and match. It should be perfectly fine to have disjoint dependency trees used in separate projects.
What are your thoughts if I just ignore supportedEnvironments
when update()
ing?
https://github.com/lalbuild/lal/blob/master/src/update.rs#L57
I'm thinking we can just check against the current environment, and not bother checking the other environments.
We should still gracefully error if the current environment doesn't have the dependency published.
I can hack a workaround for now...
@@ -53,11 +55,15 @@ pub fn update<T: CachedBackend + ?Sized>(
// First, since this potentially goes in the manifest
// make sure the version is found for all supported environments:
- let ver = backend
- .get_latest_supported_versions(comp, manifest.supportedEnvironments.clone())?
+ let supported_versions = backend
+ .get_latest_supported_versions(comp, vec![env.to_string()])?;
+
+ let ver = supported_versions
.into_iter()
.max()
$ cargo run --manifest-path ../../Cargo.toml update panic1
Compiling lal v3.9.0 (/home/bencord0/src/lal)
Finished dev [unoptimized + debuginfo] target(s) in 4.53s
Running `/home/bencord0/src/lal/target/debug/lal update panic1`
lal::update: Fetch default panic1
lal::storage::download: Last versions for panic1 in default env is {1}
lal::update: Fetch default panic1=1
lal::storage::local: get_component_info: panic1 Some(1) default
Otherwise, I think I can rework this entire section to be a bit simpler if update
doesn't have to concern itself with other environments.
lal update
panics if the backend does not have an artifact for allsupportedEnvironments
.note: using LocalBackend. also, it's fine if you specify the version of component.
Steps to reproduce:
lal update now panics
It's fine if you specify the version
Fetching from the unpublished environment also panics (as expected).
Publishing to both environments (listed in panic2's supportedEnvironments), resolves the issue.