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

msvc: fix -I"" for same directory pch header #231

Merged
merged 1 commit into from
Apr 8, 2023

Conversation

Kojoley
Copy link
Contributor

@Kojoley Kojoley commented Mar 28, 2023

It would be nice to fix that by doing :D:E=. but unfortunately it does not work.

Fixes #229

@Kojoley
Copy link
Contributor Author

Kojoley commented Mar 28, 2023

Blah, this will blow up if it will expand to an absolute path, any ideas of a simple fix? Workarounding the lack of :D:E=. requires a lot of boilerplate here :-(

@Kojoley
Copy link
Contributor Author

Kojoley commented Mar 28, 2023

I would say that the bug is in B2 :D expanding, filed #232.

@grafikrobot
Copy link
Member

The only semi non-hack solution I can think of is doing the dir/file split in the rule instead of the action. That way the dir will end up as an empty list instead of the empty string in the list:

rule compile-c-c++ ( targets + : sources * )
{
    DEPENDS $(<[1]) : [ on $(<[1]) return $(PCH_HEADER) ] ;
    DEPENDS $(<[1]) : [ on $(<[1]) return $(PCH_FILE) ] ;
    PDB_NAME on $(<) = $(<[1]:S=.pdb) ;
    LOCATE on $(<[1]:S=.pdb) = [ on $(<[1]) return $(LOCATE) ] ;
    PCH_HEADER_DIR on $(<) = $(PCH_HEADER:D) ;
    PCH_HEADER_DIR on $(<) ?= "." ;
    PCH_HEADER on $(<) = $(PCH_HEADER:D=) ;
}

And the action uses -I"$(PCH_HEADER_DIR)" -FI"$(PCH_HEADER)".

@Kojoley
Copy link
Contributor Author

Kojoley commented Mar 30, 2023

That what I called a workaround :-) I would prefer :D or :D:E=. to just work...

@Kojoley
Copy link
Contributor Author

Kojoley commented Apr 1, 2023

The only semi non-hack solution I can think of is doing the dir/file split in the rule instead of the action. That way the dir will end up as an empty list instead of the empty string in the list:

rule compile-c-c++ ( targets + : sources * )
{
    DEPENDS $(<[1]) : [ on $(<[1]) return $(PCH_HEADER) ] ;
    DEPENDS $(<[1]) : [ on $(<[1]) return $(PCH_FILE) ] ;
    PDB_NAME on $(<) = $(<[1]:S=.pdb) ;
    LOCATE on $(<[1]:S=.pdb) = [ on $(<[1]) return $(LOCATE) ] ;
    PCH_HEADER_DIR on $(<) = $(PCH_HEADER:D) ;
    PCH_HEADER_DIR on $(<) ?= "." ;
    PCH_HEADER on $(<) = $(PCH_HEADER:D=) ;
}

And the action uses -I"$(PCH_HEADER_DIR)" -FI"$(PCH_HEADER)".

That won't work for two reasons:

  • Empty variable i.e "" is not the same thing as unset, so ?= "." will do nothing here.
  • Even if it worked for empties, there could be multiple precompile headers at the same time.

@Kojoley Kojoley force-pushed the fix-msvc-samedir-pch branch from d18e8f6 to ad24683 Compare April 2, 2023 01:21
@Kojoley
Copy link
Contributor Author

Kojoley commented Apr 2, 2023

I realized it is might be wrong to create PCH with only header name, it is preferable to use the exact path the user typed, but it seems to hard to obtain it? Also made some refactoring as a drive by.

@@ -689,15 +689,15 @@ rule compile.c ( targets + : sources * : properties * )
{
set-setup-command $(targets) : $(properties) ;
get-rspline $(targets) : -TC CFLAGS ;
compile-c-c++ $(<) : $(>) [ on $(<) return $(PCH_FILE) ] [ on $(<) return $(PCH_HEADER) ] ;
compile-c-c++ $(<) : $(>) ;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually not sure why it was done like this, maybe I miss something, like it's need to run header scanning or something else?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe header scanning. But not sure on that. It does seem rather weird.

@Kojoley Kojoley force-pushed the fix-msvc-samedir-pch branch 2 times, most recently from 5801366 to 0b933da Compare April 2, 2023 02:29
Pass to `-Yc` path as was typed by user (not exactly so, B2 automatically turns / to \ on Windows, so force the / in variable expansion).
@Kojoley Kojoley force-pushed the fix-msvc-samedir-pch branch from 0b933da to 65fa7a5 Compare April 2, 2023 03:22
@grafikrobot grafikrobot self-assigned this Apr 3, 2023
@grafikrobot grafikrobot added bug Something isn't working backport version/4.10.0 Backport to release 4.10.0 labels Apr 3, 2023
@grafikrobot grafikrobot merged commit 7cc58a8 into bfgroup:main Apr 8, 2023
@grafikrobot
Copy link
Member

/backport

@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2023

Successfully created backport PR for version/4.10.0:

@Kojoley Kojoley deleted the fix-msvc-samedir-pch branch April 17, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport version/4.10.0 Backport to release 4.10.0 bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Boost.Math Github Actions error with C1083: Cannot open precompiled header file
2 participants