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

Fix: broken build; update to core22 #23

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

lucyllewy
Copy link
Member

@lucyllewy lucyllewy commented Jan 15, 2024

  • Broken build fixed by upgrading to core22
  • Add npm-deps part with a face package.json to work-around the npm plugin weirdness breaking the build step ordering that we need.

fixes: #21
fixes: #22

@lucyllewy lucyllewy requested review from a team and lengau and removed request for a team January 15, 2024 19:59
@merlijn-sebrechts
Copy link
Member

@lucyllewy any idea why pack is failing?

@lengau
Copy link

lengau commented Jan 16, 2024

Looks like the pack is failing because bin/thelounge doesn't exist. (I used snapcraft try and then snap pack to work around canonical/snapcraft#4499.)

lengau@ratel:~/Projects/thelounge$ snap pack --filename thelouge_4.4.1_amd64.snap --compression xz prime/ .
2024/01/16 11:00:28.931547 container.go:239: in snap "thelounge": path "bin/thelounge" does not exist
error: cannot pack "prime/": snap is unusable due to missing files

Maybe we need the yarn global add thelounge command like they have in the docs? https://thelounge.chat/docs/install-and-upgrade#from-npm-releases

@lucyllewy
Copy link
Member Author

I don't know why it failed in the CI. It succeeded on my system. (my wired internet is offline until I move home so I will prolly struggle to get a look at it for a bit)

Copy link

@lengau lengau left a comment

Choose a reason for hiding this comment

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

I'm just commenting here to remove this from my review queue :-)

Please let me know when you'd like a new review.

@lucyllewy
Copy link
Member Author

Please let me know when you'd like a new review.

I've updated the branch so you can add it back to your queue now.

* Broken build fixed by upgrading to core22
* Add `npm-deps` part with a face `package.json` to work-around the npm plugin weirdness breaking the build step ordering that we need.

Signed-off-by: Lucy Llewellyn <lucyllewy@ubuntu.com>
@jnsgruk
Copy link
Member

jnsgruk commented Sep 18, 2024

Feels like if we're going to trick the npm plugin with a fake package.json, maybe we should just not use the npm plugin at all and just override the build?

Might also be worth looking at the proxy config for yarn - I don't think this will immediately work for lp builds - see the (recent) signal snap commit history which has some clues in it!

@lucyllewy
Copy link
Member Author

I successfully built with snapcraft remote-build, which should be close to the build service's config?

@lucyllewy
Copy link
Member Author

also the fake package.json is purely to get nodejs installed and kept up to date. Its the easiest way to keep it updated without trying to duplicate effort and code by copying the npm plugin's download logic.

@jnsgruk jnsgruk merged commit cf6cc3a into snapcrafters:master Sep 19, 2024
1 check passed
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.

Update to 4.4.0 Update to 4.3.1 (is this snap still being maintained?)
4 participants