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

Minor Windows installer fixes/changes #1506

Closed
rcurtin opened this issue Sep 6, 2018 · 4 comments
Closed

Minor Windows installer fixes/changes #1506

rcurtin opened this issue Sep 6, 2018 · 4 comments

Comments

@rcurtin
Copy link
Member

rcurtin commented Sep 6, 2018

These are just a few leftover changes from #1485. They aren't high priority but I don't think any of them are too hard.

  1. Higher resolution image that looks less blurry. Windows WiX Installer #1485 (comment)
  2. Wrapped lines for AppVeyor config. (Not totally sure if this will work.) Windows WiX Installer #1485 (comment)
  3. Use git version in build number by parsing gitversion.hpp if it exists. Windows WiX Installer #1485 (comment)

This could be a nice intro task for someone with some Windows knowledge who wants to contribute.

@gmanlan
Copy link
Contributor

gmanlan commented Sep 6, 2018

Another improvement might be to remove the *_test.exe and the test dependency (boost_unit_test_framework-vc140-mt-1_60.dll). Having the test in the installer may not be that useful (and it fails anyways because it can't find many test files).

@KimSangYeon-DGU
Copy link
Member

KimSangYeon-DGU commented Jan 11, 2019

Hi @rcurtin. In work list, the 1 and 2 have been done and the 3 almost has been done. To use the git version in build, the version should be x.x.x.x or x.x.x... format(x is integer from 0 to 65534). However, the git version is 7 hexadecimal string like git-b23d5ce, so we should convert this string to x.x.x.x format. So, I have a idea on converting it. Combine two characters into a integer number. Below is example. Let's assume the git version is abcd207.

  1. User's git version is abcd207.
  2. Add the end of git version to '0' (like padding).
  3. So changed git version is abcd2070
  4. A binary of a, and b is 1010 and 1011 respectively, so it can be 1010 1011 (171)
  5. Likewise, the rest of pair can be converted to 205, 32, and 112.
  6. As a result, the git version abcd207 can be converted to 171.205.32.112.

Because the possible minimum and maximum character pairs are 00(0) and ff(255), so this solution does not exceed the range(0 ~ 65534). Finally, we can distinguish between git version and official mlpack version using the number of x. (if the number of x is 3, it is official mlpack version, otherwise it is 'git version')

So, is it proper way to converting it??

@rcurtin
Copy link
Member Author

rcurtin commented Jan 12, 2019

Hehe, @KimSangYeon-DGU, thanks for the extensive analysis. But I was thinking something much simpler, just like we do for existing Linux builds. :) Basically we just use the git hash... so if the hash is b23d5ce then the version is mlpack git-b23d5ce. CMake will already generate ${CMAKE_BINARY_DIR}/src/mlpack/core/util/gitversion.hpp, which will have this text in it, so I think the only thing to do would be to use a line in the AppVeyor config in order to extract that text and modify the WiX configuration accordingly.

@rcurtin
Copy link
Member Author

rcurtin commented Jan 25, 2019

With #1506 merged, we can close this now. Thanks @KimSangYeon-DGU! 👍

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

No branches or pull requests

3 participants