option-delete in portfile fails if option not yet set

[jberry@…]

Patch attached.

If an option is not yet set, option-delete, perhaps called from a variant, will raise an error.

This patch verifies that the option exists before attempting to delete components of it.

[jberry@…]

patch to base/src/port1.0/portutil.tcl

[pguyot (Paul Guyot)]

What would be a test case for this bug?

[jberry@…]

Paul:

I created a test case by building a Portfile that had no depends_lib, line, but did a depends_lib-delete foo. This caused the error to manifest in the unpatched code, and was fixed by the patched code. Similar circumstances for any other option that doesn't have a default supplied.

I've questions about this bug come up several times in the list, when the answer is "oh, just put an empty depends_lib line in." This patch should mean that's not necessary, at least for the delete case.

-jdb

[pguyot (Paul Guyot)]

My question for a test case was that I wonder if we really should silently accept option-delete when option wasn't previously set.

Indeed, currently we get a cryptic error message such as: Unable to open port: can't read "depends_run": can't unset "PortInfo(depends_run)": no such element in array

But it means there is something wrong in the Portfile, doesn't it?

[jberry@…]

Hi Paul,

I don't know that this always means there is something wrong in the Portfile, although I suspect it could usually be worked around. As it is, there is a difference between trying to delete a option that isn't there, when the option has been set to "", and deleting the same option that isn't there when the option has never been set.

I believe these cases should function identically, and that is what this patch does, and it keeps the Portfile from failing completely.

Your question, however, raises an additional point, which is whether there should be a warning message for either case, stating that you tryed to delete an option that didn't exist. I suspect this occurs mainly in fairly complex portfiles with multiple options. A warning message might be useful, but it would add more code to that ugly escaped delete-proc mess, and I'm not sure I want to tackle it ;) I don't think the delete returns an error, so you'd have to search for the option before the delete in order to report that it's not there.

[jberry@…]

Paul?

[pguyot (Paul Guyot)]

Until you showed me a Portfile that legitimately could have option-delete without the option set, I will consider that this patch silently ignores a bug in a portfile, which is a bad idea.

[jberry@…]

The current code already silently ignores delete of options that aren't there.

You can currently do the following to your hearts desire:

my_option one two three my_option-delete four

No warning or error is given for that case. You could certainly argue that we want to issue a warning for that case, and I probably wouldn't argue against you. But that's not what this patch is about.

The follow is also currently allowed with no errors:

my_option "" my_option-delete four

This patch simply keeps the following from failing with a fatal TCL error:

my_option-delete "four"

The fact that this fails, currently, while the example above succeeds, is a bug. There should be no difference between an option that is set to "", and an option that has not been set. This patch just gets rid of the difference between those two cases. If you want to additionally stipulate that there should be a warning when somebody tries to delete an option that isn't there, that's fine, but that's an issue orthagonal to this patch.

I heard somebody complaining about this bug, or one similar to it, looked at the source, found this problem, and wrote a patch for it. But I don't have enough energy to waste breath defending something so trivial. It's not worth making it this hard.

[pguyot (Paul Guyot)]

Committed. Thanks James. A test case could be: my_option foo

variant bar { my_option-delete foo }

variant foobar { my_option-delete foo }