| Message ID | 1322085218-10192-1-git-send-email-jcmvbkbc@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 23 November 2011 21:53, Max Filippov <jcmvbkbc@gmail.com> wrote: > --*dir) option pattern precede --{en,dis}able-usb-redir) patterns in the > option analysis switch, making the latter options have no effect. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> I just said this in the other thread, but to repeat it in the right place: I think we should expand out the case statement to explicitly list the --thingydir options it is supposed to be matching, and drop the wildcard: as this bug demonstrates it's rather easy to accidentally shoot yourself in the foot with it. In fact, what cases is this supposed to be matching? All the documented --thingydir options are handled explicitly earlier in the case statement. Paolo, you added this case in commit 6bde81cb0, but the commit message doesn't give any rationale; what's it for? -- PMM
On 11/23/2011 11:22 PM, Peter Maydell wrote: > I just said this in the other thread, but to repeat it in the > right place: I think we should expand out the case statement > to explicitly list the --thingydir options it is supposed to > be matching, and drop the wildcard: as this bug demonstrates > it's rather easy to accidentally shoot yourself in the foot > with it. > > In fact, what cases is this supposed to be matching? All > the documented --thingydir options are handled explicitly > earlier in the case statement. > > Paolo, you added this case in commit 6bde81cb0, but the > commit message doesn't give any rationale; what's it for? There were some --*dir that are supported by Autoconf and not by QEMU configure. The aim was to let QEMU packagers use the rpm (or similar) macro that overrides directories for their distribution.
On 24 November 2011 08:25, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 11/23/2011 11:22 PM, Peter Maydell wrote: >> In fact, what cases is this supposed to be matching? All >> the documented --thingydir options are handled explicitly >> earlier in the case statement. >> >> Paolo, you added this case in commit 6bde81cb0, but the >> commit message doesn't give any rationale; what's it for? > > There were some --*dir that are supported by Autoconf and > not by QEMU configure. The aim was to let QEMU packagers > use the rpm (or similar) macro that overrides directories > for their distribution. Ah. Can we list them explicitly with a comment about why we accept but ignore them, please? -- PMM
>>> In fact, what cases is this supposed to be matching? All >>> the documented --thingydir options are handled explicitly >>> earlier in the case statement. >>> >>> Paolo, you added this case in commit 6bde81cb0, but the >>> commit message doesn't give any rationale; what's it for? >> >> There were some --*dir that are supported by Autoconf and >> not by QEMU configure. The aim was to let QEMU packagers >> use the rpm (or similar) macro that overrides directories >> for their distribution. > > Ah. Can we list them explicitly with a comment about why > we accept but ignore them, please? I guess the list is the following: --mandir=PATH install man pages in PATH --datadir=PATH install firmware in PATH --docdir=PATH install documentation in PATH --bindir=PATH install binaries in PATH --sysconfdir=PATH install config in PATH/qemu http://www.gnu.org/prep/standards/html_node/Configuration.html says that it should also handle the following list: --bindir --sbindir --libexecdir --sysconfdir --sharedstatedir --localstatedir --libdir --includedir --oldincludedir --datarootdir --datadir --infodir --localedir --mandir --docdir --htmldir --dvidir --pdfdir --psdir Do we need anything of these?
On 24 November 2011 09:57, Max Filippov <jcmvbkbc@gmail.com> wrote: >>> There were some --*dir that are supported by Autoconf and >>> not by QEMU configure. The aim was to let QEMU packagers >>> use the rpm (or similar) macro that overrides directories >>> for their distribution. >> >> Ah. Can we list them explicitly with a comment about why >> we accept but ignore them, please? > http://www.gnu.org/prep/standards/html_node/Configuration.html says > that it should also handle the following list: > > --bindir --sbindir --libexecdir --sysconfdir > --sharedstatedir --localstatedir --libdir --includedir --oldincludedir > --datarootdir --datadir --infodir --localedir --mandir --docdir > --htmldir --dvidir --pdfdir --psdir So the ones we don't already support are: --sbindir --libexecdir --datarootdir --infodir --localedir --htmldir --dvidir --pdfdir --psdir -- PMM
diff --git a/configure b/configure index f033438..e5566c8 100755 --- a/configure +++ b/configure @@ -759,8 +759,6 @@ for opt do ;; --enable-opengl) opengl="yes" ;; - --*dir) - ;; --disable-rbd) rbd="no" ;; --enable-rbd) rbd="yes" @@ -783,6 +781,8 @@ for opt do ;; --disable-guest-agent) guest_agent="no" ;; + --*dir) + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac
--*dir) option pattern precede --{en,dis}able-usb-redir) patterns in the option analysis switch, making the latter options have no effect. Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- configure | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)