macfusion-ng / macfusion2

Macfusion2
225 stars 28 forks source link

version numbers broken #41

Closed core-code closed 5 years ago

core-code commented 6 years ago

the newest version still claims it is 2.0.5-dev although its up to 2.1.1:

https://github.com/macfusion-ng/macfusion2/blob/master/Resources/MacfusionMain-Info.plist

tessus commented 6 years ago

I've pushed a new branch version-info. I was looking into that 2 years ago, but I've never actually added it to the code.

Anyway, there might be room for improvement, but it will take the version number from the git tag. It also adds a version info pane in the preferences window. As I said, it could be improved, but have a look at it and let me know what you think.

core-code commented 6 years ago

i've checked out the "version-info" branch but was unable to build it on 10.13/Xcode 10

Preprocess /Users/julian/Library/Developer/Xcode/DerivedData/MacFusion2-hgrmguipdrlbqdcjvrbhcqfnvavx/Build/Intermediates.noindex/MacFusion2.build/Debug/MFCore.build/Preprocessed-Info.plist /Users/julian/Desktop/macfusion2-version-info/Resources/MFCore-Info.plist (in target: MFCore)
    cd /Users/julian/Desktop/macfusion2-version-info
    /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -E -P -x c -Wno-trigraphs /Users/julian/Desktop/macfusion2-version-info/Resources/MFCore-Info.plist -F/Users/julian/Library/Developer/Xcode/DerivedData/MacFusion2-hgrmguipdrlbqdcjvrbhcqfnvavx/Build/Products/Debug -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -include Resources/InfoPlist.h -o /Users/julian/Library/Developer/Xcode/DerivedData/MacFusion2-hgrmguipdrlbqdcjvrbhcqfnvavx/Build/Intermediates.noindex/MacFusion2.build/Debug/MFCore.build/Preprocessed-Info.plist

<built-in>:1:10: fatal error: 'Resources/InfoPlist.h' file not found
#include "Resources/InfoPlist.h"
         ^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Command Preprocess failed with a nonzero exit code

i am actually not sure where the VERSION_INFO etc replacements are supposed to happen.

but great to know that you are looking into this, thanks.

tessus commented 6 years ago

Hmm, this is very strange. I'm using macOS 10.13.6 17G3025 and Xcode 10.1 10B61 and it works when I compile it. I use this in all of my other projects as well. Never had any issues.

Here's how the process works:

I'm using a script in a target dependency (Generate Version Info), which creates the file Resources/InfoPlist.h. This file is used in the build settings (INFOPLIST_PREFIX_HEADER) for the project. If the setting INFOPLIST_PREPROCESS is set to yes, the placeholders in the Info.plist files will be replaced.

I suspect that for some reason the file is not created on your machine. Can you have a look at the build log? There should be more information on why that is.

Here's how the file looks on my machine:

$ cat Resources/InfoPlist.h
#define VERSION_INFO 2.1.1-dev.3.25
#define INTERNAL_VERSION_INFO 2.1.1-dev.3.25_cb725be
#define GIT_HASH_INFO cb725be
#define REVISION_INFO 210

#define VERSION "2.1.1-dev.3.25"
#define INTERNAL_VERSION "2.1.1-dev.3.25_cb725be"
#define GIT_HASH "cb725be"
#define REVISION "210"
core-code commented 6 years ago

thanks for the explanation.

i don't think this is weird at all, its quite easy to upload Xcode projects that only work on your Mac because you can introduce local dependencies or forget to check in some files without ever noticing.

i've had a look at the error, the same thing happens on a 10.14 Mac. i think i found the reason:

INFOPLIST_PREFIX_HEADER = Resources/InfoPlist.h

the file does not exist in Git: https://github.com/macfusion-ng/macfusion2/tree/version-info/Resources

tessus commented 6 years ago

No, this is fine. This file does not exist and must not exist in the repo.

It is dynamically created during the build process. This happens in the Generate Version Info step (target) that I described above.

screen shot 2018-11-14 at 20 50 14

Now, if you click on Target All, you will see Genenerate Version Info in the Target Dependencies.

Ah, how do you build the project?

You must set the target to All or run it via make on the command line. e.g. if you only build the target, let's say MacfusionMain, the file will not be created, beccause it is not in the target dependencies for that target. You could add it manually, though.

core-code commented 6 years ago

sorry took me a while to come back here

i can confirm everything builds fine if you select the 'All' target

the resulting build had version 0.0 (0) but i guess this is to be expected since the version numbers should come directly from git.

all in all looks good.

tessus commented 6 years ago

Yep, it retrieves the info from git.

As I said, the branch and changes (like the version in the window title) were just mockups. Those are easy things to change.

If you have suggestions, please let me know.

core-code commented 6 years ago

i think it looks like a good solution. the only possible issue with fetching the info from GIT i see is if the tags in GIT are not properly set up, or if the working copy is dirty and the last round of changes hasn't been committed yet - then you have the old tag in the version.

personally i haven't seen any use for embedding git tags in the version, and have used a much simpler system with a script embedded into the Xcode target that automatically increments the build number (CFBundleVersion) which consists of a single number on every build. a simple system is possible because we never have more than 1-2 targets per app, i see that your Xcode project is much more complicated so a more sophisticated solution may be preferable. also, in our case if the Info.plist is changed on every build automatically, so the working copy is dirty even if you just compile, which can get tedious.

in any case the most important thing is to make sure that all releases have the right version number. in our case this is ensured by deployment script that generates the final ZIP filename based on the CFShortVersion string, which makes it pretty much impossible to ship something with an outdated version number. if you'd be interested in a copy of the deployment script just let me know, happy to share.

ElDeveloper commented 6 years ago

@tessus this is super cool!!! Thanks for putting it together, I just had a chance to have a look at it and it is working very well.

If you want to submit a PR, that would be awesome.

i think it looks like a good solution. the only possible issue with fetching the info from GIT i see is if the tags in GIT are not properly set up, or if the working copy is dirty and the last round of changes hasn't been committed yet - then you have the old tag in the version.

That's a fair concern but if any of that happens in the future it is likely due to some manipulation on the repository itself, which should probably not be macfusion's responsibility.

tessus commented 6 years ago

Sure, although we could also merge the branch directly. ;-)

ElDeveloper commented 6 years ago

Call me old fashioned but I like code reviews :-]

On (Nov-26-18|15:24), Helmut K. C. Tessarek wrote:

Sure, although we could also merge the branch directly. ;-)

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/macfusion-ng/macfusion2/issues/41#issuecomment-441839002

ElDeveloper commented 5 years ago

This was closed by #42.