Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225075 - Review Request: ntfs-config - A front-end to Enable/Disable write support
Summary: Review Request: ntfs-config - A front-end to Enable/Disable write support
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 6
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-28 16:47 UTC by Xavier Lamien
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-03-02 18:00:25 UTC
lxtnow: fedora-review+
lxtnow: fedora-cvs+


Attachments (Terms of Use)
Mock build log of ntfs-config-0.5.2-1.fc7 (deleted)
2007-02-09 14:47 UTC, Mamoru TASAKA
no flags Details

Description Xavier Lamien 2007-01-28 16:47:58 UTC
Spec URL: http://blog.fedora-fr.org/public/smootherfrogz/SPECS/ntfs-config.spec
SRPM URL: http://blog.fedora-fr.org/public/smootherfrogz/RPMs/ntfs-config-0.5.2-1.fc6.src.rpm
Description: 

ntfs-config will allow you to enable/disable write support
for external and/or internal device with only two click.
This will configure your system to use the new ntfs-3g driver
instead of the current read-only kernel one.

-----------------

Hi, 
here is another package from me.

Comment 1 Mamoru TASAKA 2007-02-09 14:47:21 UTC
Created attachment 147773 [details]
Mock build log of  ntfs-config-0.5.2-1.fc7

Mockbuild of ntfs-config-0.5.2-1 fails on FC-devel i386.
It seems that many needes BuildRequires are missing.

Comment 2 Xavier Lamien 2007-02-09 15:18:27 UTC
yeah, 
my bad, forgot to upload fixed files.
it'll done to night

Comment 3 Xavier Lamien 2007-02-10 13:40:35 UTC
Rebuild to updated ntfs-config to 0.5.4-1
New spec and srpm files below

Spec: http://blog.fedora-fr.org/public/smootherfrogz/SPECS/ntfs-config.spec
SRPM:
http://blog.fedora-fr.org/public/smootherfrogz/RPMs/ntfs-config-0.5.4-1.fc6.src.rpm

Comment 4 Xavier Lamien 2007-02-10 13:47:12 UTC
s/updated/update

Comment 5 Mamoru TASAKA 2007-02-10 16:05:50 UTC
Well, for 0.5.4-1:

* BuildRequires/Requires:
  - This is gtk2 application and gtk+ is not necessary.
  - Also, libglade is not needed.
  - pkgconfig for BuildRequires is redundant as gtk2-devel
    requires (and should require) pkgconfig.
  - gtk2-devel for BuildRequires is redundant. libglade2-devel
    requires gtk2-devel.

* Source vs using echo
  - Personally, I don't like to use "echo ???? >> file" because:
    - it may update timestamp of the file unnecessarily.
    - this makes the spec file larger.
    Rather I like to make a file and include it as sources.

* Desktop file
  - Category
-------------------------------------------------------------
	--add-category X-Fedora				\
-------------------------------------------------------------
    This category is deprecated and should be removed.
  - Icon
-------------------------------------------------------------
Icon=gnome-dev-harddisk
-------------------------------------------------------------
    aracarte shows that this is taken from
    /usr/share/icons/Bluecurve/48x48/devices/gnome-dev-harddisk.png,
    so adding "Requires: redhat-artwork" is preferable, IMO
    (on FC-devel. on FC-6, this may differs).

* soft linking
   - soft linking should be relative.
-------------------------------------------------------------
ln -s /%{_bindir}/consolehelper $RPM_BUILD_ROOT%{_bindir}/%{name}
-------------------------------------------------------------
     should be:
-------------------------------------------------------------
ln -s consolehelper $RPM_BUILD_ROOT%{_bindir}/%{name}
-------------------------------------------------------------

* Timestamps
  - Keep timestamps on text files, for example, .fdi files in
    /usr/share/ntfs-config/. Perhaps
-------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -c -p"
-------------------------------------------------------------
    will work (check this).

* Documentation
  - Please add the following documents.
-------------------------------------------------------------
README
TODO
-------------------------------------------------------------

Comment 6 Mamoru TASAKA 2007-02-10 16:09:35 UTC
s|aracarte|alacarte|

Comment 8 Mamoru TASAKA 2007-02-13 12:53:49 UTC
I hope I can check this by tomorrow... may take a bit long..

Comment 9 Mamoru TASAKA 2007-02-14 08:52:55 UTC
For -2:

* Timestamps
  - Well, to keep timestamps on files installed from source,
    using "cp -p" or "install -p" is needed. Then please 
    "install -p -Dm 644 ...", for example.

* disttag
  - Well, disttag for %changelog entry is not needed. rpmlint
    ignored when disttag is missing on %changelog, however if
    you explicitly write .fc6 on disttag, rpmlint complains to
    me because... I use rawhide. i.e. please write:
---------------------------------------------------
* Mon Feb 13 2007 Xavier Lamien <lxtnow@gmail.com> - 0.5.4-2
---------------------------------------------------

Comment 10 Xavier Lamien 2007-02-14 22:17:30 UTC
Files updated, just click above....same location.

(in reply to comment #9)

[...]
rpmlint complains to me because... I use rawhide.
[...]

i've not this problem with mock building for rawhide.

Comment 11 Mamoru TASAKA 2007-02-15 05:45:15 UTC
Umm... where?

Would you bump release to -3 and re-upload?
At least I viewed your spec file 
http://blog.fedora-fr.org/public/smootherfrogz/SPECS/ntfs-config.spec
and "install" does not keep timestamps (still written as
install -Dm 644, not install -p -Dm 644)

Comment 12 Xavier Lamien 2007-02-15 11:27:34 UTC
timestamps already added to "install -p -Dm 644"
clean your cache and retry please.

i'll bump it to -3 -;)

Comment 13 Mamoru TASAKA 2007-02-15 14:55:37 UTC
???

Still http://blog.fedora-fr.org/public/smootherfrogz/SPECS/ntfs-config.spec
says "install -Dm 644" and -3 srpm doesn't seem to be available
from http://blog.fedora-fr.org/public/smootherfrogz/RPMs/ ...

(Please bump release tag every time you change/fix spec/srpm.
 Changing spec/srpm without changing release number causes
 confusion on people who are watching the package)

Comment 15 Mamoru TASAKA 2007-02-15 19:21:19 UTC
I should write what I meant more politely...

Well,
-----------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p -Dm 644"
-----------------------------------------------------
  should be
-----------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
-----------------------------------------------------
  Executable bits on scripts/binaries will be removed when
  adding "-Dm 644".
  And the following lines
-----------------------------------------------------
install -Dm 644 %{SOURCE1} $RPM_BUILD_ROOT/%{_sysconfdir}/pam.d/%{name}
install -Dm 644 %{SOURCE2}
$RPM_BUILD_ROOT/%{_sysconfdir}/security/console.apps/%{name}
-----------------------------------------------------
  are not fixed yet ("install -p -Dm 644" should be used also
  on the two lines above)


Comment 17 Mamoru TASAKA 2007-02-16 13:45:02 UTC
Okay. Two minor issues.

* setup directory
--------------------------------------------
%setup -q -n %{name}-%{version}
--------------------------------------------
 is okay with
--------------------------------------------
%setup -q
--------------------------------------------
 because the default directory is %{name}-%{version}

* For Requires/BuildRequires of perl modules:
--------------------------------------------
BuildRequires:	perl-XML-Parser
--------------------------------------------
  Well, for perl modules, the preferred style is
--------------------------------------------
BuildRequires: perl(XML::Parser)
--------------------------------------------

  Anything else is okay.
-----------------------------------------------
  This package (ntfs-config) is APPROVED by me.
-----------------------------------------------

  Please fill up http://fedoraproject.org/wiki/Extras/CVSSyncNeeded
  and import this package after cvsadmin does some needed procedure.

  By the way, are you in need of sponsor? I see that you
  assigned some review requests to yourself, however
  as far as I know the person who can review the bug must be in
  fedorabugs group, and then must be in cvsadmin group...

Comment 18 Xavier Lamien 2007-02-16 15:18:58 UTC
> * setup directory
> --------------------------------------------
> %setup -q -n %{name}-%{version}
> --------------------------------------------
> is okay with
> --------------------------------------------
> %setup -q
> --------------------------------------------
> because the default directory is %{name}-%{version}

It's why i let it like that ;-)

>  Well, for perl modules, the preferred style is
> --------------------------------------------
> BuildRequires: perl(XML::Parser)
> --------------------------------------------

okay, i can change it to perl(XML::Parser) ;-)

> Please fill up http://fedoraproject.org/wiki/Extras/CVSSyncNeeded
>  and import this package after cvsadmin does some needed procedure.
>
>  By the way, are you in need of sponsor? I see that you
>  assigned some review requests to yourself, however
>  as far as I know the person who can review the bug must be in
>  fedorabugs group, and then must be in cvsadmin group...

I'm in the fedorabugs group but, not in cvsadmin group yet.
And i'm in fact still looking for sponsors...
If you can do something for me.

Comment 19 Mamoru TASAKA 2007-02-16 15:30:50 UTC
(In reply to comment #18)
> > * setup directory
> It's why i let it like that ;-)
Okay, I don't force it.

> 
> > Please fill up http://fedoraproject.org/wiki/Extras/CVSSyncNeeded
> >  and import this package after cvsadmin does some needed procedure.
> >
> >  By the way, are you in need of sponsor? 
> I'm in the fedorabugs group but, not in cvsadmin group yet.
> And i'm in fact still looking for sponsors...
> If you can do something for me.

Then I will sponsor you. Please follow the procedure of
http://fedoraproject.org/wiki/Extras/Contributors .

Comment 20 Mamoru TASAKA 2007-02-16 18:11:23 UTC
Removing NEEDSPONSOR. I am now sponsoring.

Comment 22 Mamoru TASAKA 2007-03-01 17:33:48 UTC
... Why did you reopen this?

Comment 23 Xavier Lamien 2007-03-02 18:00:25 UTC
New review isn't requires for this updated release, no major changes happened.



Note You need to log in before you can comment on or make changes to this bug.