standardml / smackage

Smackage Package Manager for Standard ML
Other
122 stars 12 forks source link

Incomplete semantic version produces misleading warning message #31

Closed HarrisonGrodin closed 4 years ago

HarrisonGrodin commented 4 years ago

Given a project with an incomplete semantic version as a tag (say, v0.1), running smackage refresh produces the following error message (twice):

WARNING: When trying to pull source `package-name git package-uri', got the following error 
    "Fail: I/O error trying to access temporary file"
If this line is in sources.local, you may need to run
`smackage unsource' to remove it.

As far as I can tell, this is caused by a combination of src/get-git.sml and src/semver.sml. In src/get-git.sml, any Fail exception is propagated as Fail "I/O error trying to access temporary file": https://github.com/standardml/smackage/blob/eb383f0da0dcbed6c613ba9e569226a67460c120/src/get-git.sml#L32-L45

Inconveniently, fromString raises Fail given an incomplete semantic version: https://github.com/standardml/smackage/blob/eb383f0da0dcbed6c613ba9e569226a67460c120/src/semver.sml#L120-L124

Thus, rather than

WARNING: When trying to pull source `package-name git package-uri', got the following error 
    "Fail: `0.1` is an incomplete semantic version"
If this line is in sources.local, you may need to run
`smackage unsource' to remove it.

being mentioned, the misleading warning is produced.

It seems like this could be fixed by declaring a local exception to raise and handle rather than using Fail. If this sounds reasonable, I'd be more than happy to put together a PR!

gian commented 4 years ago

Hrm. I'm trying to remember what we intended here. It's been a long time. I feel like this wasn't quite intended to be an error, so if you think it's recoverable in this way, I'd be happy to review a PR, but I'm low confidence on everything here because of lack of recall.