PATCH: Add node_version.h to install

161 views
Skip to first unread message

SamShull

unread,
Aug 4, 2010, 1:31:03 AM8/4/10
to nodejs
I am hoping that this patch can be added to the source for the simple
task of adding node_version.h as one of the files to be installed
during 'make install'. This would make creating addons that require a
particular version of node much easier. Thanks.

SamShull

unread,
Aug 4, 2010, 1:31:40 AM8/4/10
to nodejs
Be nice if I included this http://gist.github.com/507695

Vitali Lovich

unread,
Aug 4, 2010, 1:45:42 AM8/4/10
to nod...@googlegroups.com
node_version.h doesn't tell you which version of node you are actually running on.  That requires doing a runtime check.

--
You received this message because you are subscribed to the Google Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com.
To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.


Marco Rogers

unread,
Aug 4, 2010, 2:38:01 AM8/4/10
to nodejs
Vitali this is true, but it's also reasonable to want to error on
compilation if you're not compiling against the correct version of
node.

SamShull, why not just #include node_version.h in your addon? That
should work just as well. But as far as your patch, rather than add
it to wscript, would it be reasonable to just #include node_version.h
inside node.h? I assume you're including at least that header in your
addon. So then you would have access to node version included with
that. Try it out and let us know if it works the way you want.

:Marco

On Aug 4, 1:45 am, Vitali Lovich <vlov...@gmail.com> wrote:
> node_version.h doesn't tell you which version of node you are actually
> running on.  That requires doing a runtime check.
>
>
>
> On Tue, Aug 3, 2010 at 10:31 PM, SamShull <brickysa...@gmail.com> wrote:
> > Be nice if I included thishttp://gist.github.com/507695
>
> > On Aug 4, 1:31 am, SamShull <brickysa...@gmail.com> wrote:
> > > I am hoping that this patch can be added to the source for the simple
> > > task of adding node_version.h as one of the files to be installed
> > > during 'make install'. This would make creating addons that require a
> > > particular version of node much easier. Thanks.
>
> > --
> > You received this message because you are subscribed to the Google Groups
> > "nodejs" group.
> > To post to this group, send email to nod...@googlegroups.com.
> > To unsubscribe from this group, send email to
> > nodejs+un...@googlegroups.com<nodejs%2Bunsu...@googlegroups.com>
> > .

Vitali Lovich

unread,
Aug 4, 2010, 4:36:28 AM8/4/10
to nod...@googlegroups.com
node_version.h needs to be staged - his change is correct.

Still think node should expose a generic mechanism instead:

// this module is only compatible with node older than 0.100.0 & earlier than 1.50.0
if (!node_version_required(NODE_VERSION(0, 100, 0), NODE_VERSION(1,50,0))) {
   // node before minimum 0.100.0 or after 1.50.0
   fprintf(stderr, "Incompatible node.js version for this module: %s", node_version_string());
   abort();
}

To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.

SamShull

unread,
Aug 4, 2010, 10:20:30 AM8/4/10
to nodejs
@Vitali
node_version.h as of v0.1.101 contains a macro NODE_VERSION_AT_LEAST
that allows for conditional compilation based on the version of node.

@Marco
Including the header into node.h would however still require that
node_version.h be included in the install, or that the addon take an
argument of the src location, which in my opinion defeats the purpose
of having the file in the first place. Additionally, until v0.1.100 it
was being installed, but it was being parsed, when the code was
changed to stop parsing it, it was never added to the static install
list. This patch solves that problem.
> > <nodejs%2Bunsu...@googlegroups.com<nodejs%252Buns...@googlegroups.com>

Marco Rogers

unread,
Aug 4, 2010, 11:11:16 AM8/4/10
to nodejs
You guys seem more knowledgeable about the build process than I am. I
was just trying to be helpful. It sounds like you've got a handle on
the problem though. The best way to get ry's attention to consider a
patch is to hop in the irc channel. I'm interested in the outcome of
this for my libxmljs lib as well.

:Marco
> > > <nodejs%2Bunsu...@googlegroups.com<nodejs%252Bunsubscribe@googlegroups. com>

r...@tinyclouds.org

unread,
Aug 4, 2010, 1:56:47 PM8/4/10
to nod...@googlegroups.com
On Tue, Aug 3, 2010 at 10:31 PM, SamShull <brick...@gmail.com> wrote:
> Be nice if I included this http://gist.github.com/507695

Thanks. That was an oversight. 24c6d26.

Reply all
Reply to author
Forward
0 new messages