Closed Bug 779995 Opened 12 years ago Closed 6 years ago

configure.py: --disable-foo doesn't disable foo when foo has dependent features.

Categories

(Tamarin Graveyard :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: pnkfelix, Unassigned)

References

Details

Attachments

(1 file)

% ../configure.py --target=x86_64-darwin --mac-sdk=106 --enable-debug --enable-debugger --disable-exact-tracing % make % ./shell/avmshell -Dversion shell 2.1 debug-debugger build 7483:36aa7dd8026a features AVMSYSTEM_64BIT;AVMSYSTEM_UNALIGNED_INT_ACCESS;AVMSYSTEM_UNALIGNED_FP_ACCESS;AVMSYSTEM_LITTLE_ENDIAN;AVMSYSTEM_AMD64;AVMSYSTEM_MAC;AVMFEATURE_DEBUGGER;AVMFEATURE_ALLOCATION_SAMPLER;AVMFEATURE_JIT;AVMFEATURE_COMPILEPOLICY;AVMFEATURE_ABC_INTERP;AVMFEATURE_SELFTEST;AVMFEATURE_EVAL;AVMFEATURE_PROTECT_JITMEM;AVMFEATURE_SHARED_GCHEAP;AVMFEATURE_MEMORY_PROFILER;AVMFEATURE_CACHE_GQCN;AVMFEATURE_SAFEPOINTS;AVMFEATURE_INTERRUPT_SAFEPOINT_POLL;AVMFEATURE_SWF12;AVMFEATURE_SWF13;AVMFEATURE_SWF14;AVMFEATURE_SWF15;AVMFEATURE_SWF16;AVMFEATURE_SWF17;AVMTWEAK_EXACT_TRACING; You see that bit at the end? Yeah, so the --disable-exact-tracing flag doesn't do anything of the sort. My current guess is that the way exact tracing was turned on permanently was not quite kosher with the expectations of our avmfeatures.as setup. There may be a slew of other options that fall into this category, where one needs to disable them by editing header files rather than running configure.py with particular flags. (So one plausible, if heavy-handed, way to fix this bug would be to replace the avmfeatures.as infrastructure.) Adobe Employees can see also for my recent brain dump on this topic: [~fklockii:/2012/08/01/Our ad-hoc tools for configuration and build settings] https://zerowing.corp.adobe.com/x/CoGgKw
Hmm. Looking at the generated avmfeatures.py script, I'm guessing the issue here is (at least in part) tbis difference in the generated code: if o.getBoolArg("exact-tracing"): args += "-DAVMTWEAK_EXACT_TRACING=1 -DAVMTWEAK_SELECTABLE_EXACT_TRACING=0 " source: http://hg.mozilla.org/tamarin-redux/file/5e07a25d4308/build/avmfeatures.py#l241 It only adds the arguments to set the flag to 1. There is no way this script will ever explicitly set it to 0, which is what avmfeatures.h needs to see in order to not use the default value for the tweak. Compare that with the far more frequent pattern for all the features that can be explicitly enabled/disabled: arg = o.getBoolArg("epoc-emulator") if (arg == True): args += "-DAVMTWEAK_EPOC_EMULATOR=1 " if (arg == False): args += "-DAVMTWEAK_EPOC_EMULATOR=0 " Looking at avmfeatures.as, I'm guessing that the source of this discrepancy is this conditional: // features without dependencies can be explicitly enabled or disabled if (dependent == "") { return (" arg = o.getBoolArg(\"" + feature + "\")\n" + " if (arg == True):\n" + " args += \"" + enable + "\"\n" + " if (arg == False):\n" + " args += \"" + disable + "\"\n"); } return (" if o.getBoolArg(\"" + feature + "\"):\n" + " args += \"" + enable + dependent + "\"\n"); source: http://hg.mozilla.org/tamarin-redux/file/5e07a25d4308/core/avmfeatures.as#l1191 Of course, the comment in the code snippet above immediately makes one wonder about the inverse statement: can features with dependencies not be explicitly enabled or disabled? And if so, why not -- just because it would complicate things too much to support it? That may be the case, but right now the system is extremely misleading, since if a feature like exact-tracing has a default value of being turned on, and also can only be meaningfully enabled by the logic in configure.py and never disabled, then what is the point of exposing it to configure.py at all?
This code went through a couple overly-complex revisions before I realized that one could solve this problem by simplifying! Namely, by combining the logic/features of the two pre-existing control paths. (Of course I still need to test that it all works out okay.) The comments may go over-board, but I wanted to get my notes down somewhere about the different design criteria that come into play with respect to how this should behave, and why the previous version was probably bogus (because using if o.getBoolArg(...): A else: B is very very different from arg = o.getBoolArg(...) if (arg == True): A if (arg == False): B and I don't know why we thought the former sufficed for this particular special case while we carefully used the latter everywhere else.
Summary: configure.py --disable-exact-tracing doesn't disable exact tracing (either remove the option or make it work) → configure.py: --disable-foo doesn't disable foo when foo has depedent features.
Summary: configure.py: --disable-foo doesn't disable foo when foo has depedent features. → configure.py: --disable-foo doesn't disable foo when foo has dependent features.
(In reply to Felix S Klock II from comment #2) > and I don't know why we thought the former sufficed for this particular > special case while we carefully used the latter everywhere else. Some of the relevant background can be found by reading the dialogue on Bug 487593 and by inspecting the changes associated with that bug, namely rev 2416: http://hg.mozilla.org/tamarin-redux/rev/2416#l2.30 Note in particular: The style that I am saying is bogus to use ("if o.getBoolArg(...):") was the original style of the generated code. The use of checks for the explicit True and False values was first introduced in changeset:2416, but was not universally applied. Thus this bug, as well as Bug 583742.
Blocks: 583742
Attachment #648560 - Flags: review?(brbaker)
Comment on attachment 648560 [details] [diff] [review] patch A v1: allow explicit disables in code generated via avmfeatures.as (bouncing review request over to dan)
Attachment #648560 - Flags: review?(brbaker) → review?(dschaffe)
Attachment #648560 - Flags: review?(dschaffe)
Tamarin is a dead project now. Mass WONTFIX.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Tamarin isn't maintained anymore. WONTFIX remaining bugs.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: