| Message ID | 554E1ED2.7060206@cam.ac.uk |
|---|---|
| State | Superseded |
| Headers | show |
On 2015-05-09 16:50, Thomas Strobel wrote: > Hi, > > I'm using OpenWRT on NixOS, a Linux operating system that uses a > sophisticated hash mechanism instead of the Filesystem Hierarchy > Standard. A consequence of the hashing mechanism is that files like, > e.g., /usr/bin/perl don't exist under NixOS. > > I've tried to create a patch that replaces two occurrences where > "/usr/bin/perl" would be needed with a more general calling scheme that > "shouldn't" break OpenWRT on any other platform, but allows to build it > on NixOS. > > Could you please comment on the changes suggested, e.g., if they break > anything, and whether there is an interest in merging the patch upstream? > > > Many thanks! > > Thomas > > > > diff --git a/include/feeds.mk b/include/feeds.mk > index 695b03b..27f3e8f 100644 > --- a/include/feeds.mk > +++ b/include/feeds.mk > @@ -7,7 +7,7 @@ > > -include $(TMP_DIR)/.packagefeeds > > -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) > +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) I think this is the wrong place to make the change. Why not change scripts/feeds to replace the #! line with: #!/usr/bin/env perl - Felix
On 05/10/2015 01:12 PM, Felix Fietkau wrote: > On 2015-05-09 16:50, Thomas Strobel wrote: >> Hi, >> >> I'm using OpenWRT on NixOS, a Linux operating system that uses a >> sophisticated hash mechanism instead of the Filesystem Hierarchy >> Standard. A consequence of the hashing mechanism is that files like, >> e.g., /usr/bin/perl don't exist under NixOS. >> >> I've tried to create a patch that replaces two occurrences where >> "/usr/bin/perl" would be needed with a more general calling scheme that >> "shouldn't" break OpenWRT on any other platform, but allows to build it >> on NixOS. >> >> Could you please comment on the changes suggested, e.g., if they break >> anything, and whether there is an interest in merging the patch upstream? >> >> >> Many thanks! >> >> Thomas >> >> >> >> diff --git a/include/feeds.mk b/include/feeds.mk >> index 695b03b..27f3e8f 100644 >> --- a/include/feeds.mk >> +++ b/include/feeds.mk >> @@ -7,7 +7,7 @@ >> >> -include $(TMP_DIR)/.packagefeeds >> >> -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) >> +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) > I think this is the wrong place to make the change. Why not change > scripts/feeds to replace the #! line with: > #!/usr/bin/env perl > > - Felix > > Thanks for the review. Unfortunately, "/usr/bin/env" does not exist either. In NixOS there is no "/usr", no "/sbin", and only a "/bin/sh -> /nix/store/nixos_installation_specific_hash/bash". The absolute path to, e.g., "perl" or "env" is different for each installation of NixOS, so it can't be hard-coded into the build script. That's why I thought of calling "perl" directly from the PATH environment. Thomas
On 2015-05-10 14:16, Thomas Strobel wrote: >>> diff --git a/include/feeds.mk b/include/feeds.mk >>> index 695b03b..27f3e8f 100644 >>> --- a/include/feeds.mk >>> +++ b/include/feeds.mk >>> @@ -7,7 +7,7 @@ >>> >>> -include $(TMP_DIR)/.packagefeeds >>> >>> -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) >>> +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) >> I think this is the wrong place to make the change. Why not change >> scripts/feeds to replace the #! line with: >> #!/usr/bin/env perl >> >> - Felix >> >> > Thanks for the review. > Unfortunately, "/usr/bin/env" does not exist either. In NixOS there is > no "/usr", no "/sbin", and only a "/bin/sh -> > /nix/store/nixos_installation_specific_hash/bash". The absolute path to, > e.g., "perl" or "env" is different for each installation of NixOS, so it > can't be hard-coded into the build script. That's why I thought of > calling "perl" directly from the PATH environment. Then I think we should hold off on applying changes like the one you proposed until we have a proper plan for dealing with the rest. It needs to be possible to call ./scripts/feeds and similar scripts directly. How do other perl scripts on NixOS deal with that sort of stuff? - Felix
On 05/10/2015 02:48 PM, Felix Fietkau wrote: > On 2015-05-10 14:16, Thomas Strobel wrote: >>>> diff --git a/include/feeds.mk b/include/feeds.mk >>>> index 695b03b..27f3e8f 100644 >>>> --- a/include/feeds.mk >>>> +++ b/include/feeds.mk >>>> @@ -7,7 +7,7 @@ >>>> >>>> -include $(TMP_DIR)/.packagefeeds >>>> >>>> -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) >>>> +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) >>> I think this is the wrong place to make the change. Why not change >>> scripts/feeds to replace the #! line with: >>> #!/usr/bin/env perl >>> >>> - Felix >>> >>> >> Thanks for the review. >> Unfortunately, "/usr/bin/env" does not exist either. In NixOS there is >> no "/usr", no "/sbin", and only a "/bin/sh -> >> /nix/store/nixos_installation_specific_hash/bash". The absolute path to, >> e.g., "perl" or "env" is different for each installation of NixOS, so it >> can't be hard-coded into the build script. That's why I thought of >> calling "perl" directly from the PATH environment. > Then I think we should hold off on applying changes like the one you > proposed until we have a proper plan for dealing with the rest. > It needs to be possible to call ./scripts/feeds and similar scripts > directly. > How do other perl scripts on NixOS deal with that sort of stuff? > > - Felix > > > The way how NixOS deals with absolute path names in scripts is to patch every script. If software is packed for NixOS, then scripts are automatically or manually patched to execute under NixOS. For software that is not packed but only executed in NixOS, the scripts have to be adapted manually. As OpenWRT is just executed, the scripts have to be adapted manually for each new version or checkout of OpenWRT. The only reasonable way I can think of so far to fix the scripts in OpenWRT is at the calling side, like suggested in my patch. However, there are only 3-4 places that need changing in the build system and in a standard set of packages. So it wouldn't need too much tweaking on the OpenWRT side. - Thomas
On 2015-05-10 20:49, Thomas Strobel wrote: > On 05/10/2015 02:48 PM, Felix Fietkau wrote: >> On 2015-05-10 14:16, Thomas Strobel wrote: >>>>> diff --git a/include/feeds.mk b/include/feeds.mk >>>>> index 695b03b..27f3e8f 100644 >>>>> --- a/include/feeds.mk >>>>> +++ b/include/feeds.mk >>>>> @@ -7,7 +7,7 @@ >>>>> >>>>> -include $(TMP_DIR)/.packagefeeds >>>>> >>>>> -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) >>>>> +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) >>>> I think this is the wrong place to make the change. Why not change >>>> scripts/feeds to replace the #! line with: >>>> #!/usr/bin/env perl >>>> >>>> - Felix >>>> >>>> >>> Thanks for the review. >>> Unfortunately, "/usr/bin/env" does not exist either. In NixOS there is >>> no "/usr", no "/sbin", and only a "/bin/sh -> >>> /nix/store/nixos_installation_specific_hash/bash". The absolute path to, >>> e.g., "perl" or "env" is different for each installation of NixOS, so it >>> can't be hard-coded into the build script. That's why I thought of >>> calling "perl" directly from the PATH environment. >> Then I think we should hold off on applying changes like the one you >> proposed until we have a proper plan for dealing with the rest. >> It needs to be possible to call ./scripts/feeds and similar scripts >> directly. >> How do other perl scripts on NixOS deal with that sort of stuff? >> >> - Felix >> >> >> > The way how NixOS deals with absolute path names in scripts is to patch > every script. > If software is packed for NixOS, then scripts are automatically or > manually patched to execute under NixOS. > For software that is not packed but only executed in NixOS, the scripts > have to be adapted manually. Ugh, that sounds really nasty. > As OpenWRT is just executed, the scripts have to be adapted manually for > each new version or checkout of OpenWRT. The only reasonable way I can > think of so far to fix the scripts in OpenWRT is at the calling side, > like suggested in my patch. > However, there are only 3-4 places that need changing in the build > system and in a standard set of packages. So it wouldn't need too much > tweaking on the OpenWRT side. Given that the approach is incomplete (and quite inconvenient for regular use), and any better fix will not need these changes at all, I still object to adding them. The way I see it, the issue of missing /usr/bin/env is going to come up frequently with package-internal builds as well. NixOS users should probably just create a symlink or wrapper script at /usr/bin/env, and we should make sure that all important places in OpenWrt use it. I really don't want to start adding more and more patches afterwards to deal with the quirky non-standard behavior of NixOS. - Felix
On 05/10/2015 09:02 PM, Felix Fietkau wrote: > On 2015-05-10 20:49, Thomas Strobel wrote: >> On 05/10/2015 02:48 PM, Felix Fietkau wrote: >>> On 2015-05-10 14:16, Thomas Strobel wrote: >>>>>> diff --git a/include/feeds.mk b/include/feeds.mk >>>>>> index 695b03b..27f3e8f 100644 >>>>>> --- a/include/feeds.mk >>>>>> +++ b/include/feeds.mk >>>>>> @@ -7,7 +7,7 @@ >>>>>> >>>>>> -include $(TMP_DIR)/.packagefeeds >>>>>> >>>>>> -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) >>>>>> +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) >>>>> I think this is the wrong place to make the change. Why not change >>>>> scripts/feeds to replace the #! line with: >>>>> #!/usr/bin/env perl >>>>> >>>>> - Felix >>>>> >>>>> >>>> Thanks for the review. >>>> Unfortunately, "/usr/bin/env" does not exist either. In NixOS there is >>>> no "/usr", no "/sbin", and only a "/bin/sh -> >>>> /nix/store/nixos_installation_specific_hash/bash". The absolute path to, >>>> e.g., "perl" or "env" is different for each installation of NixOS, so it >>>> can't be hard-coded into the build script. That's why I thought of >>>> calling "perl" directly from the PATH environment. >>> Then I think we should hold off on applying changes like the one you >>> proposed until we have a proper plan for dealing with the rest. >>> It needs to be possible to call ./scripts/feeds and similar scripts >>> directly. >>> How do other perl scripts on NixOS deal with that sort of stuff? >>> >>> - Felix >>> >>> >>> >> The way how NixOS deals with absolute path names in scripts is to patch >> every script. >> If software is packed for NixOS, then scripts are automatically or >> manually patched to execute under NixOS. >> For software that is not packed but only executed in NixOS, the scripts >> have to be adapted manually. > Ugh, that sounds really nasty. Well, there's a price to pay for having a purely functional, declarative distribution with atomic upgrades and rollbacks ;). It's amazing when using the package manager of NixOS for software development, as you can, e.g., define a separate working environment for each project, especially as the environments are much more lightweight than Docker or LXC or all that. Have a look, the package manager is not restricted to NixOS but is also available for other Linux distributions and for OSX! >> As OpenWRT is just executed, the scripts have to be adapted manually for >> each new version or checkout of OpenWRT. The only reasonable way I can >> think of so far to fix the scripts in OpenWRT is at the calling side, >> like suggested in my patch. >> However, there are only 3-4 places that need changing in the build >> system and in a standard set of packages. So it wouldn't need too much >> tweaking on the OpenWRT side. > Given that the approach is incomplete (and quite inconvenient for > regular use), and any better fix will not need these changes at all, I > still object to adding them. > The way I see it, the issue of missing /usr/bin/env is going to come up > frequently with package-internal builds as well. NixOS users should > probably just create a symlink or wrapper script at /usr/bin/env, and we > should make sure that all important places in OpenWrt use it. > I really don't want to start adding more and more patches afterwards to > deal with the quirky non-standard behavior of NixOS. > > Changing the scripts in OpenWRT to use /usr/bin/env sounds like a pragmatic choice that would already make life easier under NixOS. In addition to changing the scripts, what do you think about referring to $(STAGING_DIR)/host/bin only for calling host binaries from OpenWRT scripts? That is, for calling "perl", the change would be: -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) +FEEDS_AVAILABLE:=$($(STAGING_DIR)/host/bin/perl $(SCRIPT_DIR)/feeds list -n) I know that this is very similar to what I proposed before, but it would have the benefit that there is a clear interface through which OpenWRT accesses any host binaries. Package-internal builds could be restricted to only access the host environment through $(STAGING_DIR)/host/bin as well, maybe helping to execute correctly once all requirements in $(STAGING_DIR)/host/bin are filled. - Thomas
On 2015-05-11 01:12, Thomas Strobel wrote: > Changing the scripts in OpenWRT to use /usr/bin/env sounds like a > pragmatic choice that would already make life easier under NixOS. > > In addition to changing the scripts, what do you think about referring > to $(STAGING_DIR)/host/bin only for calling host binaries from OpenWRT > scripts? > That is, for calling "perl", the change would be: > > -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) > +FEEDS_AVAILABLE:=$($(STAGING_DIR)/host/bin/perl $(SCRIPT_DIR)/feeds list -n) > > I know that this is very similar to what I proposed before, but it would > have the benefit that there is a clear interface through which OpenWRT > accesses any host binaries. I think if $(SCRIPT_DIR)/feeds is changed in a way that it can be called directly, the above change becomes completely pointless, as it only adds redundant information about what format $(SCRIPT_DIR)/feeds is supposed to be. So NACK from me on this one. > Package-internal builds could be restricted to only access the host > environment through $(STAGING_DIR)/host/bin as well, maybe helping to > execute correctly once all requirements in $(STAGING_DIR)/host/bin are > filled. Right now we rely on many things outside of the staging dir - basic utilties, toolchain, etc. I don't see that changing any time soon. - Felix
diff --git a/include/feeds.mk b/include/feeds.mk index 695b03b..27f3e8f 100644 --- a/include/feeds.mk +++ b/include/feeds.mk @@ -7,7 +7,7 @@ -include $(TMP_DIR)/.packagefeeds -FEEDS_AVAILABLE:=$(shell $(SCRIPT_DIR)/feeds list -n) +FEEDS_AVAILABLE:=$(perl $(SCRIPT_DIR)/feeds list -n) FEEDS_INSTALLED:=$(notdir $(wildcard $(TOPDIR)/package/feeds/*)) FEEDS_ENABLED:=$(foreach feed,$(FEEDS_INSTALLED),$(if $(CONFIG_FEED_$(feed)),$(feed))) FEEDS_DISABLED:=$(filter-out $(FEEDS_ENABLED),$(FEEDS_AVAILABLE)) diff --git a/include/toplevel.mk b/include/toplevel.mk index d8651d9..1a0bc2e 100644 --- a/include/toplevel.mk +++ b/include/toplevel.mk @@ -74,7 +74,7 @@ prepare-tmpinfo: FORCE f=tmp/.$${type}info; t=tmp/.config-$${type}.in; \ [ "$$t" -nt "$$f" ] || ./scripts/metadata.pl $${type}_config "$$f" > "$$t" || { rm -f "$$t"; echo "Failed to build $$t"; false; break; }; \ done - [ tmp/.config-feeds.in -nt tmp/.packagefeeds ] || ./scripts/feeds feed_config > tmp/.config-feeds.in + [ tmp/.config-feeds.in -nt tmp/.packagefeeds ] || perl
Hi, I'm using OpenWRT on NixOS, a Linux operating system that uses a sophisticated hash mechanism instead of the Filesystem Hierarchy Standard. A consequence of the hashing mechanism is that files like, e.g., /usr/bin/perl don't exist under NixOS. I've tried to create a patch that replaces two occurrences where "/usr/bin/perl" would be needed with a more general calling scheme that "shouldn't" break OpenWRT on any other platform, but allows to build it on NixOS. Could you please comment on the changes suggested, e.g., if they break anything, and whether there is an interest in merging the patch upstream? Many thanks! Thomas scripts/feeds feed_config > tmp/.config-feeds.in ./scripts/metadata.pl package_mk tmp/.packageinfo > tmp/.packagedeps || { rm -f tmp/.packagedeps; false; } ./scripts/metadata.pl package_feeds tmp/.packageinfo > tmp/.packagefeeds || { rm -f tmp/.packagefeeds; false; } touch $(TOPDIR)/tmp/.build