On Wed, Apr 12, 2017 at 1:05 AM, Corentin Noël wrote: > https://mentors.debian.net/debian/pool/main/g/gsignond/gsignond_1.0.6.dsc
I don't intend to sponsor this, but here is a quick review: debian/source/format should be 3.0 (quilt) instead of 3.0 (native) and the version number should be 1.0.6-1 instead of 1.0.6. Usually DH_VERBOSE is not set in debian/rules, you might want to comment that out or remove it. DEB_CONFIGURE_EXTRA_FLAGS is a cdbs-specific make variable but your debian/rules doesn't use cdbs, I would suggest removing those lines and the associated comment from debian/rules. dh --parallel is the default with debhelper compat 10, I would suggest you read the debhelper manual page for more information about compat level 10, remove --parallel from debian/rules and bump debian/compat to 10. One thing this will do is enabling autoreconf, which should always be done, even in lower compat levels. Please remove override_dh_auto_test, all packages should run their build time tests. If they currently fail on Debian you can run them but not fail the build. Why does debian/rules override the upstream location for the configuration file? debian/README.Debian doesn't look useful, I would suggest removing it. The maintainer scripts contain a lot of boilerplate comments that can be removed. The handling of gsignond.conf in the maintainer scripts looks very strange, is that needed? I'm not sure, but I think ldconfig should never be run manually from maintainer scripts. I don't think a -dev package is the right place for dbus service definitions? debian/install should be removed because you have all the other debian/*.install files. debian/gsignond-doc.docs looks strange, I'd suggest removing it. The Vcs-* fields refer to the Debian packaging VCS, not the upstream VCS. You should use unstable rather than UNRELEASED as the target suite in debian/changelog. You should close your ITP in debian/changelog, I'd suggest retitling the RFP you filed to ITP: https://bugs.debian.org/831747 https://www.debian.org/Bugs/server-control#retitle The upstream README.md doesn't contain any information useful to users of the binary packages, I'd suggest not installing it. The upstream NEWS file is empty, I'd suggest contacting upstream to ask them to remove it or put useful information in it. Also, don't install it in the Debian binary packages. You might want to setup some tests for ci.debian.net: https://ci.debian.net/doc/ tools/archive.sh contains some commands for creating tarballs but hardcodes the version number to 0.0.2, you might want to talk to usptream about fixing that. We generally encourage upstreams to not distribute debian/ directories, even in subdirs like dists/debian/ The tests and dbus files hard-code paths in /tmp but they should never do that, instead they should only ever use random filenames for /tmp/. Are you sure the package build needs vala? There is only one *.vala file and it doesn't seem to be touched by the build. Automatic checks: build: gsignond-signonui-data.c:390: Warning: gSignond: gsignond_signonui_data_set_forgot_password: invalid return annotation gsignond-signonui-data.c:424: Warning: gSignond: gsignond_signonui_data_set_forgot_password_url: invalid return annotation gsignond-signonui-data.c:793: Warning: gSignond: gsignond_signonui_data_set_url_response: invalid return annotation gsignond-utils.c:311: Warning: gSignond: gsignond_variant_to_sequence: return value: Invalid non-constant return of bare structure or union; register as boxed type or (skip) gsignond-utils.c:366: Warning: gSignond: gsignond_array_to_sequence: return value: Invalid non-constant return of bare structure or union; register as boxed type or (skip) gsignond-utils.c:394: Warning: gSignond: gsignond_copy_array_to_sequence: return value: Invalid non-constant return of bare structure or union; register as boxed type or (skip) gsignond-session-data.c:218: Warning: gSignond: gsignond_session_data_get_allowed_realms: return value: Invalid non-constant return of bare structure or union; register as boxed type or (skip) ... gtkdoc-mkdb --module=gsignond --output-format=xml --expand-content-files="" --main-sgml-file=gsignond-docs.sgml ${_source_dir} --xml-mode --output-format=xml ../include/gsignond/gsignond.h:41: warning: Section gsignond is not defined in the gsignond-sections.txt file. ... gtkdoc-fixxref --module=gsignond --module-dir=html --html-dir=/usr/share/gtk-doc/html html/GSignondPlugin.html:154: warning: no link for: 'GStrv' -> (<span class="type">GStrv</span>). html/GSignondPlugin.html:179: warning: no link for: 'G-SIGNAL-RUN-FIRST:CAPS' -> (Run First). html/GSignondPlugin.html:247: warning: no link for: 'GObject' -> (<span class="type">GObject</span>). html/GSignondPlugin.html:637: warning: no link for: 'GError' -> (<span class="type">GError</span>). html/GSignondPlugin.html:897: warning: no link for: 'GTypeInterface' -> (<span class="type">GTypeInterface</span>). html/GSignondSessionData.html:277: warning: no link for: 'GVariant' -> (<span class="type">GVariant</span>). html/GSignondSessionData.html:525: warning: no link for: 'GSequence' -> (<span class="type">GSequence</span>). html/gsignond-Errors.html:220: warning: no link for: 'g-error-new' -> (<code class="function">g_error_new()</code>). html/GSignondDictionary.html:369: warning: no link for: 'GVariantBuilder' -> (<span class="type">GVariantBuilder</span>). html/GSignondDictionary.html:371: warning: no link for: 'g-variant-builder-unref' -> (<code class="function">g_variant_builder_unref()</code>). html/GSignondDictionary.html:1026: warning: no link for: 'GHashTable' -> (<span class="type">GHashTable</span>). html/GSignondConfig.html:113: warning: no link for: 'g-get-system-config-dirs' -> (<code class="function">g_get_system_config_dirs()</code>). html/GSignondAccessControlManager.html:184: warning: no link for: 'GList' -> (<span class="type">GList</span>). html/gsignond-General-configuration.html:116: warning: no link for: 'g-get-user-name' -> (<code class="function">g_get_user_name()</code>). ... Cannot open /usr/share/gtk-doc/html: No such file or directory ... dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/libgsignond-common0/usr/lib/x86_64-linux-gnu/libgsignond-common.so.0.0.0 was not linked against libgmodule-2.0.so.0 (it uses none of the library's symbols) ... dpkg-gencontrol: warning: Depends field of package gir1.2-gsignond-1.0: unknown substitution variable ${gir:Depends} dpkg-gencontrol: warning: Depends field of package gir1.2-gsignond-1.0: unknown substitution variable ${shlibs:Depends} lintian: I: gsignond source: duplicate-long-description gsignond libgsignond-common0 W: gsignond source: debhelper-but-no-misc-depends gsignond-doc W: gsignond source: changelog-should-mention-nmu W: gsignond source: source-nmu-has-incorrect-version-number 1.0.6 P: gsignond source: unversioned-copyright-format-uri http://dep.debian.net/deps/dep5 W: gsignond source: out-of-date-standards-version 3.9.7 (current is 3.9.8) I: libgsignond-common-dev: extended-description-is-probably-too-short I: gsignond: spelling-error-in-binary usr/bin/gsignond occured occurred I: gsignond: spelling-error-in-binary usr/bin/gsignond eror error I: gsignond: spelling-error-in-binary usr/bin/gsignond convertion conversion I: gsignond: hardening-no-bindnow usr/bin/gsignond I: gsignond: hardening-no-bindnow usr/lib/x86_64-linux-gnu/gsignond/extensions/libextension-test.so I: gsignond: hardening-no-bindnow usr/lib/x86_64-linux-gnu/gsignond/gplugins/libdigest.so I: gsignond: hardening-no-bindnow ... use --no-tag-display-limit to see all (or pipe to a file/program) I: gsignond: extended-description-is-probably-too-short W: gsignond: binary-without-manpage usr/bin/gsignond W: gsignond: maintscript-calls-ldconfig postinst W: gsignond: maintscript-calls-ldconfig postrm I: gsignond-doc: extended-description-is-probably-too-short I: libgsignond-common0: hardening-no-fortify-functions usr/lib/x86_64-linux-gnu/libgsignond-common.so.0.0.0 I: libgsignond-common0: hardening-no-bindnow usr/lib/x86_64-linux-gnu/libgsignond-common.so.0.0.0 I: libgsignond-common0: extended-description-is-probably-too-short I: libgsignond-common0: no-symbols-control-file usr/lib/x86_64-linux-gnu/libgsignond-common.so.0.0.0 check-all-the-things: # This command checks style. While a consistent style # is a good idea, people who have different style # preferences will want to ignore some of the output. # Do not bother adding non-upstreamable patches for this. $ find . -type f \( -iname '*.sh' -o -iname '*.bash' \) -exec bashate --ignore E002,E003 {} + E001: Trailing Whitespace: ' LD_PRELOAD="libduma.so" $SRC_HOME/src/daemon/.libs/gsignond ' - ./tools/setup-and-start-daemon.sh : L44 $ find .. -maxdepth 1 -type f -iwholename '../*.build' -exec blhc --all {} + <lots of LDFLAGS missing> $ find . -type f -iname '*.sh' -exec checkbashisms {} + <lots> $ env PERL5OPT=-m-lib=. cme check dpkg <lots> $ codespell --quiet-level=3 ... ./src/common/gsignond-plugin-interface.c:223: persistant ==> persistent ./src/common/gsignond-plugin-interface.c:236: occured ==> occurred ./src/common/db/gsignond-secret-storage.c:562: occured ==> occurred ./src/daemon/gsignond-identity.c:307: convertion ==> conversion ./src/daemon/gsignond-identity.c:613: occured ==> occurred ./src/daemon/gsignond-identity.c:711: occured ==> occurred ./src/daemon/dbus/gsignond-dbus-server.c:379: aquired ==> acquired $ cppcheck -j1 --quiet -f . [test/plugins/pluginremotetest.c:484]: (error) Null pointer dereference: error [test/plugins/pluginremotetest.c:496]: (error) Null pointer dereference: error $ find . \( -name .git -o -name .svn -o -name .bzr -o -name CVS -o -name .hg -o -name _darcs -o -name _FOSSIL_ -o -name .sgdrawer -o -name .pc \) -prune -o -empty -print ./ChangeLog ./NEWS $ fdupes -q -r . | grep -vE '/(\.(git|svn|bzr|hg|sgdrawer|pc)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s ./ChangeLog ./NEWS ./dists/debian/ ./debian/ $ flawfinder -Q -c . <lots> # check if these can be switched to https:// $ grep -rF http: . <lots> $ env PERL5OPT=-m-lib=. license-reconcile <some copyright year mismatches> $ licensecheck --check=. --recursive --copyright . | grep --text -F 'with incorrect FSF address' ./aclocal.m4: GPL (with incorrect FSF address) GENERATED FILE # You should not assume that paths are at most PATH_MAX characters long. # Some operating systems (e.g. Hurd) don't define PATH_MAX at all. # Others (e.g. Linux, OS X) define it, but don't enforce the limit. $ find . -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o -iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o -iname '*.hpp' \) -exec grep -HwE 'PATH_MAX|MAXPATHLEN' {} + ./test/daemon/daemon-test.c: path = g_malloc0 (PATH_MAX + 1); ./test/daemon/daemon-test.c: res = readlink (procfname, path, PATH_MAX); ./src/common/gsignond-access-control-manager.c: peerpath = g_malloc0 (PATH_MAX + 1); ./src/common/gsignond-access-control-manager.c: res = readlink (procfname, peerpath, PATH_MAX); $ rpmlint . ./dists/rpm/gsignond-tizen.spec:17: W: unversioned-explicit-provides gsignon ./dists/rpm/gsignond-tizen.spec: W: no-cleaning-of-buildroot %clean ./dists/rpm/gsignond-tizen.spec: W: no-buildroot-tag ./dists/rpm/gsignond-tizen.spec: W: no-%clean-section ./dists/rpm/gsignond-tizen.spec: W: invalid-url Source0: gsignond-1.0.6.tar.gz ./dists/rpm/gsignond-suse.spec:15: W: unversioned-explicit-provides gsignon ./dists/rpm/gsignond-suse.spec: W: no-cleaning-of-buildroot %clean ./dists/rpm/gsignond-suse.spec: W: no-buildroot-tag ./dists/rpm/gsignond-suse.spec: W: no-%clean-section ./dists/rpm/gsignond-suse.spec: E: specfile-error warning: bogus date in %changelog: Thu Feb 08 2013 Jussi Laako <jussi.la...@linux.intel.com> 0 packages and 2 specfiles checked; 1 errors, 9 warnings. $ find . -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh' \) -exec shellcheck {} + <lots, ignore the ltmain.sh ones though> $ find . -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname .svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o -iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o -iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname _sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o -iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname '~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o -iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o -iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o -iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname '*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname '*.css.min' -o -iname '*.wav' \) -exec env PERL5OPT=-m-lib=. spellintian --picky {} + <lots> $ grep -r '/tmp/' . ./test/common/Makefile.am: SSO_STORAGE_PATH=/tmp/gsignond \ ./test/plugins/Makefile.am: SSO_STORAGE_PATH=/tmp/gsignond \ ./test/daemon/daemon-test.c: fail_if (g_setenv ("SSO_STORAGE_PATH", "/tmp/gsignond", TRUE) == FALSE); ./test/daemon/daemon-test.c: if (system("rm -rf /tmp/gsignond") != 0) { ./test/daemon/daemontest.conf:StoragePath = /tmp/gsignond ./test/daemon/gsignond-dbus.conf.in: <listen>unix:tmpdir=/tmp/</listen> ./test/db/dbtest.c: dir = "/tmp/gsignond"; ./test/db/dbtest.c: gsignond_config_set_string (config, GSIGNOND_CONFIG_GENERAL_SECURE_DIR, "/tmp/gsignond"); ./test/db/dbtest.c: gsignond_config_set_string (config, GSIGNOND_CONFIG_GENERAL_SECURE_DIR, "/tmp/gsignond"); ./test/db/dbtest.c: gsignond_config_set_string (config, GSIGNOND_CONFIG_GENERAL_SECURE_DIR, "/tmp/gsignond"); ./test/db/Makefile.am:TESTS_ENVIRONMENT= SSO_STORAGE_PATH=/tmp/gsignond ./tools/setup-and-start-daemon.sh:export SSO_STORAGE_PATH="/tmp/gsignond" ./tools/setup-and-start-daemon.sh:#rm -rf /tmp/gsignond ./tools/run-tests.sh:export SSO_STORAGE_PATH=/tmp/gsignond $ grep -riE 'fixme|todo|hack|xxx+|broken' . ./src/common/gsignond-credentials.c: /* TODO: define proper invalid id */ ./src/daemon/gsignond-identity.c: /* TODO: auto-set to validated on successful process() cycle */ ./src/daemon/gsignond-identity.c: /* TODO; clear the cached secret for unstored identity */ ./src/daemon/gsignond-identity.c: /* FIXME : either username/secret changed reset the identity ./src/daemon/gsignond-identity.c: /*FIXME: Roll-back the local changes */ ./src/daemon/dbus/gsignond-dbus-identity-adapter.c: /* FIXME: Do we allow multiple calls at a given point of time */ $ find . -type f \( -iname '*.yaml' -o -iname '*.yml' -o -iwholename ./debian/upstream/metadata -o -iwholename ./debian/upstream/edam \) -exec yamllint {} + ./.gitlab-ci.yml 1:1 warning missing document start "---" (document-start) 6:3 error wrong indentation: expected 4 but found 2 (indentation) 10:81 error line too long (174 > 80 characters) (line-length) 27:32 warning too few spaces before comment (comments) -- bye, pabs https://wiki.debian.org/PaulWise
