Skip to content

Always use -fmacro-prefix-map #23212

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Always use -fmacro-prefix-map #23212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42986e0
Always use -fmacro-prefix-map
aheejin Dec 18, 2024
ba62274
Fix test_dwarf_system_lib
aheejin Dec 19, 2024
7d15c0d
Merge branch 'main' into macro_prefix
aheejin Dec 21, 2024
62b8149
Automatic rebaseline of codesize expectations. NFC
aheejin Dec 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion embuilder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ def main():
library.erase()
if do_build:
if USE_NINJA:
library.generate()
library.generate(deterministic_paths=True)
else:
library.build(deterministic_paths=True)
elif what == 'sysroot':
Expand Down
33 changes: 20 additions & 13 deletions tools/system_libs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
# link time.
USE_NINJA = int(os.environ.get('EMCC_USE_NINJA', '0'))

FAKE_EMSCRIPTEN_PATH = '/emsdk/emscripten'


def files_in_path(path, filenames):
srcdir = utils.path_from_root(path)
Expand Down Expand Up @@ -427,8 +429,8 @@ def build(self, deterministic_paths=False):
self.deterministic_paths = deterministic_paths
return cache.get(self.get_path(), self.do_build, force=USE_NINJA == 2, quiet=USE_NINJA)

def generate(self):
self.deterministic_paths = False
def generate(self, deterministic_paths=False):
self.deterministic_paths = deterministic_paths
return cache.get(self.get_path(), self.do_generate, force=USE_NINJA == 2, quiet=USE_NINJA,
deferred=True)

Expand Down Expand Up @@ -469,12 +471,15 @@ def generate_ninja(self, build_dir, libname):
utils.safe_ensure_dirs(build_dir)

cflags = self.get_cflags()
source_dir = utils.path_from_root()
relative_source_dir = os.path.relpath(source_dir, build_dir)
if self.deterministic_paths:
source_dir = utils.path_from_root()
relative_source_dir = os.path.relpath(source_dir, build_dir)
cflags += [f'-ffile-prefix-map={source_dir}=/emsdk/emscripten',
f'-ffile-prefix-map={relative_source_dir}/=',
'-fdebug-compilation-dir=/emsdk/emscripten']
cflags += [f'-ffile-prefix-map={source_dir}={FAKE_EMSCRIPTEN_PATH}',
f'-ffile-prefix-map={relative_source_dir}={FAKE_EMSCRIPTEN_PATH}',
'-fdebug-compilation-dir={FAKE_EMSCRIPTEN_PATH}']
else:
cflags += [f'-fmacro-prefix-map={source_dir}={FAKE_EMSCRIPTEN_PATH}',
f'-fmacro-prefix-map={relative_source_dir}={FAKE_EMSCRIPTEN_PATH}']
asflags = get_base_cflags(preprocess=False)
input_files = self.get_files()
ninja_file = os.path.join(build_dir, 'build.ninja')
Expand All @@ -492,13 +497,15 @@ def build_objects(self, build_dir):
commands = []
objects = set()
cflags = self.get_cflags()
source_dir = utils.path_from_root()
relative_source_dir = os.path.relpath(source_dir, build_dir)
if self.deterministic_paths:
source_dir = utils.path_from_root()
if batch_inputs:
relative_source_dir = os.path.relpath(source_dir, build_dir)
cflags += [f'-ffile-prefix-map={relative_source_dir}/=']
cflags += [f'-ffile-prefix-map={source_dir}=/emsdk/emscripten',
'-fdebug-compilation-dir=/emsdk/emscripten']
cflags += [f'-ffile-prefix-map={relative_source_dir}={FAKE_EMSCRIPTEN_PATH}']
Copy link
Collaborator

@sbc100 sbc100 Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you split this line out into its own PR? Something like "system_libs.py: Use the same deterministic path when building in batch mode". This seems like a separate and uncontroversial bugfix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done: #23222

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great!

Once #23222 lands maybe the next step should just be to unconditionally enable determinisic_paths? i.e. just remove the non-deterministic mode. We can always add it back later if it turns out someone wants it... but I doubt they wil.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought what we were going to do was to unconditionally use -fmacro-prefix-map but use -ffile-prefix-map (which also enables -fdebug-prefix-map) only in deterministic_paths? This is basically what #23195 (comment) suggested but you also said maybe even that was not necessary... But anyway this PR does not remove the undeterministic mode. I can do either.

Copy link
Member Author

Choose a reason for hiding this comment

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

I rebased this PR on top of #23222 and #23225. This still uses deterministic_paths. Please let me know if you think it'd better go unconditionally set deterministic_paths and remove the variable, i.e., you don't need your real local paths in the debug info.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think just removing deterministic_paths and always doing it this way makes sense. Sorry for the back on forth on this.

cflags += [f'-ffile-prefix-map={source_dir}={FAKE_EMSCRIPTEN_PATH}',
'-fdebug-compilation-dir={FAKE_EMSCRIPTEN_PATH}']
else:
cflags += [f'-fmacro-prefix-map={relative_source_dir}={FAKE_EMSCRIPTEN_PATH}']
cflags += [f'-fmacro-prefix-map={source_dir}={FAKE_EMSCRIPTEN_PATH}']
case_insensitive = is_case_insensitive(build_dir)
for src in self.get_files():
ext = shared.suffix(src)
Expand Down
Loading