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

Makefile: Output a path to the generated archive #166

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

marusak
Copy link
Member

@marusak marusak commented Jan 5, 2021

This is needed for packit to work.
Also reusing $(TARFILE) macro.

@marusak
Copy link
Member Author

marusak commented Jan 5, 2021

@martinpitt I thought I would take a look at this as I never really touched ostree nor packit :)

@marusak
Copy link
Member Author

marusak commented Jan 5, 2021

Oh what?

Running command: make dist-gzip
Output of ['make', 'dist-gzip']:
# if it exists already, npm install won't update it; force that so that we always get up-to-date packages
rm -f package-lock.json
# unset NODE_ENV, skips devDependencies otherwise
env -u NODE_ENV npm install
npm WARN deprecated eslint-loader@3.0.4: This loader has been deprecated. Please use eslint-webpack-plugin
npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated har-validator@5.1.5: this library is no longer supported
npm WARN deprecated popper.js@1.16.1: You can find the new Popper v2 at @popperjs/core, this package is dedicated to the legacy v1
npm WARN deprecated chokidar@2.1.8: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
npm WARN deprecated fsevents@2.1.3: Please update to v 2.2.x
npm WARN deprecated core-js@2.6.12: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
npm WARN deprecated fsevents@1.2.13: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.

Message: Preparing of the upstream to the SRPM build failed: The create-archive action did not output a path to the generated archive. Please make sure that you have valid path in the single line of the output.
Exception: PackitSRPMException('Preparing of the upstream to the SRPM build failed: The create-archive action did not output a path to the generated archive. Please make sure that you have valid path in the single line of the output.')
You can retrigger the build by adding a comment (`/packit copr-build`) into this pull request.
Please join the freenode IRC channel #packit for the latest info.

The output from make dist-git seems waay too short.

Makefile Outdated
@@ -103,6 +103,7 @@ devel-install: $(WEBPACK_TEST)
ln -s `pwd`/dist ~/.local/share/cockpit/$(PACKAGE_NAME)

dist-gzip: $(TARFILE)
echo $(TARFILE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That certainly looks right according to https://packit.dev/docs/actions/ ..

@marusak marusak added the no-test For image rebuilds and manual test requests label Jan 6, 2021
@marusak
Copy link
Member Author

marusak commented Jan 6, 2021

oh common, locally it already worked. (It needed path, not just file name, so ./ did the trick)

@marusak marusak marked this pull request as draft January 6, 2021 08:21
@marusak marusak force-pushed the packit branch 4 times, most recently from c714a7a to f919a65 Compare January 6, 2021 08:43
@marusak
Copy link
Member Author

marusak commented Jan 6, 2021

Ok, I think it actually does not create the archive.

@marusak marusak force-pushed the packit branch 4 times, most recently from 64a7aa2 to 9180591 Compare January 6, 2021 13:25
@marusak
Copy link
Member Author

marusak commented Jan 6, 2021

I am now trying to build this in usercont/sandcastle container ( I saw this in packit/sandcastle#85) and now it fails with:

ERROR: You need to install the 'sassc' package to build this project.

So either we need to install it or get it installed like we got npm? packit/sandcastle#30

@marusak
Copy link
Member Author

marusak commented Jan 6, 2021

Ok so indeed sassc needs to be installed into sandbox, sent packit/sandcastle#90
Also not all logs are shown. Talked to packit team and they will take a look at it.
Until the PR is merged and deployed, this is blocked.

@martinpitt
Copy link
Member

@marusak: The changes here look good and harmless, and can be pre-validated by our CI. If you want to re-push without "no-test", I'm happy enough to land them already.

Packit needs this to be able to use this archive.
@marusak marusak removed the no-test For image rebuilds and manual test requests label Jan 7, 2021
@marusak marusak marked this pull request as ready for review January 7, 2021 09:48
To fix this warning:
    WARNING  'upstream_project_name' configuration key was renamed to 'upstream_package_name', please update your configuration file.
@marusak
Copy link
Member Author

marusak commented Jan 7, 2021

@martinpitt tests were green. I added one more commit.
Locally packit srpm and packit local-build both work.

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danke!

@martinpitt martinpitt merged commit 0168693 into cockpit-project:master Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants