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

[vcpkg-artifacts] Fix end to end development. #586

Merged
merged 11 commits into from
Jun 24, 2022

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Jun 13, 2022

Thanks to @fearthecowboy for help on this one.

I considered ripping out the rush dependency, since it seems like we are only ever going to have one meaningful "project", and the dependency deduplication we need seems to be already done by pnpm (rather than npm), but after talking with @fearthecowboy I've decided to not go there since we still have rush linking the test project in.

Unfortunately, there does not appear to be an effiicent way to build the typescript parts out-of-source, since they depend on node_modules which is put into the source tree.

This adds a vcpkg.ps1 which does the same "environment hacking" as the in-development ce.ps1, teaches CMakeLists.txt to invoke rush and the typescript compiler as necessary, and teaches vcpkg.exe to use a hard-coded-into-the-binary path to the source tree when that in-development setting is turned on.

The previous "always download latest ce bits" behavior is retained for folks who build vcpkg.exe from source and don't want to arrange for node and rush to be available.

Resolves https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1551959

Thanks to @fearthecowboy for help on this one.

I considered ripping out the rush dependency, since it seems like we are only ever going to have one meaningful "project", and the dependency deduplication we need seems to be already done by pnpm (rather than npm), but after talking with @fearthecowboy I've decided to not go there since we still have rush linking the test project in.

Unfortunately, there does not appear to be an effiicent way to build the typescript parts out-of-source, since they depend on node_modules which is put into the source tree.

This adds a vcpkg.ps1 which does the same "environment hacking" as the in-development ce.ps1, teaches CMakeLists.txt to invoke rush and the typescript compiler as necessary, and teaches vcpkg.exe to use a hard-coded-into-the-binary path to the source tree when that in-development setting is turned on.

The previous "always download latest ce bits" behavior is retained for folks who build vcpkg.exe from source and don't want to arrange for node and rush to be available.
@BillyONeal
Copy link
Member Author

image

"outDir": "./dist",
"rootDir": ".",
"types": [
"node",
"mocha"
],
"inlineSourceMap": true,
Copy link
Member

Choose a reason for hiding this comment

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

Was inlineSourceMap causing a problem?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was inlineSourceMap causing a problem?

No, but it's already specified in the common one here:

"inlineSourceMap": true,

(I deduplicated any such settings I noticed)


add_custom_command(
OUTPUT
"${CMAKE_CURRENT_SOURCE_DIR}/ce/ce/node_modules"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an imperfect output connection; if the packages.json is modified it won't be regenerated. I'm not sure what the right definition would be at this time; maybe just having a "stamp" file that we create after running the rush commands.

Copy link
Member Author

Choose a reason for hiding this comment

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

Everything related to the ecmascript dependency detection is imperfect unfortunately. This is "good enough" to do development work; unfortunately there doesn't seem to be a way to make it conclusively correct as long as Rush is involved.

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@BillyONeal BillyONeal merged commit 426593e into microsoft:main Jun 24, 2022
@BillyONeal BillyONeal deleted the artifacts-dev branch June 24, 2022 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants