Skip to content

msvc: #1687: handle case where output file is a directory - #1836

Closed
AudranDoublet wants to merge 1 commit into
mozilla:mainfrom
AudranDoublet:bug/msvc/1687-guess-obj-name
Closed

AudranDoublet wants to merge 1 commit into
mozilla:mainfrom
AudranDoublet:bug/msvc/1687-guess-obj-name

Conversation

@AudranDoublet

Copy link
Copy Markdown
Contributor

As described in #1687, MSBuild may provide as output file (/Fo) a folder, ending with a trailing backslash, for example /FoRelease\.

msvc detects this case and chose {folder}/{input_file_name}.obj as output file. For example, for /cfoo.c /FoRelease\, the expected output path (by linker and msbuild) is Release/foo.obj.

Currently, we would use Release.obj, leading to:

  • concurrent write to the same object if the project contains multiple source
  • output file can't be found by other MSVC tools

The proposed fix simply check if the std::path::Path contains a trailing backslash (see https://users.rust-lang.org/t/trailing-in-paths/43166) and correctly guess the path if so.

Comment thread src/compiler/msvc.rs
}
Some(o) => {
if o.extension().is_none() && compilation {
if o.as_os_str()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sorry for the latency.
compilation is gone from the check, is that expected?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Np. It is intentional, at least. compilation is checked a bit earlier in this function (l760): this check was always true

@sylvestre
sylvestre force-pushed the bug/msvc/1687-guess-obj-name branch from a61a9a3 to cec7c09 Compare January 26, 2024 08:28
@sylvestre
sylvestre force-pushed the bug/msvc/1687-guess-obj-name branch from cec7c09 to d511d09 Compare February 20, 2024 12:41
@sylvestre
sylvestre force-pushed the bug/msvc/1687-guess-obj-name branch from d511d09 to 44905c6 Compare February 22, 2024 07:52
@sylvestre

Copy link
Copy Markdown
Collaborator

rustfmt isn't happy and some builds are failing
please reopen when they have been fixed

@sylvestre sylvestre closed this Feb 22, 2024
@jimis

jimis commented Oct 17, 2025

Copy link
Copy Markdown
Contributor

The bug exists in sccache-0.11: -Forelease\ does not write to the expected directory.
@AudranDoublet do you have time to refresh your patch? Else I could try to rebase it to HEAD myself.

@jimis

jimis commented Nov 6, 2025

Copy link
Copy Markdown
Contributor

Rebased and did minor updates to this pull request in #2482.

tottoto added a commit to tottoto/sccache that referenced this pull request Feb 6, 2026
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.

3 participants