-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Create build.sh #27037
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
base: master
Are you sure you want to change the base?
Create build.sh #27037
Conversation
|
We really gotta talk about using a rebase workflow for your PRs, this is what attempt number 4 at this now? Making a separate development branch for your PRs is also generally helpful, but that boat has sailed for this PR so I'll just mention it as a general suggestion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR seems close to actually being a merge-able build script for vlang.
If you need help with the workflow we can help you out with that, but if you're just gonna keep closing the old PR when you hit a problem and opening a new one then at some point me or @robertkirkman are probably just going to open a PR ourselves.
Opening multiple PRs isn't inherently an issue, but it makes reviewing and iterating on a build script much harder than it needs to be.
| TERMUX_PKG_VERSION=0.4.12 | ||
| TERMUX_PKG_SRCURL=https://github.com/vlang/v/archive/refs/tags/${TERMUX_PKG_VERSION}.tar.gz | ||
| TERMUX_PKG_SHA256=65e3579df61ae0a7314aa5f0c7eb3b0b9b45170a0275e392e9c168be23046e89 | ||
| TERMUX_PKG_DEPENDS="build-essential, clang, make, libgc-dev, libexecinfo-dev, libatomic1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These depends don't make any sense and don't match dependencies for any other package of vlang.
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=vlang-bin#n16
https://build.opensuse.org/projects/openSUSE:Factory/packages/vlang/files/vlang.spec
https://github.com/Homebrew/homebrew-core/blob/6c4e6736161b14b1345bc48990f8e56d4039fe42/Formula/v/vlang.rb#L23-L27
libgcseems to be correct, but we do not provide separate*-devpackages for headers, that's a Debian-ism.build-essential,clangandmakeare provided as base components of the build environment.- I have no idea where you got
libexecinfoorlibatomic1as dependencies from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even notice this until I looked at the CI checks, this file is not in the correct location.
It should be at packages/vlang/build.sh.
At which the CI is instantly going to throw a failure due to the non-existent *-dev, libatomic1 and libexecinfo dependencies.
Unfortunately, not quite yet - if you look closely, you can see that this PR creates a folder directly in the repository named " |
I wrote that review immediately after approving the CI runs, so I didn't see their status yet and then I was very surprised to see them "pass". |
No description provided.