Closed Bug 1426558 Opened 2 years ago Closed 2 years ago

Make autospider builds not rely on system libnspr

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

No description provided.
Comment on attachment 8938215 [details]
Bug 1426558 - Make autospider builds not rely on system libnspr.

https://reviewboard.mozilla.org/r/208966/#review214670

::: js/src/devtools/automation/autospider.py:459
(Diff revision 1)
> +    libpath = [os.path.join(OBJDIR, 'dist', 'bin')]
> +    if 'LD_LIBRARY_PATH' in os.environ:
> +        libpath.append(os.environ['LD_LIBRARY_PATH'])
> +    test_env['LD_LIBRARY_PATH'] = ':'.join(libpath)

This looks like it will only handle the jsapi-tests binary, not the invocations of the JS shell.

I think this would be better done way up above, next to where it is setting up MOZ_UPLOAD_DIR. 'env' is an alias for os.environ. Whether it was a good idea to alias it is another question, but as long as it is, it would be better to be consistent.

::: js/src/devtools/rootAnalysis/analyze.py:35
(Diff revision 1)
> +    libpath = [os.path.dirname(config['js'])]
> +    if 'LD_LIBRARY_PATH' in os.environ:
> +        libpath.append(os.environ['LD_LIBRARY_PATH'])
> +    e['LD_LIBRARY_PATH'] = ':'.join(libpath)

To be consistent, I'd rather this were

    bindir = os.path.dirname(config['js'])
    e['LD_LIBRARY_PATH'] = ':'.join(p for p in (e.get('LD_LIBRARY_PATH'), bindir) if p)

Either that, or expand out the PATH setting above similar to how you have it here. (Or make a function, or whatever. Just so long as it's consistent.)
Attachment #8938215 - Flags: review?(sphink)
Comment on attachment 8938215 [details]
Bug 1426558 - Make autospider builds not rely on system libnspr.

https://reviewboard.mozilla.org/r/208966/#review214686
Attachment #8938215 - Flags: review?(sphink) → review+
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/8e075bee2a52
Make autospider builds not rely on system libnspr. r=sfink
https://hg.mozilla.org/mozilla-central/rev/8e075bee2a52
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.