[PATCH] Eliminate config.inc.

Tue Mar 20 01:14:01 UTC 2018

On 20/03/2018 02:16, Christian Mauderer wrote:
> Most of the values in config.inc haven't been used any more. The only
> part left has been the ip addresses for some of the test.

Why not just removed the parts not being used?

> All other values are not used and might irritate new users.

Why would it? It would irritate existing "users" if the hard coded set up
conflicts with any networks they use.

I do not agree with a hard coded set of addresses and I cannot see a compelling
reason given in this patch to do this.

The way the setting of these is currently handled may end up being part of the
#3351 resolution once that ticket's path is resolved.

> 
> Generating the header for the network configuration adds quite some
> lines to the build system.

It is not that much, it exists and it works. The code being removed is about 30
lines while the generated waf script is over 3000 lines long.

How is this complicating the build system?

> On the other side it seems not really that
> necessary to make the network configuration for the tests configurable.

Why?

> Therefore this patch elimintates that too and uses fixed IP addresses in
> the (previously generated) network-config.h header instead. If someone
> really needs to run the tests with some other IP addresses, he still can
> locally change the header (instead of the config.inc).

Sorry, lets not go there. As a policy I would like to see RTEMS avoid touching
code this way so tests can run. Saying this requires developers walk patches
through repeated changes to the repo and that is an overhead for them, it
complicates continuous integration set ups and it added complexity to an audit
process wanting to run any certification effort.

> ---
>  config.inc                                         | 15 --------
>  libbsd.txt                                         | 30 +++------------
>  libbsd_waf.py                                      | 36 -----------------
>  .../test/{network-config.h.in => network-config.h} |  8 ++--
>  waf_generator.py                                   | 45 ----------------------
>  wscript                                            |  5 ---
>  6 files changed, 10 insertions(+), 129 deletions(-)
>  delete mode 100644 config.inc
>  rename testsuite/include/rtems/bsd/test/{network-config.h.in => network-config.h} (92%)
> 
> diff --git a/config.inc b/config.inc
> deleted file mode 100644
> index 3432a5fc..00000000
> --- a/config.inc
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -# Mandatory: Select your BSP and installation prefix
> -TARGET = arm-rtems4.11
> -BSP = realview_pbx_a9_qemu
> -PREFIX = /opt/rtems-4.11
> -
> -# Optional: Separate installation base directory
> -INSTALL_BASE = $(PREFIX)/$(TARGET)/$(BSP)
> -
> -# Optional: Network test configuration
> -TEST_RUNNER = $(BSP)
> -NET_CFG_SELF_IP = 10.0.2.1
> -NET_CFG_NETMASK = 255.255.0.0
> -NET_CFG_PEER_IP = 192.168.100.11
> -NET_CFG_GATEWAY_IP = 192.168.100.11
> -NET_TAP_INTERFACE = tap0
> diff --git a/libbsd.txt b/libbsd.txt
> index 71d5cc8d..d57d1884 100644
> --- a/libbsd.txt
> +++ b/libbsd.txt
> @@ -42,7 +42,6 @@ installed.
>  
>  . Clone the Git repository +git clone git://git.rtems.org/rtems-libbsd.git+.
>  . Change into the RTEMS BSD library root directory.
> -. Edit the `config.inc` configuration file and adjust it to your environment.
>  . Run +waf configure ...+.
>  . Run +waf+.
>  . Run +waf install+.
> @@ -119,29 +118,6 @@ devices (you can run multiple test instances on one virtual network).
>  The build system based on the Waf build system. To build with Waf please refer
>  to the README.waf file.
>  
> -===== Example Configuration =====
> -
> -In the BSD library source directory edit the file `config.inc`.  Continuing on
> -the above, the `config.inc` used to match the above is:
> -
> --------------------------------------------------------------------------------
> -# Mandatory: Select your BSP and installation prefix
> -TARGET = arm-rtems4.12
> -BSP = xilinx_zynq_a9_qemu
> -PREFIX = $(HOME)/sandbox/install
> -
> -# Optional: Separate installation base directory
> -INSTALL_BASE = $(PREFIX)/$(TARGET)/$(BSP)
> -
> -# Optional: Network test configuration
> -TEST_RUNNER = $(BSP)
> -NET_CFG_SELF_IP = 10.0.0.2
> -NET_CFG_NETMASK = 255.255.0.0
> -NET_CFG_PEER_IP = 10.0.0.1
> -NET_CFG_GATEWAY_IP = 10.0.0.1
> -NET_TAP_INTERFACE = tap0
> --------------------------------------------------------------------------------
> -
>  === BSD Library Initialization ===
>  
>  To initialise the BSD Library create a suitable rc.conf file. The FreeBSD man
> @@ -273,6 +249,12 @@ their own implementation of the `rtems_bsd_get_allocator_domain_size()`
>  function (for example in the module which calls `rtems_bsd_initialize()`) if
>  different values are desired.  The default size is 8MiB for all domains.
>  
> +=== Tests ===
> +
> +There are tests for some features in the `testsuite` directory. Note that some
> +of the tests use a fixed IP configuration that is defined in
> +`testsuite/include/rtems/bsd/test/network-config.h`.

It is not clear to me in this documentation how a developer who has clashing
networks is suppose to handle these static settings and nothing here explains
what what to do.

> +
>  == Network Stack Features
>  
>  http://roy.marples.name/projects/dhcpcd/index[DHCPCD(8)]:: DHCP client
> diff --git a/libbsd_waf.py b/libbsd_waf.py
> index 745512bf..642142e2 100644
> --- a/libbsd_waf.py
> +++ b/libbsd_waf.py
> @@ -91,42 +91,6 @@ def build(bld):
>      # Collect the libbsd uses
>      libbsd_use = []
>  
> -    # Network test configuration
> -    if not os.path.exists(bld.env.NET_CONFIG):
> -        bld.fatal("network configuraiton '%s' not found" % (bld.env.NET_CONFIG))
> -    net_cfg_self_ip = None
> -    net_cfg_netmask = None
> -    net_cfg_peer_ip = None
> -    net_cfg_gateway_ip = None
> -    net_tap_interface = None
> -    try:
> -        net_cfg_lines = open(bld.env.NET_CONFIG).readlines()
> -    except:
> -        bld.fatal("network configuraiton '%s' read failed" % (bld.env.NET_CONFIG))
> -    lc = 0
> -    for l in net_cfg_lines:
> -        lc += 1
> -        if l.strip().startswith("NET_CFG_"):
> -            ls = l.split("=")
> -            if len(ls) != 2:
> -                bld.fatal("network configuraiton '%s' parse error: %d: %s" % (bld.env.NET_CONFIG, lc, l))
> -            lhs = ls[0].strip()
> -            rhs = ls[1].strip()
> -            if lhs == "NET_CFG_SELF_IP":
> -                net_cfg_self_ip = rhs
> -            if lhs == "NET_CFG_NETMASK":
> -                net_cfg_netmask = rhs
> -            if lhs == "NET_CFG_PEER_IP":
> -                net_cfg_peer_ip = rhs
> -            if lhs == "NET_CFG_GATEWAY_IP":
> -                net_cfg_gateway_ip = rhs
> -            if lhs == "NET_TAP_INTERFACE":
> -                net_tap_interface = rhs
> -    bld(target = "testsuite/include/rtems/bsd/test/network-config.h",
> -        source = "testsuite/include/rtems/bsd/test/network-config.h.in",
> -        rule = "sed -e 's/@NET_CFG_SELF_IP@/%s/' -e 's/@NET_CFG_NETMASK@/%s/' -e 's/@NET_CFG_PEER_IP@/%s/' -e 's/@NET_CFG_GATEWAY_IP@/%s/' < ${SRC} > ${TGT}" % (net_cfg_self_ip, net_cfg_netmask, net_cfg_peer_ip, net_cfg_gateway_ip),
> -        update_outputs = True)
> -
>      # copy headers if necessary
>      header_build_copy_paths = [
>                                 ('freebsd/crypto/openssl/crypto', '*.h', 'openssl'),
> diff --git a/testsuite/include/rtems/bsd/test/network-config.h.in b/testsuite/include/rtems/bsd/test/network-config.h
> similarity index 92%
> rename from testsuite/include/rtems/bsd/test/network-config.h.in
> rename to testsuite/include/rtems/bsd/test/network-config.h
> index 2cef97db..c7350011 100755
> --- a/testsuite/include/rtems/bsd/test/network-config.h.in
> +++ b/testsuite/include/rtems/bsd/test/network-config.h
> @@ -54,12 +54,12 @@
>    #define NET_CFG_INTERFACE_0 "lo0"
>  #endif
>  
> -#define NET_CFG_SELF_IP "@NET_CFG_SELF_IP@"
> +#define NET_CFG_SELF_IP "10.0.2.1"

I am OK with 192.168.0.1.

... then again that is a default used by most routers and a widely used network
set up including the RTEMS server's internal networks and why my local networks
are based around 10.* networks including subnets, VPNs, VLANs and PPT high speed
server interconnects.

>  
> -#define NET_CFG_NETMASK "@NET_CFG_NETMASK@"
> +#define NET_CFG_NETMASK "255.255.0.0"

I would prefer 255.255.255.0.

> -#define NET_CFG_PEER_IP "@NET_CFG_PEER_IP@"
> +#define NET_CFG_PEER_IP "192.168.100.11"

If the address above it changed what is used here ...

>  
> -#define NET_CFG_GATEWAY_IP "@NET_CFG_GATEWAY_IP@"
> +#define NET_CFG_GATEWAY_IP "192.168.100.11"

Anyway I think I have made my point :)

Chris