refactor(upgrade): use Bazel packages to avoid symlinks in the source by petebacondarwin · Pull Request #29466 · angular/angular

petebacondarwin · 2019-03-22T10:54:57Z

Previously we had to share code between upgrade/dynamic and upgrade/static
by symlinking the src folder, which allowed both packages to access
the upgrade/common files.

These symlinks are always problematic on Windows, where we had to run
a script to re-link them, and restore them.

This change uses Bazel packages to share the upgrade/common code,
which avoids the need for symlinking the src folder.

Also, the Windows specific scripts that fixup the symlinks have also
been removed as there is no more need for them.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

Yes
No

Other information

filipesilva

Can you also remove the newly added scripts/windows/create-symlinks.cmd, and remove its usage in .codefresh/codefresh.yml?

I was trialing a new Windows CI that uses needs that script so had to add an actual windows CMD for it.

petebacondarwin · 2019-03-22T11:15:32Z

Will do!

gkalpak

Woohoo! 🎉 🎉 🎉

mhevery · 2019-03-25T16:33:07Z

http://test/OCL:240155732:BASE:240155805:1553531503722:36978456

mhevery · 2019-03-25T17:45:39Z

Breaks g3:


third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:9:84 - error TS2307: Cannot find module '../common/src/angular1'.
	to fix, run:
	taze //third_party/javascript/angular2

9 export {getAngularJSGlobal, getAngularLib, setAngularJSGlobal, setAngularLib} from '../common/src/angular1';
                                                                                     ~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:10:34 - error TS2307: Cannot find module '../common/src/downgrade_component'.
	to fix, run:
	taze //third_party/javascript/angular2

10 export {downgradeComponent} from '../common/src/downgrade_component';
                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:11:35 - error TS2307: Cannot find module '../common/src/downgrade_injectable'.
	to fix, run:
	taze //third_party/javascript/angular2

11 export {downgradeInjectable} from '../common/src/downgrade_injectable';
                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:12:23 - error TS2307: Cannot find module '../common/src/version'.
	to fix, run:
	taze //third_party/javascript/angular2

12 export {VERSION} from '../common/src/version';
                         ~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:13:31 - error TS2307: Cannot find module './src/downgrade_module'.
	to fix, run:
	taze //third_party/javascript/angular2

13 export {downgradeModule} from './src/downgrade_module';
                                 ~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:14:32 - error TS2307: Cannot find module './src/upgrade_component'.
	to fix, run:
	taze //third_party/javascript/angular2

14 export {UpgradeComponent} from './src/upgrade_component';
                                  ~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:15:29 - error TS2307: Cannot find module './src/upgrade_module'.
	to fix, run:
	taze //third_party/javascript/angular2

15 export {UpgradeModule} from './src/upgrade_module';
                               ~~~~~~~~~~~~~~~~~~~~~~

IgorMinar

I discussed this with Pete and we agreed that it would be better to move /dynamics and /common under src to prevent these folders to be accidentally considered and used as entry points and public api.

otherwise this change looks great once these adjustments are made.

petebacondarwin · 2019-03-27T19:02:19Z

Done. It'll need another presubmit.

mhevery · 2019-03-27T19:49:28Z

http://test/OCL:240155732:BASE:240621012:1553716123697:cf97eb17

ERROR: third_party/javascript/angular2/BUILD:916:1: Compiling Angular templates (ngtsc) //third_party/javascript/angular2:upgrade failed (Exit 1) ngc_wrapped failed: error executing command blaze-out/host/bin/javascript/angular2/tools/ngc_wrapped/ngc_wrapped '--node_options=--expose-gc' '--node_options=--stack-trace-limit=100' '--node_options=--max-old-space-size=2048' ... (remaining 1 argument(s) skipped).  [forge_remote_host=ikch12]
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:9:84 - error TS2307: Cannot find module '../src/common/src/angular1'.
	to fix, run:
	taze //third_party/javascript/angular2

9 export {getAngularJSGlobal, getAngularLib, setAngularJSGlobal, setAngularLib} from '../src/common/src/angular1';
                                                                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:10:34 - error TS2307: Cannot find module '../src/common/src/downgrade_component'.
	to fix, run:
	taze //third_party/javascript/angular2

10 export {downgradeComponent} from '../src/common/src/downgrade_component';
                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:11:35 - error TS2307: Cannot find module '../src/common/src/downgrade_injectable'.
	to fix, run:
	taze //third_party/javascript/angular2

11 export {downgradeInjectable} from '../src/common/src/downgrade_injectable';
                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:12:23 - error TS2307: Cannot find module '../src/common/src/version'.
	to fix, run:
	taze //third_party/javascript/angular2

12 export {VERSION} from '../src/common/src/version';
                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:13:31 - error TS2307: Cannot find module './src/downgrade_module'.
	to fix, run:
	taze //third_party/javascript/angular2

13 export {downgradeModule} from './src/downgrade_module';
                                 ~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:14:32 - error TS2307: Cannot find module './src/upgrade_component'.
	to fix, run:
	taze //third_party/javascript/angular2

14 export {UpgradeComponent} from './src/upgrade_component';
                                  ~~~~~~~~~~~~~~~~~~~~~~~~~
third_party/javascript/angular2/rc/packages/upgrade/static/public_api.ts:15:29 - error TS2307: Cannot find module './src/upgrade_module'.
	to fix, run:
	taze //third_party/javascript/angular2

15 export {UpgradeModule} from './src/upgrade_module';
                               ~~~~~~~~~~~~~~~~~~~~~~

jasonaden · 2019-04-01T18:17:33Z

Presubmit

Previously we had to share code between upgrade/dynamic and upgrade/static by symlinking the `src` folder, which allowed both packages to access the upgrade/common files. These symlinks are always problematic on Windows, where we had to run a script to re-link them, and restore them. This change uses Bazel packages to share the `upgrade/common` code, which avoids the need for symlinking the `src` folder. Also, the Windows specific scripts that fixup the symlinks have also been removed as there is no more need for them.

… source

gkalpak · 2019-04-02T18:47:06Z

\o/

…angular#29466) Previously we had to share code between upgrade/dynamic and upgrade/static by symlinking the `src` folder, which allowed both packages to access the upgrade/common files. These symlinks are always problematic on Windows, where we had to run a script to re-link them, and restore them. This change uses Bazel packages to share the `upgrade/common` code, which avoids the need for symlinking the `src` folder. Also, the Windows specific scripts that fixup the symlinks have also been removed as there is no more need for them. PR Close angular#29466

angular-automatic-lock-bot · 2019-09-14T22:22:31Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

petebacondarwin requested a review from IgorMinar as a code owner March 22, 2019 10:54

petebacondarwin requested review from a team March 22, 2019 10:54

ngbot Bot added this to the Backlog milestone Mar 22, 2019

googlebot added the cla: yes label Mar 22, 2019

petebacondarwin mentioned this pull request Mar 22, 2019

feat(upgrade): provide test helpers for wiring up injectors #16848

Closed

filipesilva suggested changes Mar 22, 2019

View reviewed changes

petebacondarwin force-pushed the ngupgrade-windows-support branch from 3d75825 to a27568b Compare March 22, 2019 11:16

petebacondarwin requested a review from filipesilva March 22, 2019 12:13

gkalpak approved these changes Mar 22, 2019

View reviewed changes

gkalpak added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Mar 22, 2019

filipesilva approved these changes Mar 22, 2019

View reviewed changes

petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Mar 22, 2019

alexeagle approved these changes Mar 22, 2019

View reviewed changes

petebacondarwin force-pushed the ngupgrade-windows-support branch from 47957ac to 90b046d Compare March 23, 2019 20:02

IgorMinar approved these changes Mar 27, 2019

View reviewed changes

IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 27, 2019

petebacondarwin removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 27, 2019

gkalpak reviewed Mar 28, 2019

View reviewed changes

Comment thread tools/public_api_guard/BUILD.bazel Resolved

Comment thread tools/public_api_guard/BUILD.bazel Resolved

jasonaden added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 1, 2019

petebacondarwin added 5 commits April 1, 2019 19:27

fixup! refactor(upgrade): use Bazel packages to avoid symlinks in the…

ee2d384

… source

fixup! refactor(upgrade): use Bazel packages to avoid symlinks in the…

c9f5a03

… source

fixup! refactor(upgrade): use Bazel packages to avoid symlinks in the…

b62cdd7

… source

fixup! refactor(upgrade): use Bazel packages to avoid symlinks in the…

a673140

… source

petebacondarwin force-pushed the ngupgrade-windows-support branch from eb5ee3d to a673140 Compare April 1, 2019 18:27

jasonaden removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 1, 2019

petebacondarwin added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Apr 2, 2019

jasonaden closed this in 9f54d76 Apr 2, 2019

petebacondarwin deleted the ngupgrade-windows-support branch April 3, 2019 07:42

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019

Conversation

petebacondarwin commented Mar 22, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

filipesilva left a comment

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Mar 22, 2019

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhevery commented Mar 25, 2019

Uh oh!

mhevery commented Mar 25, 2019

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Mar 27, 2019

Uh oh!

mhevery commented Mar 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasonaden commented Apr 1, 2019

Uh oh!

gkalpak commented Apr 2, 2019

Uh oh!

angular-automatic-lock-bot Bot commented Sep 14, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mhevery commented Mar 27, 2019 •

edited

Loading