Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

[Windows] Ease building with VS Express by checking in generated files.#4450

Closed
sblom wants to merge 1 commit into
nodejs:masterfrom
MSOpenTech:express-build
Closed

[Windows] Ease building with VS Express by checking in generated files.#4450
sblom wants to merge 1 commit into
nodejs:masterfrom
MSOpenTech:express-build

Conversation

@sblom

@sblom sblom commented Dec 21, 2012

Copy link
Copy Markdown

@piscisaureus, would love your feedback this morning, since @izs would like to use this technology to help him with today's build.

Well-tested with VS 2010 Ultimate, but I still need to test this on a VS Express box, which will have to (ironically) be at the office tomorrow morning.

Meanwhile, I feel pretty good about this patch as a way to have git complain about modified files to those of us that have the WinSDK lying around (either from Visual Studio or otherwise), but to break the WinSDK dependency for folks that don't already have it installed.

Btw, I'm not happy about the obnoxious file names, but haven't found a way to change them. MSG00001.BIN, really?

@isaacs

isaacs commented Dec 21, 2012

Copy link
Copy Markdown

Landed on 841b7f5. Thanks, @sblom!

@piscisaureus I'd still love to get a review from you when you get back. If you object to this we can revert or refactor it, but it's good enough to get 0.9.4 out the door.

@isaacs isaacs closed this Dec 21, 2012
Comment thread configure

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if ctrpp.exe is not on the path? Shouldn't there be some kind of manual override?

@bnoordhuis

Copy link
Copy Markdown
Member

-1 from me. The files should be generated at configure or build time. Checking them in is deeply wrong.

@sblom

sblom commented Dec 22, 2012

Copy link
Copy Markdown
Author

@bnoordhuis, I'm with you in principle on the generated files thing. The reason we made this change is so that builders (@izs) with only the free Visual Studio Express can build Node without also going out and downloading the half-gig Windows SDK. So it's a bit of a crappy compromise, but all of us so far think we're doing the right thing.

@sblom

sblom commented Dec 22, 2012

Copy link
Copy Markdown
Author

If we end up deciding to keep this approach after discussing it, I'll make changes to address your other feedback, @bnoordhuis.

@piscisaureus

Copy link
Copy Markdown

I agree this is the best compromise.

@sblom I think configure should tell the user whether the "cached" stuff is being used or if it's generated fresh.

@bnoordhuis

Copy link
Copy Markdown
Member

$DEITY is going to kill kittens every time someone checks out the source but I guess it's the lesser of two evils. I rest my case safe for two things:

  1. The manual override.
  2. The location of the files. I'd much rather see them outside of src/ so it's perfectly clear that they're not "real" source files.

@piscisaureus

Copy link
Copy Markdown

$DEITY is going to kill kittens every time someone checks out the source but I guess it's the lesser of two evils. I rest my case safe for two things:

No I'm not!

@bnoordhuis

Copy link
Copy Markdown
Member

$DEITY is going to kill kittens every time someone checks out the source but I guess it's the lesser of two evils. I rest my case safe for two things:

No I'm not!

Sorry, didn't know I was dealing with the Lord of the Flies here. :-)

@sblom

sblom commented Dec 22, 2012

Copy link
Copy Markdown
Author

How about deps/gen (/winsdk ?) for location? Maybe tools/msvs/genfiles?

@TooTallNate

Copy link
Copy Markdown

+1 for tools/msvs/genfiles

@piscisaureus

Copy link
Copy Markdown

I concur with @TooTallNate

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants