Bug 222594
Summary: | Review Request: seedit: SELinux Policy Editor | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Yuichi Nakamura <ynakam> | ||||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | mtasaka | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2007-01-31 14:03:23 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779 | ||||||||
Attachments: |
|
Description
Yuichi Nakamura
2007-01-15 00:56:12 UTC
Sorry, please review following. * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.7.beta6.src.rpm Modified to use disttag to describe version number. hi, - for fisrt: i checked out your spec file and...it doesn't meets with extras Packaging Guidelines. see http://fedoraproject.org/wiki/Packaging/Guidelines Theres are a lot of things to review in your SPEC file, a lot of things must be fix. %{distro} can be remove, %{?dist} do the same thing. #policy,gui subpackages are build only when arch is "noarch" --> why don't you set them to noarch in spec file ? your source0 must be point to the full path to verify the tarball. you use cd and cd .. to enter and exit from an subfolder, you can use pushd to enter and popd to exit. your desktop-file-install command miss some option such as mode and -add-categories. there is a lot of macros which's not in the right place. (In reply to comment #2) Thank you for comment. > %{distro} can be remove, %{?dist} do the same thing. Removed %{distro}. >#policy,gui subpackages are build only when arch is "noarch" >--> why don't you set them to noarch in spec file ? I wanted to make noarch.rpm not i386.rpm for doc and gui subpackage, but it seems that such sub packages are usually build as i386.rpm in fedora. So I removed %if s related to noarch, it is more simple. >your source0 must be point to the full path to verify the tarball. Fixed. >you use cd and cd .. to enter and exit from an subfolder, you can use pushd to enter and popd to exit. Fixed. > your desktop-file-install command miss some option such as mode and -add-categories. Fixed, add "-add-categories". > there is a lot of macros which's not in the right place. I looked at macros section of package guide line, but I am not sure what you mean. Can you tell me one example? Other fixes: - Added forgotten BuildRequires for libselinux,libsepol. - Added %{?_smp_mflags} to make New SPEC and SRPM: * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.8.beta6.1.src.rpm (In reply to comment #3) > (In reply to comment #2) > > >#policy,gui subpackages are build only when arch is "noarch" >--> why don't you > set them to noarch in spec file ? > > I wanted to make noarch.rpm not i386.rpm for doc and gui subpackage, > but it seems that such sub packages are usually build as i386.rpm in fedora. Right, this is impossible. All packages being built from a common src.rpm share the same arch. More comments: - MUSTFIX: Package doesn't honor RPM_OPT_FLAGS: ... gcc -c -Wall -O2 -g -DDEBUG=1 -I../include -I/usr/include/selinux lex.yy.c .. - SHOULDFIX: Incorrect include directory -I/usr/include ... cc -Werror -Wall -W -I/usr/include -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -I../include -I/usr/include/selinux -Dfc6 -c -o seedit-restorecon.o ... Passing -I/usr/include is a mistake. You should never pass -I/usr/include. - MUSTFIX: Directory /usr/lib/python2.4/site-packages/seedit is unowned. The seedit package must known this directory. (In reply to comment #4) > - MUSTFIX: Directory /usr/lib/python2.4/site-packages/seedit is unowned. > The seedit package must known this directory. s/known/own/ (In reply to comment #4) > - MUSTFIX: Directory /usr/lib/python2.4/site-packages/seedit is unowned. > The seedit package must known this directory. s/known/own/ more info: your directory /usr/lib/python2.4/site-packages/seedit should look like this --> %{python_sitelib}/%{name}/ where your defined python_sitelib must set above with something like this: %define python_sitelib %(%{__python} -c 'from distutils import sysconfig; print sysconfig.get_python_lib()') > >#policy,gui subpackages are build only when arch is "noarch" >--> why don't you > set them to noarch in spec file ? > > I wanted to make noarch.rpm not i386.rpm for doc and gui subpackage, > but it seems that such sub packages are usually build as i386.rpm in fedora. >Right, this is impossible. All packages being built from a common src.rpm share >the same arch. i meant to set subpakage section to "buildarch: noarch" more comment: vendor option in desktop-file-install should set to vendor "" set it to fedora is not required any more. in %post policy: touch %{_datadir}/share/seedit/sepolicy/need-init --> %{_datadir}/%{name}/sepolicy/need-init same thing for the other below. %doc should be place just after %defattr macros and on the same line. missing argument: %files -f %{name}.lang see Handling Locale Files section in http://fedoraproject.org/wiki/Packaging/Guidelines (In reply to comment #5) >- MUSTFIX: Package doesn't honor RPM_OPT_FLAGS: >... >gcc -c -Wall -O2 -g -DDEBUG=1 -I../include -I/usr/include/selinux lex.yy.c >.. Fixed. Modified spec file and Makefiles. >- SHOULDFIX: Incorrect include directory -I/usr/include >... >cc -Werror -Wall -W -I/usr/include -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE >-I../include -I/usr/include/selinux -Dfc6 -c -o seedit-restorecon.o >... >Passing -I/usr/include is a mistake. You should never pass -I/usr/include. Fixed. >- MUSTFIX: Directory /usr/lib/python2.4/site-packages/seedit is unowned. >The seedit package must own this directory. Fixed. (In reply to comment #6) >more info: >your directory /usr/lib/python2.4/site-packages/seedit >should look like this --> %{python_sitelib}/%{name}/ >where your defined python_sitelib must set above with something like this: >%define python_sitelib %(%{__python} -c 'from distutils import sysconfig; print >sysconfig.get_python_lib()') It is very useful, thanks! Fixed. >vendor option in desktop-file-install should set to vendor "" >set it to fedora is not required any more. Fixed. >in %post policy: >touch %{_datadir}/share/seedit/sepolicy/need-init >--> %{_datadir}/%{name}/sepolicy/need-init >same thing for the other below. Fixed. >%doc should be place just after %defattr macros and on the same line. Fixed. >missing argument: >%files -f %{name}.lang >see Handling Locale Files section in >http://fedoraproject.org/wiki/Packaging/Guidelines Fixed, but there is no locale file in seedit package. locale file exists only in gui subpackage. Current spec file and src.rpm file is following: * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.9.beta6.2.src.rpm Created attachment 146272 [details] Mock build log of on FC-devel i386 Well, * BuildRequires, etc - Firstly, I cannot check this package because mockbuild fails on FC-devel i386. Please check the attached log. * File/Directory entries - For main package, file entries include: --------------------------------------------- %{python_sitelib}/%{name} %{_datadir}/%{name} --------------------------------------------- And, for example -policy subpackage include: --------------------------------------------- %{_datadir}/%{name}/scripts/seedit-installhelper.sh ---------------------------------------------- ... Then seedit-installhelper.sh is installed in both main and -policy subpackage, because ---------------------------------------------- %{_datadir}/%{name} ---------------------------------------------- is interpretted as the directory %{_datadir}/%{name} itself _and_ all the files/directories etc under %{_datadir}/%{name}. If you want to indicate only the directory, you have to write: ---------------------------------------------- %dir %{_datadir}/%{name} ---------------------------------------------- Please remove duplicate entries, then ensure that every directories created during the install of seedit related packages are owned by one package. * Requires ----------------------------------------------- Requires: python >= 2.3 ----------------------------------------------- This is not needed. Python version dependency is automatically added by rpmbuild process. * %build process, etc... ----------------------------------------------- --add-category X-Fedora ----------------------------------------------- The category "X-???" is deprecated and should be removed. ------------------------------------------------ install -m 664 %{SOURCE2} ------------------------------------------------ Why should this be 0664, not 0644? * scriptlets: ... By the way, what if user sets selinux policy as "DISABLED"? There may be the case in which sysadmin has to disable selinux for some reason. In the case, selinux degree is "upgraded" to TARGETED? * For header description: ------------------------------------------------------ Source0: http://prdownloads.sourceforge.jp/selpe/23577/%{name}-%{version}-%{betatag}.tar.gz ------------------------------------------------------ Please change the URL of Source0 so that I can directly gain the source by "wget -N .........". Executing "wget -N" against this URL only pulls a HTML file. (In reply to comment #10) > * scriptlets: > ... By the way, what if user sets selinux policy as > "DISABLED"? There may be the case in which sysadmin > has to disable selinux for some reason. In the case, > selinux degree is "upgraded" to TARGETED? Oops, s|TARGETED|permissive|... Then: anyway should selinux degree be set as permissive when removing this anyway? If selinux policy is set correctly by this application (seedit), the selinux policy should work even after this application is removed, shouldn't it? (In reply to comment #10) Mamoru, thanks for comments. >Well, * BuildRequires, etc > - Firstly, I cannot check this package because mockbuild > fails on FC-devel i386. Please check the attached > log. Fixed. Added flex and byacc in buildrequires. > * File/Directory entries > - For main package, file entries include: > Please remove duplicate entries, then ensure that > every directories created during the install of > seedit related packages are owned by one package. Fixed. >* Requires >----------------------------------------------- >Requires: python >= 2.3 >----------------------------------------------- > This is not needed. Python version dependency is > automatically added by rpmbuild process. Fixed. >* %build process, etc... >----------------------------------------------- >--add-category X-Fedora >----------------------------------------------- > The category "X-???" is deprecated and should be > removed. Fixed. >------------------------------------------------ >install -m 664 %{SOURCE2} >------------------------------------------------ > Why should this be 0664, not 0644? It was only a mis-spell. Fixed. >* For header description: > Please change the URL of Source0 so that I can directly > gain the source by "wget -N .........". > Executing "wget -N" against this URL only pulls a HTML > file. Fixed. >* scriptlets: > ... By the way, what if user sets selinux policy as > "DISABLED"? There may be the case in which sysadmin > has to disable selinux for some reason. In the case, > selinux degree is "upgraded" to TARGETED? Hmm, I will reply in next comment. SELinux Policy Editor generates its own policy under /etc/selinux/seedit.
It can not be editted without seedit, seedit-policy packages.
And when user uses seedit for the first time,
/etc/selinux/seedit is overwritten.
SELINUXTYPE=targeted
->
SELINUXTYPE=seedit
So, when seedit is uninstalled,
/etc/selinux/seedit becomes unuseful, because it can not be editted.
So /etc/selinux/config should be editted.
SELINUXTYPE=seedit
->
SELINUXTYPE=targeted
>* scriptlets:
> ... By the way, what if user sets selinux policy as
> "DISABLED"? There may be the case in which sysadmin
> has to disable selinux for some reason. In the case,
> selinux degree is "upgraded" to TARGETED?
I've understood what you say, and I agree.
In the latest seedit.spec,
it will not touch "SELINUX=" line.
Current spec file and src.rpm file is following: * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.10.beta6.2.src.rpm Well, I * install 0.10 -gui package * changed selinux : "disabled" -> "permissive" * reboot * try "seedit-gui" Then... ------------------------------------------------- [tasaka1@localhost ~]$ seedit-gui sh: /usr/share/seedit/scripts/seedit-installhelper.sh: そのようなファイルやディ レクトリはありません ------------------------------------------------- (for non-Japanese users: "そのようなファイル..." means "no such file or directory") What is happening? OOps! It is due to resent changes about paths. I have forgotten test seedit itself.. I will fix it tomorrow, (In reply to comment #16) Following is latest version: I installed rpms and they worked.. * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.11.beta6.3.src.rpm Created attachment 146400 [details]
The output log on the execution of seedit-gui
Well, now:
* On the first time I executed seedit-gui, I was
asked to reboot the system, so did I.
* Then on the second time I executed, I got the
error attached.
* On the third time, it seems that seedit-gui works
well.
Is this the expected behavior?
For packaging issue, I will check later.
(In reply to comment #18) It is unexpected behavior, thanks for report. It is a bug when upgrading package, and I have fixed now. And please input /usr/sbin/seedit-rbac off -n before upgrade.(it is needed only this time, will not needed next time.) Fixed version.. * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.12.beta6.4.src.rpm >And please input
>/usr/sbin/seedit-rbac off -n
>before upgrade.(it is needed only this time, will not needed next time.)
I think you should past this in your updated spec file for updated release.
(In reply to comment #20) > >And please input > >/usr/sbin/seedit-rbac off -n > >before upgrade.(it is needed only this time, will not needed next time.) > I think you should past this in your updated spec file for updated release. I see, it is more kind. I fixed seedit itself. "/usr/sbin/seedit-rbac off -n" is no longer necessary. Latest version.. * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.13.beta6.5.src.rpm Well, for packaing issue: * Requires: - gnome-python2 (required by -gui) Please check if this is really required. By checking with "grep import" no module seems to be installed from gnome-python2. * File ownership issue/scriptlet - - policy package: ------------------------------------------------------- if [ $1 = 2 ]; then #Mark to initialize RBAC config when upgrade touch /usr/share/seedit/sepolicy/need-rbac-init fi ------------------------------------------------------- However: ------------------------------------------------------- [root@localhost ~]# touch /usr/share/seedit/sepolicy/need-rbac-init [root@localhost ~]# LANG=C rpm -qf /usr/share/seedit/sepolicy/need-rbac-init file /usr/share/seedit/sepolicy/need-rbac-init is not owned by any package ------------------------------------------------------- This file (and perhaps also /usr/share/seedit/sepolicy/need-init) should be marked as %ghost file And please check if any other file which should be marked as such exists so that all _unnessary_ files are correctly removed on the complete removal of seedit. * Version dependency requirement ------------------------------------------------------- Requires: seedit >= 2.1.0 ------------------------------------------------------- - Usually these types of requirement should be version-release dependent, i.e. ------------------------------------------------------- Requires: %{name} = %{version}-%{release} ------------------------------------------------------- * Desktop file: ------------------------------------------------------- Categories=Application;SystemSetup;X-Red-Hat-Base; ------------------------------------------------------- Both categories: "Application" "X-Red-Hat-Base" "SystemSetup" are now deprecated and these should be removed. From desktop-file-validate: ------------------------------------------------------- Categories values must be one of "AudioVideo", "Audio", "Video", "Development", "Education", "Game", "Graphics", "Network", "Office", "Settings", "System", "Utility", "Building", "Debugger", "IDE", "GUIDesigner", "Profiling", "RevisionControl", "Translation", "Calendar", "ContactManagement", "Database", "Dictionary", "Chart", "Email", "Finance", "FlowChart", "PDA", "ProjectManagement", "Presentation", "Spreadsheet", "WordProcessor", "2DGraphics", "VectorGraphics", "RasterGraphics", "3DGraphics", "Scanning", "OCR", "Photography", "Viewer", "DesktopSettings", "HardwareSettings", "PackageManager", "Dialup", "InstantMessaging", "IRCClient", "FileTransfer", "HamRadio", "News", "P2P", "RemoteAccess", "Telephony", "WebBrowser", "WebDevelopment", "Midi", "Mixer", "Sequencer", "Tuner", "TV", "AudioVideoEditing", "Player", "Recorder", "DiscBurning", "ActionGame", "AdventureGame", "ArcadeGame", "BoardGame", "BlocksGame", "CardGame", "KidsGame", "LogicGame", "RolePlaying", "Simulation", "SportsGame", "StrategyGame", "Art", "Construction", "Music", "Languages", "Science", "Astronomy", "Biology", "Chemistry", "Geology", "Math", "MedicalSoftware", "Physics", "Amusement", "Archiving", "Electronics", "Emulator", "Engineering", "FileManager", "TerminalEmulator", "Filesystem", "Monitor", "Security", "Accessibility", "Calculator", "Clock", "TextEditor", "Core", "KDE", "GNOME", "GTK", "Qt", "Motif", "Java", "ConsoleOnly", "Screensaver", "TrayIcon", "Applet", "Shell" ------------------------------------------------------- * Timestamps - These packages include many text files, image files and keeping timestamps on these files are generally preferred. Please fix so that the timestamps on these files are kept. ------------------------------------------------------- install -m 0644 %{SOURCE2} ${RPM_BUILD_ROOT}%{_datadir}/pixmaps/seedit-gui.png ------------------------------------------------------- Also, please use "install -p". * Macros ------------------------------------------------------- %define selinuxconf /etc/selinux/config %define auditrules /etc/audit/audit.rules ------------------------------------------------------- Please check if the directory /etc should be written as hardcoded or as %{_sysconfdir}. * Pam requirement %{_sysconfdir}/pam.d/seedit-gui includes: ------------------------------------------------------- auth include config-util ------------------------------------------------------- This sentence requires pam >= 0.80 so I think adding "Requires: pam >= 0.80" is preferable. * $RPM_BUILD_ROOT vs %{buildroot} Please use one, not both. And.. config-util (/etc/pam.d/config-util) is perhaps Fedora/Redhat specific and requires pam >= 0.80-9. Thank you for review! >* Requires: > - gnome-python2 (required by -gui) > Please check if this is really required. By checking with > "grep import" no module seems to be installed from > gnome-python2. Fixed, I am not using it now. >* File ownership issue/scriptlet > And please check if any other file which should be marked > as such exists so that all _unnessary_ files are correctly > removed on the complete removal of seedit. Fixed, I listed rbac-init, need-init in %ghost. >* Version dependency requirement >------------------------------------------------------- >Requires: seedit >= 2.1.0 >------------------------------------------------------- > - Usually these types of requirement should be version->release > dependent, i.e. >------------------------------------------------------- >Requires: %{name} = %{version}-%{release} >------------------------------------------------------- Fixed. >Requires: %{name} = %{version}-%{release} did not work("=" does not work), so I used Requires: %{name} >= %{version}-%{release} >* Desktop file: >------------------------------------------------------- >Categories=Application;SystemSetup;X-Red-Hat-Base; >------------------------------------------------------- > Both categories: "Application" "X-Red-Hat-Base" "SystemSetup" > are now deprecated and these should be removed. Fixed. >* Timestamps > - These packages include many text files, image files > and keeping timestamps on these files are generally > preferred. Please fix so that the timestamps on these > files are kept. > >------------------------------------------------------- >install -m 0644 %{SOURCE2} ${RPM_BUILD_ROOT}%{_datadir}/pixmaps/seedit-gui.png >------------------------------------------------------- > Also, please use "install -p". Fixed both spec file and Makefiles. >* Macros >------------------------------------------------------- >%define selinuxconf /etc/selinux/config >%define auditrules /etc/audit/audit.rules >------------------------------------------------------- > Please check if the directory /etc should be written > as hardcoded or as %{_sysconfdir}. Fixed. >* Pam requirement > %{_sysconfdir}/pam.d/seedit-gui includes: >------------------------------------------------------- >auth include config-util >------------------------------------------------------- > This sentence requires pam >= 0.80 so I think > adding "Requires: pam >= 0.80" is preferable. Yes, in old pam, it does not work. Fixed. >* $RPM_BUILD_ROOT vs %{buildroot} > Please use one, not both. Fixed. Using only %{buildroot} Updated version: * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.14.beta6.6.src.rpm For -0.14: (In reply to comment #24) > >------------------------------------------------------- > > - Usually these types of requirement should be version->release > > dependent, i.e. > >------------------------------------------------------- > >Requires: %{name} = %{version}-%{release} > >------------------------------------------------------- > Fixed. > >Requires: %{name} = %{version}-%{release} > did not work("=" does not work), > so I used > Requires: %{name} >= %{version}-%{release} What do you mean? Why equality is not valid for this package? Theoretically, inequality admits the different version between main package and its subpackages. > Fixed. Okay. Now I can correctly see seedit-gui entry. > > >* Timestamps > > - These packages include many text files, image files > > and keeping timestamps on these files are generally > > preferred. Please fix so that the timestamps on these > > files are kept. > Fixed both spec file and Makefiles. Still some files are installed by "install -m 755" or "cp -r", not "install -p -m 755" or "cp -pr" > >------------------------------------------------------- > >auth include config-util > >------------------------------------------------------- > > This sentence requires pam >= 0.80 so I think > > adding "Requires: pam >= 0.80" is preferable. > Yes, in old pam, it does not work. > Fixed. For this, I think "pam >= 0.80-9" is preferable as said in comment 23, because config-util is introduced on 0.80-9. Nevertheless, the left issues are rather small. So: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review request and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/Extras/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora Extras package review requests which are waiting for someone to review can be checked on: https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-NEW&hide_resolved=1 Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------ > > This sentence requires pam >= 0.80 so I think
> > adding "Requires: pam >= 0.80" is preferable.
> Yes, in old pam, it does not work.
> Fixed.
>For this, I think "pam >= 0.80-9" is preferable as said
>in comment 23, because config-util is introduced on 0.80-9.
I think that PAM is not really obligatory as requires, considering it is the
default authentication of fedora system, it's already installed from first
installation of fedora Core.
On the other hand, it would surely be necessary to add "usermode" in fields
requires.
(In reply to comment #26) > I think that PAM is not really obligatory as requires, What I mention is this requires pam ">= 0.80-9", not only pam, because "include config-util" sentence cannot be interpretted under pam 0.80-8. (In reply to comment #25) > > >Requires: %{name} = %{version}-%{release} > > did not work("=" does not work), > > so I used > > Requires: %{name} >= %{version}-%{release} > What do you mean? Why equality is not valid for this package? > Theoretically, inequality admits the different version between > main package and its subpackages. Fixed. It works now. It seems that I misspelled something :-) > Still some files are installed by "install -m 755" or "cp -r", > not "install -p -m 755" or "cp -pr" OOps, I've greped build log, and removed cp -r, install -m. > For this, I think "pam >= 0.80-9" is preferable as said > in comment 23, because config-util is introduced on 0.80-9. Fixed, About usermode, I've already have it in Requres field. (In reply to comment #25) Updated version: * SPEC http://prdownloads.sourceforge.jp/selpe/23577/seedit.spec * SRPM http://prdownloads.sourceforge.jp/selpe/23577/seedit-2.1.0-0.15.beta6.7.src.rpm > ------------------------------------------------------------- > NOTE: Before being sponsored: <snip> > Usually there are two ways to show this. > A. submit other review requests with enough quality. > B. Do a "pre-review" of other person's review request > (at the time you are not sponsored, you cannot do > a formal review) I see. I have no more package to submit, so I will take "B". It might take time, but I will do some pre-review. Please wait and see my pre-review. I have read guidelines again and did two pre-review today.. Bug 225119 and Bug 222338 . Well, * This package (seedit) is good. * md5sum of source coincides. * Your pre-review looks good to a certain content. - Note: My recognition is that setting the id of vendor as "fedora" is still recommended if upstream does not set vendor id. http://fedoraproject.org/wiki/Packaging/Guidelines Okay!! ------------------------------------------------------------ This package (seedit) is APPROVED by me. ------------------------------------------------------------ Please go forward according to http://fedoraproject.org/wiki/Extras/Contributors I will sponsor you when you do a procedure and I receive a mail which tells that you need a sponsor. I am now sponsored, thank you very much! I will import seedit this midnight. I've imported seedit now, and built successfully. Mamoru, I really appreciate reviewing and sponsoring me. To all, thanks to very useful comments ! I will close this bug. Yuichi, Well I saw you upgraded seedit to 2.1.0, however current seedit has ---------------------------------------------- ynakam AT hitachisoft.jp: seedit FE6 > FE7 (0:2.1.0-3.fc6 > 0:2.1.0-2.fc7) ---------------------------------------------- So please bump the release of FE7 seedit to -3 for now. (Note: this mail should be received if you are subscribing to fedora-maintainers mailing list). Note: when you failed to send a queue by some mistakes (e.g. just forgot to add a patch, making a tag failed with some reason..) and you just want to bump release, you can set the release as 2%{?dist}.1, for example. The details are: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#DistBump Thanks, fixed it now. |