Tim, Thank you! I hope you enjoyed making your first submitted Guix package. Some things that need to be taken care of before it can be merged: Tim Magee 写道： > + %D%/packages/patches/nextxcloud-fix-filenames.patch > \ Sneaky typo. ./pre-inst-env won't care but it would break ‘guix pull’. > --- /dev/null > +++ b/gnu/packages/nextcloud.scm > @@ -0,0 +1,397 @@ > +;;; GNU Guix --- Functional package management for GNU > +;;; Copyright © 2015, 2016, 2017, 2018, 2019 Efraim Flashner > > +;;; Copyright © 2017 Ricardo Wurmus > +;;; Copyright © 2018, 2019, 2020 Tobias Geerinckx-Rice > > +;;; Copyright © 2018 Ludovic Courtès > +;;; Copyright © 2018, 2019, 2020 Nicolas Goaziou > > +;;; Copyright © 2019 Clément Lassieur > +;;; Copyright © 2020 Jakub Kądziołka > +;;; Copyright © 2020 Tim Magee Humm… oh. I guess you first added NextCloud to sync.scm, then decided to put it in its own file (or maybe because I told you so :-). Don't forget to delete all the previous copyright lines (unless you borrowed code from elsewhere), and packages other than nextcloud-desktop. > +(define-module (gnu packages nextcloud) > + #:use-module ((guix licenses) #:prefix license:) […] Best to delete this list, start from scratch, copy back only what's needed to build nextcloud-desktop. > +(define-public nextcloud-desktop > + (package > + (name "nextcloud-desktop") > + (version "2.6.5") > + (source > + (origin > + (method url-fetch) > + (uri (string-append "https://github.com/" > + "nextcloud/desktop/archive/v" > version > ".tar.gz")) Avoid GitHub's /archive/ tarballs. They're auto-generated and provide no value over a git checkout. They can also change unexpectedly leading to hash mismatches. This has happened in the past. Instead: --8<---------------cut here---------------start------------->8--- (source (origin (method git-fetch) (uri (git-reference (url "https://github.com/nextcloud/desktop") (commit (string-append "v" version)))) (sha256 (base32 "…")) (file-name (git-file-name name version)))) --8<---------------cut here---------------end--------------->8--- COMMIT can actually be any supported git ref; here it's a tag which makes updating the package a breeze. > + ;; I cherry picked this patch from Nextcloud > + (patches (search-patches > "nextcloud-fix-filenames.patch")) Please move the comment to the patch file (you can type whatever you want above its first line) and include a link to the exact upstream commit. The patch file itself is missing from this patch, you probably need to ‘git add’ it, then ‘git commit --amend’. > + (modules '((guix build utils))) > + (snippet > + '(begin > + ;; libcrashreporter-qt has its own bundled > dependencies > + (delete-file-recursively > "src/3rdparty/libcrashreporter-qt") > + (delete-file-recursively "src/3rdparty/sqlite3") > + ;; qprogessindicator, qlockedfile, qtokenizer and > + ;; qtsingleapplication have not yet been packaged, > but all are > + ;; explicitly used from the 3rdparty folder during > build. > + #t)))) > + (build-system cmake-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (add-after 'unpack 'delete-failing-utility-test ^^^^^^ This is a tab, which is character non grata in Guix Scheme code. I won't point them all out: ‘guix lint’ should warn you about them. If you're not using emacs, check out etc/indent-code.el in the Guix source tree. It will indent your code for you using emacs and Guix's style rules. > + ;; "Could not create autostart folder" Nitpick: ;;-comments are sentences, hence capitalised & ending with a full stop. Same for the snippet comments above, but libfoo can remain lowercase there. > + (lambda _ > + (substitute* "test/CMakeLists.txt" > + (("nextcloud_add_test\\(Utility \"\"\\)" > test) > + (string-append "#" test))) > + #t)) > + (add-after 'unpack 'delete-failing-files-test These 2 can be combined into one ‘skip-failing-tests’ phase, and even one single substitute* call: --8<---------------cut here---------------start------------->8--- (substitute* "test/CMakeLists.txt" (("nextcloud_add_test\\(Utility \"\"\\)" test) (string-append "#" test)) (("nextcloud_add_test\\(AllFilesDeleted \"syncenginetestutils.h\"\\)" test) (string-append "#" test))) --8<---------------cut here---------------end--------------->8--- > + (delete 'patch-dot-desktop-files)) Please explain why in a comment. > + #:configure-flags '("-DUNIT_TESTING=ON"))) Thanks for taking the trouble to enable and fix tests! > + (native-inputs > + `(("cmocka" ,cmocka) > + ("perl" ,perl) > + ("pkg-config" ,pkg-config) > + ("qtlinguist" ,qttools))) > + (inputs > + `(("openssl" , openssl) > + ("qtbase" ,qtbase) > + ("qtdeclarative" ,qtdeclarative) > + ("qtkeychain" ,qtkeychain) > +;; ("qtquickcontrols" ,qtquickcontrols) > +;; ("qtquickcontrols2" ,qtquickcontrols2) What's the deal with these 2? If you want to keep them as a warning/promise to others, please elaborate in a comment. > + ("qtwebchannel", qtwebchannel) > + ("qtwebengine" ,qtwebengine) > + ("qtwebkit" ,qtwebkit) > + ("sqlite" ,sqlite) > + ("zlib" ,zlib))) > + (home-page "https://nextcloud.org") > + (synopsis "Folder synchronization with a Nextcloud server") s/folder/file/? Or does it synchronise more than files and directories? > + (description "Use the Nextcloud desktop client to keep your > files > synchronized between your Nextcloud server and your > desktop. Select one We always double-space sentences in Texinfo (like. this.). ‘guix lint’ should warn you of this. > or more directories on your local machine and always have access > to your > latest files wherever you are.") > + (license license:gpl2+))) I'm about to fall asleep and will call it a day. I'll actually build the package and double-check the licence tomorrow, if nobody beats me to it. Don't hesitate to holler if I forget. Thanks again for sharing this with us! Kind regards, T G-R