Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken for 0.23.1 #23

Closed
pedantic-git opened this issue Jul 14, 2017 · 9 comments
Closed

Broken for 0.23.1 #23

pedantic-git opened this issue Jul 14, 2017 · 9 comments

Comments

@pedantic-git
Copy link

Since the release of 0.23.1 I'm getting this from the buildpack:

remote: -----> Crystal app detected
remote: -----> Installing Crystal (0.23.1 due to latest release at https://github.com/crystal-lang/crystal)... 
remote: gzip: stdin: not in gzip format
remote: tar: Child returned status 1
remote: tar: Error is not recoverable: exiting now
remote: done
remote: -----> Installing Dependencies... /app/tmp/buildpacks/7cae2d464297fe40ab4f8baf661b5905138e7a4679faeb1e9b99b4ed6b787bc446f63a669c7a918dc21952d53acb6cac9426f13ed74d85acdccff4c0dc00b0f6/bin/compile: line 65: crystal: command not found

Something changed about the upstream filenames, perhaps?

@straight-shoota
Copy link
Member

This URL assumes package version 0.23.0-1 but for this release it is acually 0.23.1-3.
CRYSTAL_URL="https://github.com/crystal-lang/crystal/releases/download/$CRYSTAL_VERSION/crystal-$CRYSTAL_VERSION-1-linux-x86_64.tar.gz"

@ktaragorn
Copy link

ktaragorn commented Jul 17, 2017

Just ran into this.. am working around it by using 0.23.0 for now, by creating a file .crystal-version with that version in it.

@mverzilli
Copy link

We can either extract the correct tar.gz name from the response of curl https://api.github.com/repos/crystal-lang/crystal/releases/latest or change our naming policy so that the tar.gz name is a function of the version (that means either dropping the -1, -2, -3 suffixes so we end up with CRYSTAL_URL="https://github.com/crystal-lang/crystal/releases/download/$CRYSTAL_VERSION/crystal-$CRYSTAL_VERSION-linux-x86_64.tar.gz or adding them as a 4th component of the version so we end up with CRYSTAL_VERSION=0.23.1.3 in this case).

I'd vote to drop the -X suffixes in tar.gz names. @matiasgarciaisaia thoughts about going that way?

@matiasgarciaisaia
Copy link
Member

Hi there!

It's effectively because of the released tar.gz name. The published 0.23.1 was the third build - there was a first build published in one of the issues, a second build shared internally that was plain wrong, and this last one that worked.

I'm still not sure about The Way™ to handle this. If it's not that much work to simply extract the name from the URL, I'd love to do that for now.

We have started discussing distribution channels (some beta channel vs a stable one) that will imply betas, release candidates, etc, so I think we should first define that, and then choose between a build version (like the -1, -X we have now) and some other suffix.

But I think a suffix will be there for a while, at least.

Any other thoughts?

@bcardiff
Copy link
Member

To unblock this we can create a fallback from -1 to -5 in case a 404 is reached

@mverzilli
Copy link

The info is there, I just was wondering if it's worth having many builds per version, given the only reason that happens is because of packaging errors.

Anyway, there's an "assets" collection in the json response that holds the exact name of each file we distribute. We can iterate that and look for linux-x86_64.tar.gz suffix, and we'd be pretty sure that's the right file. I had started to work on this but ran out of time and won't be in front of a computer for two weeks. Just sharing the "research" in case someone wants to give it a try before!

@pedantic-git
Copy link
Author

Something I neglected to mention in the original bug report is that this error took my app down, because the buildpack didn't fail after the 404 - it tried to deploy the app without a compiler installed.

Whatever the solution to the above, it's probably worth making sure that future filename changes / 404s don't take apps down with them.

matiasgarciaisaia added a commit to matiasgarciaisaia/heroku-buildpack-crystal that referenced this issue Aug 7, 2017
matiasgarciaisaia added a commit to matiasgarciaisaia/heroku-buildpack-crystal that referenced this issue Aug 7, 2017
matiasgarciaisaia added a commit to matiasgarciaisaia/heroku-buildpack-crystal that referenced this issue Aug 7, 2017
matiasgarciaisaia added a commit to matiasgarciaisaia/heroku-buildpack-crystal that referenced this issue Aug 7, 2017
@matiasgarciaisaia
Copy link
Member

Please review #24 so we fix this 👍

bcardiff pushed a commit that referenced this issue Aug 8, 2017
* Discover tar.gz asset URL

See #23

* Build fails on error

See #23
@pedantic-git
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants