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: ICU shrinker needs to adapt to new output path #23245

Closed
srl295 opened this issue Oct 3, 2018 · 4 comments

Comments

@srl295
Copy link
Member

commented Oct 3, 2018

  • Version:v11.0.0-pre
  • Platform:all
  • Subsystem:tools

The location of the ICU small datafile changed from out/Release/gen/icutmp to out/Release/obj/gen/icutmp

Not critical (because there's a workaround), but would be nice to fix the default. Supports #23244

@srl295 srl295 added the intl label Oct 3, 2018
@srl295 srl295 self-assigned this Oct 3, 2018
@srl295

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2018

also, #22450 moved configure to configure.py, so fix the docs also

@srl295 srl295 added the doc label Oct 3, 2018
srl295 added a commit to srl295/node that referenced this issue Oct 3, 2018
- tools: path to ICU datafile moved
- docs: configure is now configure.py

Fixes: nodejs#23245
@refack

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

@srl295 Do the docs refer to this value:

def configure_intl(o):
  icus = [
    {
      'url': 'https://sourceforge.net/projects/icu/files/ICU4C/62.1/icu4c-62_1-src.zip',
      'md5': '408854f7b9b58311b68fab4b4dfc80be',
    },
  ]

I would actually believe is would be easy to read it from a file (say tools/icu/current_ver.dep)?

@refack

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

BTW @srl295 did you mean to post this and #23244 as PRs or issues?

@srl295

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2018

@refack issues. PR coming soon, but 63.1 is not final yet.

@srl295 srl295 referenced this issue Oct 4, 2018
3 of 3 tasks complete
refack added a commit to refack/node that referenced this issue Oct 4, 2018
@refack refack referenced this issue Oct 4, 2018
3 of 3 tasks complete
refack added a commit to refack/node that referenced this issue Oct 4, 2018
ATM on every ICU version bump we need to update these data.
Reading it from a file makes it independant of `configre.py` changes.

Refs: nodejs#23245
refack added a commit to refack/node that referenced this issue Oct 4, 2018
ATM on every ICU version bump we need to update these data.
Reading it from a file makes it independant of `configre.py` changes.

Refs: nodejs#23245
srl295 added a commit to srl295/node that referenced this issue Oct 4, 2018
- tools: path to ICU datafile moved
- docs: configure is now configure.py

Fixes: nodejs#23245

PR-URL: nodejs#23266
Reviewed-By: Refael Ackermann <refack@gmail.com>
@srl295 srl295 closed this in #23266 Oct 4, 2018
targos added a commit that referenced this issue Oct 5, 2018
- tools: path to ICU datafile moved
- docs: configure is now configure.py

Fixes: #23245

PR-URL: #23266
Reviewed-By: Refael Ackermann <refack@gmail.com>
refack added a commit to refack/node that referenced this issue Oct 5, 2018
ATM on every ICU version bump we need to update these data.
Reading it from a file makes it independant of `configre.py` changes.

Refs: nodejs#23245
targos added a commit that referenced this issue Oct 7, 2018
- tools: path to ICU datafile moved
- docs: configure is now configure.py

Fixes: #23245

PR-URL: #23266
Reviewed-By: Refael Ackermann <refack@gmail.com>
refack added a commit to refack/node that referenced this issue Oct 12, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: nodejs#23269
Refs: nodejs#23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos added a commit that referenced this issue Oct 12, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: #23269
Refs: #23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos added a commit that referenced this issue Oct 12, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: #23269
Refs: #23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
jasnell added a commit that referenced this issue Oct 17, 2018
- tools: path to ICU datafile moved
- docs: configure is now configure.py

Fixes: #23245

PR-URL: #23266
Reviewed-By: Refael Ackermann <refack@gmail.com>
jasnell added a commit that referenced this issue Oct 17, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: #23269
Refs: #23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins added a commit that referenced this issue Oct 30, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: #23269
Refs: #23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg added a commit that referenced this issue Nov 28, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: #23269
Refs: #23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins added a commit that referenced this issue Nov 29, 2018
* ATM on every ICU version bump we need to update these data. Reading
  it from a file makes it independant of `configre.py` changes.
* Update guide.

PR-URL: #23269
Refs: #23245
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.