Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Unify product name to as few places as possible #7192

Merged
merged 18 commits into from Feb 4, 2016

Conversation

Projects
None yet
Member

luke-jr commented Dec 9, 2015

This should help keep the software name consistent within translations (Bitcoin Kern, Nucleul Bitcoin, etc) or forks (Bitcoin LJR, etc).

Member

btcdrak commented Dec 9, 2015

Good.

utACK

@MarcoFalke MarcoFalke and 1 other commented on an outdated diff Dec 9, 2015

src/init.cpp
@@ -500,7 +500,7 @@ std::string HelpMessage(HelpMessageMode mode)
std::string LicenseInfo()
{
// todo: remove urls from translations on next change
- return FormatParagraph(strprintf(_("Copyright (C) 2009-%i The Bitcoin Core Developers"), COPYRIGHT_YEAR)) + "\n" +
+ return FormatParagraph(strprintf(_("Copyright (C) 2009-%i The %s Developers"), COPYRIGHT_YEAR, _(PACKAGE_NAME))) + "\n" +
@MarcoFalke

MarcoFalke Dec 9, 2015

Member

As you are touching this translation anyway. Is there any language that translates 2009 into something other than 2009? If so, this should be changed as well.

@laanwj

laanwj Dec 10, 2015

Owner

Agree, makes sense to parametrize both years, even though the first one will never change

@jonasschnelli jonasschnelli and 4 others commented on an outdated diff Dec 9, 2015

src/clientversion.h
@@ -38,7 +38,7 @@
#define DO_STRINGIZE(X) #X
//! Copyright string used in Windows .rc files
-#define COPYRIGHT_STR "2009-" STRINGIZE(COPYRIGHT_YEAR) " The Bitcoin Core Developers"
+#define COPYRIGHT_STR "2009-" STRINGIZE(COPYRIGHT_YEAR) " The " PACKAGE_NAME " Developers"
@maaku

maaku Dec 22, 2015

Contributor

As a separate issue from Jonas, are we sure the copyright name is something we want to change? I've had to deal with this as maintainer of Freicoin .. changing the product name would result in "Copyright 2009-2015 The Freicoin Core Developers" which would certainly not be okay (or legal?).

It would be nice to detect if PACKAGE_NAME is changed from the default, and if so add a another copyright string (while maintaining the hard-coded "Bitcoin Core" text).

@fanquake

fanquake Dec 22, 2015

Member

Concept ACK, haven't had a chance to test.

On Tuesday, 22 December 2015, Mark Friedenbach notifications@github.com
wrote:

In src/clientversion.h
#7192 (comment):

@@ -38,7 +38,7 @@
#define DO_STRINGIZE(X) #X

//! Copyright string used in Windows .rc files
-#define COPYRIGHT_STR "2009-" STRINGIZE(COPYRIGHT_YEAR) " The Bitcoin Core Developers"
+#define COPYRIGHT_STR "2009-" STRINGIZE(COPYRIGHT_YEAR) " The " PACKAGE_NAME " Developers"

As a separate issue from Jonas, are we sure the copyright name is
something we want to change? I've had to deal with this as maintainer of
Freicoin .. changing the product name would result in "Copyright 2009-2015
The Freicoin Core Developers" which would certainly not be okay (or legal?).

It would be nice to detect if PACKAGE_NAME is changed from the default,
and if so add a another copyright string (while maintaining the hard-coded
"Bitcoin Core" text).


Reply to this email directly or view it on GitHub
https://github.com/bitcoin/bitcoin/pull/7192/files#r48238911.

@MarcoFalke

MarcoFalke Dec 22, 2015

Member

@maaku Good catch! I also don't think this is legal.

@luke-jr

luke-jr Dec 22, 2015

Member

There is no legal entity "The Bitcoin Core developers", it is just a term used to indicate that each developer retains his own copyright. Replacing the package name in it doesn't change that.

@MarcoFalke

MarcoFalke Dec 22, 2015

Member

IANAL but for instance "The MF Core developers" would just be me whereas "The Bitcoin Core developers" are something like https://github.com/bitcoin/bitcoin/graphs/contributors. Otherwise you wouldn't need the multiline headers such as https://github.com/dogecoin/dogecoin/blob/bb4b082c086689434d67490a15224c42fefdfd13/src/main.cpp#L3?

@luke-jr

luke-jr Dec 22, 2015

Member

Why would "The MF Core developers" not similarly be https://github.com/MarcoFalke/bitcoin/graphs/contributors ?

Otherwise you wouldn't need the multiline headers such as https://github.com/dogecoin/dogecoin/blob/bb4b082c086689434d67490a15224c42fefdfd13/src/main.cpp#L3?

I don't know that they actually do.

@jonasschnelli

jonasschnelli Dec 22, 2015

Member

After thinking about this in detail, I kind of agree with @luke-jr: this line does not change the copyright of the sources itself. It's just a information string that will be transported to the user over package informations and it's under the responsibility of the package-/project-mainteiner(s).

If this (string) needs to fit all legal constraints, then it would probably require to add "LevelDB" and other third-party copyrights here.

@MarcoFalke

MarcoFalke Dec 22, 2015

Member

and it's under the responsibility of the package-/project-mainteiner(s).

Right, but at least let's not reuse the variable. Someone in IRC mentioned to split the two constants (one for package name and the other for copyright).

Member

jonasschnelli commented Dec 9, 2015

Nice!
Concept ACK.
Will test soon.

Member

jonasschnelli commented Dec 10, 2015

Currently fails windows: https://bitcoin.jonasschnelli.ch/pulls/7192/build-win.log i'm happy to test this if windows issues are solved.

@laanwj laanwj commented on the diff Dec 10, 2015

src/qt/bitcoingui.cpp
@@ -104,7 +108,7 @@ BitcoinGUI::BitcoinGUI(const PlatformStyle *platformStyle, const NetworkStyle *n
{
GUIUtil::restoreWindowGeometry("nWindow", QSize(850, 550), this);
- QString windowTitle = tr("Bitcoin Core") + " - ";
+ QString windowTitle = tr(PACKAGE_NAME) + " - ";
@laanwj

laanwj Dec 10, 2015

Owner

This won't work. And you don't want the tr. Just use:

QString windowTitle = QString(PACKAGE_NAME) + " - ";
@luke-jr

luke-jr Dec 10, 2015

Member

But then it won't be translated? I don't mean to change that status quo in this PR...

@laanwj

laanwj Dec 10, 2015

Owner

Oh, right, does this work then? Does qtranslate pick up the string out of tr(PACKAGE_NAME), even though it's a macro? Same for _(PACKAGE_NAME) in gettext? I thought only literal strings could be input to translation.

@luke-jr

luke-jr Dec 10, 2015

Member

I doubt it. I imagine there's some way to explicitly tell it? :/

@luke-jr

luke-jr Dec 22, 2015

Member

I believe this has been addressed here.

Owner

laanwj commented Dec 10, 2015

Concept ACK (for 0.13 - due to translation changes this is too late for 0.12)

@MarcoFalke MarcoFalke commented on an outdated diff Dec 10, 2015

src/qt/forms/debugwindow.ui
@@ -357,7 +357,7 @@
<item row="16" column="0">
<widget class="QPushButton" name="openDebugLogfileButton">
<property name="toolTip">
- <string>Open the Bitcoin Core debug log file from the current data directory. This can take a few seconds for large log files.</string>
+ <string>Open the %1 debug log file from the current data directory. This can take a few seconds for large log files.</string>
@MarcoFalke

MarcoFalke Dec 10, 2015

Member

Needs rebase to at least 5940cf33b35fb70979a7baa3046107a9b72d8f0f

Member

luke-jr commented Dec 11, 2015

Added a second commit reworking the binary blobs used for Mac-deploy. Haven't rebased yet pending completion/testing.

@laanwj I am probably going to need to do the translation updates for Bitcoin LJR anyway, so let me know and I can see about doing it in Transifex for 0.12 if that'd be preferable.

Contributor

pstratem commented Dec 11, 2015

concept ACK 0898436e860f695028b3fc6b612074712bbdcbc6
(reviewed the code also, but don't know anything about qt/autotools)

Member

luke-jr commented Dec 13, 2015

Ok, I believe I have addressed all previous requests, but also introduced a packaging FIXME which I hope @theuni can help out with...

Member

luke-jr commented Dec 14, 2015

Travis is happy with it now, and I've confirmed the Mac DMG background image is rendered correctly in gitian. Only thing left is the depends/ stuff I need @theuni 's guidance on... But this is possibly "good enough to merge" without that, even if not perfect (which is the enemy of the good).

Member

MarcoFalke commented Dec 14, 2015

Looks like there are current binaries for testing at https://bitcoin.jonasschnelli.ch/pulls/7192/

Had a quick look at the linux binaries and it looks ok. Haven't checked OS X, though.

Member

jonasschnelli commented Dec 14, 2015

Looks like there are current binaries for testing at https://bitcoin.jonasschnelli.ch/pulls/7192/

Yes. Just built.

@luke-jr: Somethings wrong with the background. I don't get one.

bildschirmfoto 2015-12-14 um 09 45 46

Did you made sure, that both resolutions are in the tiff file? Needs to be a @2x (HiDPI) and a normal?
Maybe read this. I hope there is a linux tool for that.

bildschirmfoto 2015-12-14 um 09 49 47

Member

jonasschnelli commented Dec 14, 2015

On linux, you probably need a tool like this: http://linux.die.net/man/1/tiffcp

Member

luke-jr commented Dec 14, 2015

Ok, try this one.

Member

jonasschnelli commented Dec 14, 2015

Build error on osx:

/usr/bin/tiffcp -c none dpi36.background.tiff dpi72.background.tiff dist/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt dist/.background/background.tiff
dist/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt: Not a TIFF or MDI file, bad magic number 64207 (0xfacf).
make: *** [dist/.background/background.tiff] Error 253

I guess the output file is wrong:
It's dist/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt dist/.background/background.tiff instead of dist/.background/background.tiff

Member

luke-jr commented Dec 16, 2015

@jonasschnelli Can you minimally modify the generated DS_Store to work, and post me the working file? (Not on your broken SSL server...)

Member

jonasschnelli commented Dec 17, 2015

DMG including background.tiff and DS_Store looks good now. Well done!
bildschirmfoto 2015-12-17 um 10 06 02

@theuni theuni and 1 other commented on an outdated diff Dec 17, 2015

.travis.yml
@@ -52,6 +52,7 @@ install:
before_script:
- unset CC; unset CXX
- mkdir -p depends/SDKs depends/sdk-sources
+ - if [ -n "$OSX_SDK" ]; then pip install --user cairosvg mac_alias ds_store; export PATH="$HOME/.local/bin:$PATH"; ( wget 'https://bitbucket.org/al45tair/ds_store/get/c80c23706eae.tar.gz' && tar -xzvpf c80c23706eae.tar.gz && cd al45tair-ds_store-c80c23706eae/ && python setup.py install --user; ) fi
@theuni

theuni Dec 17, 2015

Member

This should go in "install". Also, should depend on the host being OSX, not on the OSX_SDK availability.

@luke-jr

luke-jr Dec 17, 2015

Member

How is host==OSX determined here, besides OSX_SDK availability?

Member

luke-jr commented Dec 22, 2015

I believe this is ready for merging.

@laanwj Let me know if you would like me to prepare a 0.12 backport. Again, I am willing to fixup the translations for it. I will need to make it anyway for Bitcoin LJR, so might as well benefit Core in the process.

Contributor

maaku commented Dec 22, 2015

Nice! Concept ACK. I was just about to implement the same thing when I figured I'd check the PR history and found this pleasant surprise :)

Will test soon.

@MarcoFalke MarcoFalke commented on an outdated diff Dec 22, 2015

share/qt/Info.plist.in
@@ -17,7 +17,7 @@
<string>APPL</string>
<key>CFBundleGetInfoString</key>
- <string>@CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@CLIENT_VERSION_REVISION@, Copyright © 2009-@COPYRIGHT_YEAR@ The Bitcoin Core developers</string>
+ <string>@CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@CLIENT_VERSION_REVISION@, Copyright © 2009-@COPYRIGHT_YEAR@ The @PACKAGE_NAME@ developers</string>
@MarcoFalke

MarcoFalke Dec 22, 2015

Member

This one as well: The copyright is held by "The Bitcoin Core developers", so please leave as is or add another line.

@MarcoFalke MarcoFalke commented on an outdated diff Dec 22, 2015

@@ -513,7 +513,7 @@ std::string HelpMessage(HelpMessageMode mode)
std::string LicenseInfo()
{
// todo: remove urls from translations on next change
- return FormatParagraph(strprintf(_("Copyright (C) 2009-%i The Bitcoin Core Developers"), COPYRIGHT_YEAR)) + "\n" +
+ return FormatParagraph(strprintf(_("Copyright (C) %i-%i The %s Developers"), 2009, COPYRIGHT_YEAR, _(PACKAGE_NAME))) + "\n" +

@MarcoFalke MarcoFalke and 1 other commented on an outdated diff Dec 22, 2015

src/qt/splashscreen.cpp
QString versionText = QString("Version %1").arg(QString::fromStdString(FormatFullVersion()));
- QString copyrightText = QChar(0xA9)+QString(" 2009-%1 ").arg(COPYRIGHT_YEAR) + QString(tr("The Bitcoin Core developers"));
+ QString copyrightText = QChar(0xA9)+QString(" %1-%2 ").arg(2009).arg(COPYRIGHT_YEAR) + QString(tr("The %1 developers").arg(tr(PACKAGE_NAME)));
@jonasschnelli

jonasschnelli Dec 22, 2015

Member

I think this change is acceptable. Because it does not change the copyright of the source file header itself. The copyright informations within the GUI application may be different (additional assets attributions, libraries, etc.).

@luke-jr luke-jr changed the title from Unify product name to as few places as possible without major changes to Unify product name to as few places as possible Dec 22, 2015

@jonasschnelli jonasschnelli commented on an outdated diff Dec 22, 2015

contrib/macdeploy/background.svg
@@ -0,0 +1,34 @@
+<?xml version="1.0" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 20010904//EN"
+ "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">
+<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="1000pt" height="640pt" viewBox="0 0 1000 640" preserveAspectRatio="xMidYMid meet">
+ <!-- kate: space-indent off;
+ Copyright (c) 2011-2013 The Bitcoin Core developers

@jonasschnelli jonasschnelli commented on the diff Dec 22, 2015

contrib/macdeploy/custom_dsstore.py
+ 'textSize': 12.0,
+ 'viewOptionsVersion': 1,
+ 'backgroundImageAlias': '\x00\x00\x00\x00\x02\x1e\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xd1\x94\\\xb0H+\x00\x05\x00\x00\x00\x98\x0fbackground.tiff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x99\xd19\xb0\xf8\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\x00\x00\r\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0b.background\x00\x00\x10\x00\x08\x00\x00\xd1\x94\\\xb0\x00\x00\x00\x11\x00\x08\x00\x00\xd19\xb0\xf8\x00\x00\x00\x01\x00\x04\x00\x00\x00\x98\x00\x0e\x00 \x00\x0f\x00b\x00a\x00c\x00k\x00g\x00r\x00o\x00u\x00n\x00d\x00.\x00t\x00i\x00f\x00f\x00\x0f\x00\x02\x00\x00\x00\x12\x00\x1c/.background/background.tiff\x00\x14\x01\x06\x00\x00\x00\x00\x01\x06\x00\x02\x00\x00\x0cMacintosh HD\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xce\x97\xab\xc3H+\x00\x00\x01\x88[\x88\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02u\xab\x8d\xd1\x94\\\xb0devrddsk\xff\xff\xff\xff\x00\x00\t \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x07bitcoin\x00\x00\x10\x00\x08\x00\x00\xce\x97\xab\xc3\x00\x00\x00\x11\x00\x08\x00\x00\xd1\x94\\\xb0\x00\x00\x00\x01\x00\x14\x01\x88[\x88\x00\x16\xa9\t\x00\x08\xfaR\x00\x08\xfaQ\x00\x02d\x8e\x00\x0e\x00\x02\x00\x00\x00\x0f\x00\x1a\x00\x0c\x00M\x00a\x00c\x00i\x00n\x00t\x00o\x00s\x00h\x00 \x00H\x00D\x00\x13\x00\x01/\x00\x00\x15\x00\x02\x00\x14\xff\xff\x00\x00\xff\xff\x00\x00',
+ 'backgroundColorBlue': 1.0,
+ 'iconSize': 96.0,
+ 'backgroundColorGreen': 1.0,
+ 'arrangeBy': 'none',
+ 'showIconPreview': True,
+ 'gridSpacing': 100.0,
+ 'gridOffsetY': 0.0,
+ 'showItemInfo': False,
+ 'labelOnBottom': True,
+ 'backgroundType': 2,
+ 'backgroundColorRed': 1.0
+}
+alias = Alias.from_bytes(icvp['backgroundImageAlias'])
@jonasschnelli

jonasschnelli Dec 22, 2015

Member

This looks still a bit hackish.

Why not use alias = Alias.for_file(path_in_image)? Check: http://nullege.com/codes/show/src@d@m@dmgbuild-1.0.0@dmgbuild@core.py/337/biplist.Data

The whole byte-fiddling might break soon.

@luke-jr

luke-jr Dec 22, 2015

Member

Why not use alias = Alias.for_file(path_in_image)?

It only works on Mac...

The whole byte-fiddling might break soon.

???

@jonasschnelli

jonasschnelli Dec 22, 2015

Member

It only works on Mac...

Okay. Thats a point.

The whole byte-fiddling might break soon.

I was trying to say, that the hex construct at L31 will very likely break with upcoming OSX updates.

@luke-jr

luke-jr Dec 22, 2015

Member

I was trying to say, that the hex construct at L31 will very likely break with upcoming OSX updates.

Why is that? Nothing we can do here if so...

@jonasschnelli

jonasschnelli Dec 22, 2015

Member

Why is that? Nothing we can do here if so...

I just think, if we could let the OS or a OS near function create the byte-stream for the alias, this might help for future compatibility. But as you said, this only runs on mac...

ACK how it is in this PR right now.

Member

luke-jr commented Dec 22, 2015

Following further discussion of the copyright notice matter on IRC, I have separated the copyright notice code so that it instead uses independent COPYRIGHT_HOLDERS and COPYRIGHT_HOLDERS_SUBSTITUTION variables, so it is harder to accidentally modify them, yet when modified, translations get preserved as much as possible.

(also fixed the incorrect copyright years on the DMG background image template)

Member

btcdrak commented Dec 22, 2015

Tested ACK 4d5a3df

Member

jtimon commented Dec 22, 2015

Concept ACK

Member

jonasschnelli commented Dec 23, 2015

Tested ACK 4d5a3df

Contributor

maaku commented Dec 23, 2015

Tested ACK
On Dec 23, 2015 4:16 PM, "Jonas Schnelli" notifications@github.com wrote:

Tested ACK 4d5a3df
4d5a3df


Reply to this email directly or view it on GitHub
#7192 (comment).

@fanquake fanquake commented on the diff Dec 25, 2015

contrib/macdeploy/background.svg
@@ -0,0 +1,34 @@
+<?xml version="1.0" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 20010904//EN"
+ "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">
+<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="1000pt" height="640pt" viewBox="0 0 1000 640" preserveAspectRatio="xMidYMid meet">
+ <!-- kate: space-indent off;
@fanquake

fanquake Dec 25, 2015

Member

nit: Do we need this line, looks like a text-editor flag?

Member

luke-jr commented Dec 26, 2015

Update on 0.12: Too many languages unfortunately do not have common translations of "Bitcoin Core", so it is impractical to try to include this for 0.12.0.

Member

jtimon commented Dec 26, 2015

I still don't see why "Bitcoin Core" needs to be translated in the first place: it's a proper name.

Member

MarcoFalke commented Dec 26, 2015

I think we wanted to keep it because it was already translated in some places... What about not translating it after 0.13?

Member

jtimon commented Dec 26, 2015

What about not translating it from 0.12?
I'm sure I wouldn't translate it to spanish. Why some languages translate it and others don't?

Member

btcdrak commented Dec 26, 2015

@luke-jr

Too many languages unfortunately do not have common translations of "Bitcoin Core"

Product names are not translated. "Windows" is "Windows", even in China, Japan and Korea. Look at this as an example http://www.microsoft.com/zh-cn

Owner

sipa commented Dec 26, 2015

Member

luke-jr commented Dec 26, 2015

No opinion on whether Bitcoin Core ought to be translated or not, but it would de facto be a change in translations, which is too late for 0.12.0 as I understand it. I would think at the very least, however, transliteration would be desirable for non-Latin languages.

Member

btcdrak commented Dec 26, 2015

@luke-jr Even for non-Latin languages, the practice is not to translate/transliterate.

Member

MarcoFalke commented Dec 26, 2015

It would make sense to translate if we had some different flavors to offer like Ubuntu Kylin for instance but I can't see where this happens.

Member

luke-jr commented Dec 26, 2015

Anyhow, whether or not to translate the name is out-of-scope for this PR.

Contributor

maaku commented Dec 27, 2015

Translation is complicated. I'm not 100% sure that that Luke-Jr meant the
name was actually being changed in translation. It is also the case that in
some languages even a proper name might be inflected or have grammatical
particles attached based on context which would require a native speaker to
review, and would have prevented Luke-Jr's search-and-replace strategy.
But even if you decide on not using translations of "Bitcoin Core" as are
in use in the target demographic, it's just plain rude not to transliterate
into the native script. "Bitcoin" in Chinese is 比特币 -- a phrase which has
no real meaning but is the closest approximation of "bit-coin" to Chinese
ears. The Japanese would have it as ビットコイン, "bit-coin" rendered in a script
specifically meant for foreign transliterations. To hoist a non-native
script on them would be quite inconsiderate.

But as mentioned it is out of scope for this PR anyway.

On Dec 26, 2015 6:13 PM, "MarcoFalke" notifications@github.com wrote:

I think we wanted to keep it because it was already translated in some
places... What about not translating it after 0.13?


Reply to this email directly or view it on GitHub.

Contributor

dexX7 commented Dec 27, 2015

utACK -- this seems very useful for "customized" Bitcoin Core versions.

Member

MarcoFalke commented Dec 27, 2015

Concept ACK 4d5a3df

@MarcoFalke MarcoFalke commented on an outdated diff Jan 6, 2016

contrib/gitian-descriptors/gitian-osx.yml
- "libcap-dev"
- "libz-dev"
- "libbz2-dev"
+- "python-dev"
+- "python-setuptools"
+- "fonts-tuffy"
reference_datetime: "2015-06-01 00:00:00"
@MarcoFalke

MarcoFalke Jan 6, 2016

Member

@luke-jr reference_datetime was accidentally changed in master. Thus, this needs rebase. :(

Alternatively, it should be sufficient to move the python* packages up a bit.

Owner

laanwj commented Jan 20, 2016

Isn't the "whether to translate Bitcoin Core" discussion irrelevant to this pull? It just factors it out, doesn't make it untranslatable right?

Also: needs conflicts resolved

Member

luke-jr commented Jan 20, 2016

Isn't the "whether to translate Bitcoin Core" discussion irrelevant to this pull? It just factors it out, doesn't make it untranslatable right?

Right, it's a tangent discussion. If it's decided to make it untranslatable, someone would need to open a new PR - it's outside the scope of this one, which maintains the status quo of "translatable".

Merged master (conflicts resolved).

Member

jtimon commented Jan 21, 2016

@laanwj I thought this was out of 0.12 because of the translation (and things like "bitcoin kern" sound incredibly stupid as translations to me), but I didn't thought about languages that don't use the latin alphabet.
My apologies, let's leave that out of this PR.

Member

MarcoFalke commented Jan 21, 2016

This was left out of 0.12 because it would change some translations after translation freeze. The changes are necessary due to the refactoring and I think @luke-jr already applied the refactoring to the translations on trasifex accordingly? (If so, this could go into 0.12.1)

Owner

laanwj commented Jan 22, 2016

This gives me an empty translation string in bitcoinstrings.cpp, the output of GNU gettext:

+QT_TRANSLATE_NOOP("bitcoin-core", ""),

I'm not sure where it comes from - I can't find any direct _("") in the code. But it indicates something is wrong, maybe a _() that gets passed a macro or variable.

*Note: * after merging this, need to re-run autogen.sh and configure otherwise there will be an undefined macro error during compile.

Owner

laanwj commented Jan 25, 2016

@luke-jr It had nothing to do with what I thought above. You can apply the following diff to prevent adding an empty translation string:

diff --git a/share/qt/extract_strings_qt.py b/share/qt/extract_strings_qt.py
index 2a6e4b9..cd76fab 100755
--- a/share/qt/extract_strings_qt.py
+++ b/share/qt/extract_strings_qt.py
@@ -72,7 +72,7 @@ f.write("""
 f.write('static const char UNUSED *bitcoin_strings[] = {\n')
 f.write('QT_TRANSLATE_NOOP("bitcoin-core", "%s"),\n' % (os.getenv('PACKAGE_NAME'),))
 f.write('QT_TRANSLATE_NOOP("bitcoin-core", "%s"),\n' % (os.getenv('COPYRIGHT_HOLDERS'),))
-if os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION') != os.getenv('PACKAGE_NAME'):
+if os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION') and os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION') != os.getenv('PACKAGE_NAME'):
     f.write('QT_TRANSLATE_NOOP("bitcoin-core", "%s"),\n' % (os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION'),))
 messages.sort(key=operator.itemgetter(0))
 for (msgid, msgstr) in messages:

(probably makes sense to do this for the other getenvs as well, thinking of it)

Member

luke-jr commented Jan 25, 2016

Why would those variables ever be undefined or empty? :/

Owner

laanwj commented Jan 27, 2016

As said, COPYRIGHT_HOLDERS_SUBSTITUTION ends up empty in my tests, resulting in an empty translation string. If this is not the intention something else must be going wrong.

Member

luke-jr commented Jan 28, 2016

@laanwj Ok, fixed that.

Also fixed the text sizing logic in splashscreen to be more flexible with unexpected fonts and/or name lengths; and moved PACKAGE_URL from setup.nsi.in to configure.ac's AC_INIT (note that it uses the old bitcoin.org URL in the commit itself, and updated in the merge with master).

@laanwj laanwj commented on the diff Jan 28, 2016

configure.ac
@@ -6,7 +6,9 @@ define(_CLIENT_VERSION_REVISION, 99)
define(_CLIENT_VERSION_BUILD, 0)
define(_CLIENT_VERSION_IS_RELEASE, false)
define(_COPYRIGHT_YEAR, 2016)
-AC_INIT([Bitcoin Core],[_CLIENT_VERSION_MAJOR._CLIENT_VERSION_MINOR._CLIENT_VERSION_REVISION],[https://github.com/bitcoin/bitcoin/issues],[bitcoin])
+define(_COPYRIGHT_HOLDERS,[The %s developers])
@laanwj

laanwj Jan 28, 2016

Owner

Won't this make it much to easy to 'steal' copyright, by just changing this value?
We discussed something about only making it possible to add copyright holders this way, not replace the current ones.
Or am I misunderstanding how this works and this is true already?

@luke-jr

luke-jr Jan 28, 2016

Member

It was always easy to 'steal'. This simply makes it easy to extend without stealing: changing the package name itself (in AC_INIT) won't automatically change the substitution here (as with the original PR), and it can be easily amended to "The %s and FooCoin developers".

@laanwj

laanwj Jan 29, 2016

Owner

Yes, it's easy to do anyway, if you really want to, but it should be as hard as possible to do so accidentally.
E.g. I strongly feel it should at least involve changing the source code, not just the build metadata.
Adding potential copyright holders in the build metadata is fine with me, although those in turn may have the same concern.

@luke-jr

luke-jr Jan 29, 2016

Member

How about moving _COPYRIGHT_HOLDERS_SUBSTITUTION out of the top, down to the AC_SUBST area below?

@jtimon

jtimon Jan 29, 2016

Member

This suggestion may sound stupid...but how about repeating the line if %s != "Bitcoin Core" (or just always repeating the line if that's too complicated)?
That way, when someone uses the code and wants to add something to the copyright, "The Bitcoin Core Developers" will be maintained first, before the modified one, which is what forks of this project should always do.

@laanwj

laanwj Feb 1, 2016

Owner

Moving it doesn't solve my issue; to be clear: having the copyright in a autoconf variable means it is passed to the compiler as a -D... flag. I think this is ridiculous. Copyright should be in the code, not something that can be varied by changing a compiler argument.

@jtimon's solution sounds somewhat better to me. It needs to be impossible to get rid of our copyright by just changing build metadata, without changing the actual code.

@luke-jr

luke-jr Feb 1, 2016

Member

The problem with simply moving this to the code, is that the copyright notice appears outside of code also (typically in single-line format).

@laanwj

laanwj Feb 2, 2016

Owner

OK, fair enough. In this case I only care about the copyright instance in the code (which will be shown with --version and in the about box) that should be resilient to change of compiler arguments.

If there are other ones that are also in metadata, having them be modifiable in other metadata doesn't bother me.

@luke-jr

luke-jr Feb 3, 2016

Member

Ok, how's this look now?

@laanwj

laanwj Feb 4, 2016

Owner

Looks good to me now, going to test

Owner

laanwj commented Feb 4, 2016

Bitcoin Core Daemon version v0.12.99.0-b204181-dirty
Copyright (C) 2009-2016 The Shitcoin Core developers
Copyright (C) 2009-2016 The Bitcoin Core developers

(could nit on the years, probably derived software will have a different year range for their own development, but it's good enough for a fallback)
ACK a68bb9f

@laanwj laanwj merged commit a68bb9f into bitcoin:master Feb 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 4, 2016

Merge #7192: Unify product name to as few places as possible
027fdb8 When/if the copyright line does not mention Bitcoin Core developers, add a second line to copyrights in -version, About dialog, and splash screen (Luke Dashjr)
cc2095e Rewrite FormatParagraph to handle newlines within input strings correctly (Luke Dashjr)
cddffaf Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substitutions so it gets passed to extract-strings correctly (Luke Dashjr)
29598e4 Move PACKAGE_URL to configure.ac (Luke Dashjr)
78ec83d splashscreen: Resize text to fit exactly (Luke Dashjr)
3cae140 Bugfix: Actually use _COPYRIGHT_HOLDERS_SUBSTITUTION everywhere (Luke Dashjr)
4d5a3df Bugfix: gitian-descriptors: Add missing python-setuptools requirement for OS X (biplist module) (Luke Dashjr)
e4ab5e5 Bugfix: Correct copyright year in Mac DMG background image (Luke Dashjr)
917b1d0 Set copyright holders displayed in notices separately from the package name (Luke Dashjr)
c39a6ff Travis & gitian-osx: Use depends for ds_store and mac_alias modules (Luke Dashjr)
902ccde depends: Add mac_alias to depends (Luke Dashjr)
82a2d98 depends: Add ds_store to depends (Cory Fields)
de619a3 depends: Pass PYTHONPATH along to configure (Cory Fields)
e611b6e macdeploy: Use rsvg-convert rather than cairosvg (Luke Dashjr)
63bcdc5 More complicated package name substitution for Mac deployment (Luke Dashjr)
1a6c67c Parameterise 2009 in translatable copyright strings (Luke Dashjr)
d5f4683 Unify package name to as few places as possible without major changes (Luke Dashjr)

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Jun 30, 2016

kyuupichan added a commit to kyuupichan/BitcoinUnlimited that referenced this pull request Mar 11, 2017

Merge #7192: Unify product name to as few places as possible
027fdb8 When/if the copyright line does not mention Bitcoin Core developers, add a second line to copyrights in -version, About dialog, and splash screen (Luke Dashjr)
cc2095e Rewrite FormatParagraph to handle newlines within input strings correctly (Luke Dashjr)
cddffaf Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substitutions so it gets passed to extract-strings correctly (Luke Dashjr)
29598e4 Move PACKAGE_URL to configure.ac (Luke Dashjr)
78ec83d splashscreen: Resize text to fit exactly (Luke Dashjr)
3cae140 Bugfix: Actually use _COPYRIGHT_HOLDERS_SUBSTITUTION everywhere (Luke Dashjr)
4d5a3df Bugfix: gitian-descriptors: Add missing python-setuptools requirement for OS X (biplist module) (Luke Dashjr)
e4ab5e5 Bugfix: Correct copyright year in Mac DMG background image (Luke Dashjr)
917b1d0 Set copyright holders displayed in notices separately from the package name (Luke Dashjr)
c39a6ff Travis & gitian-osx: Use depends for ds_store and mac_alias modules (Luke Dashjr)
902ccde depends: Add mac_alias to depends (Luke Dashjr)
82a2d98 depends: Add ds_store to depends (Cory Fields)
de619a3 depends: Pass PYTHONPATH along to configure (Cory Fields)
e611b6e macdeploy: Use rsvg-convert rather than cairosvg (Luke Dashjr)
63bcdc5 More complicated package name substitution for Mac deployment (Luke Dashjr)
1a6c67c Parameterise 2009 in translatable copyright strings (Luke Dashjr)
d5f4683 Unify package name to as few places as possible without major changes (Luke Dashjr)

sickpig added a commit to sickpig/BitcoinUnlimited that referenced this pull request Mar 31, 2017

Merge #7192: Unify product name to as few places as possible
027fdb8 When/if the copyright line does not mention Bitcoin Core developers, add a second line to copyrights in -version, About dialog, and splash screen (Luke Dashjr)
cc2095e Rewrite FormatParagraph to handle newlines within input strings correctly (Luke Dashjr)
cddffaf Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substitutions so it gets passed to extract-strings correctly (Luke Dashjr)
29598e4 Move PACKAGE_URL to configure.ac (Luke Dashjr)
78ec83d splashscreen: Resize text to fit exactly (Luke Dashjr)
3cae140 Bugfix: Actually use _COPYRIGHT_HOLDERS_SUBSTITUTION everywhere (Luke Dashjr)
4d5a3df Bugfix: gitian-descriptors: Add missing python-setuptools requirement for OS X (biplist module) (Luke Dashjr)
e4ab5e5 Bugfix: Correct copyright year in Mac DMG background image (Luke Dashjr)
917b1d0 Set copyright holders displayed in notices separately from the package name (Luke Dashjr)
c39a6ff Travis & gitian-osx: Use depends for ds_store and mac_alias modules (Luke Dashjr)
902ccde depends: Add mac_alias to depends (Luke Dashjr)
82a2d98 depends: Add ds_store to depends (Cory Fields)
de619a3 depends: Pass PYTHONPATH along to configure (Cory Fields)
e611b6e macdeploy: Use rsvg-convert rather than cairosvg (Luke Dashjr)
63bcdc5 More complicated package name substitution for Mac deployment (Luke Dashjr)
1a6c67c Parameterise 2009 in translatable copyright strings (Luke Dashjr)
d5f4683 Unify package name to as few places as possible without major changes (Luke Dashjr)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7192: Unify product name to as few places as possible
027fdb8 When/if the copyright line does not mention Bitcoin Core developers, add a second line to copyrights in -version, About dialog, and splash screen (Luke Dashjr)
cc2095e Rewrite FormatParagraph to handle newlines within input strings correctly (Luke Dashjr)
cddffaf Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substitutions so it gets passed to extract-strings correctly (Luke Dashjr)
29598e4 Move PACKAGE_URL to configure.ac (Luke Dashjr)
78ec83d splashscreen: Resize text to fit exactly (Luke Dashjr)
3cae140 Bugfix: Actually use _COPYRIGHT_HOLDERS_SUBSTITUTION everywhere (Luke Dashjr)
4d5a3df Bugfix: gitian-descriptors: Add missing python-setuptools requirement for OS X (biplist module) (Luke Dashjr)
e4ab5e5 Bugfix: Correct copyright year in Mac DMG background image (Luke Dashjr)
917b1d0 Set copyright holders displayed in notices separately from the package name (Luke Dashjr)
c39a6ff Travis & gitian-osx: Use depends for ds_store and mac_alias modules (Luke Dashjr)
902ccde depends: Add mac_alias to depends (Luke Dashjr)
82a2d98 depends: Add ds_store to depends (Cory Fields)
de619a3 depends: Pass PYTHONPATH along to configure (Cory Fields)
e611b6e macdeploy: Use rsvg-convert rather than cairosvg (Luke Dashjr)
63bcdc5 More complicated package name substitution for Mac deployment (Luke Dashjr)
1a6c67c Parameterise 2009 in translatable copyright strings (Luke Dashjr)
d5f4683 Unify package name to as few places as possible without major changes (Luke Dashjr)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7192: Unify product name to as few places as possible
027fdb8 When/if the copyright line does not mention Bitcoin Core developers, add a second line to copyrights in -version, About dialog, and splash screen (Luke Dashjr)
cc2095e Rewrite FormatParagraph to handle newlines within input strings correctly (Luke Dashjr)
cddffaf Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substitutions so it gets passed to extract-strings correctly (Luke Dashjr)
29598e4 Move PACKAGE_URL to configure.ac (Luke Dashjr)
78ec83d splashscreen: Resize text to fit exactly (Luke Dashjr)
3cae140 Bugfix: Actually use _COPYRIGHT_HOLDERS_SUBSTITUTION everywhere (Luke Dashjr)
4d5a3df Bugfix: gitian-descriptors: Add missing python-setuptools requirement for OS X (biplist module) (Luke Dashjr)
e4ab5e5 Bugfix: Correct copyright year in Mac DMG background image (Luke Dashjr)
917b1d0 Set copyright holders displayed in notices separately from the package name (Luke Dashjr)
c39a6ff Travis & gitian-osx: Use depends for ds_store and mac_alias modules (Luke Dashjr)
902ccde depends: Add mac_alias to depends (Luke Dashjr)
82a2d98 depends: Add ds_store to depends (Cory Fields)
de619a3 depends: Pass PYTHONPATH along to configure (Cory Fields)
e611b6e macdeploy: Use rsvg-convert rather than cairosvg (Luke Dashjr)
63bcdc5 More complicated package name substitution for Mac deployment (Luke Dashjr)
1a6c67c Parameterise 2009 in translatable copyright strings (Luke Dashjr)
d5f4683 Unify package name to as few places as possible without major changes (Luke Dashjr)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment