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

vikunja-desktop: init at 0.24.3 #195231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kolaente
Copy link
Member

@kolaente kolaente commented Oct 9, 2022

Description of changes

This PR adds the desktop version of Vikunja. Since it is only an electron wrapper around the frontend and the frontend is already packaged in nixos, this package reuses that.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@kolaente
Copy link
Member Author

kolaente commented Oct 9, 2022

Not sure about my change to the release notes, never done that before.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation labels Oct 9, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Oct 9, 2022
@kolaente kolaente changed the title vikunja-desktop: init at 0.19.1 vikunja-desktop: init at 0.20.0 Oct 28, 2022
@mweinelt
Copy link
Member

The package is not referenced by any top-leve attribute.

@mweinelt
Copy link
Member

{"error":"invalid_request","error_description":"The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. The 'redirect_uri' parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls."}

The app recognized my instance was using SSO to login, but once I click that button I get this error.

Tried both https://example.com as well as https://example.com/api/v1.

@kolaente
Copy link
Member Author

What frontend version is that? You should be able to see that by opening the dev console in the electron shell.

@mweinelt
Copy link
Member

mweinelt commented Oct 29, 2022

@kolaente
Copy link
Member Author

The deployment still uses 0.19.1.

Let's wait until the frontend update got merged, that should fix the problem.

The redirect url is way off.

I'm afraid there's no way around that. The desktop app spins up a local server to serve frontend files, a redirect back from the provider has to happen there.

@kolaente kolaente changed the title vikunja-desktop: init at 0.20.0 vikunja-desktop: init at 0.20.1 Nov 11, 2022
@mweinelt
Copy link
Member

Please rebase and squash the commits.

@kolaente kolaente force-pushed the feature/add-vikunja-desktop branch 3 times, most recently from 8ee5fcf to e056727 Compare January 18, 2023 21:09
@kolaente kolaente changed the title vikunja-desktop: init at 0.20.1 vikunja-desktop: init at 0.20.2 Jan 18, 2023
@kolaente
Copy link
Member Author

kolaente commented Jan 18, 2023

Took me a little longer than I'd like to admit but it should all be correctly rebased now.

@kolaente
Copy link
Member Author

@mweinelt I'm not sure what I need to change so that ofborg works.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@kolaente kolaente changed the title vikunja-desktop: init at 0.20.2 vikunja-desktop: init at 0.24.3 Sep 20, 2024
@kolaente
Copy link
Member Author

Rebased and updated. The package compiles and starts successfully, but then fails with this error when running it:

App threw an error during load
Error: Cannot find module 'body-parser'
Require stack:
- /nix/store/y18anvcdqk1qk406v0m93i74gpva6f1l-vikunja-desktop-0.24.3/share/lib/vikunja-desktop/resources/app.asar/node_modules/express/lib/express.js
- /nix/store/y18anvcdqk1qk406v0m93i74gpva6f1l-vikunja-desktop-0.24.3/share/lib/vikunja-desktop/resources/app.asar/node_modules/express/index.js
- /nix/store/y18anvcdqk1qk406v0m93i74gpva6f1l-vikunja-desktop-0.24.3/share/lib/vikunja-desktop/resources/app.asar/main.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1152:15)
    at s._resolveFilename (node:electron/js2c/browser_init:2:121381)
    at Module._load (node:internal/modules/cjs/loader:993:27)
    at c._load (node:electron/js2c/node_init:2:17025)
    at Module.require (node:internal/modules/cjs/loader:1240:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (/nix/store/y18anvcdqk1qk406v0m93i74gpva6f1l-vikunja-desktop-0.24.3/share/lib/vikunja-desktop/resources/app.asar/node_modules/express/lib/express.js:15:18)
    at Module._compile (node:internal/modules/cjs/loader:1373:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1432:10)
    at Module.load (node:internal/modules/cjs/loader:1215:32)

I have absolutely no idea why it does this. The packages compiled by upstream work fine, so this does not seem to be a problem of dependency declaration here.

After having spent a few hours on this without anything really to show for, I wonder if it would be easier to fetch the built package by upstream and patch it?

@kolaente kolaente mentioned this pull request Sep 20, 2024
13 tasks
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 20, 2024
@kolaente kolaente force-pushed the feature/add-vikunja-desktop branch 2 times, most recently from 28f2d2f to aeec716 Compare September 20, 2024 16:10
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Sep 20, 2024
@mweinelt
Copy link
Member

Will defer to @lilyinstarlight and @winterqt

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 27, 2024
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 28, 2024
@maralorn
Copy link
Member

I would love to see progress on this, but I have little to no clue about the problem.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants