1100851 - Don't print $DMD = '1' when enabling DMD

Status: RESOLVED FIXED

(Core :: DMD, defect)

Description

[Mike Hommey [:glandium]]

Spawned from bug 1097507 comment 11 as per IRC:

@@ +1329,3 @@
>  
> +  if (!e) {
> +    e = "1";

This causes a "$DMD = '1'" message to be printed out immediately afterwards, which is misleading. Can you instead leave |e| as nullptr, and then do this:

> if (e) {
>   StatusMsg("$DMD = '%s'\n", e);
> } else {
>   StatusMsg("$DMD is undefined\n", e);
> }

and then make Options::Options() handle the aDMDEnvVar==nullptr case? Thanks.

Comment 1

[Eric Rahm [:erahm]]

I'm not sure I understand the background of this, but previously DMD wouldn't be enabled if $DMD isn't set. Is that no longer the case? If so perhaps a more explicit message would be useful:

  - Default options: "DMD is enabled"
  - Options via $DMD: "DMD is enabled: ${DMD}"

Comment 2

[Nicholas Nethercote [inactive]]

With the new setup just specifying the DMD library via LD_PRELOAD/MOZ_REPLACE_MALLOC_LIB/whatever is enough to enable it.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Don't lie when printing $DMD's value at start-up

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8530068 [details] [diff] [review]
Don't lie when printing $DMD's value at start-up

Review of attachment 8530068 [details] [diff] [review]:
-----------------------------------------------------------------

I'm surprised you don't change the test files. The reason I didn't do it originally was that the tests were looking for $DMD = '1' in the output.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Tweak DMD to account for the fact that $DMD can now be undefined

You asked for it! :)  Here's a new patch that does more stuff to remove almost
all traces of $DMD=1.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8534140 [details] [diff] [review]
Tweak DMD to account for the fact that $DMD can now be undefined

Review of attachment 8534140 [details] [diff] [review]:
-----------------------------------------------------------------

::: memory/replace/dmd/DMD.h
@@ +150,5 @@
>  //   // - 1: The original format. Implemented in bug 1044709.
> +//   // - 2: Added the "mode" property under "invocation". Added in bug 1094552.
> +//   // - 3: The "dmdEnvVar" property under "invocation" can now be |null| if
> +//   //      the |DMD| environment variable is not defined. Done in bug 1100851.
> +//   "version": 3,

Is that actually a backwards incompatible change that requires bumping the version number?

::: memory/replace/dmd/test/script-diff-dark-matter1.json
@@ +3,2 @@
>   "invocation": {
> +  "dmdEnvVar": "--mode=dark-matter --sample-below=127",

Adding an argument would seem to be unrelated.

::: memory/replace/dmd/test/script-diff-live-expected.txt
@@ +1,5 @@
>  #-----------------------------------------------------------------
>  # dmd.py --filter-stacks-for-testing -o script-diff-live-actual.txt script-diff-live1.json script-diff-live2.json
>  
>  Invocation 1 {
> +  $DMD = '--mode=live --sample-below=127'

Likewise

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: Print the mode in DMD output

This is useful.

Comment 8

[Nicholas Nethercote [inactive]]

> Is that actually a backwards incompatible change that requires bumping the
> version number?

No, but it gives us an opportunity to document that it happened, and DMD version changing is not a big deal, because the output files aren't intended to have a long life.

> Adding an argument would seem to be unrelated.

Yeah. I was changing some of the $DMD=1 values so I thought I'd fix some inconsistencies and it didn't seem worth separating these into a separate patch.

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/edd717f27f37
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd3c5d90a40c

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/edd717f27f37
https://hg.mozilla.org/mozilla-central/rev/bd3c5d90a40c

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/24c139438218

Comment 12

[Nicholas Nethercote [inactive]]

Two follow-ups to fix trivial bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/24c139438218
https://hg.mozilla.org/integration/mozilla-inbound/rev/65640bc0aca0

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/65640bc0aca0