Bitcoin-Qt (Windows only): extend Resource File #1607

Diapolo commented at 5:37 AM on July 18, 2012: none

extend bitcoin-qt.rc to include meta information, which is displayed on Windows, when looking in the executable properties and selecting "Details"
does currently NOT include version information, this is scheduled for later releases
for RC-file documentation see: http://msdn.microsoft.com/en-us/library/windows/desktop/aa381058%28v=vs.85%29.aspx

bitcoin-qt.exe meta information

laanwj commented at 5:44 AM on July 18, 2012: member

Hm, nice to add some metadata, but it adds yet another place to update the version number. That sucks... Maybe only add the major versions?

Diapolo commented at 5:50 AM on July 18, 2012: none

I guess this could be scripted in the Gitian process, but I'm no Linux-Script guy. I would love to see this added and I'm fine with major version numbers (e.g. 0.7.X.0), but could take a look here and update when needed :).

It's also possible to include a header with that version stuff, but it's only allowed to consist of the version #defines. So our version.h did not work, too bad.

laanwj commented at 11:38 AM on July 19, 2012: member

"makes bitcoin-qt.exe a bit more trust-worthy" that's an interesting claim :-)

Diapolo commented at 11:43 AM on July 19, 2012: none

@laanwj Why? When I use the tool ProcessExplorer, which is able to show some internal informations of running processes, handles and so on, I consider an application that provides no meta-data as suspicious on the first look. I think it's a good style to supply such meta-data.

laanwj commented at 11:46 AM on July 19, 2012: member

Heh, you meant "makes bitcoin-qt.exe look more trust-worthy". Actually becoming more trust-worthy... would be interesting, that was the joke.

I'm for merging this of course. Can't we remove the version completely, for now, until someone sets up a script to insert it automatically?

Diapolo commented at 12:47 PM on July 19, 2012: none

I meant look more trust-worthy, which is a personal thing, so yes ;).

Well I could simply comment out the version #defines but I fear no Linux dev who is able to write shell scripts is willing to put work into this ^^. I'm going to take another look at this issue now.

Edit: Btw. is version.h missing by intent from HEADERS += in the project file?

grue0 commented at 6:09 PM on July 19, 2012: none

@Diapolo

more trustworthy

wow really? that's about as effective as naming bitcoin-qt.exe to legit_bitcoin-qt.exe

in src/qt/res/bitcoin-qt.rc:None in eb90750396 outdated

  22 | +            VALUE "FileDescription",    "Bitcoin-Qt (an MIT-licenced Qt4 GUI for the original Bitcoin codebase)"
  23 | +            VALUE "FileVersion",        VER_FILEVERSION_STR
  24 | +            VALUE "InternalName",       "bitcoin-qt"
  25 | +            VALUE "LegalCopyright",     "2009-2012 The Bitcoin developers"
  26 | +            VALUE "LegalTrademarks1",   "Distributed under the MIT/X11 software license, see the accompanying"
  27 | +            VALUE "LegalTrademarks2",   "file COPYING or http://www.opensource.org/licenses/mit-license.php."

grue0 commented at 6:12 PM on July 19, 2012:

I'm pretty sure there's no point to breaking up the message into different properties.

Diapolo commented at 8:36 PM on July 19, 2012:

I'm allowed to use 2 rows and I chose the exact format used all over the source, I think that is just fine.

laanwj commented at 6:18 AM on July 20, 2012:

I'm with gruez on this one. LegalCopyright is a separate unit, but I don't see why split LegalTradeMarks1 / LegalTradeMarks2 at a illogical place between two words.

Diapolo commented at 6:28 AM on July 20, 2012:

Merged LegalTrademarks2 into LegalTrademarks1 :), you convinced me.

in src/qt/res/bitcoin-qt.rc:None in eb90750396 outdated

  17 | +    BLOCK "StringFileInfo"
  18 | +    BEGIN
  19 | +        BLOCK "040904E4" // U.S. English - Multilingual (Hex)
  20 | +        BEGIN
  21 | +            VALUE "CompanyName",        "Bitcoin"
  22 | +            VALUE "FileDescription",    "Bitcoin-Qt (an MIT-licenced Qt4 GUI for the original Bitcoin codebase)"

grue0 commented at 6:17 PM on July 19, 2012:

kind of verbose, don't you think? it should be a brief description of what bitcoin is. no need to be all technical and say it's a gui or what the licence is.

Diapolo commented at 8:39 PM on July 19, 2012:

I took that from the Wiki and was not very creative, but it describes what Bitcoin-Qt is. If you don't like is please suggest something better. I chose to put Bitcoin-Qt in front as this text is displayed e.g. when the app crashes and Windows displays the error dialog.

Qt based Bitcoin Reference Client?

grue0 commented at 2:03 AM on July 21, 2012:

How about something based on the first sentence on wikipedia?

https://en.wikipedia.org/wiki/Bitcoin "Client for Bitcoin, a decentralized electronic cash system" or "Open-source client for Bitcoin, a decentralized electronic cash system"

Diapolo commented at 8:27 AM on July 21, 2012:

This sounds less technical ... I'm going to update that string, have to think a little more how exactly though :). Thanks for your input.

laanwj commented at 9:08 AM on July 21, 2012:

Agreed that the string from the wiki is better. The focus should be on bitcoin itself, no one cares about the widget toolkit used etc

in src/qt/res/versionrc.h:None in b417315243 outdated

   0 | @@ -0,0 +1,7 @@
   1 | +#ifndef VERSIONINFO_H
   2 | +#define VERSIONINFO_H
   3 | +
   4 | +#define VER_PRODUCTVERSION          0,6,99,0
   5 | +#define VER_PRODUCTVERSION_STR      "0.6.99.0"

Diapolo commented at 9:32 PM on July 19, 2012:

This one could be generated from the #defines in version.h, but as I can't include that without breaking the windres compiler, this is a no-go :-/.

Diapolo commented at 9:34 PM on July 19, 2012: none

I split the needed version stuff into versionrc.h ... still a manual solution, but a small step perhaps to one who can help automating this :-D.

Diapolo commented at 8:47 AM on July 26, 2012: none

Updated FileDescription string and I was able to change the language property to display neutral language, which is better, because we have quite many translations in.

BitcoinPullTester commented at 5:13 AM on August 9, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/368e127e53f7dfe517b96f7fb1b365b3ef1c77a3 for binaries and test log.

laanwj commented at 4:23 AM on August 14, 2012: member

Can we merge this without the version for now? Just add base metadata for this pull, and do the scripting stuff in a later pull.

Diapolo commented at 9:00 AM on August 14, 2012: none

I can comment out the version stuff in the .rc file and remove the header for now. But to not forget about this I'll re-open a pull after the basic one is in okay?

Bitcoin-Qt (Windows only): extend Resource File

- extend bitcoin-qt.rc to include meta information, which is displayed on
  Windows, when looking in the executable properties and selecting
  "Details"
- does currently NOT include version information, this is scheduled
  for later releases
- for RC-file documentation see:
  http://msdn.microsoft.com/en-us/library/windows/desktop/aa381058%28v=vs.85%29.aspx

4b94f4e0ed

Diapolo commented at 9:20 AM on August 14, 2012: none

Last update removed all version stuff (for now), see the screenshot in the first posting.

BitcoinPullTester commented at 11:17 AM on August 14, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4b94f4e0ed38a95a3e2c3e80258fe73b88c3826c for binaries and test log.

sipa commented at 2:35 PM on August 17, 2012: member

ACK

laanwj referenced this in commit a108d3defb on Aug 17, 2012

laanwj merged this on Aug 17, 2012

laanwj closed this on Aug 17, 2012

DrahtBot locked this on Sep 8, 2021