Bug#860123: RFS: gsignond/1.0.6 [ITP] -- gSSO daemon and default plugins
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.laako@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
Reply to: