Skip to content

fix(slugbuilder): Quotes on variable causes build detect to break#23

Merged
Cryptophobia merged 1 commit into
teamhephy:masterfrom
n0n0x:fix-build-detection
Dec 11, 2020
Merged

fix(slugbuilder): Quotes on variable causes build detect to break#23
Cryptophobia merged 1 commit into
teamhephy:masterfrom
n0n0x:fix-build-detection

Conversation

@n0n0x
Copy link
Copy Markdown
Contributor

@n0n0x n0n0x commented Dec 10, 2020

$ git push deis master
Enumerating objects: 964, done.
Counting objects: 100% (964/964), done.
Delta compression using up to 8 threads
Compressing objects: 100% (601/601), done.
Writing objects: 100% (964/964), 177.31 KiB | 10.43 MiB/s, done.
Total 964 (delta 576), reused 573 (delta 340), pack-reused 0
remote: Resolving deltas: 100% (576/576), done.
Starting build... but first, coffee!
...
...
-----> Restoring cache...
       No cache file found. If this is the first deploy, it will be created now.
/builder/build.sh: line 162: /tmp/buildpacks/*/bin/detect: No such file or directory
-----> Unable to select a buildpack
slug@78a0f0b805eb:~$ buildpacks=($buildpack_root/*)
slug@78a0f0b805eb:~$ for buildpack in "${buildpacks[@]}"; do   echo "- $buildpack"; done
- /tmp/buildpacks/01-multi
- /tmp/buildpacks/02-clojure
- /tmp/buildpacks/03-go
- /tmp/buildpacks/04-gradle
- /tmp/buildpacks/05-grails
- /tmp/buildpacks/06-java
- /tmp/buildpacks/07-nodejs
- /tmp/buildpacks/08-php
- /tmp/buildpacks/09-play
- /tmp/buildpacks/10-python
- /tmp/buildpacks/11-ruby
- /tmp/buildpacks/12-scala


slug@78a0f0b805eb:~$ buildpacks=("$buildpack_root/*")
slug@78a0f0b805eb:~$ for buildpack in "${buildpacks[@]}"; do   echo "- $buildpack"; done
- /tmp/buildpacks/*
slug@78a0f0b805eb:~$

@Cryptophobia Cryptophobia self-requested a review December 11, 2020 03:05
Copy link
Copy Markdown
Member

@Cryptophobia Cryptophobia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like build broke because of quotes missing: https://travis-ci.org/github/teamhephy/slugbuilder/builds/748734755

Can you make it pass without quotes? Can you try like this:

buildpacks=(${buildpack_root}/*)

Comment thread rootfs/builder/build.sh Outdated
## Buildpack detection

buildpacks=("$buildpack_root/*")
buildpacks=($buildpack_root/*)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see if the linting will pass with this:

buildpacks=(${buildpack_root}/*)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to this: https://www.shellcheck.net/ it won't pass:

image

but we can ignore this check by adding a comment:

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, perfect @n0n0x ! Can you add the comment and rerun the CI and will merge as soon as it passes! 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed!

Copy link
Copy Markdown
Member

@Cryptophobia Cryptophobia Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TY @n0n0x for this fix! Just waiting for CI to pass before merging this.

Going to include this in the next release. It has been buggy for users.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @n0n0x , build failed. I think due to misspelling spellcheck instead of shellcheck of the comment. Can you please fix real quick when you can? :)

@n0n0x n0n0x force-pushed the fix-build-detection branch from 9f7392a to c0dfaf5 Compare December 11, 2020 18:09
Comment thread rootfs/builder/build.sh Outdated
## Buildpack detection

buildpacks=("$buildpack_root/*")
# spellcheck disable=SC2206
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be # shellcheck disable=SC2206

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, sorry! pushed!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! We have a GREEN build. :)

@n0n0x n0n0x force-pushed the fix-build-detection branch from c0dfaf5 to e0a2ae9 Compare December 11, 2020 20:24
@Cryptophobia Cryptophobia merged commit a01e637 into teamhephy:master Dec 11, 2020
@kingdonb
Copy link
Copy Markdown
Member

A report came through #support in slack today that build detection in the master branch is still not working, I have not been able to confirm this myself but I wanted to note it here. I added an item to stepsize so we don't lose track of it.

[stepsize] Sounds like build detection is still borked after slugbuilder#23 merged

@kingdonb
Copy link
Copy Markdown
Member

#24 has the rest of the fix

@n0n0x n0n0x changed the title fix(build.sh): Quotes on variable causes build detect to break fix(slugbuilder): Quotes on variable causes build detect to break Dec 19, 2020
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

Successfully merging this pull request may close these issues.

3 participants