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

2.8.0: createIntermediateGroups puts localized resources to "Recovered References" #674

Closed
bonkey opened this issue Oct 2, 2019 · 2 comments · Fixed by #679
Closed

2.8.0: createIntermediateGroups puts localized resources to "Recovered References" #674

bonkey opened this issue Oct 2, 2019 · 2 comments · Fixed by #679

Comments

@bonkey
Copy link

bonkey commented Oct 2, 2019

It looks like regression introduced in XcodeGen 2.8.0, in 2.7.0 it doesn't happen. The result is similar to #70.

When all three conditions are fulfilled:

  1. Have localized resources in .lproj folders (it doesn't matter what kind of: .json, .strings, .stringsdict)
  2. Use createIntermediateGroups: true in options
  3. Have some other non-localizable and non-assets files beside localized ones, for example fonts.

Then some of the localized resources get wrong paths and as the result go to "Recovered References"

CleanShot 2019-10-02 at 17 45 37@2x

Resulted .xcodeproj contains incorrect path:

XcodeGenLocalizedResources.xcodeproj/project.pbxproj
14:		3422F3A00BD48DB63B5645B5 /* empty.json in Resources */ = {isa = PBXBuildFile; fileRef = "TEMP_783C5EA8-19C9-44E7-B649-D2C690B1C238" /* empty.json */; };
53:		"TEMP_B814C25E-952E-4060-8ED1-22B0D59FB4E3" /* en-CA */ = {isa = PBXFileReference; name = "en-CA"; path = "en-CA.lproj/empty.json"; sourceTree = "<group>"; };
54:		"TEMP_E3D63E89-AA09-4EE1-88FE-A418BC313CC8" /* en-US */ = {isa = PBXFileReference; name = "en-US"; path = "en-US.lproj/empty.json"; sourceTree = "<group>"; };
55:		"TEMP_F472D47B-75C6-4219-AAD7-353897C3D311" /* en */ = {isa = PBXFileReference; name = en; path = en.lproj/empty.json; sourceTree = "<group>"; };
219:		"TEMP_783C5EA8-19C9-44E7-B649-D2C690B1C238" /* empty.json */ = {
222:				"TEMP_F472D47B-75C6-4219-AAD7-353897C3D311" /* en */,
223:				"TEMP_B814C25E-952E-4060-8ED1-22B0D59FB4E3" /* en-CA */,
224:				"TEMP_E3D63E89-AA09-4EE1-88FE-A418BC313CC8" /* en-US */,

Sample project: xcodegen-localized.zip

When you remove App/Resources/fonts or comment out options – the project is generated correctly.

@piellarda
Copy link
Contributor

I'm noticing the same issue with one of my project and as mentioned 2.7.0 works.

@nivanchikov
Copy link
Contributor

Looks like the issue is related to the getGroup method in the SourceGenerator class

private func getGroup(path: Path, name: String? = nil, mergingChildren children: [PBXFileElement], createIntermediateGroups: Bool, isBaseGroup: Bool) -> PBXGroup {
  let groupReference: PBXGroup

  if let cachedGroup = groupsByPath[path] {
    var cachedGroupChildren = cachedGroup.children
    for child in children {
      if !cachedGroupChildren.contains(where: { $0.path == child.path && $0.sourceTree == child.sourceTree }) {
        cachedGroupChildren.append(child)
      }
    }
    ...

the empty.json file is not correctly added to the group since it's skipped during the check for the cached value. For example for this setup the children array has 3 values:

  • fonts
    -- path: 'fonts'
    -- sourceTree: .group
  • Localizable.strings
    -- path: nil
    -- sourceTree: .group
  • empty.json
    -- path: nil
    -- sourceTree: .group

since the Localizable.strings is already being cached before the empty.json child, the json file is skipped.

It probably makes sense to also include the check for the filename along with the existing conditions, e.g.

if !cachedGroupChildren.contains(where: { $0.name == child.name && $0.path == child.path && $0.sourceTree == child.sourceTree }) {
  cachedGroupChildren.append(child)
}

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.

3 participants