Merge pull request #2726 from xmake-io/modules

Improve modules and fix rule/before_build

6 files changed, 79 insertions, 93 deletions

diff --git a/xmake/actions/build/build.lua b/xmake/actions/build/build.lua
index 3eddf1610..0213935fe 100644
--- a/xmake/actions/build/build.lua
+++ b/xmake/actions/build/build.lua
@@ -37,27 +37,25 @@ function _clean_target(target)
     end
 end
 
--- add batch jobs for rules
-function _add_batchjobs_for_rules(batchjobs, rootjob, target, suffix)
+-- add builtin batch jobs
+function _add_batchjobs_builtin(batchjobs, rootjob, target)
 
     -- uses the rules script?
     local job, job_leaf
     for _, r in irpairs(target:orderules()) do -- reverse rules order for batchjobs:addjob()
-        local scriptname = "build" .. (suffix and ("_" .. suffix) or "")
-        local script = r:script(scriptname)
+        local script = r:script("build")
         if script then
-            if r:extraconf(scriptname, "batch") then
-                job, job_leaf = assert(script(target, batchjobs, {rootjob = job or rootjob}), "rule(%s):%s(): no returned job!", r:name(), scriptname)
+            if r:extraconf("build", "batch") then
+                job, job_leaf = assert(script(target, batchjobs, {rootjob = job or rootjob}), "rule(%s):on_build(): no returned job!", r:name())
             else
-                job = batchjobs:addjob("rule/" .. r:name() .. "/" .. scriptname, function (index, total)
+                job = batchjobs:addjob("rule/" .. r:name() .. "/build", function (index, total)
                     script(target, {progress = (index * 100) / total})
                 end, {rootjob = job or rootjob})
             end
         else
-            scriptname = "buildcmd" .. (suffix and ("_" .. suffix) or "")
-            local buildcmd = r:script(scriptname)
+            local buildcmd = r:script("buildcmd")
             if buildcmd then
-                job = batchjobs:addjob("rule/" .. r:name() .. "/" .. scriptname, function (index, total)
+                job = batchjobs:addjob("rule/" .. r:name() .. "/build", function (index, total)
                     local batchcmds_ = batchcmds.new({target = target})
                     buildcmd(target, batchcmds_, {progress =  (index * 100) / total})
                     batchcmds_:runcmds({dryrun = option.get("dry-run")})
@@ -65,14 +63,6 @@ function _add_batchjobs_for_rules(batchjobs, rootjob, target, suffix)
             end
         end
     end
-    return job, job_leaf or job
-end
-
--- add builtin batch jobs
-function _add_batchjobs_builtin(batchjobs, rootjob, target)
-
-    -- add batchjobs for rules
-    local job, job_leaf = _add_batchjobs_for_rules(batchjobs, rootjob, target)
 
     -- uses the builtin target script
     if not job and (target:is_static() or target:is_binary() or target:is_shared() or target:is_object()) then
@@ -135,6 +125,19 @@ function _add_batchjobs_for_target(batchjobs, rootjob, target)
         if after_build then
             after_build(target, {progress = progress})
         end
+        for _, r in ipairs(target:orderules()) do
+            local after_build = r:script("build_after")
+            if after_build then
+                after_build(target, {progress = progress})
+            else
+                local after_buildcmd = r:script("buildcmd_after")
+                if after_buildcmd then
+                    local batchcmds_ = batchcmds.new({target = target})
+                    after_buildcmd(target, batchcmds_, {progress = progress})
+                    batchcmds_:runcmds({dryrun = option.get("dry-run")})
+                end
+            end
+        end
 
         -- restore environments
         if oldenvs then
@@ -143,14 +146,8 @@ function _add_batchjobs_for_target(batchjobs, rootjob, target)
 
     end, {rootjob = rootjob})
 
-    -- add batchjobs for rules/build_after
-    local rules_job_build_after, rules_job_build_after_leaf = _add_batchjobs_for_rules(batchjobs, job_build_after, target, "after")
-
     -- add batch jobs for target, @note only on_build script support batch jobs
-    local job_build, job_build_leaf = _add_batchjobs(batchjobs, rules_job_build_after_leaf or job_build_after, target)
-
-    -- add batchjobs for rules/build_before
-    local rules_job_build_before, rules_job_build_before_leaf = _add_batchjobs_for_rules(batchjobs, job_build_leaf, target, "before")
+    local job_build, job_build_leaf = _add_batchjobs(batchjobs, job_build_after, target)
 
     -- add before_build job for target
     local job_build_before = batchjobs:addjob(target:name() .. "/before_build", function (index, total)
@@ -164,12 +161,26 @@ function _add_batchjobs_for_target(batchjobs, rootjob, target)
         end
 
         -- do before_build
+        -- we cannot add batchjobs for this rule scripts, @see https://github.com/xmake-io/xmake/issues/2684
         local progress = (index * 100) / total
         local before_build = target:script("build_before")
         if before_build then
             before_build(target, {progress = progress})
         end
-    end, {rootjob = rules_job_build_before_leaf or job_build_leaf})
+        for _, r in ipairs(target:orderules()) do
+            local before_build = r:script("build_before")
+            if before_build then
+                before_build(target, {progress = progress})
+            else
+                local before_buildcmd = r:script("buildcmd_before")
+                if before_buildcmd then
+                    local batchcmds_ = batchcmds.new({target = target})
+                    before_buildcmd(target, batchcmds_, {progress = progress})
+                    batchcmds_:runcmds({dryrun = option.get("dry-run")})
+                end
+            end
+        end
+    end, {rootjob = job_build_leaf})
     return job_build_before, job_build, job_build_after
 end
 
@@ -255,3 +266,4 @@ end
 
 
 
+
diff --git a/xmake/modules/private/async/jobpool.lua b/xmake/modules/private/async/jobpool.lua
index dfcf2b6d5..696515f08 100644
--- a/xmake/modules/private/async/jobpool.lua
+++ b/xmake/modules/private/async/jobpool.lua
@@ -148,10 +148,12 @@ end
 -- enter group
 --
 -- @param name      the group name
+-- @param opt       the options, e.g. {rootjob = ..}
 --
-function jobpool:group_enter(name)
+function jobpool:group_enter(name, opt)
+    opt = opt or {}
     assert(not self._group, "jobpool: cannot enter group(%s)!", name)
-    self._group = {name = name, group = true}
+    self._group = {name = name, group = true, rootjob = opt.rootjob}
 end
 
 -- leave group
@@ -161,8 +163,13 @@ end
 function jobpool:group_leave()
     local group = self._group
     self._group = nil
-    if group and group._parents then
-        return group
+    if group then
+        if group._parents then
+            return group
+        else
+            -- we just return the rootjob if there is not any jobs in this group
+            return group.rootjob
+        end
     end
 end
 
@@ -214,7 +221,7 @@ end
 -- tostring
 function jobpool:__tostring()
     local refs = {}
-    return string.serialize(self:_gentree(self:rootjob(), refs), {indent = 2})
+    return string.serialize(self:_gentree(self:rootjob(), refs), {indent = 2, orderkeys = true})
 end
 
 -- new a jobpool
diff --git a/xmake/rules/c++/modules/modules_support/clang.lua b/xmake/rules/c++/modules/modules_support/clang.lua
index fb3a2d1fa..ea11cbe1c 100644
--- a/xmake/rules/c++/modules/modules_support/clang.lua
+++ b/xmake/rules/c++/modules/modules_support/clang.lua
@@ -153,7 +153,7 @@ function generate_dependencies(target, sourcebatch, opt)
             changed = true
 
             local dependinfo = io.readfile(jsonfile)
-            return { moduleinfo = dependinfo }
+            return {moduleinfo = dependinfo}
         end, {dependfile = dependfile, files = {sourcefile}})
     end
     return changed
diff --git a/xmake/rules/c++/modules/modules_support/common.lua b/xmake/rules/c++/modules/modules_support/common.lua
index 92c92d465..ae1b1ca9e 100644
--- a/xmake/rules/c++/modules/modules_support/common.lua
+++ b/xmake/rules/c++/modules/modules_support/common.lua
@@ -459,41 +459,15 @@ function get_module_dependencies(target, sourcebatch, opt)
     return modules
 end
 
--- generate headerunits for batchjobs
-function generate_headerunits_for_batchjobs(target, batchjobs, sourcebatch, modules, opt)
-
-    -- get headerunits info
-    local headerunits, stl_headerunits = get_headerunits(target, sourcebatch, modules)
-
-    -- generate headerunits
-    -- build stl header units as other headerunits may need them
-    local headerunits_flags
-    if stl_headerunits then
-        headerunits_flags = headerunits_flags or {}
-        table.join2(headerunits_flags, modules_support(target).generate_stl_headerunits_for_batchjobs(target, batchjobs, stl_headerunits, opt))
-    end
-    if headerunits then
-        headerunits_flags = headerunits_flags or {}
-        table.join2(headerunits_flags, modules_support(target).generate_user_headerunits_for_batchjobs(target, batchjobs, headerunits, opt))
-    end
-    return headerunits_flags
-end
-
 -- generate headerunits for batchcmds
 function generate_headerunits_for_batchcmds(target, batchcmds, sourcebatch, modules, opt)
-
-    -- get headerunits info
     local user_headerunits, stl_headerunits = get_headerunits(target, sourcebatch, modules)
-
-    -- generate headerunits
     -- build stl header units as other headerunits may need them
-    if stl_headerunits or user_headerunits then
-        if stl_headerunits then
-            modules_support(target).generate_stl_headerunits_for_batchcmds(target, batchcmds, stl_headerunits, opt)
-        end
-        if user_headerunits then
-            modules_support(target).generate_user_headerunits_for_batchcmds(target, batchcmds, user_headerunits, opt)
-        end
+    if stl_headerunits then
+        modules_support(target).generate_stl_headerunits_for_batchcmds(target, batchcmds, stl_headerunits, opt)
+    end
+    if user_headerunits then
+        modules_support(target).generate_user_headerunits_for_batchcmds(target, batchcmds, user_headerunits, opt)
     end
 end
 
@@ -553,7 +527,7 @@ end
 function append_dependency_objectfiles(target)
     local cachekey = target:name() .. "dependency_objectfiles"
     local cache = localcache():get(cachekey)
-    if cache then 
+    if cache then
         if target:is_binary() then
             target:add("ldflags", cache, {force = true})
         elseif target:is_static() then
diff --git a/xmake/rules/c++/modules/modules_support/msvc.lua b/xmake/rules/c++/modules/modules_support/msvc.lua
index f7dc7451c..4439a578e 100644
--- a/xmake/rules/c++/modules/modules_support/msvc.lua
+++ b/xmake/rules/c++/modules/modules_support/msvc.lua
@@ -154,7 +154,7 @@ end
 -- generate header unit module bmi for batchjobs
 function generate_headerunit_for_batchjob(target, name, flags, objectfile, index, total)
     -- don't generate same header unit bmi at the same time across targets
-    if not common.memcache():get2(name, "generating") then 
+    if not common.memcache():get2(name, "generating") then
         local compinst = target:compiler("cxx")
         local toolchain = target:toolchain("msvc")
         local vcvars = toolchain:config("vcvars")
@@ -234,7 +234,7 @@ function generate_stl_headerunits_for_batchcmds(target, batchcmds, headerunits,
         local bmifile = path.join(stlcachedir, headerunit.name .. get_bmi_extension())
         local objectfile = bmifile .. ".obj"
         local flags = {
-            exportheaderflag, 
+            exportheaderflag,
             headernameflag .. ":angle",
             headerunit.name,
             ifcoutputflag,
@@ -429,14 +429,6 @@ function build_modules_for_batchjobs(target, batchjobs, objectfiles, modules, op
                         deps = table.keys(module.requires),
                         sourcefile = module.cppfile,
                         job = batchjobs:newjob(module.cppfile, function(index, total)
-                            function contains(t, v)
-                                for _, flag in ipairs(t) do
-                                    if table.contains(flag, v) then
-                                        return true
-                                    end
-                                end
-                                return false
-                            end
                             -- append module mapper flags
                             -- @note we add it at the end to ensure that the full modulemap are already stored in the mapper
                             local requiresflags = get_requiresflags(target, module.requires)
diff --git a/xmake/rules/c++/modules/xmake.lua b/xmake/rules/c++/modules/xmake.lua
index 062dea52b..4a7193710 100644
--- a/xmake/rules/c++/modules/xmake.lua
+++ b/xmake/rules/c++/modules/xmake.lua
@@ -52,32 +52,33 @@ rule("c++.build.modules.builder")
     set_sourcekinds("cxx")
     set_extensions(".mpp", ".mxx", ".cppm", ".ixx")
 
-    -- generate headerunits
-    -- parallel build support to accelerate `xmake build` to build headerunits
-    before_build(function(target, batchjobs, opt)
-        local job
-        if target:data("cxx.has_modules") then
-            import("modules_support.common")
-            local sourcebatch = target:sourcebatches()["c++.build.modules.builder"]
-            common.patch_sourcebatch(target, sourcebatch, opt)
-
-            -- generate headerunits
-            local modules = common.get_module_dependencies(target, sourcebatch, opt)
-            batchjobs:group_enter(target:name() .. "/generate_headerunits")
-            common.generate_headerunits_for_batchjobs(target, batchjobs, sourcebatch, modules, opt)
-            job = batchjobs:group_leave()
-        end
-        return job or opt.rootjob
-    end, {batch = true})
-
-    -- build modules
     -- parallel build support to accelerate `xmake build` to build modules
     before_build_files(function(target, batchjobs, sourcebatch, opt)
         if target:data("cxx.has_modules") then
             import("modules_support.common")
             common.patch_sourcebatch(target, sourcebatch, opt)
             local modules = common.get_module_dependencies(target, sourcebatch, opt)
+
+            -- build modules
             common.build_modules_for_batchjobs(target, batchjobs, sourcebatch, modules, opt)
+
+            -- generate headerunits and we need do it before building modules
+            local user_headerunits, stl_headerunits = common.get_headerunits(target, sourcebatch, modules)
+            if user_headerunits or stl_headerunits then
+                -- we need new group(headerunits)
+                -- e.g. group(build_modules) -> group(headerunits)
+                opt.rootjob = batchjobs:group_leave() or opt.rootjob
+                batchjobs:group_enter(target:name() .. "/generate_headerunits", {rootjob = opt.rootjob})
+                local modules_support = common.modules_support(target)
+                if stl_headerunits then
+                    -- build stl header units as other headerunits may need them
+                    -- TODO maybe we need new group(build_modules) -> group(user_headerunits) -> group(stl_headerunits)
+                    modules_support.generate_stl_headerunits_for_batchjobs(target, batchjobs, stl_headerunits, opt)
+                end
+                if user_headerunits then
+                    modules_support.generate_user_headerunits_for_batchjobs(target, batchjobs, user_headerunits, opt)
+                end
+            end
         else
             -- avoid duplicate linking of object files of non-module programs
             sourcebatch.objectfiles = {}