SiarheiFedartsou / fastlane-plugin-versioning

Extends fastlane versioning actions. Allows to set/get versions without using agvtool and do some other small tricks.
MIT License
504 stars 60 forks source link

Running actions replaces Swift Package Manager project file labels with generic names #59

Closed simondelphia closed 2 years ago

simondelphia commented 3 years ago

When I run e.g. increment_version_number_in_xcodeproj:

 increment_version_number_in_xcodeproj(
     xcodeproj: PROJECT_PATH,
     bump_type: options[:bump_type],
     scheme: SCHEME_PROD
)

It runs xcodebuild with a -resolvePackageDependencies flag, and then it replaces all the SPM specific names in the project file with generic things like RemoteSwiftPackageReference SwiftPackageProductDependency and BuildFile. Running regular fastlane actions does not do this. The project still compiles and runs though, since I guess these names are just commented out labels, but it would still be preferable for this not to happen (because those labels are helpful and because I don't want to have to discard git changes every time I run fastlane with this plugin).

For example:

B2CC9E5525647C9C00AC7158 /* Sentry in Frameworks */ = {isa = PBXBuildFile; productRef = B2CC9E5425647C9C00AC7158 /* Sentry */; };

becomes:

B2CC9E5525647C9C00AC7158 /* BuildFile in Frameworks */ = {isa = PBXBuildFile; productRef = B2CC9E5425647C9C00AC7158 /* SwiftPackageProductDependency */; };

This SO issue is similar but it is talking about SwiftLint and has no solution yet.

Also, if you add / remove files within your Xcode project (in the Xcode application) the project file reverts all these changes.

isadon commented 3 years ago

I am seeing this same issue and it is very annoying 😞. Any idea when this will be looked at?

jdouglas-nz commented 3 years ago

Hey Guys,

Do one of you have a small dummy project that I can repo in and add to the tests to fix? I have a feeling this is a behaviour of Xcodeproj - but I'll dig in deeper later today. EDIT: or indeed, my usage of Xcodeproj - as maybe SwiftPM dependencies need to be handled differently. I haven't actually worked with a SwiftPM based Xcodeproj - so maybe is time to have a play

isadon commented 3 years ago

@jdouglas-nz I did a bit of digging and also believe this is a behavior of Xcodeproj. I believe they may have fixed this here: https://github.com/CocoaPods/Xcodeproj/commit/bfe0a6d8082f7adb59d662272b6943af37d20976 but it hasn't made it to an actual release from them. The latest release 1.19.0 doesn't include that commit. I could be wrong on if it was really fixed in that commit.

jdouglas-nz commented 3 years ago

@donileo I'm sure ( hopefully ) it's possible for me to target a commit for a dependency of this lib. I'll run this through a unit test and confirm that this happens for me, target that commit in particular (temporarily until it is released for realsies), release, and then re-release by removing that commit freeze "hack". How's that sound?

isadon commented 3 years ago

@jdouglas-nz Here is a test project. Has some swift dependencies and a couple of schemes for dev/stage/prod, you will need to add a fastlane file with a lane and your versioning plugin to test I'm assuming.

Sounds good. Hopefully you can pin xcodeproj to the latest master commit, I actually tried but wasn't able to, I'm not a ruby/gem expert in anyway though. Remember though I'm just guessing that commit above fixes the issue so maybe something else will be required for a fix to this issue.

jdouglas-nz commented 3 years ago

@donileo TY for the test project! I can repro - which is always a good start :) stay tuned caller, working on this today!

jdouglas-nz commented 3 years ago

Small update: I've pointed Xcodeproj at a commit after the change in serialisation (ty @donileo!). Unfortunately, my test still fails :( I'm going to call it a day for today - but if anyone wants to have a go at it - my WIP is here.

I noticed that with @simondelphia's invocation - all the Schemes in @donileo's test project are updated - that seems like a bug to me 😅

jdouglas-nz commented 3 years ago

Decided to change tack and look at hacking a bit. If I assume that there is a containing git repo - I'm might monkey around with the diff by writing it out to a file, then reapplying an edited diff without those changes. I might raise a guidance issue with Xcodeproj too.

Quick question - this is a reasonable amount of work - if you guys were to commit the changes (as in, the BuildFile in Framework and friends) - would your normal workflow result in the non generic form (Alamofire in Framework) returning ?

simondelphia commented 3 years ago

Yes, any time the project file is modified (e.g. add/remove a file or moving a file in the project navigator) the non-generic form returns.

isadon commented 3 years ago

@jdouglas-nz Before you take on any work that may be too much can you just run xcodeproj against my test repo. First run the latest version and then the latest commit in master. If you can see that the culprit of this issue is in fact xcodeproj and that the latest commit fixes it I would just push them to release a new version and pin your project to that new version. As to what Xcodeproj commands to run I would run exactly the calls that your versioning plugin calls and see what you get.

isadon commented 3 years ago

@simondelphia @jdouglas-nz I believe this issue has actually been fixed on xcodeproj. Its just that a release with this fix has not been made as it was fixed after 1.19 which is their latest version. Please see https://github.com/CocoaPods/Xcodeproj/pull/799. I wouldn't do any additional work beyond seeing if you can get fastlane-plugin-versioning to pin to the latest commit of xcodeproj as a required library for this project. Hopefully if that can be done bundle will make sure to install that version of xcodeproj and therefore resolve the problem. If not we will need xcodeproj to release a new version.

simondelphia commented 3 years ago

Yea looks like they're a bit overdue for a release

jdouglas-nz commented 3 years ago

@donileo @simondelphia I definitely can pin to whatever commit is on the latest master. I'd just rather be able to make sure that it actually fixes the issue 😅 I couldn't confirm that when I tried, but if you guys are ok, I can push out a gem targeting the latest master

isadon commented 3 years ago

In my case I added gem 'xcodeproj', github: 'CocoaPods/Xcodeproj', ref: 'f9203916f67163cf739141c36162ffc679bf30da' to my fastlane/Pluginfile + re-ran bundle update and I think that’s giving me the desired effect of having fastlane-plugin-versioning using it as well. 🤞

isadon commented 3 years ago

@simondelphia can you test my suggested workaround above ^^^ and see if it works to resolve this issue? As far as I can see from my end I believe it does but can't say 100%. I went ahead and reset my swift package cache and committed the pbxproj back to the way that it should be (without the modifications this issue performed.) I then re-ran increment_build_number_in_xcodeproj and noticed that it just changed the build as expected which is 👍 .

simondelphia commented 3 years ago

Hm well after doing that I don't see the issue with the generic labels but now I am seeing files previously listed as (null) turning into BuildFile and Swift package references changing from eg XCRemoteSwiftPackageReference "firebase-ios-sdk" to XCRemoteSwiftPackageReference "firebase-ios-sdk.git"

And these also go away if you change something in the project file (e.g. move a file around).

So I guess the issue isn't resolved (at least not on that commit). Or this is a separate issue.

isadon commented 3 years ago

These are two separate issues:

  1. The null turning into BuildFile is a good thing. I think that’s an artifact of something else that changed it (erroneously) to Null.. So this is just undoing that.. I believe Xcode will do the same thing on you touching anything in Swift packages.

  2. The XCRemoteSwiftPackageReference "firebase-ios-sdk" to XCRemoteSwiftPackageReference "firebase-ios-sdk.git" is something I am also seeing but I actually believe this is a separate issue. This original issue looks fixed to me but now this one remains 😄 . I think we should file this one on xcodeproj and then we will need to retest again by pinning to the commit that fixes this one.

What I really don't get is why xcodeproj needs to touch additional stuff beyond just change the version number if its being told to do so.

jdouglas-nz commented 3 years ago

@donileo @simondelphia - hold the phone folks. I just realised my test project was the result of running fastlane-versioning-plugin on the project, as in, it was already Buildfile in <>. I have a green tick now! EDIT: soon to be preparing a release.

simondelphia commented 3 years ago

I believe Xcode will do the same thing on you touching anything in Swift packages.

Haven't seen Xcode do this, do you have any steps to achieve it?

isadon commented 3 years ago

I believe Xcode will do the same thing on you touching anything in Swift packages.

Haven't seen Xcode do this, do you have any steps to achieve it?

@simondelphia

  1. Undo any changes that you may have added (put back your .pbxproj back to the way it was.. with the (null) in place of the BuildFile.
  2. Go into Xcode and then tap on the Project (not the target) and then Swift Packages. Then double click any swift package you have there.. In the popup leave everything as is. Then hit ok. This is my procedure for resetting pbxproj back to normal.
  3. Check if your (null)'s got modified into Buildfile or removed somehow.
simondelphia commented 3 years ago

No, nothing changed unfortunately.

isadon commented 3 years ago

@jdouglas-nz So some more digging..

I'm curious why for something as simple as bumping a version number which entails incrementing a number: FastlaneCore::Project.new() needs to invoke xcodebuild with resolvePackageDependencies. Looking at https://github.com/fastlane/fastlane/blob/d6256d62c3da0a559acfa6f66a6b4a1955e9ecd8/fastlane_core/lib/fastlane_core/project.rb#L382 it seems that there is a way to tell it NOT to do any swift package resolution.

Given that, it means it won't ever touch the file which is what we want.. I mean why would we want any side effects of swift package resolution to be done for something as simple as bumping a build..

I'm not sure myself how to tell it to skip the package resolution but it seems maybe that’s an option you can pass when invoking FastlaneCore::Project.new(config) and XcodeProj::Project.open(params[:xcodeproj]).

simondelphia commented 3 years ago

Oh yea I was wondering the same thing a while ago

wassupdoc commented 3 years ago

Any update on this? I just started using the plugin today and ran across this issue with the SPM packages being changed to generic names

jdouglas-nz commented 2 years ago

Folks! Xcodeproj just pushed an update out - I wrote some tests on my fork, and the tests pass! could one of you pls try target my PR Branch and see if this fixes it for you? TY

simondelphia commented 2 years ago

Thanks for following up — I'm not seeing any of the problematic changes I noted before, so that's an improvement.

Unfortunately I am getting four lines of things labeled as "(null)" converting to "BuildFile" (not sure what they are). e.g.,:

- B207D0DE25B8A39C006D2BFF / (null) in Sources / = {isa = PBXBuildFile; }; + B207D0DE25B8A39C006D2BFF / BuildFile in Sources / = {isa = PBXBuildFile; };

And then a whole bunch of Swift Package references (not all of them, oddly) getting ".git" appended to them e.g.,:

- B2C23C08256D74180071A78F / XCRemoteSwiftPackageReference "firebase-ios-sdk" /, - B216930C256D85CB00F98045 / XCRemoteSwiftPackageReference "ios-client-sdk" /, + B2C23C08256D74180071A78F / XCRemoteSwiftPackageReference "firebase-ios-sdk.git" /, + B216930C256D85CB00F98045 / XCRemoteSwiftPackageReference "ios-client-sdk.git" /,

And these changes disappear if I then cause any changes to my project file (e.g., moving a file around in Xcode project navigator).

simondelphia commented 2 years ago

Seems to be the same result as what I posted here https://github.com/SiarheiFedartsou/fastlane-plugin-versioning/issues/59#issuecomment-804354889

simondelphia commented 2 years ago

Note I just updated my Pluginfile to

gem 'fastlane-plugin-versioning', git: 'https://github.com/jdouglas-nz/fastlane-plugin-versioning.git', branch: 'bugfix/#59-swiftpm-buildfile'

I assume there wasn't anything else I would have needed to do?

jdouglas-nz commented 2 years ago

yeah, @simondelphia , I would assume that is all you have to do.

This (null) stuff might be a different issue. It certainly would be easier to track it as a separate issue - if you don't mind 😅 .

I was in the process of writing a horrible hack to fix this - which involves moving the project file to a temp directory, creating a git repo, making the change, then seeing if there are any bashed SwiftPM stuff.. It feels.. pretty horrible tbh.

I'm not sure what else I can/should do -- so are you saying that even adding a new file through Xcode will randomly stuff around with these as well?

jdouglas-nz commented 2 years ago

I'll push out a release (which is basically just a bundle update, and a test that used to fail). Afterwards, I'll create a new issue re: the magical (null), and then see what I can do about that. It could well be that something about the gem 'fastlane-plugin-versioning', git: 'my-fork', branch: 'my branch' could be doing something too.

simondelphia commented 2 years ago

What about the .git suffixes appended to some of the Swift Package manager references? You're not seeing that at all in your tests? It didn't happen on all Swift Packages… I'm not sure why it would happen only for some.

randomly stuff around with these

I'm not sure what that means.

If I just move a file around in Xcode the only changes to my project are the file that's been moved around. I was just saying that after running this versioning plugin, I get all these additional changes in my project file. That would be fine if that was just a one time thing, but Xcode automatically reverts these changes if you modify the project file. So if you committed these changes each time you would be constantly going back and forth and having a lot of extraneous changes in git hiding the actual changes you want. So my temporary solution has been to introduce a manual step where you have to manually move a file back and forth to remove these extra changes. If I could automate that somehow that would be helpful, but I haven't come up with a simple way to do that yet.

lukeredpath commented 2 years ago

I can confirm that the latest version of this gem from master fixes this issue for me.

I also noticed the same issue as @simondelphia with .git suffixes being added - this appears to only happen if you are referencing that package using a .git URL in either your Xcode project SPM package list or in any local Package.swift files you might have in your project. I have resolved this issue by ensuring all of our package dependencies exclude the .git suffix in their URL.

Xcode doesn't let you edit the package URL for any packages you've added but you can find/replace in the xcodeproj file to fix this. I believe there are some known issues with Xcode package resolution getting confused if you have multiple references to an SPM package with and without the .git suffix so its probably a good idea to standardise on one of the other anyway.

simondelphia commented 2 years ago

Yea that works, thank you! — now I just need to figure out what the deal is with these (null) lines

Seems I can just delete those — I guess they're left over from files being deleted and Xcode not cleaning them up properly.

simondelphia commented 2 years ago

I'm going to go ahead and close this because it's working for me and seems like it worked for @donileo too. Thanks again for the follow-up @jdouglas-nz, this will be really helpful for our CD.

jdouglas-nz commented 2 years ago

Amazing !!

I might put something in the readme, at least until xcodeproj/Xcode SwiftPM stabilises 😅