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

Visual Studio object file collision detection needs tweaking #1187

Closed
ratzlaff opened this issue Nov 2, 2018 · 2 comments · Fixed by #1191
Closed

Visual Studio object file collision detection needs tweaking #1187

ratzlaff opened this issue Nov 2, 2018 · 2 comments · Fixed by #1191

Comments

@ratzlaff
Copy link
Contributor

ratzlaff commented Nov 2, 2018

In verifying that #1182 has been addressed for the situation described by @WiwilaJuicebox, I realized that premake can still create conflicts with respect to object file naming.

With a file/folder structure of

files {	"dir1/file.cpp"
,	"dir2/file.cpp"
,	"file1.cpp"
}

premake will generate the following:
visual studio (2017) vcxproj:
The output location for file1.cpp is using the defaults, which eventually expands to $(IntDir)\file1.obj

  <ItemGroup>
    <ClCompile Include="dir1\file.cpp" />
    <ClCompile Include="dir2\file.cpp">
      <ObjectFileName>$(IntDir)\file1.obj</ObjectFileName>
    </ClCompile>
    <ClCompile Include="file1.cpp" />
  </ItemGroup>

gmake2 Makefile:
This generator one appears to do the right thing.

OBJECTS += $(OBJDIR)/file.o
OBJECTS += $(OBJDIR)/file1.o
OBJECTS += $(OBJDIR)/file11.o

gmake Makefile:
Outputs file1.cpp -> file1.o even though the duplicate file.cpp -> file1.o

OBJECTS := \
	$(OBJDIR)/file.o \
	$(OBJDIR)/file1.o \
	$(OBJDIR)/file1.o \

xcode4 project.pbxproj
This one appears to do the right thing. Everything is hashed uniquely.

/* Begin PBXBuildFile section */
 BDDCD3E181489593BFEC3A21 /* file.cpp in Sources */ = {isa = PBXBuildFile; fileRef = F69216E9EFF8185BE3937529 /* file.cpp */; };
 C919B364F8AB8256D3B8C1A4 /* file1.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A222D74C4510FDFED436DD8C /* file1.cpp */; };
 F91D0D82BC88CF34FB2C73C2 /* file.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 31D2508A2B3851FC1ED3AECA /* file.cpp */; };
/* End PBXBuildFile section */

Sure, it is trivial to rename files so that this doesn't happen in my own projects, but this is actually a situation I had to deal with recently while building fftw. I ended up building a static library with the 'file1.cpp'-type sources just to force a project configuration that would build them to unique outputs.

I'll try and work on a fix this weekend (no promises), but I have no problem if someone wants to get to it first. Mostly wanted to get the issue recorded so I won't forget about it later.

@tdesveauxPKFX
Copy link
Contributor

Thanks for reporting this and volunteering to fix this (even if you are unable to work on it in the end, really appreciate it).

Also, just fixing the vs generator should be enough, gmake2 will replace gmake at some point so there isn't much worth in spending time there.

@ratzlaff
Copy link
Contributor Author

ratzlaff commented Nov 6, 2018

Currently this appears to work in my simple test case (sorry, no PR yet)

diff --git a/src/base/oven.lua b/src/base/oven.lua
index e1911bab..b0ec46c1 100644
--- a/src/base/oven.lua
+++ b/src/base/oven.lua
@@ -691,6 +691,8 @@
 -- conflicting object file names (i.e. src/hello.cpp and tests/hello.cpp both
 -- create hello.o).
 --
+-- a file list of: src/hello.cpp, tests/hello.cpp and src/hello1.cpp also generates
+-- conflicting object file names - hello1.o
 
        function oven.assignObjectSequences(prj)
 
@@ -716,11 +718,27 @@
 
                        local sequences = bases[file.basename]
 
+                       local function addSequence(f, cfg)
+                               f.sequence = sequences[cfg] or 0
+                               sequences[cfg] = f.sequence + 1
+
+                               if f.objname ~= f.basename then
+                                       -- put objname into bases to show that file base has already been seen
+                                       if not bases[f.objname] then
+                                               bases[f.objname] = {}
+                                       end
+
+                                       if bases[f.objname][cfg] then
+                                               print("ERROR: Overwriting sequence information for cfg: " .. cfg.name .. " file: " .. cfg.objname)
+                                       end
+                                       bases[f.objname][cfg] = 1
+                               end
+                       end
+
                        for cfg in p.project.eachconfig(prj) do
                                local fcfg = p.fileconfig.getconfig(file, cfg)
                                if fcfg ~= nil and not fcfg.flags.ExcludeFromBuild then
-                                       fcfg.sequence = sequences[cfg] or 0
-                                       sequences[cfg] = fcfg.sequence + 1
+                                       addSequence(fcfg, cfg)
                                end
                        end
 
@@ -728,9 +746,7 @@
                        -- this around until they do. At which point I might consider just
                        -- storing the sequence number instead of the whole object name
 
-                       file.sequence = sequences[prj] or 0
-                       sequences[prj] = file.sequence + 1
-
+                       addSequence(file, prj)
                end)
        end

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 a pull request may close this issue.

2 participants