From: kruceter <kruceter@users.noreply.github.com>
To: ml@inbox.vuxu.org
Subject: Re: [PR REVIEW] cmt: update to 1.18 and make it buildable
Date: Sat, 07 Jan 2023 22:49:51 +0100 [thread overview]
Message-ID: <20230107214951.TmtBrmTM4COYVmxcPkfAJz4MUwwrfyoo0OY_9S6E2OI@z> (raw)
In-Reply-To: <gh-mailinglist-notifications-41a7ca26-5023-4802-975b-f1789d68868e-void-packages-39583@inbox.vuxu.org>
[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]
New review comment by kruceter on void-packages repository
https://github.com/void-linux/void-packages/pull/39583#discussion_r1064050817
Comment:
My apologies for the delay.
> I don't think sed would even be necessary when make_use_env wouldn't be set.
cmt's makefile defines CFLAGS and CXXFLAGS with `=` which completely overrides external variables as far as I can tell.
Here is what the makefile contains:
```
CFLAGS = $(INCLUDES) -Wall -Werror -O2 -fPIC
CXXFLAGS = $(CFLAGS)
PLUGIN_LIB = ../plugins/cmt.so
```
> But both of these approaches have a flaw: they remove the `-fPIC` flag from `CFLAGS`.
That would be my fault for overlooking this.
Using sed to stuff it with void's flags just because `-fPIC` is present there looks overcomplicated and unnecessary.
In this case I can suggest the following (I forgot `vsed` must be used instead of `sed`):
```diff
diff --git a/srcpkgs/cmt/template b/srcpkgs/cmt/template
index a07ca8a069..6c676b62b9 100644
--- a/srcpkgs/cmt/template
+++ b/srcpkgs/cmt/template
@@ -14,9 +14,9 @@ distfiles="https://www.ladspa.org/download/${pkgname}_${version}.tgz"
checksum=a82f8636de1f4ada386a199a017a9cd775a49b49e716b11e8dd3f723c93df6ca
post_extract() {
- sed -e "/^CFLAGS/ s/-O2/${CFLAGS}/" \
+ vsed -e "s,^\(CFLAGS.*\)=,\1+=," \
-e 's|-Werror||g' \
- -i "${wrksrc}/src/Makefile"
+ -i "${build_wrksrc}/Makefile"
}
do_install() {
```
`=` is substituted with `+=`, thus making externally defined `CFLAGS` and `CXXFLAGS` usable.
`make_use_env` now has its use here unlike earlier:
```markdown
This build style tries to compensate for makefiles
that do not respect environment variables, so well written makefiles, those
that do such things as append (`+=`) to variables, should have `make_use_env`
set in the body of the template.
```
With this approach only two lines have to be changed without meddling with upstream flags.
> `-fPIC` could be added to `make_build_args`
Based on my previous answer (I overlooked this flag for reasons unknown), I believe it should be left in the makefile since the fix above allows to use void's flags with those in the makefile together without that ugly hack.
next prev parent reply other threads:[~2023-01-07 21:49 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-01 13:27 [PR PATCH] " meator
2022-12-31 1:55 ` github-actions
2023-01-02 7:15 ` [PR REVIEW] " kruceter
2023-01-02 7:15 ` kruceter
2023-01-02 7:15 ` kruceter
2023-01-03 13:08 ` [PR PATCH] [Updated] " meator
2023-01-03 13:09 ` meator
2023-01-03 13:19 ` [PR REVIEW] " meator
2023-01-03 13:20 ` meator
2023-01-03 13:21 ` meator
2023-01-07 21:49 ` kruceter [this message]
2023-01-07 22:18 ` kruceter
2023-01-10 15:00 ` meator
2023-01-10 15:23 ` kruceter
2023-04-11 1:51 ` github-actions
2023-04-25 1:53 ` [PR PATCH] [Closed]: " github-actions
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230107214951.TmtBrmTM4COYVmxcPkfAJz4MUwwrfyoo0OY_9S6E2OI@z \
--to=kruceter@users.noreply.github.com \
--cc=ml@inbox.vuxu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).