Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

Refector: Do the npm compile as a Build step in docker #1150

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

toxix
Copy link

@toxix toxix commented Jul 3, 2021

This removes the requirement of npm on the host system and removes an
additional manual step during the building of the docker image.

Updated the documentation accordingly and fixed a typo.

This removes the requirement of npm on the host system and removes an
additional manual step during the building of the docker image.

Updated the documentation accordingly and fixed a typo.
@toxix toxix changed the base branch from master to dev July 3, 2021 22:19
@toxix toxix changed the title Refector: Do the npm compile as a Build step Refector: Do the npm compile as a Build step in docker Jul 3, 2021
@jonaswinkler
Copy link
Owner

Well, the bad thing about this is that it causes the GitHub Actions pipeline to build the front end three times, and its pretty slow on armhf.

I also still need to compile the front-end for the bare metal release.

@TheFehr
Copy link

TheFehr commented Jul 4, 2021

I did wonder about this as well like @toxix . But I then realised the same problem as @jonaswinkler . The only way I'd see to get both whitch is hacky at best and worse then the current situation at worst. Would be to include the frontend-building in the docker. But somehow manage to make it conditional. This way the Github Action could build it once and copy it in. And a person building it localy could just run the Dockerfile without having to precompile the frontend.

@toxix
Copy link
Author

toxix commented Jul 4, 2021

If you are concerned about building speed we could implement some build caching in the pipeline. For example like mentioned here: https://gist.github.com/UrsaDK/f90c9632997a70cfe2a6df2797731ac8
Let me know if I should check if I can tweek the pipeline so it caches the things. (Looks like I broke it anyway in the current commit)

Regards local/bar metal deployments you can run something like

docker run -it -v ./:/src/ node bash -c "cd /src/src-ui && npm install && ./node_modules/.bin/ng build --prod"

as the directory is mounted it will be also stay compiled outside, or if you like just run the npm commands on bare metal.

@jonaswinkler
Copy link
Owner

Uhm well, you can try adapting the pipeline of course. I recall that the build would not even finish on armhf.

The failure is because you deleted the script that's used by the pipeline to build the frontend.

@toxix
Copy link
Author

toxix commented Jul 5, 2021

Overlooked the github pipeline implications in my first commit. Have changed things a bit, let's see if that is already doing it's job or if I need to work more on it.

Toxix added 2 commits July 5, 2021 03:02
This should allow to use docker building the frontend artifacts and
reusing them in the bare metal release.
As frontend compile step is done now inside docker container, we do not
need to do it before. This is updating the documentation so it reflects
the code changes made.
@toxix toxix force-pushed the multistep-build branch from 3189ea4 to feb34c0 Compare July 5, 2021 01:04
From automatic codereview. We should use a specific node version.

We are using the same version that was used before in the github
pipeline.
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.

3 participants