Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebuild webpack during RPM build #65

Merged
merged 5 commits into from Sep 9, 2021
Merged

Rebuild webpack during RPM build #65

merged 5 commits into from Sep 9, 2021

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Sep 2, 2021

This is necessary to comply with Fedora's packaging policy:
https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/

Include the node cache in the source rpm, unpack it into the main source
dir, and force a webpack rebuild in %build.


Tested the integration with cockpituous with:

$ export RELEASE_SOURCE="_release/source"
$ ~/upstream/cockpituous/release/release-source
$ ~/upstream/cockpituous/release/release-github
> Creating release draft: 2
> Uploading file: _release/source/cockpit-certificates-2.tar.gz
####################################################################################################################### 100.0%
> Uploading file: _release/source/cockpit-certificates-node-2.tar.xz
####################################################################################################################### 100.0%
> Publishing draft: 2

This created a proper release (which I deleted again, sorry for the noise!)

@martinpitt
Copy link
Member Author

This is a no-op by itself, but cockpit-project/cockpituous#438 will use this to automatically build and publish the node-cache.

@martinpitt
Copy link
Member Author

This is the first part for addressing https://bugzilla.redhat.com/show_bug.cgi?id=1969450 properly, and rebuilding the webpack during rpm build.

@martinpitt
Copy link
Member Author

@skobyda : Would you mind if I clean up all the old and merged branches on this project? https://github.com/skobyda/cockpit-certificates/branches/all (or do it yourself, of course).

I also recommend you to enable this in your project settings:

image

Then you don't have to do this manually.

@martinpitt martinpitt marked this pull request as draft September 3, 2021 05:43
@martinpitt
Copy link
Member Author

Let's not merge that yet -- it races with the dist-gzip tarball build. I'm working on the full thing now.

This is ugly as it races with other make targets which expect
node_modules/ to exist. It's also unnecessary, we can just tell tar to
ignore node_modules/.
@martinpitt martinpitt changed the title Add a make node-cache Rebuild webpack during RPM build Sep 3, 2021
@martinpitt
Copy link
Member Author

Our own CI log shows that the webpack gets built during RPM, and tests succeed.

It does not work for packit yet.

@martinpitt
Copy link
Member Author

I don't see how to fix packit right now -- I filed packit/packit#1355 for help/fixing this.

@martinpitt
Copy link
Member Author

martinpitt commented Sep 8, 2021

With the help of packit maintainers I found a solution/workaround for packit/packit#1355, pushed. packit is down right now, but hopefully it comes back soon.

@martinpitt martinpitt removed the blocked label Sep 8, 2021
Avoid the indirection, and directly call webpack. This avoids adding an
`npm` build dependency if/when the RPM wants to rebuild the webpack
during package build.
Otherwise it is incomplete and can't be rebuilt.
The node_modules cache tarball will be published as release artifact, to
make sure that any release can be rebuilt in a reproducible way.

Cockpituous' release-source will call this if available:
cockpit-project/cockpituous#438
This is necessary to comply with Fedora's packaging policy:
https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/

Include the node cache in the source rpm, unpack it into the main source
dir, and force a webpack rebuild in `%build`.
@martinpitt
Copy link
Member Author

Yay! COPR rpm build succeeded and correctly rebuilt the webpack. Waiting for TMT tests now, but the crucial parts are working.

@martinpitt martinpitt marked this pull request as ready for review September 8, 2021 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants