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

Variant groups #124

Closed
wants to merge 4 commits into from
Closed

Variant groups #124

wants to merge 4 commits into from

Conversation

eAi
Copy link

@eAi eAi commented Feb 20, 2017

This is an amended version of the variant groups branch. I'm not sure if this is the best way to submit changes.

While testing it, I noticed a few issues which are fixed in this pull request:

  • attempting to add a PBXVariantGroup as a child of a PBXGroup gives an exception as it isn't one of the expected types.
  • adding multiple localisation files adds duplicate PBXBuildFiles for the variant group, which causes builds to fail as Xcode tries to copy the same file repeatedly.

With these two fixes, I can confirm that the variant groups work as expected for our use case (localising the app icon name by localising the Info.plist file).

But...

One thing I did find is that the system will automatically add the variants to any existing variant group that exists with the same name - even when that variant group is for a different target. For example, our project has a 'tests' target that contains a InfoPlist.strings variant group that's stored in a separate folder. With the system as it is, any InfoPlist.strings files we add to the project get added to this variant group - even though we want to add them for our main target. This code in question is this:

for variant in self.objects.get_objects_in_section(u'PBXVariantGroup'):
    if variant.name == expected_name:

I don't entirely understand how all of this works, but to me this seems like the incorrect - or at least, unexpected behaviour. I believe you should be able to have multiple variant groups for the same file. I've worked around this by removing the exiting variant group.

I also noticed that adding a strings file that's outside the project causes an exception on this line in ProjectFiles.py - I have't fixed this:

library_path = os.path.join(u'$(SRCROOT)', os.path.split(file_ref.path)[0])

AttributeError: 'PBXVariantGroup' object has no attribute 'path'

Edwin Lyons added 2 commits February 20, 2017 14:02
… added. Without this change, there will be as many PBXBuildFiles are there are files in the PBXVariantGroup. This may not be the neatest way to do this!
@eAi eAi mentioned this pull request Feb 20, 2017
@eAi
Copy link
Author

eAi commented Feb 20, 2017

I see this breaks the test 'testAddVariantGroupFileToExistingGroup' - but I'd question if this test is correct. At least, with the behaviour that the test is expecting, my Xcode project won't build.

@eAi
Copy link
Author

eAi commented Feb 20, 2017

Additionally I found that I had to add the newly created variant group to the main group or it wouldn't show up in the Xcode project, which I did like this:

	variant_group = project.objects.get_objects_in_section(u'PBXVariantGroup')[0]

	for group in project.objects._sections[u'PBXGroup']:
		if group.get_name() == "CustomTemplate":
			group.add_child(variant_group)

This worked, but I'm almost certain it's the incorrect way to handle this in the general case.

@kronenthaler
Copy link
Owner

Thanks for the contribution. I need some information before merging this PR.

  1. For the AttributeError: 'PBXVariantGroup' object has no attribute 'path' good catch. this shouldn't be added as the file is not a library/framework.
  2. For what i saw on xcode generating, the files with the same name were added to the same variant group name. If your use case shows something different i would like to see the xcode project generated files, or at least to know what sequence of steps you took to arrive to that result.
  3. for the main group addition, again, it would be great if you can do the steps by hand and submit the xcode project file (project.pbxproj) so i can take a look of what is being added where in order to show variants on groups.
  4. I will check about creating/getting buildfile out the phase, but in a cleaner way.

@sebrk
Copy link

sebrk commented Mar 13, 2018

I've been looking through the threads here but I don't get what the status is on PBXVariantGroups? Is this merged or will the feature not be developed? Sorry if I missed something but please enlighten me.

Edit: I now see that #113 is open. So I guess this is still in the works sort of?

@kronenthaler
Copy link
Owner

kronenthaler commented Mar 13, 2018 via email

@sebrk
Copy link

sebrk commented Mar 13, 2018

OK thanks for the info. Would I be stupid to try the variants-group branch? I want to add InfoPlist.strings files only.

@kronenthaler
Copy link
Owner

Not at all, go ahead and try, if you can provide feedback it would be valuable.

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.

3 participants