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

WIP: Rust: Make rlib depends on rmeta intermediary output #11707

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xclaesse
Copy link
Member

When compiling rlib, rustc can generate an intermediary rmeta file that
is enough to build another rlib that depends on it.

@@ -2317,7 +2323,8 @@ def generate_cython_compile_rules(self, compiler: 'Compiler') -> None:

def generate_rust_compile_rules(self, compiler):
rule = self.compiler_to_rule_name(compiler)
command = compiler.get_exelist() + ['$ARGS', '$in']
command = compiler.get_exelist() + ['--notify', '$notifyfile', '--emit', 'metadata', '--json=artifacts', '--error-format=json', '$ARGS', '$in']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like something we should generate two of, one to build rlibs with a wrapper and the other to execute rustc directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a temporary hack to experiment, ideally rustc should support --notify so we don't need a wrapper and don't need to switch to json output.

args[nargs++] = argv[i];
}
}
args[0] = "rustc";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't respect machine file overrides, I guess... ;)

Probably should take it as a CLI argument.

@@ -1805,6 +1805,12 @@ def _add_rust_project_entry(self, name: str, main_rust_file: str, args: Compiler

self.rust_crates[name] = crate

def _get_rust_dependency_name(self, target: build.BuildTarget, dependency: LibTypes) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If only there was a common backend class for this :(

@@ -1884,6 +1884,16 @@ def generate_rust_target(self, target: build.BuildTarget) -> None:
raise InvalidArguments('Unknown target type for rustc.')
args.extend(['--crate-type', cratetype])

# dependencies need to cause a relink, they're not just for ordering.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run-on sentence. It would be nice to have a comment with a link to what an rmeta file is.

type: dict[str]
since: 1.2.0
description: |
On rust targets this provides a map of library names to the crate name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalize rust. @sdroege is the use of library here correct?

@@ -1236,6 +1236,13 @@ def process_kwargs(self, kwargs):
if self.gnu_symbol_visibility not in permitted:
raise InvalidArguments('GNU symbol visibility arg {} not one of: {}'.format(self.gnu_symbol_visibility, ', '.join(permitted)))

rust_dependency_map = kwargs.get('rust_dependency_map', {})
if not isinstance(rust_dependency_map, dict):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like yet another thing that @dcbaker would have to rebase his build.py refactor on. Perhaps we should try to get that in first?

When compiling rlib, rustc can generate an intermediary rmeta file that
is enough to build another rlib that depends on it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants