search

crystal-lang / shards

Dependency manager for the Crystal language
Other
763 stars 100 forks source link

Release 0.13.0 #467

Closed bcardiff closed 3 years ago

bcardiff commented 3 years ago

Since the Windows CI is experimental/WIP and no release for that will be shipped in this version I think we are ok with doing the release despite those failures.

oprypin commented 3 years ago

So wait, you merged a PR that broke Windows support and then shipped a release within 24 hours of that? I'm not very OK :/

Sija commented 3 years ago

Seems like #444 broke it /cc @straight-shoota

oprypin commented 3 years ago

~~Hm I just tried a trivial usage of Shards on Windows and it seems to work. https://github.com/oprypin/crystal-nonograms/runs/1745551030 Maybe it is just the CI environment that broke.~~

To be sure, I restarted CI runs before and after that commit. We will see.

before after

oprypin commented 3 years ago

Yes, #444 broke it.

oprypin commented 3 years ago

I would like to request another release after that is fixed. https://github.com/oprypin/install-crystal has been just using the latest commit always (EDIT: specifically on Windows) because there hasn't been a stable release to refer to. And well, now there still isn't.

Blacksmoke16 commented 3 years ago

Might be a good reason to check the 2nd box to ensure each PR is tested against latest code to prevent regressions.

image

bcardiff commented 3 years ago

Since it named 0.13.0 something ought to come wrong, I guess.

We can release 0.13.1 after this is fixed. But I don't think is a blocker issue to release 0.36. It was wanted to have a new shards release for 0.36 with the merged features.

Regarding the failure #444 in particular, it's odd. that file_spec are included in crystal's windows spec ... 🤔 . I see @straight-shoota is working on the fix/windows-touch branch so maybe 0.13.1 is close.

oprypin commented 3 years ago

Thanks. Yea it's not a blocker. I found that nobody's using Shards+Windows in CI at the moment, it seems, but yes, new usages would be prevented.

straight-shoota commented 3 years ago

Yeah it's really odd. Specs pass with this minimal patch:

https://github.com/crystal-lang/shards/compare/fix/windows-touch

That branch rescue ex: Shards::ParseError shouldn't even be reached in most of the failing examples. So it may just be related to STDERR being used somewhere in the code... but stdlib runtime uses STDERR as well, so no idea 🤷 There's still no obvious relation to #444 either.

oprypin commented 3 years ago

@straight-shoota No, the error is quite simple and is what I predicted. Can't set modification time of a directory on Windows.

Error setting time on file: 'D:\\foo\\lib': Access is denied. (File::AccessDeniedError)

https://github.com/oprypin/crab/runs/1753448315

straight-shoota commented 3 years ago

Hm, then the successful run with STDOUT change seems to be an error because it should fail.

oprypin commented 3 years ago

Wow, https://github.com/crystal-lang/crystal/pull/10284 fixed it altogether, don't even need to do anything here! :clap:

Someone with access to this repo can restart the build and it will pass this time

straight-shoota commented 3 years ago

Still have to wait for the nightly Crystal build.

oprypin commented 3 years ago

"nightly" is "commit"-ly for Windows, it already finished https://github.com/crystal-lang/crystal/runs/1757395736

oprypin commented 3 years ago

Aah it's beautiful https://github.com/oprypin/install-crystal/runs/1758696003#step:15:1