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.

Sign up for GitHub

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

Jump to bottom

closure-make-deps generates incorrect paths on Windows. #1159

Closed
yamadayutaka opened this issue Mar 28, 2022 · 4 comments
Closed

closure-make-deps generates incorrect paths on Windows. #1159

yamadayutaka opened this issue Mar 28, 2022 · 4 comments

Comments

@yamadayutaka
Copy link
Contributor

yamadayutaka commented Mar 28, 2022 •
edited

To Reproduce

  1. Create working directory (e.g. C:\work\test).
  2. Change directory (e.g. cd C:\\work\\test).
  3. Clone closure-library (e.g. git clone https://github.com/google/closure-library.git).
  4. Change directory (e.g. cd closure-library).
  5. Install npm package (e.g. npm install).

Following the above steps will generate closure/goog/deps.js.

When you open deps.js, it has the following contents.

goog.addDependency('../C:\work\test\closure-library\closure\goog\a11y\aria\announcer.js', ['goog.a11y.aria.Announcer'], ['goog.Disposable', 'goog.Timer', 'goog.a11y.aria', 'goog.a11y.aria.LivePriority', 'goog.a11y.aria.State', 'goog.dom', 'goog.dom.TagName', 'goog.object', 'goog.string'], {'lang': 'es6'});
goog.addDependency('../C:\work\test\closure-library\closure\goog\a11y\aria\aria.js', ['goog.a11y.aria'], ['goog.a11y.aria.Role', 'goog.a11y.aria.State', 'goog.a11y.aria.datatables', 'goog.array', 'goog.asserts', 'goog.dom', 'goog.dom.TagName', 'goog.object', 'goog.string']);

Expected behavior

The following content will be generated.

goog.addDependency('a11y/aria/announcer.js', ['goog.a11y.aria.Announcer'], ['goog.Disposable', 'goog.Timer', 'goog.a11y.aria', 'goog.a11y.aria.LivePriority', 'goog.a11y.aria.State', 'goog.dom', 'goog.dom.TagName', 'goog.object', 'goog.string'], {'lang': 'es6'});
goog.addDependency('a11y/aria/aria.js', ['goog.a11y.aria'], ['goog.a11y.aria.Role', 'goog.a11y.aria.State', 'goog.a11y.aria.datatables', 'goog.array', 'goog.asserts', 'goog.dom', 'goog.dom.TagName', 'goog.object', 'goog.string']);

Additional context

In "depswriter.py", the path is normalized by the following code.

https://github.com/google/closure-library/blob/master/closure/bin/build/depswriter.py#L125-L136
@yamadayutaka
Copy link
Contributor Author

I have a solution and I am preparing for PR.

@yamadayutaka yamadayutaka mentioned this issue Mar 28, 2022
Merged
@joeyparrish
Copy link
Member

Can this be considered closed now that the PR has been merged? Or does this project only close issues when a release is completed?

@joeyparrish joeyparrish mentioned this issue Mar 30, 2022
Closed
joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Mar 30, 2022
@joeyparrish
fix: Fix broken deps file generation on Windows
598915c 
This fixes broken deps file generation on Window by upgrading to a
pre-release version of the deps tool.  The upstream issue was filed in
google/closure-library#1159 and fixed in
google/closure-library#1160.

This issue has never affected a release version of Shaka Player, and
was introduced when we switched deps tools in 6f274cb.

Closes shaka-project#4085
@joeyparrish joeyparrish mentioned this issue Mar 30, 2022
Merged
@joeyparrish
Copy link
Member

For anyone else on this issue looking for a solution before the next release is published, this is how you use a pre-release version at the SHA1 where this issue was fixed: https://github.com/shaka-project/shaka-player/pull/4086/files

@shicks
Copy link
Member

shicks commented Mar 30, 2022

We usually try to set it up so that merging the PR auto-closes the issue. I guess that didn't happen here.

@shicks shicks closed this as completed Mar 30, 2022
joeyparrish added a commit to shaka-project/shaka-player that referenced this issue Mar 31, 2022
@joeyparrish
fix: Fix broken deps file generation on Windows (#4086)
9660ce8 
This fixes broken deps file generation on Window by upgrading to a
pre-release version of the deps tool.  The upstream issue was filed in
google/closure-library#1159 and fixed in
google/closure-library#1160.

This issue has never affected a release version of Shaka Player, and
was introduced when we switched deps tools in 6f274cb.

Closes #4085
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@shicks @joeyparrish @yamadayutaka