Install dependencies - Clear up logs - Rebuild pre-built modules

RocketChat / Docker.Official.Image

Docker hub - community managed image

295 stars 218 forks source link

Install dependencies - Clear up logs - Rebuild pre-built modules #60

Open lag-linaro opened 5 years ago

lag-linaro commented 5 years ago

Fixes arm build see #54

geekgonecrazy commented 5 years ago

is it necessary to rebuild sharp and grpc here?

lag-linaro commented 5 years ago

is it necessary to rebuild sharp and grpc here?

Did you read the commit message?

Rebuilding essentially gives you multi-arch support.

geekgonecrazy commented 5 years ago

Ah right! Yes we actually have to do this step in our snap builds also. Also had to do something similar for my alpine PR for our other image.. 🤔 In this case for libc

https://github.com/RocketChat/Rocket.Chat/pull/12548

I wonder if we shouldn't instead have these installed via npm instead of bundled that way always built for the appropriate architecture during npm install instead of having to do this "hack" in multiple ways on multiple distribution methods.

@sampaiodiego is that even possible to move from bundled dependency to installed by end user? Looks like grpc (from google vision package) and sharp

lag-linaro commented 5 years ago

Yes, if that's possible then it would be better.

Although, it would be good to merge this pull-request in the mean time, as a quick win.

Happy to help you work on the enduring solution too if that's required.

sampaiodiego commented 5 years ago

is that even possible to move from bundled dependency to installed by end user? Looks like grpc (from google vision package) and sharp

@geekgonecrazy I don't know :disappointed: the directory bundle/programs/server/npm is added by meteor build command, idk where its content comes from =/

lag-linaro commented 5 years ago

Penny for your thoughts guys?

lag-linaro commented 5 years ago

Any updates on this please?

It's been a long time since I submitted this.

geekgonecrazy commented 5 years ago

@lag-linaro sorry for the delay we are actually hung up by #57 please see conversation there. But basically at the moment would do no good to merge because would not be able to be published :(

lag-linaro commented 5 years ago

Looks like #57 is now closed.

geekgonecrazy commented 5 years ago

Yes the dockerfile has changed some so might require a sync and test to make sure works still. Would gladly merge though if we can get in sync

lag-linaro commented 5 years ago

Rebased, moved back over to using node:<xxx>-slim and removed debian:jessie-slim hack.

lag-linaro commented 5 years ago

What if I dropped that patch? Is the rest still acceptable?

lag-linaro commented 5 years ago

Okay Aaron (@geekgonecrazy), I have added the hack back in.

Please re-consider this pull-request.

lag-linaro commented 5 years ago

@Sing-Li please take a look at this pull-request.

It should provide support for AArch64.

lag-linaro commented 5 years ago

We've put considerable work into making this work with aarch64.

It would be a real shame to see it go to waste.

Could someone @Sing-Li, @geekgonecrazy please take a look at this?

CLAassistant commented 1 year ago

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Lee Jones seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}