On Tue, Mar 19, 2019 at 06:55:10AM +0000, Roi Dayan wrote:
> 
> 
> On 18/03/2019 23:58, Ben Pfaff wrote:
> > On Sun, Mar 10, 2019 at 05:31:31AM +0000, Eli Britstein wrote:
> >> Generate datapath ovs key fields offset and size array macros as a
> >> pre-step for bit-wise comparing fields, with no functional change.
> >>
> >> Signed-off-by: Eli Britstein <[email protected]>
> >> Reviewed-by: Roi Dayan <[email protected]>
> > 
> > Thanks!
> > 
> > This introduces a script that uses /bin/bash.  I don't think that we
> > have other build or runtime  dependencies on bash, so it would be better
> > to avoid introducing one here.  I would rewrite it in terms of portable
> > Bourne shell constructs.
> > 
> 
> Hi Ben,
> 
> just to clarify, when you see portable Bourne shell constructs,
> you mean so it can work with other shells like dash on ubuntu?

Yes.  I have always implicitly thought that any shell that matches
Debian's requirements should be acceptable.  See the policy document at
https://www.debian.org/doc/debian-policy/policy.txt in section 10.4
"Scripts".  The most important part of this for OVS purposes is the
following bullet:

* "local" to create a scoped variable must be supported, including
  listing multiple variables in a single local command and assigning a
  value to a variable at the same time as localizing it. "local" may
  or may not preserve the variable value from an outer scope if no
  assignment is present. Uses such as:

     fname () {
         local a b c=delta d
         # ... use a, b, c, d ...
     }

  must be supported and must set the value of "c" to "delta".

> There are other scripts depending on bash.
> examples are tests in tests/system-traffic.at, the bash auto completion 
> scripts, ovs-sim.
> do you still prefer us to be compatible and use /bin/sh shebang ?

I wasn't aware that system-traffic.at depended on bash.  It is not
ideal.  Do you know what dependencies it has?  However, we do not expect
every developer to run the system-traffic tests, and I believe that it
necessarily depends on Linux-specific and third-party utilities anyhow,
so it is less important that it work everywhere.

The bash auto completion scripts are optional.  You can use them if you
already use bash and you want them, but there is no downside if you do
not.

ovs-sim is also somewhat specialized.  I couldn't figure out how to do
what I wanted without bash, so I decided that it could use bash.

This patch, however, introduces a new script without which OVS cannot
build at all.  It should not depend on bash, absent general agreement in
the project that bash is an acceptable new dependency (and absent
documentation in the build instructions that bash is a build
dependency).  I think it would be easy to remove the dependency on bash
here, so I think that it should be removed.

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to