Spec URL: http://mmahut.fedorapeople.org/reviews/smstools/smstools.spec SRPM URL: http://mmahut.fedorapeople.org/reviews/smstools/smstools-2.2.18-1.src.rpm Mock: http://mmahut.fedorapeople.org/reviews/smstools/mock/ Description: The SMS Server Tools send and receive short messages through GSM modems or mobile phones. You can send short messages by simply storing text files into a special spool directory.
Thanks for the package, taking this for a review.
Marek: The package looks superb. Just a few minor notes before I can approve it: Summary is not correct american english. I believe the License is GPLv2+, not GPLv2 RPMlint output: Lot of smstools.i386: W: wrong-file-end-of-line-encoding /usr/share/doc/smstools-2.2.18/index.html and smstools.i386: W: wrong-file-end-of-line-encoding /usr/share/doc/smstools-2.2.18/license.html you probably need some recode/iconv/dos2unix magic smstools.src:22: W: setup-not-quiet just add -q to %setup smstools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 1) safe to ignore
We already have smstools under review in bug 235637. The submitter hasn't however been at all responsive to my comments in ~7 months so I think it's better to continue here. It would be useful to check if any of my findings in that bug apply here too though, and check if it makes more sense to package the original 2.x version or the enhanced one from http://smstools3.kekekasvi.com/ (which is what was submitted in bug 235637).
At least the /usr/bin/sendsms conflict with gnokii from bug 235637 appears to apply to this package too.
Hello Ville, Hello Lubomir, So, I've modified the spec file from Andreas. http://mmahut.fedorapeople.org/reviews/smstools/3.0.6/ I've renamed the script to smssend, which is better from my point of view, because other smstools scripts starts with "sms*" too. I will try to implement the feature of running with no-root user later, I will check with upstream if it's possible. But for now, running as root is enough for me (we will use this package in my company). Let me know in case of any other suggestions.
Sooo, Marek: In my opinion you shouldn't change modes in threee different places; (ugly chmods in %prep, install -m %install and %attr in %files). And RPMlint: smstools.i386: W: file-not-utf8 /usr/share/doc/smstools-3.0.6/manual/softwarecomp.html ... Alreay discussed that, not a problem. smstools.i386: W: file-not-utf8 /usr/share/doc/smstools-3.0.6/examples/send_sms_unicode.sms UNICODE example, whouldn't make sense if was utf8 smstools.i386: E: non-standard-dir-perm /var/spool/sms/failed 0750 smstools.i386: E: non-standard-dir-perm /var/spool/sms/checked 0750 smstools.i386: E: non-standard-dir-perm /var/spool/sms/outgoing 0750 smstools.i386: E: non-readable /etc/smsd.conf 0600 smstools.i386: E: non-standard-dir-perm /var/spool/sms/sent 0750 smstools.i386: E: non-standard-dir-perm /var/spool/sms/incoming 0750 smstools.i386: E: non-standard-dir-perm /var/spool/sms 0750 I think this is all intended smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools Using /var/lock/subsys/smsd for a subsys lock instead of /var/lock/subsys/smstools would make more sense in my opinion. smstools.i386: W: incoherent-init-script-name smsd Perfectly ok smstools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 66, tab: line 1) Run expand on the spec.
(In reply to comment #6) > Sooo, Marek: > > In my opinion you shouldn't change modes in threee different places; (ugly > chmods in %prep, install -m %install and %attr in %files). Done, let me know if it's ok. > And RPMlint: > > smstools.i386: W: file-not-utf8 > /usr/share/doc/smstools-3.0.6/manual/softwarecomp.html > ... > > Alreay discussed that, not a problem. > > smstools.i386: W: file-not-utf8 > /usr/share/doc/smstools-3.0.6/examples/send_sms_unicode.sms > > UNICODE example, whouldn't make sense if was utf8 > > smstools.i386: E: non-standard-dir-perm /var/spool/sms/failed 0750 > smstools.i386: E: non-standard-dir-perm /var/spool/sms/checked 0750 > smstools.i386: E: non-standard-dir-perm /var/spool/sms/outgoing 0750 > smstools.i386: E: non-readable /etc/smsd.conf 0600 > smstools.i386: E: non-standard-dir-perm /var/spool/sms/sent 0750 > smstools.i386: E: non-standard-dir-perm /var/spool/sms/incoming 0750 > smstools.i386: E: non-standard-dir-perm /var/spool/sms 0750 > > I think this is all intended Indeed. > smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools > smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools > smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools > > Using /var/lock/subsys/smsd for a subsys lock instead of > /var/lock/subsys/smstools would make more sense in my opinion. Done. > smstools.i386: W: incoherent-init-script-name smsd > > Perfectly ok > > smstools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 66, tab: line 1) > > Run expand on the spec. Done. Updated spec file: http://mmahut.fedorapeople.org/reviews/smstools/3.0.6/
New version is here: http://mmahut.fedorapeople.org/reviews/smstools/3.0.10/
A few other things; Please fix the source URL, and bump to latest upstream version.
Done.
smstools3/src/stats.c does: 40 #ifndef NOSTATS 41 MM_create(DEVICES*sizeof(_stats),tempnam(0,0)); 42 #endif and libmm does: 235 #if defined(MM_SHMT_MMPOSX) || defined(MM_SHMT_MMFILE) 236 sprintf(shmfilename, "%s.mem", file); 237 fnmem = shmfilename; 238 #endif 257 shm_unlink(fnmem); /* Ok when it fails */ 258 if ((fdmem = shm_open(fnmem, O_RDWR|O_CREAT|O_EXCL, MM_CORE_FILEMODE)) == -1) 259 FAIL(MM_ERR_CORE|MM_ERR_SYSTEM, "failed to open tempfile"); This leaves possibility that some user guesses the temporary file name generated by adding ".mem" suffix to result of tempnam() call and cause smsd to fail. As this can be considered a low severity security flaw, it needs to be addressed. It would be hard to fix it, since the problem lies partly in the mm library (which mangles the file name by adding the .mem suffix), probably by using a private temporary directory. I think the best solution would be to disable stats support (which is even disabled by upstream by default and would permit dropping of mm dependency).
All outstanding inssues have been addressed in http://mmahut.fedorapeople.org/reviews/smstools/3.0.10/ APPROVED
*** Bug 235637 has been marked as a duplicate of this bug. ***
Thank you Lubomir for your patience :)
New Package CVS Request ======================= Package Name: smstools Short Description: Tools to send and receive short messages through GSM modems or mobile phones Owners: mmahut Branches: F-7 F-8 EL-4 EL-5 Cvsextras Commits: yes
cvs done.
Thank you all!
smstools-3.0.10-1.fc7 has been pushed to the Fedora 7 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update smstools'
smstools-3.0.10-1.fc7 has been pushed to the Fedora 7 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: smstools New Branches: epel7 Owners: mmahut pcfe
Git done (by process-git-requests).
This was left opened following a new branch request. Closing again in an effort to clean up the queue.