Replace many occurrences of the old (non-https and HTTP/301 moved permanently anyway) URL to MIT license by newer link.
(Archive) First_ attempt was: In two steps, update the links as described in the issue.
First commit: sed everything.
Second commit: Fix whitespace syntactic sugar that got bitter due to different length of URLs.
0 | @@ -1,7 +1,7 @@ 1 | /********************************************************************* 2 | * Copyright (c) 2016 Pieter Wuille * 3 | * Distributed under the MIT software license, see the accompanying * 4 | - * file COPYING or http://www.opensource.org/licenses/mit-license.php.* 5 | + * file COPYING or https://opensource.org/licenses/mit-license.php. *
Should be submitted upstream for the ctaes subtree, if wanted
Good point, I do not know the codebase very well yet. Issued PR to ctaes in https://github.com/bitcoin-core/ctaes/pull/13 .
@fwolfst Please see the notes on Scripted Diffs in doc/developer-notes.md.
Please note that there has been a large increase in PR activity recently (a good problem to have!) and reviewers are struggling to keep up, so simple refactoring, language changes or other cleanup sometimes don't get a lot of immediate attention.
You could also help with review, if you feel like it. Due to the amount of open pull requests, there are some easier ones to get started.
Even with limited understanding of the code, just compiling them and passing in edge-cases until they break is useful.
https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+is%3Aopen+label%3ADocs https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+is%3Aopen+label%3ATests https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+is%3Aopen+label%3ARPC%2FREST%2FZMQ
@fwolfst Please see the notes on Scripted Diffs in doc/developer-notes.md.
@morcos Thanks for pointing that out, I'll happily do that. Sorry for being such a pita-noob, whats the suggested work flow now? Closing this PR and reopen a new one?
Edit Will figure that one out - will re-read CONTRIBUTE.md and just try if a changed commit message influences the PR.
0 | @@ -1,7 +1,7 @@ 1 | /********************************************************************** 2 | * Copyright (c) 2015 Pieter Wuille * 3 | * Distributed under the MIT software license, see the accompanying * 4 | - * file COPYING or http://www.opensource.org/licenses/mit-license.php.* 5 | + * file COPYING or https://opensource.org/licenses/mit-license.php. * 6 | **********************************************************************/
Please send secp256k1 changes upstream.
Thanks for pointing that out. Issued in https://github.com/bitcoin-core/secp256k1/issues/503 .
0 | @@ -1,6 +1,6 @@ 1 | // Copyright 2014 BitPay Inc. 2 | // Distributed under the MIT software license, see the accompanying 3 | -// file COPYING or http://www.opensource.org/licenses/mit-license.php. 4 | +// file COPYING or https://opensource.org/licenses/mit-license.php.
Univalue changes also need to go upstream.
Thanks for pointing that out. Issued at https://github.com/jgarzik/univalue/issues/47 .
@fwolfst Please don't close PRs and open new ones with the same changes. For this to be merged, you'll need to remove any changes to sub-trees in this repository (you can open PRs with the changes upstream if you'd like).
Then rebase and update the commits to be proper scripted diffs, see developer-notes.
A large diff like this will likely be merged after a branch off.
Needs rebase if still relevant
will do that next week
On Sat, Feb 10, 2018 at 8:13 PM, MarcoFalke notifications@github.com wrote:
Needs rebase if still relevant
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/12192#issuecomment-364681639, or mute the thread https://github.com/notifications/unsubscribe-auth/AANY7XmNYdqhXMOtKkrRfdjKc5pFEDYeks5tTepFgaJpZM4Reeff .
From a 'use https everywhere' perspective I agree with this change, but changing every single source file seems a bit much.
New URL ACK:
curl -v http://www.opensource.org/licenses/mit-license.php
< HTTP/1.1 301 Moved Permanently
< Location: http://opensource.org/licenses/mit-license.php
curl -v http://opensource.org/licenses/mit-license.php
< HTTP/1.1 301 Moved Permanently
< Location: https://opensource.org/licenses/mit-license.php
curl -v https://opensource.org/licenses/mit-license.php
< HTTP/1.1 200 OK
Agree with @morcos regarding scripted diff. @laanwj does this change conflicts with other branches?
@laanwj does this change conflicts with other branches?
Depending on how github computes conflicts (I'm not sure if, and if so how much, lines of context it uses - but how easy this builds up a list of conflicts below is one omen), possibly every single one.
I will rebase soon and include a scripted_diff commit (I will have to figure out how this works WITHIN this PR). The target is moving a bit, because changes have already been accepted in included projects (https://github.com/jgarzik/univalue/issues/47 and I guess https://github.com/bitcoin-core/secp256k1/issues/503 will follow).
I am not git native yet, but I think https://github.com/bitcoin/bitcoin/pull/12192/commits/8d162082a744cd4688be615df0f007936f0bdba3 comes close to what should be (have been) done:
I reset my branch, merged upstream and created a new "scripted" diff.
There are still some 70 occurences of the old URL, which I will gladly scripted-diff-replace once I get green lights here.
@fanquake Dealt with in https://github.com/fwolfst/bitcoin/commit/a164d7f6dd5df1dbb9a77c56a57350f469e4b2da . Thanks.
utACK a164d7f6dd5df1dbb9a77c56a57350f469e4b2da
At this point you might as well bump the copyright headers altogether. Please add another commit with the dates bumped to 2018 (see ./contrib/devtools/README.md).
@MarcoFalke Yep, cool. Tough my preferred way to deal with the copyright extension would be the following: you create an issue for that and assign it to me - I'd personally prefer to keep that stuff in a separate PR, as it has a different class of legal implications, but if the majority thinks otherwise I'd give it a go.
Well its a legal statement indicating when (in certain countries) these (versions of the) files will enter the public domain. Anyway. Done.
Needs rebase
@MarcoFalke Thanks for seeing that. Done. Now, needs merge ;)
0 | @@ -1,8 +1,8 @@ 1 | #!/usr/bin/env python3 2 | -# Copyright (c) 2015-2017 The Bitcoin Core developers 3 | +# Copyright (c) 2015-2018 The Bitcoin Core developers 4 | # Distributed under the MIT software license, see the accompanying 5 | -# file COPYING or http://www.opensource.org/licenses/mit-license.php. 6 | -"""Test transaction signing using the signrawtransaction* RPCs.""" 7 | +# file COPYING or https://opensource.org/licenses/mit-license.php. 8 | +"""Test transaction signing using the signrawtransaction* RPC."""
Why this change?
My mistake. Glitch in my workflow after new rebase experience. Will fix soon. If I might ask: currently I replace old commits in this PR/my branch with hard resets and push -f, to keep changes small and honor the scripted_diff checks (except in last case, my apologies) - is that a correct thing to do?
Jup, correct. You can also check the scripted diffs with the ./contrib/devtools/commit-script-check.sh script.
Corrected in cdc7d3c . And also, I adjusted the URL. The canonical URL (and the one linked already in Bitcoins README as well as from within the opensource.org site) is https://opensource.org/licenses/MIT .
curl --silent --head https://opensource.org/licenses/mit-license.php -o - | grep canon
Link: </licenses/MIT>; rel="canonical",</node/66>; rel="shortlink"And also, I adjusted the URL. The canonical URL (and the one linked already in Bitcoins README as well as from within the opensource.org site) is https://opensource.org/licenses/MIT .
Yup, good catch! If we're going to change this, let's be 100% sure we're using the correct address, and the one that is most likely to stay the same going forward.
Which makes me wonder (sorry for always being a heretic) - what is the motivation for an external link at all? We carry a copy of the license in the repository, after all. Is the answer to cover for individually copied files?
Gregory asked that as well and I had a good answer that I don't find in the archives - probably it was "moderated away". It'd find it odd if header stuff were to be removed, but would not consider it a serious issue to dramatize about.
Why not skip all the header stuff?
Btw afaik same is true for the copyright headers in many legislatives. They are not necessarily needed to protect your IP.
It'd opt for the headers though, to make the statement that a conscious choice and responsability was taken, and that these decisions can be seen as part of a bigger picture.
On Fri, Feb 23, 2018 at 11:46 AM, Wladimir J. van der Laan < notifications@github.com> wrote:
And also, I adjusted the URL. The canonical URL (and the one linked already in Bitcoins README as well as from within the opensource.org site) is https://opensource.org/licenses/MIT .
Yup, good catch! If we're going to change this, let's be 100% sure we're using the correct address, and the one that is most likely to stay the same going forward.
Which makes me wonder (sorry for always being a heretic) - what is the motivation for an external link at all? We carry a copy of the license in the repository, after all. Is the answer to cover for individually copied files?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/12192#issuecomment-367975068, or mute the thread https://github.com/notifications/unsubscribe-auth/AANY7SQS1XuFIDPuEzPJWXy5qCZPqh-vks5tXpcbgaJpZM4Reeff .
Thanks. To be clear I'm not advocating for removing the headers completely! that would require an infinite amount of discussion which I'm not going to spend my time left on earth partaking in. I was just curious as to an external URL link, as it is both more[1] and less[2] self-contained at the same time.
*1 disambiguation of "MIT license" for individual files, as you say, without having to copy the whopping thing into every single file *2 the link is external, so it could potentially change under us
But to be clear, keeping it like this is OK to me. utACK. I've verified that commit 506c92ce208f2e620314dd9952516ebbd68b3f20 and cdc7d3ce48df4575ba1313a7225372008763c31b match the output of their scripts.
Hmm, find . will recurse into .git and potentially corrupt the directory. Could you change that to git ls-files | xargs sed ..., please?
Sorry for mentioning that so late.
This commit deals with one part of #12190 by replacing many occurrences
of
http://www.opensource.org/licenses/mit-license.php
with
https://opensource.org/licenses/MIT
. A second commit could tackle the other occurrences where either
"comment-star-boxes" (/* 80 chars */) are used or the URL appears
in a different context.
The src/univalue/ path is ignored, as it references
https://github.com/jgarzik/univalue , where a similar issue is
ady
closed (jgarzik/univalue#47) and waiting to be
included into this (bitcoin) repository.
-BEGIN VERIFY SCRIPT-
git ls-files | grep --invert-match 'src/univalue' | xargs sed -i \
-e 's,http://www.opensource.org/licenses/mit-license.php.$,https://opensource.org/licenses/MIT.,g'
-END VERIFY SCRIPT-
good point @MarcoFalke. I ran it on a temporary clone to avoid corrupting unrelated stuff such as depends downloads. But I did not think of git's internal state. git ls-files is much better.
Used the contrib/devtools/copyright_header.py script to extend
Copyright in files edited in 2018.
-BEGIN VERIFY SCRIPT-
contrib/devtools/copyright_header.py update .
-END VERIFY SCRIPT-
@laanwj Yes, you are right. Unfortunately the --exclude option of git ls-files seems to only be able to exclude untracked files.
But facepalm - messed up the commit message in https://github.com/bitcoin/bitcoin/pull/12192/commits/23f4029ad1656f266274980674f9460f497436a0 (ady instead of already). Also there are still occurrences of the old URL in those shiny /* boxes */. If there is green lights, I'd like to change the commit message, get this merged and come back with the next step in a couple of days/weeks.
utACK b3047f45a8c01aa7477a3d0752ed83ab56beb093
@fwolfst Needs rebase