Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

iOS Bump Version is overwriting CFBundleShortVersionString #26

Open
chkpnt opened this issue Dec 16, 2019 · 8 comments
Open

iOS Bump Version is overwriting CFBundleShortVersionString #26

chkpnt opened this issue Dec 16, 2019 · 8 comments
Assignees

Comments

@chkpnt
Copy link

chkpnt commented Dec 16, 2019

When ios-bump-version is used in a classic build pipeline, the task's behaviour regarding versionName corresponds to the description ("leave blank to use existing value in info.plist"): CFBundleShortVersionString isn't overwritten.

But when the same task is used in a yaml-pipeline, CFBundleShortVersionString is overwritten by 1.0.$(Build.BuildId), which is the defaultValue from task.json.

- task: ios-bundle-version@1
  displayName: 'Bump iOS Versions in path/to/Info.plist'
  inputs:
    sourcePath: path/to/Info.plist
    versionCode: '$(Build.BuildNumber)'
@chkpnt
Copy link
Author

chkpnt commented Dec 16, 2019

Okay, I have to specify versionName explicitly:

- task: ios-bundle-version@1
  displayName: 'Bump iOS Versions in path/to/Info.plist'
  inputs:
    sourcePath: path/to/Info.plist
    versionCode: '$(Build.BuildNumber)'
    versionName: 

The yaml in my previous example have been generated by the yaml-export in the classic build pipleline, but obviously it doesn't behave in the same way. :-(

In my opinion, an empty versionName in the yaml is quite confusing. Therefore I suggest introducing a boolean like updateVersionName or a special versionName: keep to keep the yaml more readable.

@SkyeHoefling
Copy link
Collaborator

Can you include an example Info.plist, I think that will help in determining what the problem is. I just took a look at the code and if the versionName isn't specified it shouldn't do anything with it.

if(!isNullOrUndefined(versionName))
{
// ---- Current Bundle Short Version String:
tl.execSync("/usr/libexec/PlistBuddy", "-c \"Print CFBundleShortVersionString\" " + "\"" + sourcePath + "\"");
// ---- Set Bundle Short Version String:
tl.execSync("/usr/libexec/PlistBuddy", "-c \"Set :CFBundleShortVersionString " + versionName + "\" " + "\"" + sourcePath + "\"");
}

Having an example Info.plist will be useful to reproduce the problem to make an appropriate fix. Otherwise I'll just be taking educated guesses

@chkpnt
Copy link
Author

chkpnt commented Feb 26, 2020

Yeah, but versionName doesn't seem to be null or undefined anymore at this line, if a yaml-pipeline is used. It seems that Azure is replacing it with the defaultValue from task.json, which makes sense, of course, because that's what a default value is used to be. ;-)

I think the main issue I had was that I were confused by the fact I have to specify versionName explicitly. Maybe it's Azure's yaml-exporter, which could include the empty fields in the result.... But on the other hand, if you had a huge task with many fields left empty in the classic interface, the yaml would be huge....

I still think the yaml would be more readable if it contained a boolean or a special value as suggested in my second comment. But from my perspective, you can also close this issue.

I do not have any special in my Info.plist:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
	<key>CFBundleDevelopmentRegion</key>
	<string>$(DEVELOPMENT_LANGUAGE)</string>
	<key>CFBundleDisplayName</key>
	<string>DemoApp</string>
	<key>CFBundleExecutable</key>
	<string>$(EXECUTABLE_NAME)</string>
	<key>CFBundleIdentifier</key>
	<string>$(PRODUCT_BUNDLE_IDENTIFIER)</string>
	<key>CFBundleInfoDictionaryVersion</key>
	<string>6.0</string>
	<key>CFBundleName</key>
	<string>$(PRODUCT_NAME)</string>
	<key>CFBundlePackageType</key>
	<string>APPL</string>
	<key>CFBundleShortVersionString</key>
	<string>$(MARKETING_VERSION)</string>
	<key>CFBundleVersion</key>
	<string>1</string>
	<key>LSRequiresIPhoneOS</key>
	<true/>
	<key>UILaunchStoryboardName</key>
	<string>LaunchScreen</string>
	<key>UIRequiredDeviceCapabilities</key>
	<array>
		<string>armv7</string>
	</array>
	<key>UISupportedInterfaceOrientations</key>
	<array>
		<string>UIInterfaceOrientationPortrait</string>
		<string>UIInterfaceOrientationLandscapeLeft</string>
		<string>UIInterfaceOrientationLandscapeRight</string>
	</array>
	<key>UISupportedInterfaceOrientations~ipad</key>
	<array>
		<string>UIInterfaceOrientationPortrait</string>
		<string>UIInterfaceOrientationPortraitUpsideDown</string>
		<string>UIInterfaceOrientationLandscapeLeft</string>
		<string>UIInterfaceOrientationLandscapeRight</string>
	</array>
</dict>
</plist>

@SkyeHoefling
Copy link
Collaborator

You are absolutely right, I am thinking it may be best to update the versionName to have a default value of an empty string. Here is the code in question

{
"name": "versionName",
"type": "string",
"label": "Version Name (CFBundleShortVersionString)",
"defaultValue": "1.0.$(Build.BuildId)",
"required": false,
"helpMarkDown": "The version number shown to users (leave blank to use existing value in info.plist)."
},

The only problem with creating a default value of an empty string is this could be a breaking change across other integrations that expect it to be defaulted to "1.0.$(Build.BuildId)".

With all of this being said, I think we can safely bump the Major Version of this task to make this fix. That will prevent any breaking changes for people using v1.

Example Integration

- task: ios-bundle-version@2
  displayName: 'Bump iOS Versions in path/to/Info.plist'
  inputs:
    sourcePath: path/to/Info.plist
    versionCode: '$(Build.BuildNumber)'

Note that the task name is now ios-bundle-version@2 which will use the new version.

@SkyeHoefling
Copy link
Collaborator

The defaultValue is very useful in the Visual Editor because it gives the user an idea of what the versionName should be. If we do set the default value to an empty string, we should move the "1.0.$(Build.BuildId)" as an example to the helpMarkDown

@chkpnt
Copy link
Author

chkpnt commented Feb 26, 2020

Yes, that sounds to be a good idea!

@jamesmontemagno
Copy link
Owner

makes sense previously there was only the visual part :) open to PRS :)

@SkyeHoefling
Copy link
Collaborator

I need to look at the docs, but I think this means we need to support both a v1 and a v2 implementation side-by-side. If you don't do that how would a new installation know where to get the v1 code vs the v2code. At least that is my working theory, only the docs have the true answer.

I'll do some research and submit a PR when it is ready for review.

@SkyeHoefling SkyeHoefling self-assigned this Feb 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants