search

digital-asset / daml

The Daml smart contract language
https://www.digitalasset.com/developers
797 stars 199 forks source link

`daml install` fails if there exists a version it cannot parse on GitHub, even when trying to install one that does parse #18714

Open akrmn opened 6 months ago

akrmn commented 6 months ago

Found by @garyverhaegen-da

I'm observing surprising behaviour:

$ curl https://get.daml.com/ | sh -s v2.8.0
[...]
daml: Snapshot versions list from https://api.github.com/repos/digital-asset/daml/releases does not contain valid JSON
  context: Installing extracted SDK release.
  details: Error in $: Couldn't find Linux SDK in release version: 'v3.0.0-snapshot.20240311.0'

I've been scratching my head about it for a while - it seems unlikely get.daml.com has been broken since it was last modified on Feb 7 and no one noticed - but now I think I have a working theory: this is working up until the point where we have the daml assistant, and it tries to build up its list of versions to get the mapping, and it cannot parse the brand new version I just published because the name of the linux binaries have changed.

Indeed, this function in daml-assistant expects the suffix -linux.tar.gz in at least one of the assets listed on github and fails otherwise

https://github.com/digital-asset/daml/blob/540a746043ec2fd84b41fb5b9a521ac6ea96bf51/daml-assistant/src/DA/Daml/Assistant/Version.hs#L469-L476

It's a simple fix to extend this to support suffixes that include the processor architecture, which would fix the issue with get.daml.com, but I'm not sure if daml install <sdk> will work for existing installations of daml

garyverhaegen-da commented 6 months ago

From my perspective, it's expected that current assistants don't know about the platform suffix, but it's a bug that they crash. The existence of a "bad" release should not prevent people from installing "good" ones.

So while I do want us to move towards supporting platform versions, I think the most urgent fix would be to change that code to ignore/skip versions that don't have the expected artifact(s), rather than die when one exists.

garyverhaegen-da commented 6 months ago

To clarify: the bug I wanted to raise is "The daml install command fails if there exists a version it cannot parse on GitHub, even when trying to install one that does parse."

get.daml.com is incidental here.

moisesackerman-da commented 4 months ago

Update: this bug doesn't currently occur because 1. Gary removed the "bad" releases (those where none of the artifacts matched the expected patterns) and 2. ensured that all releases include both the expected patterns and the newer patterns with processor architectures. If we ever stop producing the artifacts with the "plain" patterns (those that don't mention processor architecture) we will see this bug again, so, as Gary stated,

the most urgent fix would be to change that code to ignore/skip versions that don't have the expected artifact(s), rather than die when one exists.