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

Don't treat "dependson" libraries as "links" libraries in Xcode #644

Merged
merged 3 commits into from
Jan 20, 2017

Conversation

macsforme
Copy link
Contributor

This patch corrects an issue with the xcode module where if you list a library you're building under "dependson," Xcode would add it to the target dependencies but also to the link dependencies, which is undesired. With this fix, you can now use "links" and "dependson" as desired and they both work as intended.

Fixes #631.

@macsforme
Copy link
Contributor Author

Any thoughts? I'm trying to port the BZFlag build system to premake5, but it will be easier for me to sell our other developers on it if our premake scripts don't require workarounds, such as for this bug.

Copy link
Member

@samsinsane samsinsane left a comment

Choose a reason for hiding this comment

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

This solution is expensive for large workspaces with many projects that depend on each other but don't link.

Consider making the internals of the loop a function instead with a simple build/link parameter.

Furthermore, instead of using nobuild to flag whether or not something should build, instead consider using build/link and checking if node.build != false. This will assume true and be much easier to read than "not no build".

@macsforme
Copy link
Contributor Author

I can rework it to get rid of the double negative wording.

Consider making the internals of the loop a function instead with a simple build/link parameter.

Can you explain more what you mean by this? I'm new to lua, but from what I've read the only way to test whether a value is in a table is by iterating through it. So, as long as we depend on the existing project.getdependencies to distinguish between links and dependson, I don't see any other way except iterating through the list.

I could, however, probably move my call to project.getdependencies("dependOnly") outside of the parent loop, since that result doesn't change between dependencies (I just realized that). That should increase efficiency somewhat.

@samsinsane
Copy link
Member

Basically, something like this:

function xcode.addDependency(tr, dep, build)
  -- create a child node for the dependency's xcodeproj
  local xcpath = xcode.getxcodeprojname(dep)
  local xcnode = tree.insert(tr.projects, tree.new(path.getname(xcpath)))
  xcnode.path = xcpath
  xcnode.project = dep
  xcnode.productgroupid = xcode.newid(xcnode.name, "prodgrp")
  xcnode.productproxyid = xcode.newid(xcnode.name, "prodprox")
  xcnode.targetproxyid  = xcode.newid(xcnode.name, "targprox")
  xcnode.targetdependid = xcode.newid(xcnode.name, "targdep")

  -- create a grandchild node for the dependency's link target
  local lprj = premake.workspace.findproject(prj.workspace, dep.name)
  local cfg = project.findClosestMatch(lprj, prj.configurations[1])
  node = tree.insert(xcnode, tree.new(cfg.linktarget.name))
  node.path = cfg.linktarget.fullpath
  node.cfg = cfg

  if build == false then
    node.nobuild = true
  end
end

-- Back inside xcode.buildprjtree
for _, dep in ipairs(project.getdependencies(prj, "linkOnly")) do
  addDependency(tr, dep, true)
end

for _, dep in ipairs(project.getdependencies(prj, "dependOnly")) do
  addDependency(tr, dep, false)
end

@macsforme
Copy link
Contributor Author

I fixed the inefficiency per your suggestion.

I looked at alternatives for the not nobuild line, but because some nodes are created outside of that loop, if there was a build variable which could be true, false, or nil, we would have to include items where build is either true or nil. Otherwise, we could test for build != false as you suggested, and exclude those files, but it seems to me that having a significant distinction between false and nil would be more confusing than the original not nobuild test (and kind of backwards from the conventional if test). If that's what you want I'll change it, but to me it seems less confusing the way it is now.

@samsinsane
Copy link
Member

Maybe renaming nobuild to exclude would be the better solution? It removes the issue of the variable name, and avoids the issue you described above. At this point I feel like I'm being a bit pedantic about this, but I've seen this trip people up before, plus it reads a bit silly "if not no build then" as opposed to "if build then" or "if not exclude then".

@macsforme
Copy link
Contributor Author

Changed nobuild to excludefrombuild (so it's clear what the item is being excluded from).

@tvandijck tvandijck merged commit 0e765b5 into premake:master Jan 20, 2017
@macsforme macsforme deleted the fix-xcode-dependson branch January 20, 2017 05:43
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.

Xcode: App that "dependson" a SharedLib creates "links" relationship instead
3 participants