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

Fix regression from "newapkbuild: remove obsolete cd statements " #75

Conversation

ollieparanoid
Copy link
Contributor

Since the obsolete 'cd "$builddir"' statements have been removed in f83d19c, build(), check() and package() can generate empty functions if no build system is specified or if there is no default for the given build system. newapkbuild will then fail, as it tries to parse the script it generated:

$ cd /home/pmos && newapkbuild test
/usr/bin/abuild: /home/pmos/test/APKBUILD: line 18: syntax error: unexpected "}"
$ cat test/APKBUILD
...
build() {
}
...

Fix this by placing "true" in functions that would be empty.

@ollieparanoid
Copy link
Contributor Author

@ncopa: getting this in before freezing abuild would be important.

@Cogitri
Copy link
Member

Cogitri commented Jun 6, 2019

Not that I know much shell, but wouldn't doing : instead of true work too?

Oliver Smith added 2 commits June 6, 2019 22:01
Since the obsolete 'cd "$builddir"' statements have been removed in [1],
build(), check() and package() can generate empty functions if no build
system is specified or if there is no default for the given build
system. newapkbuild will then fail, as it tries to parse the script it
generated:

$ cd /home/pmos && newapkbuild test
/usr/bin/abuild: /home/pmos/test/APKBUILD: line 18: syntax error: unexpected "}"
$ cat test/APKBUILD
...
build() {
}
...

Fix this by placing ":" in functions that would be empty.

[1]: f83d19c
Add a '# Check sections' comment, for consistency with the equally
commented build and package sections.
@ollieparanoid
Copy link
Contributor Author

Not that I know much shell, but wouldn't doing : instead of true work too?

Sure, updated to use :.

@ncopa, can you take a look at this regression fix?

Copy link
Contributor

@CosmicToast CosmicToast left a comment

Choose a reason for hiding this comment

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

LGTM

@algitbot
Copy link

Merged in 635a699, da4aca2 by @ncopa. Thanks for your contribution!

(This pull request has been closed automatically by GitHub PR Closer. If you think that it’s not resolved yet, please add a comment.)

@algitbot algitbot closed this Jun 12, 2019
z3ntu pushed a commit to z3ntu/pmbootstrap that referenced this pull request Jul 20, 2019
Half of this test is failing, because of a regression in newapkbuild.

This patch will fix it:
alpinelinux/abuild#75

Until then, let's disable this test, so our testsuite runs through
again. If somebody is curious on how I've developed and tested this
patch with pmbootstrap, I've added a guide to the wiki:
https://wiki.postmarketos.org/wiki/Hacking_on_abuild
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.

4 participants