This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 225075 - Review Request: ntfs-config - A front-end to Enable/Disable write support
Review Request: ntfs-config - A front-end to Enable/Disable write support
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
6
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-28 11:47 EST by Xavier Lamien
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-02 13:00:25 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
lxtnow: fedora‑review+
lxtnow: fedora‑cvs+


Attachments (Terms of Use)
Mock build log of ntfs-config-0.5.2-1.fc7 (31.91 KB, text/plain)
2007-02-09 09:47 EST, Mamoru TASAKA
no flags Details

  None (edit)
Description Xavier Lamien 2007-01-28 11:47:58 EST
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 09:47:21 EST
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 10:18:27 EST
yeah, 
my bad, forgot to upload fixed files.
it'll done to night
Comment 3 Xavier Lamien 2007-02-10 08:40:35 EST
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 08:47:12 EST
s/updated/update
Comment 5 Mamoru TASAKA 2007-02-10 11:05:50 EST
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 11:09:35 EST
s|aracarte|alacarte|
Comment 8 Mamoru TASAKA 2007-02-13 07:53:49 EST
I hope I can check this by tomorrow... may take a bit long..
Comment 9 Mamoru TASAKA 2007-02-14 03:52:55 EST
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 17:17:30 EST
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 00:45:15 EST
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 06:27:34 EST
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 09:55:37 EST
???

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 14:21:19 EST
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 08:45:02 EST
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 10:18:58 EST
> * 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 10:30:50 EST
(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 13:11:23 EST
Removing NEEDSPONSOR. I am now sponsoring.
Comment 22 Mamoru TASAKA 2007-03-01 12:33:48 EST
... Why did you reopen this?
Comment 23 Xavier Lamien 2007-03-02 13:00:25 EST
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.