You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Dmitry Blotsky <db...@microsoft.com> on 2015/10/26 23:06:48 UTC

[DISCUSS] Copying node_modules during platform install

Hey folks,

I’ve come across a bug with symlinks and platform installation recently. The point of interest is this line: https://github.com/apache/cordova-ios/blob/4039aeb6f87c6803df5814b8cdefb8c2058504a0/bin/lib/create.js#L93.

Is copying node_modules a safe operation? In my case there was a relative symlink inside it when it was copied and as a result some dependencies broke. The symlink was created by a previous invocation of “npm link”.

Kindly,
Dmitry

Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
Gotcha. I’m now thoroughly convinced that shipping node_modules directly is the 100% way to go. I guess having out of date dependencies is just an unavoidable side effect of shipping software as-is. We’ll just have to be diligent in updating cordova-common in platforms quickly enough to unblock CI when bugs come up. Thanks for the detailed explanation, Carlos.

Kindly,
Dmitry

> On Oct 28, 2015, at 8:08 PM, Carlos Santana <cs...@gmail.com> wrote:
> 
> Sorry for being late to the party.
> +1 to conclusion
> 
> But leaving 2 cents
> 
> - Using "npm install" or "npm link" is a development time (i.e. cordova
> contributors) only, When a product is shipped we should not rely on using
> this process to satisfy dependencies
> - The way dependencies are manage in npm/node is not controllable by
> package.json, most people they think they can by fixing/pinning the
> dependencies versions, but what they don't realize is that the dependencies
> children/graph don't usually use fixed versions of their dependencies and
> so on thru the whole deep graph
> - This is why npm-shrinkwrap exist [1] in the first place
> - Bundling all dependencies there are multiple benefits
> 1. we shipped the exact version of the dependency that we clear with legal
> 2. we avoid less headaches for end user to be responsible to install
> dependencies on the fly
> 3. we shipped what we tested
> 4. every end user get's the exact version of the software, an user running
> cordova 5.3.3 that install a week ago, using same bytes of software of a
> user that installed cordova 5.3.3 yesterday
> Making 3 and 4, the most important to me.
> 
> [1] https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fdocs.npmjs.com%2fcli%2fshrinkwrap&data=01%7c01%7cdblotsky%40microsoft.com%7cc13f7ce7572044ed272a08d2e00e3856%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=1CsQ118SBKKLj0YcYUV9TMt0Gjlv1UUBfwYHj4tmUmo%3d
> 
> 
> 
> On Wed, Oct 28, 2015 at 8:37 PM Steven Gill <st...@gmail.com> wrote:
> 
>> +1 nice conclusion.
>> 
>> On Wed, Oct 28, 2015 at 5:30 PM, Jesse <pu...@gmail.com> wrote:
>> 
>>> Yes, I believe the cordova-lib case should be linked, and definitely this
>>> is where a relative link makes sense, since it's all in the same repo.
>>> 
>>> 
>>> @purplecabbage
>>> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7cc13f7ce7572044ed272a08d2e00e3856%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=8JUuLcFvlvO%2b5n9S78bS7yO0UZdl%2fzhkMS04lay%2fPyo%3d
>>> 
>>> On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <db...@microsoft.com>
>>> wrote:
>>> 
>>>>> trust that each platform will be updated
>>>>> with a newer cordova-common when it makes sense
>>>> 
>>>> That sounds reasonable to me. I’m removing the link step for
>>>> cordova-common in platforms.
>>>> 
>>>> Last question: the cordova-common used by cordova-lib should still be
>>>> linked, yes?
>>>> 
>>>> Kindly,
>>>> Dmitry
>>>> 
>>>>> On Oct 28, 2015, at 2:44 PM, Jesse <pu...@gmail.com> wrote:
>>>>> 
>>>>> It is completely likely, and maybe even expected that the specific
>>>> version
>>>>> of cordova-common that sits in any particular platform template can
>> be
>>>>> different than what is used by cordova-lib.
>>>>> They may even be different amongst platforms in a single project.
>>>>> 
>>>>> As long as the lib->platform api is consistent, this is not an issue.
>>> We
>>>>> use cordova-common solely to reduce duplicated code in our repos; We
>> do
>>>> not
>>>>> use cordova-common to reduce the duplicated code on app developers'
>>>>> machines between multiple projects.
>>>>> 
>>>>> re: >> Follow-up: the reason we link them is so that we test master
>>> with
>>>>> master at all times.
>>>>> I would change this so that a platform is tested as a complete
>> entity.
>>>> Test
>>>>> what is in platform-master and trust that each platform will be
>> updated
>>>>> with a newer cordova-common when it makes sense.
>>>>> 
>>>>> 
>>>>> 
>>>>> @purplecabbage
>>>>> 
>>>> 
>>> 
>> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d
>>>>> 
>>>>> On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <
>>> dblotsky@microsoft.com>
>>>>> wrote:
>>>>> 
>>>>>> This use case does mostly affect Cordova contributors. Usually the
>>>>>> platforms will come packaged, you’re right. The discussion of
>>>>>> packaged-vs-installed is separate, but just as a nod to it: I don’t
>>> see
>>>> a
>>>>>> reason we can’t just automatically call “npm install” in
>>>>>> platforms/ios/cordova. Copying over a package.json with fixed
>> versions
>>>> is
>>>>>> no more complex than copying over node_modules.
>>>>>> 
>>>>>> Kindly,
>>>>>> Dmitry
>>>>>> 
>>>>>>> On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com>
>>>> wrote:
>>>>>>> 
>>>>>>> Currently, those modules ship with the platform. Example,
>> cordova-ios
>>>>>> will
>>>>>>> have all necessary modules bundled in. When you create a cordova
>>>> project
>>>>>>> and add a platform, it takes those modules and moves them into your
>>>> newly
>>>>>>> created cordova project. To have the cordova project run `npm
>>> install`
>>>>>> and
>>>>>>> install those modules would require us include those modules in a
>>>> project
>>>>>>> level `package.json` file for every cordova project that adds that
>>>>>>> platform. This would confuse developers for sure. I don't think we
>>>> would
>>>>>>> have to modify requires to have this work.
>>>>>>> 
>>>>>>> To run `npm install` on those directories would also be very poor
>>>>>> practice.
>>>>>>> By those directories, I assume you mean
>>>>>>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
>>>>>>> node_modules from cordova-ios get copied). It would mean we would
>>> have
>>>> to
>>>>>>> have a `package.json` be copied from `cordova-ios` into
>>>>>>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would
>>> then
>>>> at
>>>>>>> least have as many package.json files as platforms have been
>> added. I
>>>>>> think
>>>>>>> this is poor practice.
>>>>>>> 
>>>>>>> Your usecase of npm linking modules in platforms
>>>>>> (cordova-ios/node_modules)
>>>>>>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova`
>> is
>>>>>>> unique. I still think the best solution is to add some sort of
>> check
>>> to
>>>>>>> make sure you haven't npm linked or handle the npm linked case when
>>>>>>> copying. Guess this problem will arise more often with
>>> cordova-common.
>>>>>> That
>>>>>>> usecase pretty much only affects cordova contributors.
>>>>>>> 
>>>>>>> -Steve
>>>>>>> 
>>>>>>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <
>>>> dblotsky@microsoft.com>
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> Is it possible to do “npm install” in those directories instead?
>> Or
>>> to
>>>>>>>> adjust the path so that require() works with the original
>>> node_modules
>>>>>>>> directory?
>>>>>>>> 
>>>>>>>> Kindly,
>>>>>>>> Dmitry
>>>>>>>> 
>>>>>>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <
>> stevengill97@gmail.com>
>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> I don't think we thought of symlinks in this usecase. Probably
>>> worth
>>>>>>>> adding
>>>>>>>>> in code that checks for symlinks before the copy. I don't see us
>>>>>> removing
>>>>>>>>> this copy as the cordova scripts (build, run, install, etc)
>> require
>>>>>> those
>>>>>>>>> modules.
>>>>>>>>> 
>>>>>>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
>>>>>> dblotsky@microsoft.com>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> Ping. Anyone have any information on this? Is it safe to "cp
>> -r” a
>>>>>>>>>> node_modules directory?
>>>>>>>>>> 
>>>>>>>>>> Kindly,
>>>>>>>>>> Dmitry
>>>>>>>>>> 
>>>>>>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <
>>>> dblotsky@microsoft.com>
>>>>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> Hey folks,
>>>>>>>>>>> 
>>>>>>>>>>> I’ve come across a bug with symlinks and platform installation
>>>>>>>> recently.
>>>>>>>>>> The point of interest is this line:
>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>> 
>>> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>>>>>>>>>> 
>>>>>>>>>>> Is copying node_modules a safe operation? In my case there was
>> a
>>>>>>>>>> relative symlink inside it when it was copied and as a result
>> some
>>>>>>>>>> dependencies broke. The symlink was created by a previous
>>> invocation
>>>>>> of
>>>>>>>>>> “npm link”.
>>>>>>>>>>> 
>>>>>>>>>>> Kindly,
>>>>>>>>>>> Dmitry
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>>> 
>>> 
>> 


Re: [DISCUSS] Copying node_modules during platform install

Posted by Carlos Santana <cs...@gmail.com>.
Sorry for being late to the party.
+1 to conclusion

But leaving 2 cents

- Using "npm install" or "npm link" is a development time (i.e. cordova
contributors) only, When a product is shipped we should not rely on using
this process to satisfy dependencies
- The way dependencies are manage in npm/node is not controllable by
package.json, most people they think they can by fixing/pinning the
dependencies versions, but what they don't realize is that the dependencies
children/graph don't usually use fixed versions of their dependencies and
so on thru the whole deep graph
- This is why npm-shrinkwrap exist [1] in the first place
- Bundling all dependencies there are multiple benefits
 1. we shipped the exact version of the dependency that we clear with legal
 2. we avoid less headaches for end user to be responsible to install
dependencies on the fly
 3. we shipped what we tested
 4. every end user get's the exact version of the software, an user running
cordova 5.3.3 that install a week ago, using same bytes of software of a
user that installed cordova 5.3.3 yesterday
Making 3 and 4, the most important to me.

[1] https://docs.npmjs.com/cli/shrinkwrap



On Wed, Oct 28, 2015 at 8:37 PM Steven Gill <st...@gmail.com> wrote:

> +1 nice conclusion.
>
> On Wed, Oct 28, 2015 at 5:30 PM, Jesse <pu...@gmail.com> wrote:
>
> > Yes, I believe the cordova-lib case should be linked, and definitely this
> > is where a relative link makes sense, since it's all in the same repo.
> >
> >
> > @purplecabbage
> > risingj.com
> >
> > On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <db...@microsoft.com>
> > wrote:
> >
> > > > trust that each platform will be updated
> > > > with a newer cordova-common when it makes sense
> > >
> > > That sounds reasonable to me. I’m removing the link step for
> > > cordova-common in platforms.
> > >
> > > Last question: the cordova-common used by cordova-lib should still be
> > > linked, yes?
> > >
> > > Kindly,
> > > Dmitry
> > >
> > > > On Oct 28, 2015, at 2:44 PM, Jesse <pu...@gmail.com> wrote:
> > > >
> > > > It is completely likely, and maybe even expected that the specific
> > > version
> > > > of cordova-common that sits in any particular platform template can
> be
> > > > different than what is used by cordova-lib.
> > > > They may even be different amongst platforms in a single project.
> > > >
> > > > As long as the lib->platform api is consistent, this is not an issue.
> > We
> > > > use cordova-common solely to reduce duplicated code in our repos; We
> do
> > > not
> > > > use cordova-common to reduce the duplicated code on app developers'
> > > > machines between multiple projects.
> > > >
> > > > re: >> Follow-up: the reason we link them is so that we test master
> > with
> > > > master at all times.
> > > > I would change this so that a platform is tested as a complete
> entity.
> > > Test
> > > > what is in platform-master and trust that each platform will be
> updated
> > > > with a newer cordova-common when it makes sense.
> > > >
> > > >
> > > >
> > > > @purplecabbage
> > > >
> > >
> >
> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d
> > > >
> > > > On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <
> > dblotsky@microsoft.com>
> > > > wrote:
> > > >
> > > >> This use case does mostly affect Cordova contributors. Usually the
> > > >> platforms will come packaged, you’re right. The discussion of
> > > >> packaged-vs-installed is separate, but just as a nod to it: I don’t
> > see
> > > a
> > > >> reason we can’t just automatically call “npm install” in
> > > >> platforms/ios/cordova. Copying over a package.json with fixed
> versions
> > > is
> > > >> no more complex than copying over node_modules.
> > > >>
> > > >> Kindly,
> > > >> Dmitry
> > > >>
> > > >>> On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com>
> > > wrote:
> > > >>>
> > > >>> Currently, those modules ship with the platform. Example,
> cordova-ios
> > > >> will
> > > >>> have all necessary modules bundled in. When you create a cordova
> > > project
> > > >>> and add a platform, it takes those modules and moves them into your
> > > newly
> > > >>> created cordova project. To have the cordova project run `npm
> > install`
> > > >> and
> > > >>> install those modules would require us include those modules in a
> > > project
> > > >>> level `package.json` file for every cordova project that adds that
> > > >>> platform. This would confuse developers for sure. I don't think we
> > > would
> > > >>> have to modify requires to have this work.
> > > >>>
> > > >>> To run `npm install` on those directories would also be very poor
> > > >> practice.
> > > >>> By those directories, I assume you mean
> > > >>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
> > > >>> node_modules from cordova-ios get copied). It would mean we would
> > have
> > > to
> > > >>> have a `package.json` be copied from `cordova-ios` into
> > > >>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would
> > then
> > > at
> > > >>> least have as many package.json files as platforms have been
> added. I
> > > >> think
> > > >>> this is poor practice.
> > > >>>
> > > >>> Your usecase of npm linking modules in platforms
> > > >> (cordova-ios/node_modules)
> > > >>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova`
> is
> > > >>> unique. I still think the best solution is to add some sort of
> check
> > to
> > > >>> make sure you haven't npm linked or handle the npm linked case when
> > > >>> copying. Guess this problem will arise more often with
> > cordova-common.
> > > >> That
> > > >>> usecase pretty much only affects cordova contributors.
> > > >>>
> > > >>> -Steve
> > > >>>
> > > >>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <
> > > dblotsky@microsoft.com>
> > > >>> wrote:
> > > >>>
> > > >>>> Is it possible to do “npm install” in those directories instead?
> Or
> > to
> > > >>>> adjust the path so that require() works with the original
> > node_modules
> > > >>>> directory?
> > > >>>>
> > > >>>> Kindly,
> > > >>>> Dmitry
> > > >>>>
> > > >>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <
> stevengill97@gmail.com>
> > > >>>> wrote:
> > > >>>>>
> > > >>>>> I don't think we thought of symlinks in this usecase. Probably
> > worth
> > > >>>> adding
> > > >>>>> in code that checks for symlinks before the copy. I don't see us
> > > >> removing
> > > >>>>> this copy as the cordova scripts (build, run, install, etc)
> require
> > > >> those
> > > >>>>> modules.
> > > >>>>>
> > > >>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
> > > >> dblotsky@microsoft.com>
> > > >>>>> wrote:
> > > >>>>>
> > > >>>>>> Ping. Anyone have any information on this? Is it safe to "cp
> -r” a
> > > >>>>>> node_modules directory?
> > > >>>>>>
> > > >>>>>> Kindly,
> > > >>>>>> Dmitry
> > > >>>>>>
> > > >>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <
> > > dblotsky@microsoft.com>
> > > >>>>>> wrote:
> > > >>>>>>>
> > > >>>>>>> Hey folks,
> > > >>>>>>>
> > > >>>>>>> I’ve come across a bug with symlinks and platform installation
> > > >>>> recently.
> > > >>>>>> The point of interest is this line:
> > > >>>>>>
> > > >>>>
> > > >>
> > >
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> > > >>>>>>>
> > > >>>>>>> Is copying node_modules a safe operation? In my case there was
> a
> > > >>>>>> relative symlink inside it when it was copied and as a result
> some
> > > >>>>>> dependencies broke. The symlink was created by a previous
> > invocation
> > > >> of
> > > >>>>>> “npm link”.
> > > >>>>>>>
> > > >>>>>>> Kindly,
> > > >>>>>>> Dmitry
> > > >>>>>>
> > > >>>>>>
> > > >>>>
> > > >>>>
> > > >>
> > > >>
> > >
> > >
> >
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Steven Gill <st...@gmail.com>.
+1 nice conclusion.

On Wed, Oct 28, 2015 at 5:30 PM, Jesse <pu...@gmail.com> wrote:

> Yes, I believe the cordova-lib case should be linked, and definitely this
> is where a relative link makes sense, since it's all in the same repo.
>
>
> @purplecabbage
> risingj.com
>
> On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
>
> > > trust that each platform will be updated
> > > with a newer cordova-common when it makes sense
> >
> > That sounds reasonable to me. I’m removing the link step for
> > cordova-common in platforms.
> >
> > Last question: the cordova-common used by cordova-lib should still be
> > linked, yes?
> >
> > Kindly,
> > Dmitry
> >
> > > On Oct 28, 2015, at 2:44 PM, Jesse <pu...@gmail.com> wrote:
> > >
> > > It is completely likely, and maybe even expected that the specific
> > version
> > > of cordova-common that sits in any particular platform template can be
> > > different than what is used by cordova-lib.
> > > They may even be different amongst platforms in a single project.
> > >
> > > As long as the lib->platform api is consistent, this is not an issue.
> We
> > > use cordova-common solely to reduce duplicated code in our repos; We do
> > not
> > > use cordova-common to reduce the duplicated code on app developers'
> > > machines between multiple projects.
> > >
> > > re: >> Follow-up: the reason we link them is so that we test master
> with
> > > master at all times.
> > > I would change this so that a platform is tested as a complete entity.
> > Test
> > > what is in platform-master and trust that each platform will be updated
> > > with a newer cordova-common when it makes sense.
> > >
> > >
> > >
> > > @purplecabbage
> > >
> >
> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d
> > >
> > > On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <
> dblotsky@microsoft.com>
> > > wrote:
> > >
> > >> This use case does mostly affect Cordova contributors. Usually the
> > >> platforms will come packaged, you’re right. The discussion of
> > >> packaged-vs-installed is separate, but just as a nod to it: I don’t
> see
> > a
> > >> reason we can’t just automatically call “npm install” in
> > >> platforms/ios/cordova. Copying over a package.json with fixed versions
> > is
> > >> no more complex than copying over node_modules.
> > >>
> > >> Kindly,
> > >> Dmitry
> > >>
> > >>> On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com>
> > wrote:
> > >>>
> > >>> Currently, those modules ship with the platform. Example, cordova-ios
> > >> will
> > >>> have all necessary modules bundled in. When you create a cordova
> > project
> > >>> and add a platform, it takes those modules and moves them into your
> > newly
> > >>> created cordova project. To have the cordova project run `npm
> install`
> > >> and
> > >>> install those modules would require us include those modules in a
> > project
> > >>> level `package.json` file for every cordova project that adds that
> > >>> platform. This would confuse developers for sure. I don't think we
> > would
> > >>> have to modify requires to have this work.
> > >>>
> > >>> To run `npm install` on those directories would also be very poor
> > >> practice.
> > >>> By those directories, I assume you mean
> > >>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
> > >>> node_modules from cordova-ios get copied). It would mean we would
> have
> > to
> > >>> have a `package.json` be copied from `cordova-ios` into
> > >>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would
> then
> > at
> > >>> least have as many package.json files as platforms have been added. I
> > >> think
> > >>> this is poor practice.
> > >>>
> > >>> Your usecase of npm linking modules in platforms
> > >> (cordova-ios/node_modules)
> > >>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is
> > >>> unique. I still think the best solution is to add some sort of check
> to
> > >>> make sure you haven't npm linked or handle the npm linked case when
> > >>> copying. Guess this problem will arise more often with
> cordova-common.
> > >> That
> > >>> usecase pretty much only affects cordova contributors.
> > >>>
> > >>> -Steve
> > >>>
> > >>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <
> > dblotsky@microsoft.com>
> > >>> wrote:
> > >>>
> > >>>> Is it possible to do “npm install” in those directories instead? Or
> to
> > >>>> adjust the path so that require() works with the original
> node_modules
> > >>>> directory?
> > >>>>
> > >>>> Kindly,
> > >>>> Dmitry
> > >>>>
> > >>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
> > >>>> wrote:
> > >>>>>
> > >>>>> I don't think we thought of symlinks in this usecase. Probably
> worth
> > >>>> adding
> > >>>>> in code that checks for symlinks before the copy. I don't see us
> > >> removing
> > >>>>> this copy as the cordova scripts (build, run, install, etc) require
> > >> those
> > >>>>> modules.
> > >>>>>
> > >>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
> > >> dblotsky@microsoft.com>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
> > >>>>>> node_modules directory?
> > >>>>>>
> > >>>>>> Kindly,
> > >>>>>> Dmitry
> > >>>>>>
> > >>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <
> > dblotsky@microsoft.com>
> > >>>>>> wrote:
> > >>>>>>>
> > >>>>>>> Hey folks,
> > >>>>>>>
> > >>>>>>> I’ve come across a bug with symlinks and platform installation
> > >>>> recently.
> > >>>>>> The point of interest is this line:
> > >>>>>>
> > >>>>
> > >>
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> > >>>>>>>
> > >>>>>>> Is copying node_modules a safe operation? In my case there was a
> > >>>>>> relative symlink inside it when it was copied and as a result some
> > >>>>>> dependencies broke. The symlink was created by a previous
> invocation
> > >> of
> > >>>>>> “npm link”.
> > >>>>>>>
> > >>>>>>> Kindly,
> > >>>>>>> Dmitry
> > >>>>>>
> > >>>>>>
> > >>>>
> > >>>>
> > >>
> > >>
> >
> >
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Jesse <pu...@gmail.com>.
Yes, I believe the cordova-lib case should be linked, and definitely this
is where a relative link makes sense, since it's all in the same repo.


@purplecabbage
risingj.com

On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <db...@microsoft.com>
wrote:

> > trust that each platform will be updated
> > with a newer cordova-common when it makes sense
>
> That sounds reasonable to me. I’m removing the link step for
> cordova-common in platforms.
>
> Last question: the cordova-common used by cordova-lib should still be
> linked, yes?
>
> Kindly,
> Dmitry
>
> > On Oct 28, 2015, at 2:44 PM, Jesse <pu...@gmail.com> wrote:
> >
> > It is completely likely, and maybe even expected that the specific
> version
> > of cordova-common that sits in any particular platform template can be
> > different than what is used by cordova-lib.
> > They may even be different amongst platforms in a single project.
> >
> > As long as the lib->platform api is consistent, this is not an issue.  We
> > use cordova-common solely to reduce duplicated code in our repos; We do
> not
> > use cordova-common to reduce the duplicated code on app developers'
> > machines between multiple projects.
> >
> > re: >> Follow-up: the reason we link them is so that we test master with
> > master at all times.
> > I would change this so that a platform is tested as a complete entity.
> Test
> > what is in platform-master and trust that each platform will be updated
> > with a newer cordova-common when it makes sense.
> >
> >
> >
> > @purplecabbage
> >
> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d
> >
> > On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <db...@microsoft.com>
> > wrote:
> >
> >> This use case does mostly affect Cordova contributors. Usually the
> >> platforms will come packaged, you’re right. The discussion of
> >> packaged-vs-installed is separate, but just as a nod to it: I don’t see
> a
> >> reason we can’t just automatically call “npm install” in
> >> platforms/ios/cordova. Copying over a package.json with fixed versions
> is
> >> no more complex than copying over node_modules.
> >>
> >> Kindly,
> >> Dmitry
> >>
> >>> On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com>
> wrote:
> >>>
> >>> Currently, those modules ship with the platform. Example, cordova-ios
> >> will
> >>> have all necessary modules bundled in. When you create a cordova
> project
> >>> and add a platform, it takes those modules and moves them into your
> newly
> >>> created cordova project. To have the cordova project run `npm install`
> >> and
> >>> install those modules would require us include those modules in a
> project
> >>> level `package.json` file for every cordova project that adds that
> >>> platform. This would confuse developers for sure. I don't think we
> would
> >>> have to modify requires to have this work.
> >>>
> >>> To run `npm install` on those directories would also be very poor
> >> practice.
> >>> By those directories, I assume you mean
> >>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
> >>> node_modules from cordova-ios get copied). It would mean we would have
> to
> >>> have a `package.json` be copied from `cordova-ios` into
> >>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would then
> at
> >>> least have as many package.json files as platforms have been added. I
> >> think
> >>> this is poor practice.
> >>>
> >>> Your usecase of npm linking modules in platforms
> >> (cordova-ios/node_modules)
> >>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is
> >>> unique. I still think the best solution is to add some sort of check to
> >>> make sure you haven't npm linked or handle the npm linked case when
> >>> copying. Guess this problem will arise more often with cordova-common.
> >> That
> >>> usecase pretty much only affects cordova contributors.
> >>>
> >>> -Steve
> >>>
> >>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <
> dblotsky@microsoft.com>
> >>> wrote:
> >>>
> >>>> Is it possible to do “npm install” in those directories instead? Or to
> >>>> adjust the path so that require() works with the original node_modules
> >>>> directory?
> >>>>
> >>>> Kindly,
> >>>> Dmitry
> >>>>
> >>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
> >>>> wrote:
> >>>>>
> >>>>> I don't think we thought of symlinks in this usecase. Probably worth
> >>>> adding
> >>>>> in code that checks for symlinks before the copy. I don't see us
> >> removing
> >>>>> this copy as the cordova scripts (build, run, install, etc) require
> >> those
> >>>>> modules.
> >>>>>
> >>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
> >> dblotsky@microsoft.com>
> >>>>> wrote:
> >>>>>
> >>>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
> >>>>>> node_modules directory?
> >>>>>>
> >>>>>> Kindly,
> >>>>>> Dmitry
> >>>>>>
> >>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <
> dblotsky@microsoft.com>
> >>>>>> wrote:
> >>>>>>>
> >>>>>>> Hey folks,
> >>>>>>>
> >>>>>>> I’ve come across a bug with symlinks and platform installation
> >>>> recently.
> >>>>>> The point of interest is this line:
> >>>>>>
> >>>>
> >>
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> >>>>>>>
> >>>>>>> Is copying node_modules a safe operation? In my case there was a
> >>>>>> relative symlink inside it when it was copied and as a result some
> >>>>>> dependencies broke. The symlink was created by a previous invocation
> >> of
> >>>>>> “npm link”.
> >>>>>>>
> >>>>>>> Kindly,
> >>>>>>> Dmitry
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
> >>
>
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
> trust that each platform will be updated
> with a newer cordova-common when it makes sense

That sounds reasonable to me. I’m removing the link step for cordova-common in platforms.

Last question: the cordova-common used by cordova-lib should still be linked, yes?

Kindly,
Dmitry

> On Oct 28, 2015, at 2:44 PM, Jesse <pu...@gmail.com> wrote:
> 
> It is completely likely, and maybe even expected that the specific version
> of cordova-common that sits in any particular platform template can be
> different than what is used by cordova-lib.
> They may even be different amongst platforms in a single project.
> 
> As long as the lib->platform api is consistent, this is not an issue.  We
> use cordova-common solely to reduce duplicated code in our repos; We do not
> use cordova-common to reduce the duplicated code on app developers'
> machines between multiple projects.
> 
> re: >> Follow-up: the reason we link them is so that we test master with
> master at all times.
> I would change this so that a platform is tested as a complete entity. Test
> what is in platform-master and trust that each platform will be updated
> with a newer cordova-common when it makes sense.
> 
> 
> 
> @purplecabbage
> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d
> 
> On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
> 
>> This use case does mostly affect Cordova contributors. Usually the
>> platforms will come packaged, you’re right. The discussion of
>> packaged-vs-installed is separate, but just as a nod to it: I don’t see a
>> reason we can’t just automatically call “npm install” in
>> platforms/ios/cordova. Copying over a package.json with fixed versions is
>> no more complex than copying over node_modules.
>> 
>> Kindly,
>> Dmitry
>> 
>>> On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com> wrote:
>>> 
>>> Currently, those modules ship with the platform. Example, cordova-ios
>> will
>>> have all necessary modules bundled in. When you create a cordova project
>>> and add a platform, it takes those modules and moves them into your newly
>>> created cordova project. To have the cordova project run `npm install`
>> and
>>> install those modules would require us include those modules in a project
>>> level `package.json` file for every cordova project that adds that
>>> platform. This would confuse developers for sure. I don't think we would
>>> have to modify requires to have this work.
>>> 
>>> To run `npm install` on those directories would also be very poor
>> practice.
>>> By those directories, I assume you mean
>>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
>>> node_modules from cordova-ios get copied). It would mean we would have to
>>> have a `package.json` be copied from `cordova-ios` into
>>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would then at
>>> least have as many package.json files as platforms have been added. I
>> think
>>> this is poor practice.
>>> 
>>> Your usecase of npm linking modules in platforms
>> (cordova-ios/node_modules)
>>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is
>>> unique. I still think the best solution is to add some sort of check to
>>> make sure you haven't npm linked or handle the npm linked case when
>>> copying. Guess this problem will arise more often with cordova-common.
>> That
>>> usecase pretty much only affects cordova contributors.
>>> 
>>> -Steve
>>> 
>>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
>>> wrote:
>>> 
>>>> Is it possible to do “npm install” in those directories instead? Or to
>>>> adjust the path so that require() works with the original node_modules
>>>> directory?
>>>> 
>>>> Kindly,
>>>> Dmitry
>>>> 
>>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
>>>> wrote:
>>>>> 
>>>>> I don't think we thought of symlinks in this usecase. Probably worth
>>>> adding
>>>>> in code that checks for symlinks before the copy. I don't see us
>> removing
>>>>> this copy as the cordova scripts (build, run, install, etc) require
>> those
>>>>> modules.
>>>>> 
>>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
>> dblotsky@microsoft.com>
>>>>> wrote:
>>>>> 
>>>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
>>>>>> node_modules directory?
>>>>>> 
>>>>>> Kindly,
>>>>>> Dmitry
>>>>>> 
>>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
>>>>>> wrote:
>>>>>>> 
>>>>>>> Hey folks,
>>>>>>> 
>>>>>>> I’ve come across a bug with symlinks and platform installation
>>>> recently.
>>>>>> The point of interest is this line:
>>>>>> 
>>>> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>>>>>> 
>>>>>>> Is copying node_modules a safe operation? In my case there was a
>>>>>> relative symlink inside it when it was copied and as a result some
>>>>>> dependencies broke. The symlink was created by a previous invocation
>> of
>>>>>> “npm link”.
>>>>>>> 
>>>>>>> Kindly,
>>>>>>> Dmitry
>>>>>> 
>>>>>> 
>>>> 
>>>> 
>> 
>> 


Re: [DISCUSS] Copying node_modules during platform install

Posted by Jesse <pu...@gmail.com>.
It is completely likely, and maybe even expected that the specific version
of cordova-common that sits in any particular platform template can be
different than what is used by cordova-lib.
They may even be different amongst platforms in a single project.

As long as the lib->platform api is consistent, this is not an issue.  We
use cordova-common solely to reduce duplicated code in our repos; We do not
use cordova-common to reduce the duplicated code on app developers'
machines between multiple projects.

re: >> Follow-up: the reason we link them is so that we test master with
master at all times.
I would change this so that a platform is tested as a complete entity. Test
what is in platform-master and trust that each platform will be updated
with a newer cordova-common when it makes sense.



@purplecabbage
risingj.com

On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <db...@microsoft.com>
wrote:

> This use case does mostly affect Cordova contributors. Usually the
> platforms will come packaged, you’re right. The discussion of
> packaged-vs-installed is separate, but just as a nod to it: I don’t see a
> reason we can’t just automatically call “npm install” in
> platforms/ios/cordova. Copying over a package.json with fixed versions is
> no more complex than copying over node_modules.
>
> Kindly,
> Dmitry
>
> > On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com> wrote:
> >
> > Currently, those modules ship with the platform. Example, cordova-ios
> will
> > have all necessary modules bundled in. When you create a cordova project
> > and add a platform, it takes those modules and moves them into your newly
> > created cordova project. To have the cordova project run `npm install`
> and
> > install those modules would require us include those modules in a project
> > level `package.json` file for every cordova project that adds that
> > platform. This would confuse developers for sure. I don't think we would
> > have to modify requires to have this work.
> >
> > To run `npm install` on those directories would also be very poor
> practice.
> > By those directories, I assume you mean
> > `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
> > node_modules from cordova-ios get copied). It would mean we would have to
> > have a `package.json` be copied from `cordova-ios` into
> > `MyCordovaProject/platforms/ios/cordova`. A cordova project would then at
> > least have as many package.json files as platforms have been added. I
> think
> > this is poor practice.
> >
> > Your usecase of npm linking modules in platforms
> (cordova-ios/node_modules)
> > that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is
> > unique. I still think the best solution is to add some sort of check to
> > make sure you haven't npm linked or handle the npm linked case when
> > copying. Guess this problem will arise more often with cordova-common.
> That
> > usecase pretty much only affects cordova contributors.
> >
> > -Steve
> >
> > On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
> > wrote:
> >
> >> Is it possible to do “npm install” in those directories instead? Or to
> >> adjust the path so that require() works with the original node_modules
> >> directory?
> >>
> >> Kindly,
> >> Dmitry
> >>
> >>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
> >> wrote:
> >>>
> >>> I don't think we thought of symlinks in this usecase. Probably worth
> >> adding
> >>> in code that checks for symlinks before the copy. I don't see us
> removing
> >>> this copy as the cordova scripts (build, run, install, etc) require
> those
> >>> modules.
> >>>
> >>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
> dblotsky@microsoft.com>
> >>> wrote:
> >>>
> >>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
> >>>> node_modules directory?
> >>>>
> >>>> Kindly,
> >>>> Dmitry
> >>>>
> >>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
> >>>> wrote:
> >>>>>
> >>>>> Hey folks,
> >>>>>
> >>>>> I’ve come across a bug with symlinks and platform installation
> >> recently.
> >>>> The point of interest is this line:
> >>>>
> >>
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> >>>>>
> >>>>> Is copying node_modules a safe operation? In my case there was a
> >>>> relative symlink inside it when it was copied and as a result some
> >>>> dependencies broke. The symlink was created by a previous invocation
> of
> >>>> “npm link”.
> >>>>>
> >>>>> Kindly,
> >>>>> Dmitry
> >>>>
> >>>>
> >>
> >>
>
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
This use case does mostly affect Cordova contributors. Usually the platforms will come packaged, you’re right. The discussion of packaged-vs-installed is separate, but just as a nod to it: I don’t see a reason we can’t just automatically call “npm install” in platforms/ios/cordova. Copying over a package.json with fixed versions is no more complex than copying over node_modules.

Kindly,
Dmitry

> On Oct 28, 2015, at 1:33 PM, Steven Gill <st...@gmail.com> wrote:
> 
> Currently, those modules ship with the platform. Example, cordova-ios will
> have all necessary modules bundled in. When you create a cordova project
> and add a platform, it takes those modules and moves them into your newly
> created cordova project. To have the cordova project run `npm install` and
> install those modules would require us include those modules in a project
> level `package.json` file for every cordova project that adds that
> platform. This would confuse developers for sure. I don't think we would
> have to modify requires to have this work.
> 
> To run `npm install` on those directories would also be very poor practice.
> By those directories, I assume you mean
> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
> node_modules from cordova-ios get copied). It would mean we would have to
> have a `package.json` be copied from `cordova-ios` into
> `MyCordovaProject/platforms/ios/cordova`. A cordova project would then at
> least have as many package.json files as platforms have been added. I think
> this is poor practice.
> 
> Your usecase of npm linking modules in platforms (cordova-ios/node_modules)
> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is
> unique. I still think the best solution is to add some sort of check to
> make sure you haven't npm linked or handle the npm linked case when
> copying. Guess this problem will arise more often with cordova-common. That
> usecase pretty much only affects cordova contributors.
> 
> -Steve
> 
> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
> 
>> Is it possible to do “npm install” in those directories instead? Or to
>> adjust the path so that require() works with the original node_modules
>> directory?
>> 
>> Kindly,
>> Dmitry
>> 
>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
>> wrote:
>>> 
>>> I don't think we thought of symlinks in this usecase. Probably worth
>> adding
>>> in code that checks for symlinks before the copy. I don't see us removing
>>> this copy as the cordova scripts (build, run, install, etc) require those
>>> modules.
>>> 
>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
>>> wrote:
>>> 
>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
>>>> node_modules directory?
>>>> 
>>>> Kindly,
>>>> Dmitry
>>>> 
>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
>>>> wrote:
>>>>> 
>>>>> Hey folks,
>>>>> 
>>>>> I’ve come across a bug with symlinks and platform installation
>> recently.
>>>> The point of interest is this line:
>>>> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>>>> 
>>>>> Is copying node_modules a safe operation? In my case there was a
>>>> relative symlink inside it when it was copied and as a result some
>>>> dependencies broke. The symlink was created by a previous invocation of
>>>> “npm link”.
>>>>> 
>>>>> Kindly,
>>>>> Dmitry
>>>> 
>>>> 
>> 
>> 


Re: [DISCUSS] Copying node_modules during platform install

Posted by Steven Gill <st...@gmail.com>.
Currently, those modules ship with the platform. Example, cordova-ios will
have all necessary modules bundled in. When you create a cordova project
and add a platform, it takes those modules and moves them into your newly
created cordova project. To have the cordova project run `npm install` and
install those modules would require us include those modules in a project
level `package.json` file for every cordova project that adds that
platform. This would confuse developers for sure. I don't think we would
have to modify requires to have this work.

To run `npm install` on those directories would also be very poor practice.
By those directories, I assume you mean
`MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
node_modules from cordova-ios get copied). It would mean we would have to
have a `package.json` be copied from `cordova-ios` into
`MyCordovaProject/platforms/ios/cordova`. A cordova project would then at
least have as many package.json files as platforms have been added. I think
this is poor practice.

Your usecase of npm linking modules in platforms (cordova-ios/node_modules)
that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is
unique. I still think the best solution is to add some sort of check to
make sure you haven't npm linked or handle the npm linked case when
copying. Guess this problem will arise more often with cordova-common. That
usecase pretty much only affects cordova contributors.

-Steve

On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
wrote:

> Is it possible to do “npm install” in those directories instead? Or to
> adjust the path so that require() works with the original node_modules
> directory?
>
> Kindly,
> Dmitry
>
> > On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
> wrote:
> >
> > I don't think we thought of symlinks in this usecase. Probably worth
> adding
> > in code that checks for symlinks before the copy. I don't see us removing
> > this copy as the cordova scripts (build, run, install, etc) require those
> > modules.
> >
> > On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
> > wrote:
> >
> >> Ping. Anyone have any information on this? Is it safe to "cp -r” a
> >> node_modules directory?
> >>
> >> Kindly,
> >> Dmitry
> >>
> >>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
> >> wrote:
> >>>
> >>> Hey folks,
> >>>
> >>> I’ve come across a bug with symlinks and platform installation
> recently.
> >> The point of interest is this line:
> >>
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> >>>
> >>> Is copying node_modules a safe operation? In my case there was a
> >> relative symlink inside it when it was copied and as a result some
> >> dependencies broke. The symlink was created by a previous invocation of
> >> “npm link”.
> >>>
> >>> Kindly,
> >>> Dmitry
> >>
> >>
>
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
Follow-up: the reason we link them is so that we test master with master at all times. As of right now, that workflow breaks because my version of "npm link” makes relative links.

Kindly,
Dmitry

> On Oct 28, 2015, at 1:28 PM, Jesse <pu...@gmail.com> wrote:
> 
> They are safe to copy.
> However, I don't see why anyone would have run npm link in the template
> folder of a platform, or why anyone would need to.
> We should probably just document that if you do this, it will break.
> 
> Can anyone think of a reason why they would 'need' to npm link those
> modules?
> 
> 
> 
> 
> My team is hiring!
> @purplecabbage
> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c3c9216089163474b4bc908d2dfd65b55%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=HfO9SlDi0pQSdOvhx5gWHrxooCcQWKqrXI0LP08CVDI%3d
> 
> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
> 
>> Is it possible to do “npm install” in those directories instead? Or to
>> adjust the path so that require() works with the original node_modules
>> directory?
>> 
>> Kindly,
>> Dmitry
>> 
>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
>> wrote:
>>> 
>>> I don't think we thought of symlinks in this usecase. Probably worth
>> adding
>>> in code that checks for symlinks before the copy. I don't see us removing
>>> this copy as the cordova scripts (build, run, install, etc) require those
>>> modules.
>>> 
>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
>>> wrote:
>>> 
>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
>>>> node_modules directory?
>>>> 
>>>> Kindly,
>>>> Dmitry
>>>> 
>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
>>>> wrote:
>>>>> 
>>>>> Hey folks,
>>>>> 
>>>>> I’ve come across a bug with symlinks and platform installation
>> recently.
>>>> The point of interest is this line:
>>>> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>>>> 
>>>>> Is copying node_modules a safe operation? In my case there was a
>>>> relative symlink inside it when it was copied and as a result some
>>>> dependencies broke. The symlink was created by a previous invocation of
>>>> “npm link”.
>>>>> 
>>>>> Kindly,
>>>>> Dmitry
>>>> 
>>>> 
>> 
>> 


Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
Right now there is a difference between the cordova-common that’s in master, the cordova-common that’s released on NPM, and the cordova-common that comes with a platform. If we’re fine with these differences, then there isn’t really an issue. Are we fine with them?

Kindly,
Dmitry

> On Oct 28, 2015, at 1:28 PM, Jesse <pu...@gmail.com> wrote:
> 
> They are safe to copy.
> However, I don't see why anyone would have run npm link in the template
> folder of a platform, or why anyone would need to.
> We should probably just document that if you do this, it will break.
> 
> Can anyone think of a reason why they would 'need' to npm link those
> modules?
> 
> 
> 
> 
> My team is hiring!
> @purplecabbage
> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c3c9216089163474b4bc908d2dfd65b55%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=HfO9SlDi0pQSdOvhx5gWHrxooCcQWKqrXI0LP08CVDI%3d
> 
> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
> 
>> Is it possible to do “npm install” in those directories instead? Or to
>> adjust the path so that require() works with the original node_modules
>> directory?
>> 
>> Kindly,
>> Dmitry
>> 
>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
>> wrote:
>>> 
>>> I don't think we thought of symlinks in this usecase. Probably worth
>> adding
>>> in code that checks for symlinks before the copy. I don't see us removing
>>> this copy as the cordova scripts (build, run, install, etc) require those
>>> modules.
>>> 
>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
>>> wrote:
>>> 
>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
>>>> node_modules directory?
>>>> 
>>>> Kindly,
>>>> Dmitry
>>>> 
>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
>>>> wrote:
>>>>> 
>>>>> Hey folks,
>>>>> 
>>>>> I’ve come across a bug with symlinks and platform installation
>> recently.
>>>> The point of interest is this line:
>>>> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>>>> 
>>>>> Is copying node_modules a safe operation? In my case there was a
>>>> relative symlink inside it when it was copied and as a result some
>>>> dependencies broke. The symlink was created by a previous invocation of
>>>> “npm link”.
>>>>> 
>>>>> Kindly,
>>>>> Dmitry
>>>> 
>>>> 
>> 
>> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org

Re: [DISCUSS] Copying node_modules during platform install

Posted by Jesse <pu...@gmail.com>.
They are safe to copy.
However, I don't see why anyone would have run npm link in the template
folder of a platform, or why anyone would need to.
We should probably just document that if you do this, it will break.

Can anyone think of a reason why they would 'need' to npm link those
modules?




My team is hiring!
@purplecabbage
risingj.com

On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <db...@microsoft.com>
wrote:

> Is it possible to do “npm install” in those directories instead? Or to
> adjust the path so that require() works with the original node_modules
> directory?
>
> Kindly,
> Dmitry
>
> > On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com>
> wrote:
> >
> > I don't think we thought of symlinks in this usecase. Probably worth
> adding
> > in code that checks for symlinks before the copy. I don't see us removing
> > this copy as the cordova scripts (build, run, install, etc) require those
> > modules.
> >
> > On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
> > wrote:
> >
> >> Ping. Anyone have any information on this? Is it safe to "cp -r” a
> >> node_modules directory?
> >>
> >> Kindly,
> >> Dmitry
> >>
> >>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
> >> wrote:
> >>>
> >>> Hey folks,
> >>>
> >>> I’ve come across a bug with symlinks and platform installation
> recently.
> >> The point of interest is this line:
> >>
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> >>>
> >>> Is copying node_modules a safe operation? In my case there was a
> >> relative symlink inside it when it was copied and as a result some
> >> dependencies broke. The symlink was created by a previous invocation of
> >> “npm link”.
> >>>
> >>> Kindly,
> >>> Dmitry
> >>
> >>
>
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
Is it possible to do “npm install” in those directories instead? Or to adjust the path so that require() works with the original node_modules directory?

Kindly,
Dmitry

> On Oct 27, 2015, at 10:31 PM, Steven Gill <st...@gmail.com> wrote:
> 
> I don't think we thought of symlinks in this usecase. Probably worth adding
> in code that checks for symlinks before the copy. I don't see us removing
> this copy as the cordova scripts (build, run, install, etc) require those
> modules.
> 
> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
> 
>> Ping. Anyone have any information on this? Is it safe to "cp -r” a
>> node_modules directory?
>> 
>> Kindly,
>> Dmitry
>> 
>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
>> wrote:
>>> 
>>> Hey folks,
>>> 
>>> I’ve come across a bug with symlinks and platform installation recently.
>> The point of interest is this line:
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>> 
>>> Is copying node_modules a safe operation? In my case there was a
>> relative symlink inside it when it was copied and as a result some
>> dependencies broke. The symlink was created by a previous invocation of
>> “npm link”.
>>> 
>>> Kindly,
>>> Dmitry
>> 
>> 


Re: [DISCUSS] Copying node_modules during platform install

Posted by Steven Gill <st...@gmail.com>.
I don't think we thought of symlinks in this usecase. Probably worth adding
in code that checks for symlinks before the copy. I don't see us removing
this copy as the cordova scripts (build, run, install, etc) require those
modules.

On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <db...@microsoft.com>
wrote:

> Ping. Anyone have any information on this? Is it safe to "cp -r” a
> node_modules directory?
>
> Kindly,
> Dmitry
>
> > On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com>
> wrote:
> >
> > Hey folks,
> >
> > I’ve come across a bug with symlinks and platform installation recently.
> The point of interest is this line:
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> >
> > Is copying node_modules a safe operation? In my case there was a
> relative symlink inside it when it was copied and as a result some
> dependencies broke. The symlink was created by a previous invocation of
> “npm link”.
> >
> > Kindly,
> > Dmitry
>
>

Re: [DISCUSS] Copying node_modules during platform install

Posted by Dmitry Blotsky <db...@microsoft.com>.
Ping. Anyone have any information on this? Is it safe to "cp -r” a node_modules directory?

Kindly,
Dmitry

> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <db...@microsoft.com> wrote:
> 
> Hey folks,
> 
> I’ve come across a bug with symlinks and platform installation recently. The point of interest is this line: https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
> 
> Is copying node_modules a safe operation? In my case there was a relative symlink inside it when it was copied and as a result some dependencies broke. The symlink was created by a previous invocation of “npm link”.
> 
> Kindly,
> Dmitry