Bug 527 - misc fixes for dnsmasq package
Summary: misc fixes for dnsmasq package
Status: RESOLVED FIXED
Alias: None
Product: buildroot
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: PC Linux
: P5 normal
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-02 13:32 UTC by Daniele Salvatore Albano
Modified: 2009-09-01 21:09 UTC (History)
2 users (show)

See Also:
Host: Ubuntu 9.0.4 Server 32 Bit
Target: i686
Build: i686


Attachments
Fix dnsmasq compilation with dbus enabled (1.03 KB, patch)
2009-08-02 13:33 UTC, Daniele Salvatore Albano
Details
Fix dnsmasq dependencies if dbus is enabled (638 bytes, patch)
2009-08-02 13:34 UTC, Daniele Salvatore Albano
Details
dnsmasq doesn't take into account ldflags for target (681 bytes, patch)
2009-08-02 13:34 UTC, Daniele Salvatore Albano
Details
Replaces spaces in tabs (it's for the first patch) (1.37 KB, patch)
2009-08-02 13:35 UTC, Daniele Salvatore Albano
Details
dnsmasq.mk converted to new autotools package format (1.42 KB, text/plain)
2009-08-03 18:36 UTC, Daniele Salvatore Albano
Details
dnsmasq.mk converted to new autotools package format (1.45 KB, text/plain)
2009-08-28 17:19 UTC, Daniele Salvatore Albano
Details
dnsmasq 2.50 Makefile.autotools.in plus some other improvements (4.34 KB, patch)
2009-09-01 15:18 UTC, Gustavo Zacarias
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniele Salvatore Albano 2009-08-02 13:32:40 UTC
Hi,

i've fixed some stuff into dnsmasq package:
- paths in dbus support
- dbus dependencies
- make doesn't take into account ldflags

Now it compiles fine, but i haven't yet tested it (actually i can't)

I'm attacching four patches, because the fourth replaces spaces with tabs for dbus path support fix
Comment 1 Daniele Salvatore Albano 2009-08-02 13:33:24 UTC
Created attachment 551 [details]
Fix dnsmasq compilation with dbus enabled
Comment 2 Daniele Salvatore Albano 2009-08-02 13:34:15 UTC
Created attachment 553 [details]
Fix dnsmasq dependencies if dbus is enabled
Comment 3 Daniele Salvatore Albano 2009-08-02 13:34:44 UTC
Created attachment 555 [details]
dnsmasq doesn't take into account ldflags for target
Comment 4 Daniele Salvatore Albano 2009-08-02 13:35:21 UTC
Created attachment 557 [details]
Replaces spaces in tabs (it's for the first patch)
Comment 5 Gustavo Zacarias 2009-08-03 14:40:24 UTC
Can you try converting it to the new Makefile.autofools.in format?
Thanks.
Comment 6 Daniele Salvatore Albano 2009-08-03 15:11:57 UTC
I'll try this evening
Comment 7 Daniele Salvatore Albano 2009-08-03 18:36:22 UTC
Hi,

i've converted the package to new autotools format, actually it works with and without dbus with my external toolchain.

Can someone check if it works with buildroot toolchain?

The attached file isin't a patch but he entire .mk file
Comment 8 Daniele Salvatore Albano 2009-08-03 18:36:52 UTC
Created attachment 569 [details]
dnsmasq.mk converted to new autotools package format
Comment 9 Gustavo Zacarias 2009-08-03 19:21:33 UTC
There's one broken bit:
The old package set the different DNSMASQ_COPTS according to IPV6, LARGEFILE and TFTP settings and then passed a make COPTS="$(DNSMASQ_COPTS)" parameter when building.
You are setting DNSMASQ_MAKE_OPT which doesn't fill up those values into the COPTS parameter for make. So if one of them is true then make gets passed -DWHATEVER which makes it fail.

Then there's the redundant bits:
DNSMASQ_AUTORECONF = NO -> it's default no, so no need to say that again
DNSMASQ_INSTALL_TARGET = YES -> again, it's default
DNSMASQ_INSTALL_TARGET_OPT .. -> you're using the same
DNSMASQ_DEPENDENCIES = uclibc -> default too

And the "why" thing:
DNSMASQ_INSTALL_STAGING = YES -> why?
As far as i know dnsmasq doesn't provide any library so it doesn't get linked to.
It doesn't install any header required by some other package either.

Detail:
It's up to version 2.49, you could bump to the new one while doing this.

Improvements:
dnsmasq doesn't have a proper uninstall, you could write one that does this into dnsmasq.mk
dnsmasq-2.49 introduced an option to disable the dhcp server thus a new option if you want to do something more.
Comment 10 Gustavo Zacarias 2009-08-03 19:27:25 UTC
My mistake, DNSMASQ_INSTALL_TARGET_OPT is required since it doesn't understand install-strip.
Comment 11 Daniele Salvatore Albano 2009-08-03 20:03:17 UTC
I've used gamin package as base, however, tomorrow i'll fix this stuff
Comment 12 Peter Korsgaard 2009-08-26 21:06:31 UTC
(In reply to comment #11)
> I've used gamin package as base, however, tomorrow i'll fix this stuff

Care to send a fixed patch so it can get included in the 2009.08 release? 

Comment 13 Daniele Salvatore Albano 2009-08-27 15:55:59 UTC
Sorry for the long delay, but i got some troubles with my git copy and being full of work i didn't have the time to fix. I just dropping all the stuff making a copy of my old tree and reappling patches.

I hope to send patches for this evening
Comment 14 Daniele Salvatore Albano 2009-08-28 17:18:31 UTC
Hi,

i've fixed the package, i think.

Could you take a look to it or try to build it using uclibc?

The package is attached
Comment 15 Daniele Salvatore Albano 2009-08-28 17:19:06 UTC
Created attachment 613 [details]
dnsmasq.mk converted to new autotools package format
Comment 16 Peter Korsgaard 2009-08-30 06:08:41 UTC
(In reply to comment #15)
> Created an attachment (id=613) [details]
> dnsmasq.mk converted to new autotools package format

Thanks. I noticed:

+	$(SED) 's^--ldflags dbus-1^--ldflags dbus-1 \| sed s\\\#-I/\\\#-I$(STAGING_DIR)/\\\#g^' \
		$(DNSMASQ_DIR)/Makefile

There's no such line in the dnsmasq makefile and afaik pkg-config doesn't even accept the --ldflags option. Did you mean --libs instead?
Comment 17 Gustavo Zacarias 2009-09-01 15:18:21 UTC
Created attachment 625 [details]
dnsmasq 2.50 Makefile.autotools.in plus some other improvements

How do you like this take Daniele / Peter?

* Bump to version 2.50 fixes security issues CVE 2009-2957 and 2009-2958.
* No need for INSTALL_STAGING
* Add uninstall target
* Use PREFIX=/usr to avoid files in /usr/local
* Install manpage conditionally
* Introduce new DHCP server option
* Add Peter's dbus fix (not tested, i don't use dbus)
Comment 18 Peter Korsgaard 2009-09-01 21:09:31 UTC
(In reply to comment #17)
> Created an attachment (id=625) [details]
> dnsmasq 2.50 Makefile.autotools.in plus some other improvements
> 
> How do you like this take Daniele / Peter?

Looks good, except that it still tries to edit a line with --ldflags dbus-1, which isn't a valid pkg-config argument, so I left that out.
Why was that line added in the first place?

Committed to git, thanks both of you.