Add support for windows linker args#1670
Add support for windows linker args#1670n1ght-hunter wants to merge 1 commit intowild-linker:mainfrom
Conversation
| arch: default_target_arch(), | ||
| // Infrastructure | ||
| should_fork: true, | ||
| arch: const { Architecture::from_target_lexicon(target_lexicon::HOST.architecture) }, |
There was a problem hiding this comment.
the default should probably be the build target architecture not the build host
n1ght-hunter
force-pushed
the
push-mmnvvqsnqxtl
branch
from
f5586b2 to
49fd37a
Compare
n1ght-hunter
force-pushed
the
push-mmnvvqsnqxtl
branch
from
49fd37a to
e841eb9
Compare
There was a problem hiding this comment.
I'd prefer not to have a separate file for consts. We don't do that kind of thing elsewhere. It's a bit like having a module called "traits". It looks like these constants are a mix of ELF-specific constants and constants that should just be in the top-level args module.
There was a problem hiding this comment.
Github is showing the entire file as new, which means we can't see what has changed. I'd say either leave all the linux argument parsing in args.rs, or we could do a PR before this one where we just copy the file and don't edit it.
| /// - `-m <emulation>` — overrides format to ELF when present | ||
| /// | ||
| /// Priority: `-m` overrides format from `--target`. Architecture comes from `--target` only. | ||
| pub(crate) fn detect_target(args: &[String]) -> Result<DetectedTarget> { |
There was a problem hiding this comment.
I think the --target flag might be too much for this PR, especially since it's non-standard. I worry that the discussion might get bogged down. What about splitting that to a later PR and for this one just accepting --flavor gnu or --flavor link as the first argument like LLD does?
| .long("APPCONTAINER") | ||
| .help("/APPCONTAINER - Specifies whether the app must run within an appcontainer process environment.") | ||
| .execute(|_, _| unimplemented_option("/APPCONTAINER")); | ||
| // /ARM64XFUNCTIONPADMINX64 - Specifies the minimum number of bytes of padding between x64 functions in ARM64X images. 17.8 |
There was a problem hiding this comment.
The comments seem to duplicate the help text. Also, they seem to be copied from Microsoft. I suspect for copyright reasons, we need to come up with our own descriptions of each flag. For unimplemented flags, it'd be fine to have the help text just say "unimplemented" and we can fill in our own descriptions as we implement them.
| r#"C:\Users\Samuel\AppData\Local\Temp\rustc7RL5Io\symbols.o"#, | ||
| "dummy.dummy.6cfbe55db138f4b-cgu.0.rcgu.o", | ||
| "dummy.3wxfnlvokcqcl6j45c8xeicgz.rcgu.o", | ||
| r#"C:\Users\Samuel\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libstd-efa6c7783284bd31.rlib"#, |
There was a problem hiding this comment.
Perhaps edit these lines to make them a bit shorter.
|
im pretty busy with work stuff but will get to this over the weekend |
3 participants
Requires #1629 to be merged before this can.
This pr adds windows style flags to the args parsing.
It also moves linux flags to linux modules.
This introduces target-lexicon as a new dep. for parsing -target which follows the llvm target triple spec.
Questions to be answered.