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

Dependency scanning for .rc files on Windows #420

Closed
2 tasks done
dimitry-unified-streaming opened this issue Oct 24, 2024 · 5 comments · Fixed by #425
Closed
2 tasks done

Dependency scanning for .rc files on Windows #420

dimitry-unified-streaming opened this issue Oct 24, 2024 · 5 comments · Fixed by #425
Labels
question Further information is requested

Comments

@dimitry-unified-streaming
Copy link
Contributor

Make sure you completed the following tasks

Environment and version details

  • Operating System+version: Windows 11 23H2
  • Compiler+version: Visual Studio 2022 v17.11.5
  • Shell: cmd.exe (VS2022 Command Prompt)
  • B2 Version: B2 5.3.0 (OS=NT, jobs=8) (from b996299)

Describe your use case

In our build, we are using a number of version.rc files to define VS_VERSION_INFO resources. The resulting version.res files are linked into DLLs and executables to provide some user-visible version information in those binaries.

However, the version.rc files use C preprocessor statements such as #include "revision.h" to retrieve the exact build revision number, and it looks like B2 does not scan dependencies for .rc files at all?

That is, we have checked that touching a header file that is a dependency of these version.rc files will not result in B2 rebuilding the .rc files with rc.exe.

Looking at msvc.jam in the main branch, I see there are generators and compile rules for .rc files, but I cannot find anything relating to dependency checking.

How difficult would it be to add dependency checking similar to .c and .cpp files for .rc files?

At the moment this missing feature prevents us from doing proper incremental builds. This is only a problem because full builds take too long, though. :)

Describe the solution you'd like

Ideally, B2 should scan .rc files similarly to what it does now for .c and .cpp files, check whether any dependencies are updated, and if so, recompile the .rc files.

@dimitry-unified-streaming dimitry-unified-streaming added the enhancement New feature or request label Oct 24, 2024
@grafikrobot grafikrobot moved this to 🔖 Ready in BFG Tasks Oct 24, 2024
@grafikrobot
Copy link
Member

Thank you for bringing this up. Indeed there's no dependency scanning for RC files. Should be fairly easy to add though as it's the same as the C header scanning.

@grafikrobot grafikrobot added question Further information is requested and removed enhancement New feature or request labels Nov 15, 2024
@grafikrobot
Copy link
Member

Hm.. After checking on this realized that we already implement header dependency scanning for RC resource files. Do you have an example RC file that fails to be rebuilt when a dependency is changed?

@grafikrobot grafikrobot moved this from 🔖 Ready to 🆕 New in BFG Tasks Nov 15, 2024
@DimitryAndric
Copy link

Indeed, with our fairly large project we saw that b2 didn't rebuild whenever a generated header was updated, and this header was included in yet another header that was eventually included in both a cpp file and a rc file. I made a simple sample project, but there it does work correctly. I am currently unsure what is causing the behavior, so I will have to come up with a complete test case.

@DimitryAndric
Copy link

FWIW I have been successfully testing with the following diff applied:

diff --git a/src/tools/rc.jam b/src/tools/rc.jam
index ce94b9b82..ef4ef6e85 100644
--- a/src/tools/rc.jam
+++ b/src/tools/rc.jam
@@ -93,12 +93,17 @@ class res-scanner : scanner
     import path ;
     import regex ;
     import scanner ;
+    import toolset ;
     import virtual-target ;

     rule __init__ ( includes * )
     {
         scanner.__init__ ;
-        self.includes = $(includes) ;
+
+        # toolset.handle-flag-value is a bit of overkill, but it
+        # does correctly handle the topological sort of && separated
+        # include paths
+        self.includes = [ toolset.handle-flag-value <include> : $(includes) ] ;
     }

     rule pattern ( )

This is similar to the change in cpp.jam introduced in 290e284 ("topological sort ordered includes") by @swatanabe. For some reason the way our project is built up out of jamfiles all over the place, it triggers a condition where this toolset.handle-flag-value is needed to correctly find the dependencies of our .rc files.

Note there is also another improvement made to cpp.jam that did not get applied to rc.jam. When looking at the difference between cpp.jam and rc.jam I noticed that the regexes used to find the include statements were different. This was added in 13b8a68 ("Make C/C++/ObjC include directive scanning pattern more strict. (#362)") by @Lastique.

@dimitry-unified-streaming
Copy link
Contributor Author

I created a small project which illustrates the issue here: https://github.com/dimitry-unified-streaming/b2-rc-depends . The README contains a bit of info on how it is supposed to be built, but it is fairly straightforward.

I believe this issue can be fixed by the above diff, and adding the regex changes from #362 would also be nice.

dimitry-unified-streaming added a commit to dimitry-unified-streaming/b2 that referenced this issue Nov 19, 2024
This applies the same change as 290e284 did for `cpp.jam`.

Fixes bfgroup#420
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in BFG Tasks Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants