Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try enabling lld #179

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Try enabling lld #179

wants to merge 1 commit into from

Conversation

jdm
Copy link
Member

@jdm jdm commented Apr 29, 2019

This is an experiment.


This change is Reviewable

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #172) made this pull request unmergeable. Please resolve the merge conflicts.

@nox
Copy link
Contributor

nox commented Oct 8, 2019

Would that improve build speeds? If so, what are we waiting for? 😁

@jdm
Copy link
Member Author

jdm commented Oct 8, 2019

We were waiting for CI to actually work.

@nox
Copy link
Contributor

nox commented Oct 8, 2019

What happens if this uses lld but not the rest of our stuff btw? Isn't that a problem?

@jdm
Copy link
Member Author

jdm commented Oct 8, 2019

Fun fact: we use lld on Windows for everything already!

@nox
Copy link
Contributor

nox commented Oct 8, 2019

I'm a bit confused but I guess that wasn't my business anyway: I'm just wondering what's going on if we hardcode that flag in, but cargo itself doesn't use lld for its own linking step.

@jdm
Copy link
Member Author

jdm commented Oct 8, 2019

We already tell cargo to use lld on Windows in Servo's .cargo/config.

@nox
Copy link
Contributor

nox commented Oct 8, 2019

We already tell cargo to use lld on Windows in Servo's .cargo/config.

Sure, what I meant to say is: lld probably shouldn't be hardcoded here, we should probably pass RUSTC_LINKER or something like that.

RUSTC_LINKER - The path to the linker binary that Cargo has resolved to use for the current target, if specified. The linker can be changed by editing .cargo/config; see the documentation about cargo configuration for more information.

@jdm
Copy link
Member Author

jdm commented Oct 8, 2019

Windows CI failed with:

Traceback (most recent call last):
  File "c:/projects/mozjs/mozjs/js/src/../../configure.py", line 127, in <module>
    sys.exit(main(sys.argv))
  File "c:/projects/mozjs/mozjs/js/src/../../configure.py", line 29, in main
    sandbox.run(os.path.join(os.path.dirname(__file__), 'moz.configure'))
  File "c:\projects\mozjs\mozjs\python\mozbuild\mozbuild\configure\__init__.py", line 409, in run
    self._value_for(option)
  File "c:\projects\mozjs\mozjs\python\mozbuild\mozbuild\configure\__init__.py", line 477, in _value_for
    return self._value_for_option(obj)
  File "c:\projects\mozjs\mozjs\python\mozbuild\mozbuild\util.py", line 944, in method_call
    cache[args] = self.func(instance, *args)
  File "c:\projects\mozjs\mozjs\python\mozbuild\mozbuild\configure\__init__.py", line 542, in _value_for_option
    % option_string.split('=', 1)[0])
mozbuild.configure.options.InvalidOptionError: --enable-linker is not available in this configuration
mozmake: *** [maybe-configure] Error 1
thread 'main' panicked at 'assertion failed: result.success()', build.rs:142:5

No idea why.

@jdm
Copy link
Member Author

jdm commented Oct 8, 2019

Caused by

@depends(target)
def is_linker_option_enabled(target):
if target.kernel not in ('WINNT', 'SunOS'):
return True
.

@micwoj92
Copy link

This branch has conflicts that must be resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants