| Message ID | 1417562463-26561-2-git-send-email-bluemrp9@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Ryan, On Tue, Dec 2, 2014 at 5:21 PM, Ryan Coe <bluemrp9@gmail.com> wrote: > Signed-off-by: Ryan Coe <bluemrp9@gmail.com> > --- > package/rp-pppoe/rp-pppoe.mk | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/package/rp-pppoe/rp-pppoe.mk b/package/rp-pppoe/rp-pppoe.mk > index 70b4437..9c3e3e4 100644 > --- a/package/rp-pppoe/rp-pppoe.mk > +++ b/package/rp-pppoe/rp-pppoe.mk > @@ -11,6 +11,8 @@ RP_PPPOE_LICENSE_FILES = doc/LICENSE > RP_PPPOE_DEPENDENCIES = pppd > RP_PPPOE_SUBDIR = src > RP_PPPOE_TARGET_FILES = pppoe pppoe-server pppoe-relay pppoe-sniff > +RP_PPPOE_TARGET_SCRIPTS = pppoe-connect pppoe-init pppoe-setup pppoe-start \ > + pppoe-stop > RP_PPPOE_MAKE_OPTS = PLUGIN_DIR=/usr/lib/pppd/$(PPPD_VERSION) > RP_PPPOE_CONF_OPTS = --disable-debugging > RP_PPPOE_CONF_ENV = \ > @@ -23,6 +25,9 @@ define RP_PPPOE_INSTALL_TARGET_CMDS > for ff in $(RP_PPPOE_TARGET_FILES); do \ > $(INSTALL) -m 0755 $(@D)/src/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > done > + for ff in $(RP_PPPOE_TARGET_SCRIPTS); do \ > + $(INSTALL) -m 0755 $(@D)/scripts/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > + done Acked-by: Matt Weber <matthew.weber@rockwellcollins.com> Thanks for submitting this, I was about to work on a similar patch set! > endef > > $(eval $(autotools-package)) > -- > 2.2.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Dear Ryan Coe, On Tue, 2 Dec 2014 15:21:03 -0800, Ryan Coe wrote: > Signed-off-by: Ryan Coe <bluemrp9@gmail.com> > --- > package/rp-pppoe/rp-pppoe.mk | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/package/rp-pppoe/rp-pppoe.mk b/package/rp-pppoe/rp-pppoe.mk > index 70b4437..9c3e3e4 100644 > --- a/package/rp-pppoe/rp-pppoe.mk > +++ b/package/rp-pppoe/rp-pppoe.mk > @@ -11,6 +11,8 @@ RP_PPPOE_LICENSE_FILES = doc/LICENSE > RP_PPPOE_DEPENDENCIES = pppd > RP_PPPOE_SUBDIR = src > RP_PPPOE_TARGET_FILES = pppoe pppoe-server pppoe-relay pppoe-sniff > +RP_PPPOE_TARGET_SCRIPTS = pppoe-connect pppoe-init pppoe-setup pppoe-start \ > + pppoe-stop Any reason why we need a new variable instead of adding new values to RP_PPPOE_TARGET_FILES ? > @@ -23,6 +25,9 @@ define RP_PPPOE_INSTALL_TARGET_CMDS > for ff in $(RP_PPPOE_TARGET_FILES); do \ > $(INSTALL) -m 0755 $(@D)/src/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > done > + for ff in $(RP_PPPOE_TARGET_SCRIPTS); do \ > + $(INSTALL) -m 0755 $(@D)/scripts/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > + done Both variables do the same thing. Thomas
>>>>> "Ryan" == Ryan Coe <bluemrp9@gmail.com> writes: > Signed-off-by: Ryan Coe <bluemrp9@gmail.com> Committed, thanks.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > Dear Ryan Coe, > On Tue, 2 Dec 2014 15:21:03 -0800, Ryan Coe wrote: >> Signed-off-by: Ryan Coe <bluemrp9@gmail.com> >> --- >> package/rp-pppoe/rp-pppoe.mk | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/package/rp-pppoe/rp-pppoe.mk b/package/rp-pppoe/rp-pppoe.mk >> index 70b4437..9c3e3e4 100644 >> --- a/package/rp-pppoe/rp-pppoe.mk >> +++ b/package/rp-pppoe/rp-pppoe.mk >> @@ -11,6 +11,8 @@ RP_PPPOE_LICENSE_FILES = doc/LICENSE >> RP_PPPOE_DEPENDENCIES = pppd >> RP_PPPOE_SUBDIR = src >> RP_PPPOE_TARGET_FILES = pppoe pppoe-server pppoe-relay pppoe-sniff >> +RP_PPPOE_TARGET_SCRIPTS = pppoe-connect pppoe-init pppoe-setup pppoe-start \ >> + pppoe-stop > Any reason why we need a new variable instead of adding new values to > RP_PPPOE_TARGET_FILES ? >> @@ -23,6 +25,9 @@ define RP_PPPOE_INSTALL_TARGET_CMDS >> for ff in $(RP_PPPOE_TARGET_FILES); do \ >> $(INSTALL) -m 0755 $(@D)/src/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ >> done >> + for ff in $(RP_PPPOE_TARGET_SCRIPTS); do \ >> + $(INSTALL) -m 0755 $(@D)/scripts/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ >> + done > Both variables do the same thing. No, they copy from two different subdirs?
Dear Peter Korsgaard, On Wed, 03 Dec 2014 23:32:08 +0100, Peter Korsgaard wrote: > >> @@ -23,6 +25,9 @@ define RP_PPPOE_INSTALL_TARGET_CMDS > >> for ff in $(RP_PPPOE_TARGET_FILES); do \ > >> $(INSTALL) -m 0755 $(@D)/src/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > >> done > >> + for ff in $(RP_PPPOE_TARGET_SCRIPTS); do \ > >> + $(INSTALL) -m 0755 $(@D)/scripts/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > >> + done > > > Both variables do the same thing. > > No, they copy from two different subdirs? Gaah, my bad, I overlooked that. Indeed, both variables are needed then. So the patch looks OK to me. Sorry for the noise :/ Thomas
Dear Thomas Petazzoni, On Wed, 3 Dec 2014 23:38:32 +0100, Thomas Petazzoni wrote: > Dear Peter Korsgaard, > > On Wed, 03 Dec 2014 23:32:08 +0100, Peter Korsgaard wrote: > > >> @@ -23,6 +25,9 @@ define RP_PPPOE_INSTALL_TARGET_CMDS > > >> for ff in $(RP_PPPOE_TARGET_FILES); do \ > > >> $(INSTALL) -m 0755 $(@D)/src/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > > >> done > > >> + for ff in $(RP_PPPOE_TARGET_SCRIPTS); do \ > > >> + $(INSTALL) -m 0755 $(@D)/scripts/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > > >> + done > > > > > Both variables do the same thing. > > > > No, they copy from two different subdirs? > > Gaah, my bad, I overlooked that. Indeed, both variables are needed > then. So the patch looks OK to me. > > Sorry for the noise :/ On a second thought, we could do: RP_PPPOE_TARGET_FILES = src/foo src/bar scripts/baz scripts/bleh and then do: for ff in $(RP_PPPOE_TARGET_FILES); do \ $(INSTALL) -m 0755 $(@D)/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ done Not sure it matters much, though. Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: Hi, > On a second thought, we could do: > RP_PPPOE_TARGET_FILES = src/foo src/bar scripts/baz scripts/bleh > and then do: > for ff in $(RP_PPPOE_TARGET_FILES); do \ > $(INSTALL) -m 0755 $(@D)/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ > done Then you whould need to strip the directory part of $$ff for the destination to not end up in $(TARGET_DIR)/usr/sbin/src/foo. > Not sure it matters much, though. I think keeping it in two loops is a bit more readable.
diff --git a/package/rp-pppoe/rp-pppoe.mk b/package/rp-pppoe/rp-pppoe.mk index 70b4437..9c3e3e4 100644 --- a/package/rp-pppoe/rp-pppoe.mk +++ b/package/rp-pppoe/rp-pppoe.mk @@ -11,6 +11,8 @@ RP_PPPOE_LICENSE_FILES = doc/LICENSE RP_PPPOE_DEPENDENCIES = pppd RP_PPPOE_SUBDIR = src RP_PPPOE_TARGET_FILES = pppoe pppoe-server pppoe-relay pppoe-sniff +RP_PPPOE_TARGET_SCRIPTS = pppoe-connect pppoe-init pppoe-setup pppoe-start \ + pppoe-stop RP_PPPOE_MAKE_OPTS = PLUGIN_DIR=/usr/lib/pppd/$(PPPD_VERSION) RP_PPPOE_CONF_OPTS = --disable-debugging RP_PPPOE_CONF_ENV = \ @@ -23,6 +25,9 @@ define RP_PPPOE_INSTALL_TARGET_CMDS for ff in $(RP_PPPOE_TARGET_FILES); do \ $(INSTALL) -m 0755 $(@D)/src/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ done + for ff in $(RP_PPPOE_TARGET_SCRIPTS); do \ + $(INSTALL) -m 0755 $(@D)/scripts/$$ff $(TARGET_DIR)/usr/sbin/$$ff || exit 1; \ + done endef $(eval $(autotools-package))
Signed-off-by: Ryan Coe <bluemrp9@gmail.com> --- package/rp-pppoe/rp-pppoe.mk | 5 +++++ 1 file changed, 5 insertions(+)