Spec URL: http://per.bothner.com/DomTerm/domterm.spec SRPM URL: http://per.bothner.com/DomTerm/domterm-0.71-1.fc25.src.rpm Description: terminal emulator based on web technologies Fedora Account System Username: bothner I am the upstream maintain or this package (as well as long-time Fedora user). Website: http://domterm.org Source: https://github.com/PerBothner/DomTerm An LWN article (about a year old) describing DomTerm: https://lwn.net/Articles/670062/ Note I had some problems with "domterm" vs "DomTerm". For example the command 'fedpkg --release f25 mockbuild' didn't work because it looks for DomTerm-0.71-1.fc25.src.rpm. However, fedora-review and other tests passed.
> Note I had some problems with "domterm" vs "DomTerm". > For example the command 'fedpkg --release f25 mockbuild' didn't > work because it looks for DomTerm-0.71-1.fc25.src.rpm. fedpkg mockbuild works within distgit. No idea how you've used it to test-build a separate src.rpm. About upper-case vs. mixed cased: https://fedoraproject.org/wiki/Packaging:Naming#Case_Sensitivity > Version: 0.71 Build output preprocessor definitions refers to version "0.50", because that's what configure.ac defines. > License: BSD https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text > BuildRequires: qt5-qtbase-devel qt5-qtwebchannel-devel qt5-qtwebengine-devel Notice the following very useful "SHOULD" item in the guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_based_on_pkg-config > %package -n qtdomterm > Requires: qt5-qtbase qt5-qtwebchannel qt5-qtwebengine https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires > %install > rm -rf $RPM_BUILD_ROOT https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections > # Let alternatives manage the symlink > echo after install link %{buildroot}%{_bindir}/domterm Make it a comment instead. Nobody will see this echo output. > %files > %{_bindir}/ldomterm > %{_datadir}/domterm/application.ini > %{_datadir}/domterm/chrome.manifest > %{_datadir}/domterm/defaults/preferences/prefs.js As long as RPM does not include directories automatically, you must specify every directory you want to be included in the package. Either with explicit %dir entries or by including entire directory trees: https://fedoraproject.org/wiki/Packaging:UnownedDirectories > %{_mandir}/man1/domterm.1.gz > %{_mandir}/man1/ldomterm.1.gz As rpmbuild compresses manual pages on-the-fly, changes to the global/local configuration could change the compression program or disable compression. That's why a growing number of packagers use wildcards to include manual pages with any extension: %{_mandir}/man1/domterm.1* %{_mandir}/man1/ldomterm.1*
Thanks for the comments! I made a number of fixes, available at: Spec URL: http://per.bothner.com/DomTerm/domterm.spec SRPM URL: http://per.bothner.com/DomTerm/domterm-0.72-1.fc25.src.rpm - Fixed DomTerm to use the version number in configure.ac. - Added %license entries to %files - BuildRequires: qt5-qtbase-devel qt5-qtwebchannel-devel qt5-qtwebengine-devel These seem to be needed; otherwise fedpkg mockbuild doesn't work. - Used pkgconfig in BuildRequires. - Removed 'rm -rf $RPM_BUILD_ROOT' from %install. - Removed bogus echo command. - Added '%dir %{_datadir}/domterm' to %files. - Changed man page netries in %files to use ildcards. There is still some confusion between "domterm" vs "DomTerm". I could change the package name to DomTerm (and maybe qtDomTerm ?), to match the git name (on GitHub), and that might be easiest. However, I have no problems calling the package "domterm" if that doesn't cause problems. I'm also unsure how I should specify %Source0. I haven't managed to get working automatic downloads from GitHub - if that is actually desirable.
(In reply to Per Bothner from comment #2) > There is still some confusion between "domterm" vs "DomTerm". > I could change the package name to DomTerm (and maybe qtDomTerm ?), > to match the git name (on GitHub), and that might be easiest. > However, I have no problems calling the package "domterm" if > that doesn't cause problems. All lower case is definitely more common for the package name and executable file name. Your spec uses lower case everywhere except where capitalization is hard-coded into the GitHub repo and the tarball directory, so maybe you've already fixed this issue? `fedpkg --release f25 mockbuild` works fine for me. > I'm also unsure how I should specify %Source0. I haven't managed to get > working automatic downloads from GitHub - if that is actually desirable. When you have the correct Source0 URL, it lets spectool and various other packaging tools fetch the source tarball for you. Maybe not a huge deal if you don't, but I think it's convenient enough to be worth using the correct macro for the source URL. And it's actually pretty simple. When I went to your GitHub repo's releases page, I noticed the version 0.72 release could be downloaded at this URL: https://github.com/PerBothner/DomTerm/archive/0.72.tar.gz All you have to do is replace the version number -- 0.72 -- with the version macro. Thus, the Source0 URL you want would look like this: https://github.com/PerBothner/DomTerm/archive/%{version}.tar.gz
@bothner Do you have a sponsor yet? I can't sponsor you, but DomTerm seems interesting, so I could certainly help with the basic review.
No, I don't have a sponsor. Thanks, I have a few additional changes I've been working on, some of relate to packaging: * It would be helpful to update the libwebsockets package (to version 2.2), though some more testing (relating to domterm) is desirable. Once that is done, I can remove a bunch of crud from the source code. * I want domterm to install a 'dt-util' script, which does useful things when running under DomTerm. The script is written, so it mainly a matter of updating the documentation, plus a binary spec file change (to install the script itself and perhaps a stub man page), * I'm thinking of including some documentation (in html form) that can be loaded by a hotkey. BUt that can wait. * I'm working on an integrated pager (a la less). It's promising, but not quite there yet. A lot of it is working out the UI. On the other hand, I can always release a package, and update it as I improve it.
(In reply to Per Bothner from comment #5) > No, I don't have a sponsor. Okay. If you haven't already, you'd probably want to post on the "devel" mailing list asking for a sponsor. https://lists.fedoraproject.org/archives/list/devel%40lists.fedoraproject.org/ If you *have* asked already and didn't get a response within a few days, you can just bump the email thread. It's a busy mail list, so you have potentially many readers, but also many other messages to compete against. So if no one notices, just post again :) Include a description of your package, DomTerm, and a link back to this Bugzilla thread. > * It would be helpful to update the libwebsockets package (to version 2.2), Looks like updates are in the works. This thread is full of automated messages about detecting and trying to build for 2.2, then falling back to 2.1.1. https://bugzilla.redhat.com/show_bug.cgi?id=1422477 If that thread closes with only a 2.1.1 build, you could manually open a ticket about updating to 2.2.x. > * I want domterm to install a 'dt-util' script, which does useful things > when running under DomTerm. Hmm, that may or may not be something you'd want or need to put in a subpackage. I haven't needed to do anything like that yet, so I'm not sure. The Fedora RPM Guide talks only a little about subpackages... https://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch10s04.html And there's not much in the Wiki about when to use a subpackage or not, so this might be something to ask your future sponsor. > * I'm thinking of including some documentation (in html form) that can be > loaded by a hotkey. BUt that can wait. You've been writing your docs in Troff format so far. Especially if you're planning on also doing HTML or other formats, you might like the idea of writing the documentation in Markdown, or another more readable markup language, and then converting to whatever other formats with tools like pandoc or cmark, or other document converters. Just a thought. I personally would to maintain a nontrivial document in Troff. > On the other hand, I can always release a package, and update it as I > improve it. True :) If DomTerm is usable now, then I don't think there's much point in waiting to package it. I mean, the timing is up to you. But you'll surely get more and better feedback when it's easier to install the application.
I added 1422477 as a dep, mostly to have an easy-to-access link to it. It's almost complete anyway. fedora-review says: - Package uses either %{buildroot} or $RPM_BUILD_ROOT Note: Using both %{buildroot} and $RPM_BUILD_ROOT See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros My advice is to stick to %buildroot (or %{buildroot}). That's the commonly used modern form. In fact, it'd be best to replace the make invocation with %make_install, which expands to the exact same line. Those long lines with multiple BuildRequires should be split into separate lines. It's not required, but in my opinion is makes it much easier to spot mistakes and diffs look *much* better. domterm.src:10: W: macro-in-comment %{commit0} domterm.src:44: W: macro-in-comment %{_bindir} → The second one should be replaced with %%{_bindir} to avoid the warning. The first one can stay, if you want to have a valid link in rpmspec -P output. https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires says that you need Requires: javapackages-tools, Requires: java. (Either directly or indirectly, but I don't see it either way.) Two big things which is missing, and which is strongly recommended by the guidelines are a .desktop file [https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files] and an .appdata file [https://fedoraproject.org/wiki/Packaging:AppData]. In this case an appdata file will be particularly useful, advertising your software and allowing potential users to see how if differers from the many other terminal emulators. In the %description, please add some meat. Why is this package better, what does it do differently than other terminal emulators, what cool features does it sport. This text shouldn't be long, just one or two paragraphs to pique interest. You can reuse most of the text in the appdata file anyway ;) I built and installed the package, on F25. Seems not to work: - in the terminal: $ ldomterm [9066:9066:0328/230720.355575:ERROR:child_thread_impl.cc(762)] Request for unknown Channel-associated interface: ui::mojom::GpuMain and there's also a big blank window which only says "Waiting for localhost..." in the lower left corner. qtdomterm works OK, but it doesn't seem to support HiDPI very well (see attached screenshot). From what I know, it's a general problem with Qt, so I don't expect you to fix it. Just pointing this out in case you need stuff to improve ;) -- I can sponsor you. My requirements apart from this package will be two or three reviews of other packages. Your package is pretty complex, so you'll soak up a lot of the packaging guidelines just for it, but it's always good to see and critique what other people are doing. Please see http://fedoraproject.org/PackageReviewStatus/NEW.html for a list of interesting candidates. Please set up mock, if you haven't already, and base your reviews on fedora-review output, but note that fedora-review does get stuff wrong occasionally and leaves a lot of boxes to be filled manually. In the reviews make a comment that your review is informal because you are not in the packagers group yet. After you are, you can finish those reviews, if nobody else beats you to it. In case of questions, feel free to query me on IRC (I'm zbyszek in #fedora-devel), or by mail.
Created attachment 1267223 [details] qtdomterm screenshot I have a HiDPI screen: XWAYLAND0 connected 2560x1440+0+0 310mm x 170mm 2560x1440 59.96*+ and the default fonts in qtdomterm don't work out too well. Also note that unicode characters are not really displayed properly. It doesn't seem to just a case of missing fonts, instead multi-byte utf8 characters are converted to some ascii noise.
Created attachment 1267224 [details] qtdomterm menu
Re comments #8 and #9 [Qt font problem]: The Emacs HELLO file is not Unicode. The 'Local Variables' section of the file says it is 'coding: iso-2022-7bit' . ISO 2022 is not terribly interesting these days. Neither xterm nor gnome-terminal handle it, at least not by default. "It doesn't seem to just a case of missing fonts, instead multi-byte utf8 characters are converted to some ascii noise." That's because they're not multi-byte utf8 characters. About the ugly menu display: I've seen similar qtdomterm font issues myself, but they want away. Not sure what caused it or what fixed it. :-( I have 4k monitor but it's 42", so it probably wouldn't count as HiDPI ... [I'll get to the more complex comment #7 later.]
Ooops. For some reason I assumed it's unicode. I tested with https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-demo.txt, and that displays beautifully.
I've updated DomTerm to take into account your comments Spec URL: http://per.bothner.com/DomTerm/domterm.spec SRPM URL: http://per.bothner.com/DomTerm/domterm-0.75-1.fc25.src.rpm * It no longer uses RPM_BUILD_ROOT. * It uses %make_install. * I split up BuildRequires into one per dependency. * I removed %{_bindir} in a comment. * I added: Recommends: java See the comment in the spec file. * I added .desktop and .appdata.xml files. * I added some more features in the %description. You reported that the following didn't work for you: $ ldomterm I noticed that starting google-chome in '-app' mode doesn't work if google-chome (as a regular browser) isn't already running. Could you try: $ ldomterm --firefox and/or $ ldomterm --browser It might be reasonable to change the default to --firefox, which is a "XUL" wrapper. The big problem is that Mozilla intends to remove XUL support this year, and I haven't found a good replacement yet. So perhaps --browser (uses the user's default desktop browser) is the safest default. There is a new dt-util script, which is meant to be used when running in a DomTerm terminal: http://domterm.org/dt-util.html Note I recently implemented a major new feature, paging: http://domterm.org/Paging.html However, this only works with the ldomterm server, and only with libwebsockets 2.2. Plus the UI (including key-bindings) needs work.
I can see the spec, but trying to download the SRPM returns Error 404.
Oops, that should have been 0.74. not 0.75. Hence: Spec URL: http://per.bothner.com/DomTerm/domterm.spec SRPM URL: http://per.bothner.com/DomTerm/domterm-0.74-1.fc25.src.rpm
Your spec file still has several Source0 tags, all but one commented out. Again, the macro-ized URL we would normally expect for a project hosted on GitHub is this: https://github.com/PerBothner/DomTerm/archive/%{version}.tar.gz One issue with this, though, is that the project's GitHub seems to be a bit behind the source you're using to build the SRPM. You're building DomTerm 0.74, but the latest release I see tagged on GitHub is still version 0.72. If you do use that URL, you'd probably also have to update the directory name used in the autosetup line (currently, `%autosetup -n DomTerm-%{commit0}`). Or if, as the %{commit} macro suggests, you plan to build a prerelease from arbitrary commits, then you'd use this very similar macro-ized URL: https://github.com/PerBothner/DomTerm/archive/%{commit0}.tar.gz And then you probably ought to change the spec Release tag to reflect the fact that it's a prerelease (that is, use a number like 0.1 instead of 1). The most recent entry in the spec %changelog is a little behind, as it mentions version 0.72-1, rather than 0.74-1. I don't think it's super important to have a detailed spec changelog before this package has been initially approved, but rpmlint complains when the spec Version tag and the version mentioned in the latest changelog entry are out of sync. At least *replace* the changelog entry, if not adding new ones --- until the package is approved. *Then* you should keep the several most recent entries, so users can at their option check the changelog before they install updates. In the %build section, I noticed this line: %configure --disable-pty --with-qtwebengine --with-java --with-libwebsockets If you're configuring to build with java, shouldn't java be a Requires instead of a Recommends? I think .desktop files should be installed using desktop-file-install and/or desktop-file-validate, to ensure they validate. rpmlint caught an issue anyway, though: the .desktop file contains the line `Exec=domterm`, but there is no `domterm` file under /usr/bin/ -- just ldomterm and qtdomterm. The built domterm package contains the directory /usr/lib/.build-id/ -- which looks to me like leftovers from the build. Probably this isn't needed in the binary RPM, right? fedora-review detects some directories without a known owner. Add macro-ized versions of these to your %files section. /usr/share/domterm/defaults/ /usr/share/domterm/defaults/preferences/ The spec License tag simply says "BSD". Since there are so many variants on the BSD license, I don't think that's specific enough. You would need to use "BSD-simplified" or whatever the standard abbreviation for the current license is... Moreover, fedora-review's license check finds 3 files in /qtdomterm/ which have comments stating that they use the GNU GPL, which I think might mean that all of DomTerm should also use GPL(?). I also suspect that all these files containing copied code might amount to library bundling, which is at least discouraged in Fedora. But I'll let Zbigniew have the final say on this.
Concerning setting Source0, the problem is I can't get fedpkg to work if I set Source0 to the github download. I'm probably invoking fedpkg incorrectly - but I'm trying to follow the examples in https://fedoraproject.org/wiki/How_to_create_an_RPM_package When I set: Source0: https://github.com/PerBothner/DomTerm/archive/%{commit0}.tar.gz and try to run: $ fedpkg --release f25 local I get: sources file doesn't exist. Source files download skipped. error: File /home/bothner/DomTerm/d66241a62eb4cc9ffe90760ef39c7d890ef627a0.tar.gz: No such file or directory If I set: Source0: https://github.com/PerBothner/DomTerm/archive/%{commit0}.tar.gz#/DomTerm-%{commit0}.tar.gz I at least get the correct tar file: sources file doesn't exist. Source files download skipped. error: File /home/bothner/DomTerm/DomTerm-d66241a62eb4cc9ffe90760ef39c7d890ef627a0.tar.gz: No such file or directory The DomTerm-d66241a62eb4cc9ffe90760ef39c7d890ef627a0.tar.gz at least matches the filename if I open the specified URL in a browser. Same problem with: fedpkg --release f25 mockbuild
Concerning: "If you're configuring to build with java, shouldn't java be a Requires instead of a Recommends?" - see this comment in the spec file: Java is needed for the stylesheet-manipulating subcommands of dt-util. This may change (it's just an implementation legacy). The domterm package also includes Java client and server classes that are useful for Java applications (for example Kawa). It is convenient to put them in the same domterm.jar as the much-bigger JavaScript and style files. However, these Java classes are not needed for the ldomterm or qtdomterm applications.
(In reply to Per Bothner from comment #16) > sources file doesn't exist. Source files download skipped. This part you can ignore for the moment. This isn't documented very well, and I'm planning on fixing that as soon as I can set aside the time for it: After your package is approved and you've been added to the packagers group, you can use fedpkg to generate the `sources` file and upload the source tarballs to the "lookaside cache" for building on the Fedora infrastructure. The warning about the sources file is sort of pointless for a package that hasn't been approved yet. > error: File > /home/bothner/DomTerm/d66241a62eb4cc9ffe90760ef39c7d890ef627a0.tar.gz: No > such file or directory It's funny, fedora-review will download files for you, but for fedpkg, you have to download the source tarball before fedpkg can build with it. When the Source0 resolves to a valid online URL, you can download the tarball with `spectool --get-files ./domterm.spec`. You could also manually feed the tarball URL to wget or curl, but downloading via spectool helps to make sure your RPM spec has the correct URL. fedpkg should build just fine after that. Doing a `mockbuild` instead of a `local` build provides a more consistent build environment, and helps make sure you've properly listed all dependencies. I would usually do something like this: fedpkg --release f27 mockbuild --no-clean-all ...Or, this *should* work. When I use the most recently linked spec file, and download https://github.com/PerBothner/DomTerm/archive/%{commit0}.tar.gz and do a fedpkg mockbuild, it fails with this error message: File not found: /builddir/build/BUILDROOT/domterm-0.74-1.fc26.x86_64/usr/share/applications/domterm.desktop And likewise for the qtdomterm.desktop. The Makefile.am does create the directory `$(DESTDIR)$(datadir)/applications`, but doesn't seem to actually install the .desktop files. In fact, I didn't see any mention of "desktop" in the Makefile at all, so I added these lines to the spec file, at the end of the %install section. cp -p domterm.desktop %{buildroot}/%{_datadir}/applications/domterm.desktop cp -p qtdomterm.desktop %{buildroot}/%{_datadir}/applications/qtdomterm.desktop The fedpkg mockbuild for DomTerm runs successfully then. The equivalent Makefile commands might be better than appending to the spec file, though. (In reply to Per Bothner from comment #17) > - see this comment in the spec file: > > Java is needed for the stylesheet-manipulating subcommands of dt-util... Right, DomTerm itself doesn't really *need* Java, it's optional for only a couple features. But you are choosing to *build* with Java. I haven't tested this, but I wondered if DomTerm would crash or throw errors when launching on a system that does not have Java installed, if you're configuring it to expect Java. If it would only be an issue when trying to use those optional features, and preferably if DomTerm can gracefully handle Java's absence, then I guess you're fine.
Updated (same locations): Spec URL: http://per.bothner.com/DomTerm/domterm.spec SRPM URL: http://per.bothner.com/DomTerm/domterm-0.74-1.fc25.src.rpm I'll clean up the Source0 commans and version numbers when when everything else is OK. It probably makes sense until libwebsockets 2.2 is updated and I can test against that. ".desktop files should be installed using ... desktop-file-validate" Fixed, "rpmlint caught an issue anyway, though: the .desktop file contains the line `Exec=domterm`, but there is no `domterm` file under /usr/bin/ -- just ldomterm and qtdomterm." Well, there is be a /usr/bin/domterm installed, using alternates, but I changed the .desktop line to Exec=ldomterm. "The built domterm package contains the directory /usr/lib/.build-id/ -- which looks to me like leftovers from the build. Probably this isn't needed in the binary RPM, right?" I'm somewhat mystified by this. I noticed the /usr/lib/.build-id directory is full of links on Rawhide, but doesn't exist on F25. It seems to be related to this: https://fedoraproject.org/wiki/Releases/FeatureBuildId "fedora-review detects some directories without a known owner." Fixed. "The spec License tag simply says "BSD". Since there are so many variants on the BSD license, I don't think that's specific enough." From what I can tell, "BSD" is correct: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Valid_License_Short_Names https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses "Moreover, fedora-review's license check finds 3 files in /qtdomterm/ which have comments stating that they use the GNU GPL, which I think might mean that all of DomTerm should also use GPL(?)." Only qtdomterm needs to be GPL. This is only an issue with certain "backend" files, and I think it will make sense to rip those out in favor of using ldomterm, for the sake of simplicity. But until then I changed the qtdomterm license to "GPLv2+". "I also suspect that all these files containing copied code might amount to library bundling, which is at least discouraged in Fedora." Do you mean the third-party JavaScript files I use? They're relatively small, and I think trying to package a bunch of JavaScript client files would be a pain. "Right, DomTerm itself doesn't really *need* Java, it's optional for only a couple features. ... If it would only be an issue when trying to use those optional features, and preferably if DomTerm can gracefully handle Java's absence, then I guess you're fine." I think so. And I think it may be preferable to forcing installation of a bunch of Java packages.
(In reply to Per Bothner from comment #19) > "The built domterm package contains the directory /usr/lib/.build-id/ -- > which looks to me like leftovers from the build. Probably this isn't needed > in the binary RPM, right?" That's generated by rpm itself. It was added in https://fedoraproject.org/wiki/Changes/ParallelInstallableDebuginfo. rpmlint doen't know about this yet. > "The spec License tag simply says "BSD". Since there are so many variants on > the BSD license, I don't think that's specific enough." > > From what I can tell, "BSD" is correct: > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/ > LicensingGuidelines#Valid_License_Short_Names > https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses Yes. > "Moreover, fedora-review's license check finds 3 files in /qtdomterm/ which > have comments stating that they use the GNU GPL, which I think might mean > that all of DomTerm should also use GPL(?)." > > Only qtdomterm needs to be GPL. This is only an issue with certain > "backend" files, and I think it will make sense to rip those out in favor of > using ldomterm, for the sake of simplicity. But until then I changed the > qtdomterm license to "GPLv2+". > > "I also suspect that all these files containing copied code might amount to > library bundling, which is at least discouraged in Fedora." Bundling is allowed since https://pagure.io/fesco/issue/1483. Bundling is reluctantly accepted. This makes a lot of sense especially for javascript. Most people have given up on unbundling javascript. The appdata file should be validated using appstream-util validate-relax --nonet. (If the make file does this already by itself, ignore my comment.)
It's been a while. Per Bothner, are you still interested in working on this package?
Yes, I'm still interested. Since the previous "push" I'm made major changes that effect its packaging, especially the qtdomterm sub-package (which now only handles the UI), and help/man pages. Plus lots of features and polishing and speed-up and ports (to Atom, to Theia, to Ubuntu, to Windows using WSL). I've also posted articles to OpenSource.com and dzone.com (submitted yesterday). I've done some updates to domterm.spec, but it needs more testing and review.
Can you post the updated spec and srpm?
Sorry it took a while - I needed to re-write focus handling which led to a re-write of "input line editing" http://domterm.org/Input-line-editing.html (kind of a builtin readline library). OTOH I'm very happy with the result and I think we can declare DomTerm 1.0 any time. (There is always a wishlist of things I'd like to get done, but none I think are show-stoppers - except more testing is needed.) http://per.bothner.com/tmp/domterm.spec http://per.bothner.com/tmp/domterm-0.99-1.fc27.src.rpm http://per.bothner.com/tmp/domterm-0.99-1.fc27.x86_64.rpm http://per.bothner.com/tmp/qtdomterm-0.99-1.fc27.x86_64.rpm
Wow, I have to say that it has come a long way. I tried using both the browser and qt versions, and they work very nicely. Packaging looks good. One thing seems to be missing — validation of the appstream files. See https://fedoraproject.org/wiki/Packaging:AppData#app-data-validate_usage. validate-relax passes, but validate does not: $ appstream-util validate /usr/share/appdata/domterm.appdata.xml /usr/share/appdata/qtdomterm.appdata.xml /usr/share/appdata/domterm.appdata.xml: FAILED: • tag-missing : <translation> not specified • tag-missing : <update_contact> is not present • style-invalid : Not enough <screenshot> tags, minimum is 1 • tag-missing : <summary> is not present /usr/share/appdata/qtdomterm.appdata.xml: FAILED: • tag-missing : <translation> not specified • tag-missing : <update_contact> is not present • style-invalid : Not enough <screenshot> tags, minimum is 1 • tag-missing : <summary> is not present Validation of files failed (The requirements for appdata files keep evolving. So something that was passing a few months back might not anymore.) Package fails to build in rawhide mock: + autoreconf BUILDSTDERR: configure.ac:14: error: required file 'autotools-aux/compile' not found BUILDSTDERR: configure.ac:14: 'automake --add-missing' can install 'compile' BUILDSTDERR: configure.ac:10: error: required file 'autotools-aux/missing' not found BUILDSTDERR: configure.ac:10: 'automake --add-missing' can install 'missing' BUILDSTDERR: autoreconf: automake failed with exit status: 1 RPM build errors: BUILDSTDERR: error: Bad exit status from /var/tmp/rpm-tmp.mosYgv (%build) BUILDSTDERR: Bad exit status from /var/tmp/rpm-tmp.mosYgv (%build) Child return code was: 1
BR: gcc or g++ is also necessary now, see https://fedoraproject.org/wiki/Changes/Remove_GCC_from_BuildRoot.
Thanks for the feedback! A question about the *.appdata.xml files. I didn't find any real documentation on the <translation> tag in either https://fedoraproject.org/wiki/Packaging:AppData#app-data-validate_usage or https://www.freedesktop.org/software/appstream/docs/sect-Metadata-Application.html#spec-appdata-filespec. The only thing I found was https://blogs.gnome.org/hughsie/2016/01/25/appdata-and-the-gettext-domain/ I could add: <translation type="gettext">domterm</translation> but that seems kind-of silly when there is no translation support at this point.
No idea. Probably best to leave it out for now.
Updated to deal with the problems you noted, plus some others. The most important is that the Source0 URL is for a release tar-ball (as produced by 'make dist'), rather than a git snapshot: The former includes "missing" files added by 'make dist'. http://per.bothner.com/tmp/domterm-0.99.2-1.fc27.src.rpm http://per.bothner.com/tmp/domterm.spec fedora-review reports some weird warnings/errors, including: domterm.x86_64: W: only-non-binary-in-usr-lib No idea what that means. domterm.x86_64: E: zero-length /usr/share/domterm/help/domterm-attach.txt (and more) I don't know what this means - after rpm -i those files are installed but they're not zero-length.
I've released DomTerm 1.0 (adn announced it on LWN: https://lwn.net/Articles/750319/) See https://github.com/PerBothner/DomTerm/releases/tag/1.0 which includes rpms. Specfile: https://github.com/PerBothner/DomTerm/blob/master/domterm.spec
Hi, sorry for the delay. Please in the future use real links to the spec file and rpm. fedora-review than can download the srpm and spec automatically. In this case it should be something like: https://github.com/PerBothner/DomTerm/releases/download/1.0/domterm-1.0-1.fc27.src.rpm https://raw.githubusercontent.com/PerBothner/DomTerm/master/domterm.spec Source0 is wrong. It seems you didn't actually tag 1.0.2? > %global version ... This is not needed. Macro %version is automatically created from the Version field. > License: BSD1 The license tag should be one from https://fedoraproject.org/wiki/Licensing:Main#Software_License_List. I think you need License: BSD with advertising. But please consider switching to two-clause BSD, which is compatible with GPL. In %files, I'd suggest just useing %{_datadir}/domterm/help/ (without %dir). Unless you really want to list all files there. This will make version upgrades a bit easier. Apart from this package, can you please do the reviews mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1424890#c7. I'd like to wrap this up ;)
In terms of I reviewing other packages, I'm having trouble finding packages that haven't already gotten the kind of review I'd be qualified to give. There there are some packages in my areas of interest/expertise, but they already have a number of comments. One exception is js-viewport-navigation (https://bugzilla.redhat.com/show_bug.cgi?id=1571742). There I focused on the package itself, rather than the packaging, and opined it was not suitable/ready for Fedora - hopefully in a constructive way. I tried fedora-review on pgcli (https://bugzilla.redhat.comshow_bug.cgi?id=1570551) but ran into problems, as you can see.
"Source0 is wrong. It seems you didn't actually tag 1.0.2?" I've made a habit of updating the version number in domterm.spec when I update the version number in the git repository itself. It probably makes more sense to only update the version number in domterm.spec when creating a tag or release. The "BSD1" is because some package-test program didn't like plain "BSD" - don't remember which. I'll change it to the latter.
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time, but it seems that the review is still being working out by you. If this is right, please respond to this comment clearing the NEEDINFO flag and try to reach out the submitter to proceed with the review. If you're not interested in reviewing this ticket anymore, please clear the fedora-review flag and reset the assignee, so that a new reviewer can take this ticket. Without any reply, this request will shortly be resetted.
This is an automatic action taken by review-stats script. The ticket reviewer failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we reset the status and the assignee of this ticket.
This is an automatic action taken by review-stats script. The ticket submitter failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we consider this ticket as DEADREVIEW and proceed to close it.