Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure script reject legitimate arguments #6628

Closed
vicuna opened this Issue Oct 26, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

commented Oct 26, 2014

Original bug ID: 6628
Reporter: michipili
Assigned to: @damiendoligez
Status: closed (set by @xavierleroy on 2016-12-07T10:49:07Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: all
OS: all
OS Version: all
Version: 4.02.1
Fixed in version: 4.02.2+dev / +rc1
Category: misc
Has duplicate: #6867

Bug description

The configuration script says

----8<----
if echo "$configure_options" | grep -q -e '--?[a-zA-Z0-9-]+='; then
err "Arguments to this script look like '-prefix /foo/bar', not '-prefix=/foo/bar' (note the '=')."
fi
---->8----

This is broken because it matches legitimate arguments, like:

----8<----
-cc "gcc48 -O2 -pipe -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing"
---->8----

Fix: Do not try to pseudo-validate input in a broken way. We could report suspicious arguments in the case analyse of these, if this is important.

Steps to reproduce

./configure -cc "gcc48 -O2 -pipe -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing"

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 26, 2014

Comment author: @gasche

I suppose it is important for usability, because otherwise users could try to use this syntax and be surprised by obscure failures.

Would you propose a good patch to fix this issue?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 27, 2014

Comment author: michipili

Here it is!

I also propose another error message, as the previous is not very clear. Until I read the source, I understood it as if it referred to the actual arguments of the script. A more correct statement could have been “This script expects arguments looking like '-prefix /foo/bar', not '-prefix=/foo/bar' (note the '=').”.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 30, 2014

Comment author: @gasche

Damien fixed this upstream (using a slightly different test and error message), thanks!

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented May 3, 2015

Comment author: michi

Can we reopen this issue? The argument validation stroke two new FreeBSD users, and I asked them
to describe their use-case here.

Argument validation is expected to make people life's easier. Is there some of these people?
I am asking because there is a few people whose life is made a bit harder because of this
validation! :)

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented May 3, 2015

Comment author: @gasche

The reason I closed this issue is because of this commit by Damien:
48871e6

This change is not part of any released OCaml version so far (it should be part of the next release), so when you say "stroke two new users" I'm not sure whether you mean that even this refined test is not correct, or just that they were hit with the same old bug with the released version.

I tested that the configure command you provide works fine under not-yet-released versions (for example the stable 4.02 branch). If you are not sure whether the above patch fixes the issues that those two new users have, yet don't want to setup an experimental version just for this, I would be glad to test proposed inputs and report whether they are still problematic.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented May 3, 2015

Comment author: michi

This change is not part of any released OCaml version so far

Ooooh, there is a bit of a misunderstanding here! Thank you for pointing out that!

I erroneously assumed the change would be in 4.02.1 – maybe I should have read better the “fixed in” field. So we can keep this issue closed, add the patch in FreeBSD and remove it as 4.02.2 arrives.

Thank you for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.