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

Change our version habits #3628

Closed
Beep6581 opened this Issue Jan 24, 2017 · 44 comments

Comments

Projects
None yet
7 participants
@Beep6581
Owner

Beep6581 commented Jan 24, 2017

This issue is very important - git builds won't work well until it's fixed. I need everyone's help and input.

I'm writing this as a summary of hours of effort today to find a reliable solution to a problem I didn't know we had; to help you understand the problem, and to help myself see it clearly given the opportunity to lay it out here.

The way we form version numbers in CMake+other has become broken. I believe our system became obsolete the moment we switched to git. Up to now we would create a version of the form 4.2.1234. The form is wrong in the gitosphere, because you could end up with that number simultaneously on branches master, gtk3, locallab and pixelshift, for example. What does that number tell us then? Nothing. That sort of version was created using regular expressions to parse several chunks of git output, and the only reason it "worked" till now is because we didn't tag any releases of the form 4.2-beta1, 4.2-rc1, etc. It happened to work because we used numeric versions. Well now we have annotated tags 5.0-gtk2 and 5.0-gtk3, and everyone who compiles from git will run into a problem; even if the build succeeds, the version in AboutThisBuild and stored in the binary are wrong, and will cause trouble. Quote from an email:

When I run the app, I am getting the error:
Error: malformed version string; "5.0.5.0-gtk3" must follow this format: xx.xx.xx.xx. Admitting it's a developer version...
This is coming from ReleaseInfo.cmake which states:
set(GIT_VERSION 5.0.5.0-gtk2)

Where the version is used:

  • PP3 file header AppVersion=4.2.1234,
  • Cache key files, ~/.cache/RawTherapee/data/amsterdam.pef.0cb(...)32f2.txt:20:Version=4.2.1234
  • AboutThisBuild, Version: 4.2.1234
  • Window title,
  • Saved file metadata, -EXIF:Software=RawTherapee 4.2.1234
  • Console output
  • Options file
  • Some files in tools/, notably WindowsInnoSetup.iss.in

Git is flexible, but as such we cannot or should not stick to old models. RawTherapee can be compiled using several "starting points":

  • By checking out a branch, git checkout pixelshift
  • By checking out a commit, git checkout 4649e130
  • By checking out a tag the detached way, git checkout 5.0-gtk2
  • By checking out a tag the branch way, git checkout -b master-5.0 5.0-gtk2

Checking out in various ways means you have access to different things. It has serious consequences, it makes getting our old 4.2.1234 version form difficult or impossible or very dirty. I don't like it. Even if we did manage to script a way to get the 4.2.1234 version format, it wouldn't mean anything without more information such as a branch name, and most of the items listed above don't store one. Even if they did, we cannot always find a branch name!

There are several situations builders can find themselves in, I will give one example:
git checkout <branch>, then you would think you could find the "latest tag distance" using git describe --tags --always, and it could work, except... It will return different things depending on what your repo's history is.

/tmp/test $ git checkout master
Switched to branch 'master'
/tmp/test $ git branch
  gtk3
* master
/tmp/test $ git describe --tags --always
209c672
/tmp/test $ git tag -a "6.0-rc1" -m "Tagged RawTherapee 6.0 release candidate 1"
/tmp/test $ git describe --tags --always
6.0-rc1
/tmp/test $ echo "A" >> foo
/tmp/test $ git commit -a -m "second"
[master 00554e0] second
 1 file changed, 1 insertion(+)
/tmp/test $ git describe --tags --always
6.0-rc1-1-g00554e0

Sure there are a bunch of commands I could try, but in a script that is not reliable. And worse, this needs to be doable not only from a bash script, but also from AboutThisBuild.cmake.
Maybe there is a fool-proof way to get the info we want, but I haven't found it, and neither have these people 2 3 4

I propose we stop using our 4.2.1234 format.
We have at our disposal tags and commit hashes. A commit hash tells us all we need to know, but a tag is more friendly. We could use one, or both.

  • PP3 files don't need AppVersion at all, it's not used for anything.
  • I don't know about cache key files.
  • AboutThisBuild is for developers, it can use a commit hash.
  • Window title is for users, it can use a tag and/or commit hash.
  • Saved file metadata as above.
  • Console output as above.
  • Options file I don't know yet whether the version is used.
  • WindowsInnoSetup.iss.in I don't know.

Branch versionfix2 sets out to fix this.

I need your help to revise this and make sure it works.

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 24, 2017

Owner

The versionfix2 branch currently uses the commit hash. We need to figure out where and when to use the tag. Take into account feature branches such as locallab and pixelshift.
The script tools/generateReleaseInfo will need to be updated once we decide that.

Owner

Beep6581 commented Jan 24, 2017

The versionfix2 branch currently uses the commit hash. We need to figure out where and when to use the tag. Take into account feature branches such as locallab and pixelshift.
The script tools/generateReleaseInfo will need to be updated once we decide that.

@heckflosse

This comment has been minimized.

Show comment
Hide comment
@heckflosse

heckflosse Jan 24, 2017

Collaborator

👍 for not using meaningless suffixes. The commit hash is all we need.

Collaborator

heckflosse commented Jan 24, 2017

👍 for not using meaningless suffixes. The commit hash is all we need.

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 25, 2017

Owner

This also needs a solution: https://github.com/Beep6581/RawTherapee/blob/versionfix/rtgui/rtwindow.cc#L347
Do we pop it up on every commit change? Or only every tag change?
I'd go for on tag change only.

Owner

Beep6581 commented Jan 25, 2017

This also needs a solution: https://github.com/Beep6581/RawTherapee/blob/versionfix/rtgui/rtwindow.cc#L347
Do we pop it up on every commit change? Or only every tag change?
I'd go for on tag change only.

@Beep6581 Beep6581 changed the title from Change out version habits to Change our version habits Jan 25, 2017

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 25, 2017

I noticed that there was a problem with the new tagging method but was not sure how to file a bug report.

What I can say about windows building and innosetup

1-

for me the gtk2 or gtk3 appended to the version number is not useful as I can get this information in the AboutThisBuild.txt/field GTKMM, once make installation is finished.
Nevertheless, if you want to tag RC1,RC2,alpha.. the problem remains.

2-

To be able to generate a lot of installations, and be able to have a few RT installed simultaneously, I have to avoid collisions between

  • generated innosetup installation exe files
  • generated zip files to be uploaded
  • rawtherapee installation directory
  • uninstall registry entry

I modified for dev builds the windowsInnosetup.iss.in to include in names

  • branch name
  • version number
  • build type
  • bit depth

With this naming scheme, I am able to automatically perform building and uploading task

3- version number in innosetup (named MyAppFullVersion)

  • it should be the same as in the aboutthisbuild.txt
  • it should contain a sequence number to permit at first sight to compare age of two builds in same branch. (latesttagdistance was useful for that)
    In my builder and user opinion, the hash is meaningless in that purpose, and furthermore if I need it I can find it in the aboutthisbuild.txt.

4- to be sure what commits are included, I generate a changelog included in zipfile.

5- Yes pop up the window only on tag change

6- I think that stable release should have a version number 5.0,5.1.. At contrary 5.1.XXX should identify a dev release

7- I did not test yet versionfix

When decisions are made, the help of @Hombre57 will be needed to update windowsinnosetup

My windowsinnosetup.iss.in for development releases: https://paste.pound-python.org/show/KLDWuRAvk7WXqJG5Lc5Y/

André

gaaned92 commented Jan 25, 2017

I noticed that there was a problem with the new tagging method but was not sure how to file a bug report.

What I can say about windows building and innosetup

1-

for me the gtk2 or gtk3 appended to the version number is not useful as I can get this information in the AboutThisBuild.txt/field GTKMM, once make installation is finished.
Nevertheless, if you want to tag RC1,RC2,alpha.. the problem remains.

2-

To be able to generate a lot of installations, and be able to have a few RT installed simultaneously, I have to avoid collisions between

  • generated innosetup installation exe files
  • generated zip files to be uploaded
  • rawtherapee installation directory
  • uninstall registry entry

I modified for dev builds the windowsInnosetup.iss.in to include in names

  • branch name
  • version number
  • build type
  • bit depth

With this naming scheme, I am able to automatically perform building and uploading task

3- version number in innosetup (named MyAppFullVersion)

  • it should be the same as in the aboutthisbuild.txt
  • it should contain a sequence number to permit at first sight to compare age of two builds in same branch. (latesttagdistance was useful for that)
    In my builder and user opinion, the hash is meaningless in that purpose, and furthermore if I need it I can find it in the aboutthisbuild.txt.

4- to be sure what commits are included, I generate a changelog included in zipfile.

5- Yes pop up the window only on tag change

6- I think that stable release should have a version number 5.0,5.1.. At contrary 5.1.XXX should identify a dev release

7- I did not test yet versionfix

When decisions are made, the help of @Hombre57 will be needed to update windowsinnosetup

My windowsinnosetup.iss.in for development releases: https://paste.pound-python.org/show/KLDWuRAvk7WXqJG5Lc5Y/

André

@Floessie

This comment has been minimized.

Show comment
Hide comment
@Floessie

Floessie Jan 25, 2017

Collaborator

@gaaned92

6- I think that stable release should have a version number 5.0,5.1.. At contrary 5.1.XXX should identify a dev release

What about a bugfix for the stable release then?

Collaborator

Floessie commented Jan 25, 2017

@gaaned92

6- I think that stable release should have a version number 5.0,5.1.. At contrary 5.1.XXX should identify a dev release

What about a bugfix for the stable release then?

@sguyader

This comment has been minimized.

Show comment
Hide comment
@sguyader

sguyader Jan 25, 2017

Hi, should I wait for a definitive version scheme before making a gtk+ 3.18 build, or should I go ahead and release one from the versionfix branch already?

Edit: forget my stupid question, the versionfix branch is based on master of course... So I'll wait until there's a gtk3-based "versionfix" branch or tarball.

sguyader commented Jan 25, 2017

Hi, should I wait for a definitive version scheme before making a gtk+ 3.18 build, or should I go ahead and release one from the versionfix branch already?

Edit: forget my stupid question, the versionfix branch is based on master of course... So I'll wait until there's a gtk3-based "versionfix" branch or tarball.

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 25, 2017

@Beep6581
Versionfix :

as it is, the only indication of hash in RT window header will puzzle users.
I think that at least the tag (e.g. 5.1) should appear in aboutthisbuild.txt and in the RT Window (e.g. 5.1.96bf912) .
When you announce 5.1 is out, this number must appear somewhere.
If you tag for instance 5.1.RC2, where RC2 is intended to appear? It seems an important info.

And with this approach we loose the benefit of sequential numbering.

Is present Innosetup.iss.in compatible with this modification? No as In Innosetup.iss, MyAppFullVersion=5.0.1

How you intend to identify stable versions versus dev versions?

@Floessie At this time I can build stable version with slight modif in innosetup.
André

gaaned92 commented Jan 25, 2017

@Beep6581
Versionfix :

as it is, the only indication of hash in RT window header will puzzle users.
I think that at least the tag (e.g. 5.1) should appear in aboutthisbuild.txt and in the RT Window (e.g. 5.1.96bf912) .
When you announce 5.1 is out, this number must appear somewhere.
If you tag for instance 5.1.RC2, where RC2 is intended to appear? It seems an important info.

And with this approach we loose the benefit of sequential numbering.

Is present Innosetup.iss.in compatible with this modification? No as In Innosetup.iss, MyAppFullVersion=5.0.1

How you intend to identify stable versions versus dev versions?

@Floessie At this time I can build stable version with slight modif in innosetup.
André

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 26, 2017

Owner

@sguyader you can use the fixed source tarballs http://rawtherapee.com/shared/source/

Owner

Beep6581 commented Jan 26, 2017

@sguyader you can use the fixed source tarballs http://rawtherapee.com/shared/source/

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 26, 2017

Owner

Development versions are not releases, they are created only to help developers test things.

One option is to parse
git describe --tags --always --long
which gives us the tag + commits + hash.

Another option is to use tag+date for development builds:
git show -s --format=%cd --date=format:%Y-%m-%d

Owner

Beep6581 commented Jan 26, 2017

Development versions are not releases, they are created only to help developers test things.

One option is to parse
git describe --tags --always --long
which gives us the tag + commits + hash.

Another option is to use tag+date for development builds:
git show -s --format=%cd --date=format:%Y-%m-%d

@sguyader

This comment has been minimized.

Show comment
Hide comment
@sguyader

sguyader Jan 26, 2017

Hi, I just made a new "version-fixed" Gtk+ 3.18 build for Windows 64: https://filebin.net/m2cb0imbf03jnd9i
I tested very quickly, and it looks like the TooWaBlue theme cannot be selected in the settings menu.

Hi, I just made a new "version-fixed" Gtk+ 3.18 build for Windows 64: https://filebin.net/m2cb0imbf03jnd9i
I tested very quickly, and it looks like the TooWaBlue theme cannot be selected in the settings menu.

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 26, 2017

Owner
  • When checking out a branch or commit which has no tag in its history, git describe --tags --always --long shows only the commit hash.
  • When checking out a branch or commit which does have a tag in its history, git describe --tags --always --long shows <tag>-<numcommits>-g<commithash>
    Same when checking out a tag, or a branch on which there are no commits after the most recent tag.

When checking out a tag or commit, there is no branch information.

Owner

Beep6581 commented Jan 26, 2017

  • When checking out a branch or commit which has no tag in its history, git describe --tags --always --long shows only the commit hash.
  • When checking out a branch or commit which does have a tag in its history, git describe --tags --always --long shows <tag>-<numcommits>-g<commithash>
    Same when checking out a tag, or a branch on which there are no commits after the most recent tag.

When checking out a tag or commit, there is no branch information.

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 26, 2017

Owner

Proposed solution follows later today or tomorrow.

Owner

Beep6581 commented Jan 26, 2017

Proposed solution follows later today or tomorrow.

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 27, 2017

Owner

Branch versionfix2 pushed and ready for testing and merge.

Everyone please test!

The new versioning scheme revolves around git describe --tags --always.
There are generally two kinds of builds:

  • Releases
  • Development builds

Releases are only compiled from tags. Any compilation not made from a tag is not a release.

When you checkout a tag, RawTherapee will use that tag as the version everywhere. That satisfies the requirement for having releases use human-friendly names and paths and shortcut names. The version will be e.g. 5.0-gtk2 or 5.1-rc1 or 5.1.

When you checkout anything which is not a tag (a branch, a commit), RawTherapee will use the form
tag-commitcount-hash for example 5.0-gtk2-123-g87654321 or 5.1-rc1-3-g12ab34cd.

The form of the version, as you see, changes automatically based on what you check out, which guarantees that releases look nice, and development builds have a version which is actually precise and doesn't look bad.

This proper version is now used everywhere possible - AboutThisBuild, window title, PP3 header, installer, etc.

The Inno Setup installer, along with some aspects of Windows (right-click on the exe and look at properties), requires a numerical version only, nothing else of value. I was very inclined to set this to 0.0.0.0 and be done with this nonsense, because a numerical version alone means nothing. But because RawTherapee will still use a meaningful version everywhere else, and because some people feel that 5.0.1234 is better than 0.0.0.0 even if 5.0.1234 means nothing and is very misleading, I added support for a numerical version to be generated. This version is marked as unreliable, do not use it unless you have no other choice. InnoSetup's MyAppVersionNumeric requires the version to have the form n.n.n.n, so it uses this unreliable numeric version. I set MyAppVersion to the proper version (5.0-gtk2 or 5.0-gtk2-123-g87654321 depending on what you checkout), but I can't test whether Inno Setup will eat it. @gaaned92 @sguyader please test the installer.
Other changes to the installer are that it will install RawTherapee to
C:\Program Files\RawTherapee\5.0-gtk2\
C:\Program Files\RawTherapee\5.0-gtk3\
C:\Program Files\RawTherapee\5.0-gtk2-123-g87654321\
Users should be able to change the path and install anywhere they want to, the above are just the defaults. If the current script prevents the user from changing the path, then that still needs to be changed.

Owner

Beep6581 commented Jan 27, 2017

Branch versionfix2 pushed and ready for testing and merge.

Everyone please test!

The new versioning scheme revolves around git describe --tags --always.
There are generally two kinds of builds:

  • Releases
  • Development builds

Releases are only compiled from tags. Any compilation not made from a tag is not a release.

When you checkout a tag, RawTherapee will use that tag as the version everywhere. That satisfies the requirement for having releases use human-friendly names and paths and shortcut names. The version will be e.g. 5.0-gtk2 or 5.1-rc1 or 5.1.

When you checkout anything which is not a tag (a branch, a commit), RawTherapee will use the form
tag-commitcount-hash for example 5.0-gtk2-123-g87654321 or 5.1-rc1-3-g12ab34cd.

The form of the version, as you see, changes automatically based on what you check out, which guarantees that releases look nice, and development builds have a version which is actually precise and doesn't look bad.

This proper version is now used everywhere possible - AboutThisBuild, window title, PP3 header, installer, etc.

The Inno Setup installer, along with some aspects of Windows (right-click on the exe and look at properties), requires a numerical version only, nothing else of value. I was very inclined to set this to 0.0.0.0 and be done with this nonsense, because a numerical version alone means nothing. But because RawTherapee will still use a meaningful version everywhere else, and because some people feel that 5.0.1234 is better than 0.0.0.0 even if 5.0.1234 means nothing and is very misleading, I added support for a numerical version to be generated. This version is marked as unreliable, do not use it unless you have no other choice. InnoSetup's MyAppVersionNumeric requires the version to have the form n.n.n.n, so it uses this unreliable numeric version. I set MyAppVersion to the proper version (5.0-gtk2 or 5.0-gtk2-123-g87654321 depending on what you checkout), but I can't test whether Inno Setup will eat it. @gaaned92 @sguyader please test the installer.
Other changes to the installer are that it will install RawTherapee to
C:\Program Files\RawTherapee\5.0-gtk2\
C:\Program Files\RawTherapee\5.0-gtk3\
C:\Program Files\RawTherapee\5.0-gtk2-123-g87654321\
Users should be able to change the path and install anywhere they want to, the above are just the defaults. If the current script prevents the user from changing the path, then that still needs to be changed.

@Beep6581 Beep6581 referenced this issue Jan 27, 2017

Closed

New major release - RT5 #3300

26 of 26 tasks complete
@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 27, 2017

Owner

AboutThisBuild from master branch:

Branch: gtk3
Version: 5.0
Changeset: 7fe7c4f60f85b47bc1d24b2cfece43444028e3a6
(...)

AboutThisBuild from this branch:

Version: 5.0-gtk2-2-gea27213d
Branch: versionfix
Commit: ea27213d
Commit date: 2017-01-26
(...)
Owner

Beep6581 commented Jan 27, 2017

AboutThisBuild from master branch:

Branch: gtk3
Version: 5.0
Changeset: 7fe7c4f60f85b47bc1d24b2cfece43444028e3a6
(...)

AboutThisBuild from this branch:

Version: 5.0-gtk2-2-gea27213d
Branch: versionfix
Commit: ea27213d
Commit date: 2017-01-26
(...)
@heckflosse

This comment has been minimized.

Show comment
Hide comment
@heckflosse

heckflosse Jan 27, 2017

Collaborator

@Beep6581 Compiles and works fine

Collaborator

heckflosse commented Jan 27, 2017

@Beep6581 Compiles and works fine

@Floessie

This comment has been minimized.

Show comment
Hide comment
@Floessie

Floessie Jan 27, 2017

Collaborator

@Beep6581

When you checkout a tag, RawTherapee will use that tag as the version everywhere. That satisfies the requirement for having releases use human-friendly names and paths and shortcut names. The version will be e.g. 5.0-gtk2 or 5.1-rc1 or 5.1.

When you checkout anything which is not a tag (a branch, a commit), RawTherapee will use the form
tag-commitcount-hash for example 5.0-gtk2-123-g87654321 or 5.1-rc1-3-g12ab34cd.

That is simply brilliant! 👍

Collaborator

Floessie commented Jan 27, 2017

@Beep6581

When you checkout a tag, RawTherapee will use that tag as the version everywhere. That satisfies the requirement for having releases use human-friendly names and paths and shortcut names. The version will be e.g. 5.0-gtk2 or 5.1-rc1 or 5.1.

When you checkout anything which is not a tag (a branch, a commit), RawTherapee will use the form
tag-commitcount-hash for example 5.0-gtk2-123-g87654321 or 5.1-rc1-3-g12ab34cd.

That is simply brilliant! 👍

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 27, 2017

@Beep6581 I think the principle is quite good.
It should work for innosetup except that

#define MyAppVersion ""
#define MyAppVersionNumeric ""

are blank
Should'nt the GIT variables be redefined in RTDATA/cmakelist.txt?

Do I understand well : if you checkout a flag it is a stable release and if you checkout a branch it is a dev release?

gaaned92 commented Jan 27, 2017

@Beep6581 I think the principle is quite good.
It should work for innosetup except that

#define MyAppVersion ""
#define MyAppVersionNumeric ""

are blank
Should'nt the GIT variables be redefined in RTDATA/cmakelist.txt?

Do I understand well : if you checkout a flag it is a stable release and if you checkout a branch it is a dev release?

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 27, 2017

Owner

@gaaned92 I'll have a look at why it's blank.
If you checkout a tag (not flag) then it's a release - always will be. If you checkout anything which is not a tag, then you're by definition not checking out a release, and automatically get more info.

Owner

Beep6581 commented Jan 27, 2017

@gaaned92 I'll have a look at why it's blank.
If you checkout a tag (not flag) then it's a release - always will be. If you checkout anything which is not a tag, then you're by definition not checking out a release, and automatically get more info.

@Hombre57

This comment has been minimized.

Show comment
Hide comment
@Hombre57

Hombre57 Jan 27, 2017

Collaborator

Just to avoid confusion : wouldn't it be better to use the branch name release-candidate instead of release ?

Collaborator

Hombre57 commented Jan 27, 2017

Just to avoid confusion : wouldn't it be better to use the branch name release-candidate instead of release ?

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 27, 2017

Owner

@Hombre57 what are you referring to?

Owner

Beep6581 commented Jan 27, 2017

@Hombre57 what are you referring to?

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 28, 2017

Owner

@sguyader @gaaned92 could you please check whether versionfix2 produces a WindowsInnoSetup.iss file which Inno Setup doesn't choke on?

Owner

Beep6581 commented Jan 28, 2017

@sguyader @gaaned92 could you please check whether versionfix2 produces a WindowsInnoSetup.iss file which Inno Setup doesn't choke on?

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 28, 2017

@Beep6581

#define MyAppName "RawTherapee"
#define MyAppVersion "5.0-gtk2-4-g2356af62"
#define MyAppVersionNumeric "5.0.4"
#define MyAppPublisher "rawtherapee.com"
#define MyAppURL "http://www.rawtherapee.com/"
#define MyAppExeName "rawtherapee.exe"
#define MyBuildBasePath ""
#define MySourceBasePath "Y:/RTSOURCE/rawtherapee"
#define MyBitDepth ""
#define MyTargetArchitecture ""
#define MyInstallMode ""
#define MySystemName ""

MyBuildBasePath must be defined to be able to pack all the files in the installer

MyBitDepth, MySystemName (e.g. WinXP), MyInstallMode : I would like them filled to avoid collisions when I generate installers. (To do that I modify innosetup.iss.in on the fly) and avoid collisions in the registry for dev release.
An other solution is to generate 7zip .exe file for dev release and only use installer for stable release.

gaaned92 commented Jan 28, 2017

@Beep6581

#define MyAppName "RawTherapee"
#define MyAppVersion "5.0-gtk2-4-g2356af62"
#define MyAppVersionNumeric "5.0.4"
#define MyAppPublisher "rawtherapee.com"
#define MyAppURL "http://www.rawtherapee.com/"
#define MyAppExeName "rawtherapee.exe"
#define MyBuildBasePath ""
#define MySourceBasePath "Y:/RTSOURCE/rawtherapee"
#define MyBitDepth ""
#define MyTargetArchitecture ""
#define MyInstallMode ""
#define MySystemName ""

MyBuildBasePath must be defined to be able to pack all the files in the installer

MyBitDepth, MySystemName (e.g. WinXP), MyInstallMode : I would like them filled to avoid collisions when I generate installers. (To do that I modify innosetup.iss.in on the fly) and avoid collisions in the registry for dev release.
An other solution is to generate 7zip .exe file for dev release and only use installer for stable release.

@Hombre57

This comment has been minimized.

Show comment
Hide comment
@Hombre57

Hombre57 Jan 28, 2017

Collaborator

I'll take this.

Collaborator

Hombre57 commented Jan 28, 2017

I'll take this.

@Desmis

This comment has been minimized.

Show comment
Hide comment
@Desmis

Desmis Jan 28, 2017

Collaborator

I compile versionfix2, all is fine :)
I am not sure to all understand, I am even on the contrary, but my English is insufficient to take part in the debate, which exceeds me a little ... for me, it is Chinese;

What I understand, is to wait... :)

Collaborator

Desmis commented Jan 28, 2017

I compile versionfix2, all is fine :)
I am not sure to all understand, I am even on the contrary, but my English is insufficient to take part in the debate, which exceeds me a little ... for me, it is Chinese;

What I understand, is to wait... :)

@Hombre57

This comment has been minimized.

Show comment
Hide comment
@Hombre57

Hombre57 Jan 28, 2017

Collaborator

Comme nous sommes 4 français ici, pourquoi ne pas parler un peu français ? Il y a eu un changement mineur dans la manière dont sont créé les informations envoyées à InnoSetup. J'ai vérifié que les versions étaient bien présentes dans le fichier WindowsInnoSetup.iss, mais j'avoue n'avoir pas fait attention aux paramètres devenu vide. Je corrigerais cela ce soir car je suis pris toute la journée.

@Desmis As-tu compris le nouveau mécanisme de gestion des branchs de dev, release et master ou veux-tu que je traduise ? Pour ce qui est de produire une version installable, cela concerne surtout les personnes en charge du packaging.

@Beep6581

what are you referring to?

Sorry, was an answer to #3300

Collaborator

Hombre57 commented Jan 28, 2017

Comme nous sommes 4 français ici, pourquoi ne pas parler un peu français ? Il y a eu un changement mineur dans la manière dont sont créé les informations envoyées à InnoSetup. J'ai vérifié que les versions étaient bien présentes dans le fichier WindowsInnoSetup.iss, mais j'avoue n'avoir pas fait attention aux paramètres devenu vide. Je corrigerais cela ce soir car je suis pris toute la journée.

@Desmis As-tu compris le nouveau mécanisme de gestion des branchs de dev, release et master ou veux-tu que je traduise ? Pour ce qui est de produire une version installable, cela concerne surtout les personnes en charge du packaging.

@Beep6581

what are you referring to?

Sorry, was an answer to #3300

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 28, 2017

@Hombre57 CMAKE_SIZEOF_VOID_P seems not defined inside UpdateInfo.cmake
As I don't have nepali.islu inside my innosetup installation, could you suppress the line? or indicate where I can find it.
Que penses-tu de générer seulement un executable 7-zip pour les versions de developpement.?

gaaned92 commented Jan 28, 2017

@Hombre57 CMAKE_SIZEOF_VOID_P seems not defined inside UpdateInfo.cmake
As I don't have nepali.islu inside my innosetup installation, could you suppress the line? or indicate where I can find it.
Que penses-tu de générer seulement un executable 7-zip pour les versions de developpement.?

@Desmis

This comment has been minimized.

Show comment
Hide comment
@Desmis

Desmis Jan 28, 2017

Collaborator

@Hombre57
Salut Hombre...Je pense avoir vaguement compris, mais ce qui m'étonne le plus, c'est que a priori, nous sommes hérétiques depuis plus d'un an... Peut être qu'il va falloir aller faire un tour à Rome (ou ailleurs...)
Mais si tu peux faire une traduction ce serait bien.

Merci d'avance

Collaborator

Desmis commented Jan 28, 2017

@Hombre57
Salut Hombre...Je pense avoir vaguement compris, mais ce qui m'étonne le plus, c'est que a priori, nous sommes hérétiques depuis plus d'un an... Peut être qu'il va falloir aller faire un tour à Rome (ou ailleurs...)
Mais si tu peux faire une traduction ce serait bien.

Merci d'avance

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 28, 2017

Owner

@gaaned92 @Hombre57 just committed a fix, could you confirm that:

  • The variables in WindowsInnoSetup.iss are filled in,
  • Installation and uninstallation work,
  • One can install to a custom folder.

I will then merge if there are no objections.

Owner

Beep6581 commented Jan 28, 2017

@gaaned92 @Hombre57 just committed a fix, could you confirm that:

  • The variables in WindowsInnoSetup.iss are filled in,
  • Installation and uninstallation work,
  • One can install to a custom folder.

I will then merge if there are no objections.

@Hombre57

This comment has been minimized.

Show comment
Hide comment
Collaborator

Hombre57 commented Jan 28, 2017

@Desmis

This comment has been minimized.

Show comment
Hide comment
@Desmis

Desmis Jan 29, 2017

Collaborator

@Hombre57
Merci :)

Collaborator

Desmis commented Jan 29, 2017

@Hombre57
Merci :)

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 29, 2017

  • Variables are filled in
  • Installation with RawTherapee_5.0-gtk2-5-g52dca967_WinVista_64.exe installer ok
  • installation in custom folder ok
  • uninstallation ok

Two remarks I already made:

  • The installer name should include the build type and also the branch name as users who want to test a feature branch for instance will be unable to choose the good installer.
  • If you already made a previous installation, as the appid stay the same, the folder name is not asked for again and thus you overwrite the previous installation.

I propose to change appid for example
AppId= {#MyAppName}{#MyAppVersion}{#MyBuildType}

Thanks for the effort
André

gaaned92 commented Jan 29, 2017

  • Variables are filled in
  • Installation with RawTherapee_5.0-gtk2-5-g52dca967_WinVista_64.exe installer ok
  • installation in custom folder ok
  • uninstallation ok

Two remarks I already made:

  • The installer name should include the build type and also the branch name as users who want to test a feature branch for instance will be unable to choose the good installer.
  • If you already made a previous installation, as the appid stay the same, the folder name is not asked for again and thus you overwrite the previous installation.

I propose to change appid for example
AppId= {#MyAppName}{#MyAppVersion}{#MyBuildType}

Thanks for the effort
André

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 29, 2017

Owner

The branch name is only reliably available if you checkout a branch, so if you want it to be part of the installer's name, it should most likely be done manually.
You could use GIT_BRANCH, but that will in some cases be the hash.
See:
https://github.com/Beep6581/RawTherapee/blob/versionfix2/UpdateInfo.cmake#L30
https://github.com/Beep6581/RawTherapee/blob/versionfix2/UpdateInfo.cmake#L52
If e.g. Ingo asks for a pixelshift build one commit before HEAD because HEAD is broken, you checkout HEAD~1 and so you don't have a branch name because you checked out a commit, not a branch.

Re: AppId. The installer must let the user install to a custom folder even if a previous installation of a different version exists.
Either of these are fine:

  • AppId={#MyAppName}{#MyAppVersion}
  • AppId={#MyAppName}{#MyAppVersion}{#MyBuildType}
    (requires #define MyBuildType "${CMAKE_BUILD_TYPE}")

But I don't get why Windows releases ship debug executables separately. Why not just include rawtherapee.exe and rawtherapee_debug.exe in one installer? Saves space and effort.
P.S. In versionfix2 I fixed WindowsInnoSetup.iss.in to look for all executables beginning with "rawtherapee", the file header comment in master said that was how it worked but the code didn't match the comment.
See: https://github.com/Beep6581/RawTherapee/blob/versionfix2/tools/win/InnoSetup/WindowsInnoSetup.iss.in#L94
If you ship both release+debug executables in one installer, there will be no need for MyBuildType and less work uploading and the whole process is simplified.

@gaaned92 As I can't test, could you make the AppId change and test it?

Owner

Beep6581 commented Jan 29, 2017

The branch name is only reliably available if you checkout a branch, so if you want it to be part of the installer's name, it should most likely be done manually.
You could use GIT_BRANCH, but that will in some cases be the hash.
See:
https://github.com/Beep6581/RawTherapee/blob/versionfix2/UpdateInfo.cmake#L30
https://github.com/Beep6581/RawTherapee/blob/versionfix2/UpdateInfo.cmake#L52
If e.g. Ingo asks for a pixelshift build one commit before HEAD because HEAD is broken, you checkout HEAD~1 and so you don't have a branch name because you checked out a commit, not a branch.

Re: AppId. The installer must let the user install to a custom folder even if a previous installation of a different version exists.
Either of these are fine:

  • AppId={#MyAppName}{#MyAppVersion}
  • AppId={#MyAppName}{#MyAppVersion}{#MyBuildType}
    (requires #define MyBuildType "${CMAKE_BUILD_TYPE}")

But I don't get why Windows releases ship debug executables separately. Why not just include rawtherapee.exe and rawtherapee_debug.exe in one installer? Saves space and effort.
P.S. In versionfix2 I fixed WindowsInnoSetup.iss.in to look for all executables beginning with "rawtherapee", the file header comment in master said that was how it worked but the code didn't match the comment.
See: https://github.com/Beep6581/RawTherapee/blob/versionfix2/tools/win/InnoSetup/WindowsInnoSetup.iss.in#L94
If you ship both release+debug executables in one installer, there will be no need for MyBuildType and less work uploading and the whole process is simplified.

@gaaned92 As I can't test, could you make the AppId change and test it?

@Hombre57

This comment has been minimized.

Show comment
Hide comment
@Hombre57

Hombre57 Jan 29, 2017

Collaborator

It means that each new Release version and revision will be installed next to each other, even for Release version with minor version update (5.0, 5.1, ...). I guess that we should keep the same AppId for new minor version but change it for any new major version and non Release builds. @gaaned92 @Beep6581, are you okay with that ?

Collaborator

Hombre57 commented Jan 29, 2017

It means that each new Release version and revision will be installed next to each other, even for Release version with minor version update (5.0, 5.1, ...). I guess that we should keep the same AppId for new minor version but change it for any new major version and non Release builds. @gaaned92 @Beep6581, are you okay with that ?

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 29, 2017

Owner

We know from the forums and G+ that users want to keep several versions of RT concurrently, for example 5.0 and some dev version. The installer should make that possible.

I guess that we should keep the same AppId for new minor version but change it for any new major version and non Release builds.

How should that work, and who will do it?

Owner

Beep6581 commented Jan 29, 2017

We know from the forums and G+ that users want to keep several versions of RT concurrently, for example 5.0 and some dev version. The installer should make that possible.

I guess that we should keep the same AppId for new minor version but change it for any new major version and non Release builds.

How should that work, and who will do it?

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 29, 2017

@Hombre57 I think there is no inconvenient to have a different appid for each version revision and much simpler to implement.
@Beep6581 If I want to go back on a branch, I think I prefer to do a git reset of the branch. But I have no objection to keep the naming as it is.

Yes for dev builds, I should be able to pack GDB and rawtherapee_debug in same installer. And thus no need to append the BUILD_TYPE to names. But the installer will be bigger.

Appid change: ok with AppId={#MyAppName}{#MyAppVersion}. The new installation don't overwrite a previous one.

You never answered my proposal to use the installer for stable releases and 7zip.exe for development releases.

@Hombre57 I think there is no inconvenient to have a different appid for each version revision and much simpler to implement.
@Beep6581 If I want to go back on a branch, I think I prefer to do a git reset of the branch. But I have no objection to keep the naming as it is.

Yes for dev builds, I should be able to pack GDB and rawtherapee_debug in same installer. And thus no need to append the BUILD_TYPE to names. But the installer will be bigger.

Appid change: ok with AppId={#MyAppName}{#MyAppVersion}. The new installation don't overwrite a previous one.

You never answered my proposal to use the installer for stable releases and 7zip.exe for development releases.

@Hombre57

This comment has been minimized.

Show comment
Hide comment
@Hombre57

Hombre57 Jan 29, 2017

Collaborator

@gaaned92

Appid change: ok with AppId={#MyAppName}{#MyAppVersion}. The new installation don't overwrite a previous one.

Ok with that.

You never answered my proposal to use the installer for stable releases and 7zip.exe for development releases.

I don't know... In a general way, if this is supposed to be available through the download page, then an installer is necessary. If provided in forums or everywhere else, zip files are acceptable. (unstable) Dev builds should be clearly identified in Windows' Program and Functionalities. Do we have to set a special #MyAppName to do that or does it display #MyAppVersion too ?

Collaborator

Hombre57 commented Jan 29, 2017

@gaaned92

Appid change: ok with AppId={#MyAppName}{#MyAppVersion}. The new installation don't overwrite a previous one.

Ok with that.

You never answered my proposal to use the installer for stable releases and 7zip.exe for development releases.

I don't know... In a general way, if this is supposed to be available through the download page, then an installer is necessary. If provided in forums or everywhere else, zip files are acceptable. (unstable) Dev builds should be clearly identified in Windows' Program and Functionalities. Do we have to set a special #MyAppName to do that or does it display #MyAppVersion too ?

@gaaned92

This comment has been minimized.

Show comment
Hide comment
@gaaned92

gaaned92 Jan 29, 2017

As I understood, #MyAppversion should be sufficient to distinguish between dev and stable release. So If I name the 7zip exe file the same name as innosetup exe file (using #MyAppName), it could be sufficient to distinguish betwen dev and stable release.
drawback: 7zip exe doesn't propose an extraction location.

(unstable) Dev builds should be clearly identified in Windows' Program and Functionalities.
Could you clarify?

As I understood, #MyAppversion should be sufficient to distinguish between dev and stable release. So If I name the 7zip exe file the same name as innosetup exe file (using #MyAppName), it could be sufficient to distinguish betwen dev and stable release.
drawback: 7zip exe doesn't propose an extraction location.

(unstable) Dev builds should be clearly identified in Windows' Program and Functionalities.
Could you clarify?

@Hombre57

This comment has been minimized.

Show comment
Hide comment
@Hombre57

Hombre57 Jan 30, 2017

Collaborator

@gaaned92 I meant that Innosetup installer create an entry in Windows "Program and Functionalities" IIRC. It should have a different label there if we set different AppID for same version but different revision or for Dev installer.

Collaborator

Hombre57 commented Jan 30, 2017

@gaaned92 I meant that Innosetup installer create an entry in Windows "Program and Functionalities" IIRC. It should have a different label there if we set different AppID for same version but different revision or for Dev installer.

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 30, 2017

Owner

@gaaned92

@Beep6581 If I want to go back on a branch, I think I prefer to do a git reset of the branch. But I have no objection to keep the naming as it is.

What are you saying?

You never answered my proposal to use the installer for stable releases and 7zip.exe for development releases.

Shipping release+debug is the optimal solution, at least for now. If we were to ship the debug executable separately, plain 7z (or zip or whatever) is better than exe.

@Hombre57

Dev builds should be clearly identified in Windows' Program and Functionalities

I'm not sure what "Program and Functionalities" are - if you mean the start menu, then the shortcut uses the name:
Name: "{group}\{#MyAppName} {#MyAppVersion}"; Filename: "{app}\{#MyAppExeName}"
Which means releases will be called "RawTherapee 5.0" and development builds will be called "RawTherapee 5.0-gtk2-123-g87654321".

As I understood, #MyAppversion should be sufficient to distinguish between dev and stable release.

I think so too.

Owner

Beep6581 commented Jan 30, 2017

@gaaned92

@Beep6581 If I want to go back on a branch, I think I prefer to do a git reset of the branch. But I have no objection to keep the naming as it is.

What are you saying?

You never answered my proposal to use the installer for stable releases and 7zip.exe for development releases.

Shipping release+debug is the optimal solution, at least for now. If we were to ship the debug executable separately, plain 7z (or zip or whatever) is better than exe.

@Hombre57

Dev builds should be clearly identified in Windows' Program and Functionalities

I'm not sure what "Program and Functionalities" are - if you mean the start menu, then the shortcut uses the name:
Name: "{group}\{#MyAppName} {#MyAppVersion}"; Filename: "{app}\{#MyAppExeName}"
Which means releases will be called "RawTherapee 5.0" and development builds will be called "RawTherapee 5.0-gtk2-123-g87654321".

As I understood, #MyAppversion should be sufficient to distinguish between dev and stable release.

I think so too.

Beep6581 added a commit that referenced this issue Jan 30, 2017

Merge pull request #3648 from Beep6581/versionfix2
Revision of CMake and related files to support meaningful git versions within our extensive use of branches and development builds. #3628
@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 30, 2017

Owner

versionfix2 merged into master.

Owner

Beep6581 commented Jan 30, 2017

versionfix2 merged into master.

@Desmis

This comment has been minimized.

Show comment
Hide comment
@Desmis

Desmis Jan 31, 2017

Collaborator

@Beep6581
Can I, now, commit again and with what precautions? (locallabgtk3)
thank you for this work :)

Collaborator

Desmis commented Jan 31, 2017

@Beep6581
Can I, now, commit again and with what precautions? (locallabgtk3)
thank you for this work :)

@Beep6581

This comment has been minimized.

Show comment
Hide comment
@Beep6581

Beep6581 Jan 31, 2017

Owner

@Desmis yes, you can commit to "feature branches" such as locallab and locallabgtk3, but please wait another 2 days before committing anything to master or gtk3.
Thank You for the work :)

Owner

Beep6581 commented Jan 31, 2017

@Desmis yes, you can commit to "feature branches" such as locallab and locallabgtk3, but please wait another 2 days before committing anything to master or gtk3.
Thank You for the work :)

@Desmis

This comment has been minimized.

Show comment
Hide comment
@Desmis

Desmis Jan 31, 2017

Collaborator
Collaborator

Desmis commented Jan 31, 2017

@Desmis

This comment has been minimized.

Show comment
Hide comment
@Desmis

Desmis Feb 1, 2017

Collaborator
Collaborator

Desmis commented Feb 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment