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

tools: Fix license-builder.sh for ICU #4762

Closed
wants to merge 2 commits into from

Conversation

richardlau
Copy link
Member

The generated LICENSE using tools/license-builder.sh added by #4194 has omitted the entire Third-Party Software Licenses for ICU. This pull request modifies the license-builder.sh tool to add them back.

Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.
@richardlau
Copy link
Member Author

Perhaps consider for v5.5.0?

cc @rvagg @evanlucas

@Fishrock123 Fishrock123 added the meta Issues and PRs related to the general management of the project. label Jan 19, 2016
@Fishrock123
Copy link
Contributor

cc @srl295 re: ICU license

I don't know enough about sed to comment really

@@ -33,7 +33,7 @@ addlicense "c-ares" "deps/cares" \
"$(sed -e '/^ \*\/$/,$d' -e '/^$/d' -e 's/^[/ ]\* *//' ${rootdir}/deps/cares/src/ares_init.c)"
addlicense "HTTP Parser" "deps/http_parser" "$(cat deps/http_parser/LICENSE-MIT)"
addlicense "ICU" "deps/icu" \
"$(sed -e '1,/COPYRIGHT AND PERMISSION NOTICE/d' -e '/^<hr/,$d' -e 's/^<\/*p>$//' ${rootdir}/deps/icu/license.html)"
"$(sed -e '1,/ICU License - ICU 1\.8\.1 and later/d' -e :a -e 's/<[^>]*>//g;/</N;//ba' ${rootdir}/deps/icu/license.html)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you regex that exact version number out of this so we don't have to update this tool with each upgrade?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You don't have to Regex the version. The "1.8.1" is not going to change.
  • sorry about the wrap
  • Icu is going to move from a HTML to a text license. Suggestions welcome.

S

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, missed the "and later", just saw a version number and red lights went off in my brain! I'll get this sorted out now, thanks @richardlau and @srl295!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem.

!! I had manually formatted the license html into text. How long has this been omitted? Can you stick the license file into the download dir or something so it can be compliant with Icu and icu's dependents?

Icu's license.html I repeat is going away. I'll probably name the replacement LICENSE.txt. You could future proof now by checking for such a file and using cat instead.

Feed back welcome here http://bugs.icu-project.org/trac/ticket/12037

Enviado desde nuestro iPhone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srl295 license-builder.sh was introduced in 031b87d and I don't believe we've had a release since then. The idea is to touch LICENSE as little as possible and have a tool that automatically builds it and keeps it clean, so it overwrote your previous work (sorry, but it was too manual!).

Just let us know when we get LICENSE.txt and we can update it here, or you could PR it yourself, when you do away with the HTML it'll be much easier!

rvagg pushed a commit that referenced this pull request Jan 20, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: #4762
Reviewed-By: Rod Vagg <[email protected]>
rvagg pushed a commit that referenced this pull request Jan 20, 2016
@rvagg
Copy link
Member

rvagg commented Jan 20, 2016

In the interests of getting this done for a release I've gone ahead and landed this @ 2b9bd0f & c9dbd2b. While I was at it I modified the first commit to get rid of tabs and trailing whitespace in that license text.

@rvagg rvagg closed this Jan 20, 2016
evanlucas pushed a commit that referenced this pull request Jan 20, 2016
evanlucas pushed a commit that referenced this pull request Jan 20, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: #4762
Reviewed-By: Rod Vagg <[email protected]>
@srl295
Copy link
Member

srl295 commented Jan 20, 2016

@rvagg no problem… I have a test case now!

evanlucas pushed a commit that referenced this pull request Jan 20, 2016
evanlucas pushed a commit that referenced this pull request Jan 20, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: #4762
Reviewed-By: Rod Vagg <[email protected]>
@richardlau
Copy link
Member Author

I see @rvagg has landed #4194 on v4.x-staging so this PR will also need to land with it before the next release that uses the rebuilt LICENSE.

cc @srl295

rvagg pushed a commit that referenced this pull request Jan 28, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: #4762
Reviewed-By: Rod Vagg <[email protected]>
rvagg pushed a commit that referenced this pull request Jan 28, 2016
@rvagg
Copy link
Member

rvagg commented Jan 28, 2016

thanks @richardlau, also landed on v4.x-staging @ 6e6f20a & 97a5b16

MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: #4762
Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: nodejs#4762
Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: nodejs#4762
Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
@srl295
Copy link
Member

srl295 commented Feb 27, 2016

@rvagg fyi - http://bugs.icu-project.org/trac/ticket/12037 landed in ICU4C. ICU 57 will have a plain text license file.

scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Modify tools/license-builder.sh to restore the Third-Party Software
licenses for ICU.

Also fix arguments to tail to work on Linux.

rvagg: modified sed command for ICU to replace tabs with spaces and
       remove whitespace at the end of lines

PR-URL: nodejs#4762
Reviewed-By: Rod Vagg <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants