[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] tools/configure: drop BASH configure variable


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Ian Jackson <ian.jackson@xxxxxxxxxx>
  • Date: Mon, 29 Jun 2020 14:34:23 +0100
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Mon, 29 Jun 2020 13:34:38 +0000
  • Ironport-sdr: 2mQVr80Hqteu/U4L0F00N2/ZXrAA1kSi7Vfl8ESmr15CM8oJ5WIsJzxs4mF5AluKPB1icZZPNL eIswkUW/5mxZ01J3Zso5qQHusmNJX3U6acsUHqCPMqWk7dDLyO+dPu8MJqZG91H9a4gACOANl/ ENMPsa3xCc/8rimKeIGEeMILi8iXuZ/0kQMghzK6LP+imVcvjUy/Es5Iz5tq0AGo6fKQxNOykq YHnOfXeKiTevRW0W2VIX0IIoJ6cF0GHWf1x5V3veMr9wFYAg1iyAS+jNB4r7ujb3dyzzR6GckT BwY=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Andrew Cooper writes ("[PATCH] tools/configure: drop BASH configure variable"):
> This is a weird variable to have in the first place.  The only user of it is
> XSM's CONFIG_SHELL, which opencodes a fallback to sh, and the only two scripts
> run with this are shebang sh anyway, so don't need bash in the first place.

Thanks for this cleanup.  I agree with the basic idea.

However, did you run these scripts with dash, or review them, to check
for bashisms ?  It is not unusual to find scripts which need bash but
which mistaken have #!/bin/sh - especially if the usual arrangements
for running them always use bash anyway.  So the presence of the
#!/bin/sh is only rather weak evidence that these scripts will be fine
when run with sh.

> Make the mkflask.sh and mkaccess_vector.sh scripts executable, drop the
> CONFIG_SHELL, and drop the $BASH variable to prevent further use.

Since the build currently uses bash for these, a more neutral change
would be to change to #!/bin/bash at the same time.

> RFC for 4.14.  This is a cleanup to the build system.

I see this already has a release-ack.  However, I would not have
recommended granting one at least on the basis of the description
above.

I agree that this is cleanup.  But the current situation is not buggy.
I'm not sure exactly what the release criteria are but ISTM that this
patch adds risk to the release rather than removing it.

Thanks for your attention.

Ian.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.