Closed Bug 1229541 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: cpearce, Unassigned)

References

Details

Attachments

(2 files)

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.
Bug 1229541 - Assume config.substs.get() already returns a list in VS mach build-backend backend. r?gps
Attachment #8694477 - Flags: review?(gps)
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.
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 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 :)
Attachment #8694477 - Flags: review+
Attachment #8694536 - Flags: review?(mshal) → review+
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.
Attachment #8694477 - Flags: review?(gps)
https://hg.mozilla.org/mozilla-central/rev/16ee2f31ce5b
https://hg.mozilla.org/mozilla-central/rev/0221c49f76d2
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Depends on: 1296502
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.