1229541 - `mach build-backend --backend=VisualStudio` fails with "AttributeError: 'list' object has no attribute 'split'"

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

I pulled mozilla-central today, and now `./mach build-backend -b VisualStudio` doesn't work:

$ ./mach build-backend -b VisualStudio
 0:00.63 c:\Users\cpearce\src\mozilla\purple\objdir\_virtualenv\Scripts\python.exe c:\Users\cpearce\src\mozilla\purple\objdir\config.status --backend VisualStudio
Reticulating splines...
Traceback (most recent call last):
  File "c:\Users\cpearce\src\mozilla\purple\objdir\config.status", line 1007, in <module>
    config_status(**args)
  File "c:\Users\cpearce\src\mozilla\purple\python\mozbuild\mozbuild\config_status.py", line 178, in config_status
    the_backend.consume(definitions)
  File "c:\Users\cpearce\src\mozilla\purple\python\mozbuild\mozbuild\backend\base.py", line 133, in consume
    self.consume_finished()
  File "c:\Users\cpearce\src\mozilla\purple\python\mozbuild\mozbuild\backend\visualstudio.py", line 197, in consume_finished
    args = config.substs.get(v, '').split()
AttributeError: 'list' object has no attribute 'split'

It worked yesterday.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: MozReview Request: Bug 1229541 - Assume config.substs.get() already returns a list in VS mach build-backend backend. r?gps

Bug 1229541 - Assume config.substs.get() already returns a list in VS mach build-backend backend. r?gps

Comment 2

[Mike Hommey [:glandium]]

https://reviewboard.mozilla.org/r/26773/#review24221

::: python/mozbuild/mozbuild/backend/visualstudio.py:197
(Diff revision 1)
> -                args = config.substs.get(v, '').split()
> +                args = config.substs.get(v, '')

Of all the variables iterated here, only MOZ_PIXMAN_CFLAGS is a list. All the others are still strings, so this change would break their handling.

Note the change of type for MOZ_PIXMAN_CFLAGS comes from bug 1228463.

The best thing to do is to change the type of the other variables as well.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://reviewboard.mozilla.org/r/26773/#review24221

> Of all the variables iterated here, only MOZ_PIXMAN_CFLAGS is a list. All the others are still strings, so this change would break their handling.
> 
> Note the change of type for MOZ_PIXMAN_CFLAGS comes from bug 1228463.
> 
> The best thing to do is to change the type of the other variables as well.

I spoke to glandium on IRC, and he said he'd make the change to the build system to change the types of  ('NSPR_CFLAGS', 'NSS_CFLAGS', 'MOZ_JPEG_CFLAGS', 'MOZ_PNG_CFLAGS', 'MOZ_ZLIB_CFLAGS') to lists.

In the meantime, we could make this:

    args = config.substs.get(v, '')
    if type(args) is str:
        args = args.split()
                  
And then we'll handle both types here. Then we can land this and unblock people.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Turn NSPR_CFLAGS, NSS_CFLAGS, MOZ_JPEG_CFLAGS, MOZ_PNG_CFLAGS and MOZ_ZLIB_CFLAGS into AC_SUBST_LISTs

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8694477 [details]
MozReview Request: Bug 1229541 - Assume config.substs.get() already returns a list in VS mach build-backend backend. r?gps

https://reviewboard.mozilla.org/r/26773/#review24233

::: python/mozbuild/mozbuild/backend/visualstudio.py:197
(Diff revision 1)
> -                args = config.substs.get(v, '').split()
> +                args = config.substs.get(v, '')

This is r=me with [] instead of '', if this lands along my additional patch :)

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/16ee2f31ce5b
https://hg.mozilla.org/integration/mozilla-inbound/rev/0221c49f76d2

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8694477 [details]
MozReview Request: Bug 1229541 - Assume config.substs.get() already returns a list in VS mach build-backend backend. r?gps

I'll take glandium's r+ and land with it.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/16ee2f31ce5b
https://hg.mozilla.org/mozilla-central/rev/0221c49f76d2