| Message ID | 20130212163938.GA12339@twin.jikos.cz (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi, On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote: > +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck' > +btrfsck: btrfs > + @echo " [CP] $@" > + $(Q)cp btrfs btrfsck > + I think the idea was that btrfsck becomes a link (either symbolic or hardlink works) to btrfs... Maybe just replace cp with ln? Cheers, Filipe -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/12/2013 06:37 PM, Filipe Brandenburger wrote: > Hi, > > On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote: >> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck' >> +btrfsck: btrfs >> + @echo " [CP] $@" >> + $(Q)cp btrfs btrfsck >> + > > I think the idea was that btrfsck becomes a link (either symbolic or > hardlink works) to btrfs... > > Maybe just replace cp with ln? I agree with Filipe, or even a script is reasonable. So we have only one binary to update, and we avoid the risk to have a version mismatch between btrfsck and btrfs. This could lead to a different behaviour when the user call btrfsck instead btrfs. Finally this could save some bytes of space. Anyway my opinion would be to left this kind to decision to the distribution. We (as upstream) should only remove the old btrfsck and issue an WARNING/REMARK in the release note to notify this change. Unfortunately btrfsck is old; now we must provide an alternative file to overwrite this binary in order to avoid the mismatch above when the user is used to recompile the binary from the source. BR Goffredo
On Tue, Feb 12, 2013 at 07:01:33PM +0100, Goffredo Baroncelli wrote: > On 02/12/2013 06:37 PM, Filipe Brandenburger wrote: > > Hi, > > > > On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote: > >> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck' > >> +btrfsck: btrfs > >> + @echo " [CP] $@" > >> + $(Q)cp btrfs btrfsck > >> + > > > > I think the idea was that btrfsck becomes a link (either symbolic or > > hardlink works) to btrfs... > > > > Maybe just replace cp with ln? > > I agree with Filipe, or even a script is reasonable. So we have only one > binary to update, and we avoid the risk to have a version mismatch > between btrfsck and btrfs. This could lead to a different behaviour > when the user call btrfsck instead btrfs. Finally this could save some > bytes of space. Ok, I'll replace it with a hardlink. A symlink is not reliable (cannot be copied without breaking the path). > Anyway my opinion would be to left this kind to decision to the > distribution. We (as upstream) should only remove the old btrfsck and > issue an WARNING/REMARK in the release note to notify this change. > Unfortunately btrfsck is old; now we must provide an alternative file to > overwrite this binary in order to avoid the mismatch above when the user > is used to recompile the binary from the source. A warning is a good idea and it will start a deprecation period of btrfsck as separate utility. Unil some point in future, I'd rather stay conservative and let it exist. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/12/2013 11:52 PM, David Sterba wrote: > On Tue, Feb 12, 2013 at 07:01:33PM +0100, Goffredo Baroncelli wrote: >> On 02/12/2013 06:37 PM, Filipe Brandenburger wrote: >>> Hi, >>> >>> On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote: >>>> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck' >>>> +btrfsck: btrfs >>>> + @echo " [CP] $@" >>>> + $(Q)cp btrfs btrfsck >>>> + >>> >>> I think the idea was that btrfsck becomes a link (either symbolic or >>> hardlink works) to btrfs... >>> >>> Maybe just replace cp with ln? >> >> I agree with Filipe, or even a script is reasonable. So we have only one >> binary to update, and we avoid the risk to have a version mismatch >> between btrfsck and btrfs. This could lead to a different behaviour >> when the user call btrfsck instead btrfs. Finally this could save some >> bytes of space. > > Ok, I'll replace it with a hardlink. A symlink is not reliable (cannot > be copied without breaking the path). ...mmm... the install command (invoked by the Makefile during the installation phase) doesn't seem to preserve both the hard-link and the soft-link: $ touch test $ ln test test2 $ ln -sf test lntest $ mkdir t3 $ install test2 t3/ $ install lntest t3/ $ ls -li lntest test test2 t3/test2 t3/lntest 3005857 lrwxrwxrwx 1 ghigo ghigo 4 Feb 13 00:03 lntest -> test 3005858 -rwxr-xr-x 1 ghigo ghigo 0 Feb 13 00:03 t3/lntest 3005854 -rwxr-xr-x 1 ghigo ghigo 0 Feb 13 00:00 t3/test2 3005852 -rw-r--r-- 2 ghigo ghigo 0 Feb 13 00:00 test 3005852 -rw-r--r-- 2 ghigo ghigo 0 Feb 13 00:00 test2 I think that a bash script is a better choice. >> Anyway my opinion would be to left this kind to decision to the >> distribution. We (as upstream) should only remove the old btrfsck and >> issue an WARNING/REMARK in the release note to notify this change. >> Unfortunately btrfsck is old; now we must provide an alternative file to >> overwrite this binary in order to avoid the mismatch above when the user >> is used to recompile the binary from the source. > > A warning is a good idea and it will start a deprecation period of > btrfsck as separate utility. Unil some point in future, I'd rather stay > conservative and let it exist. > > david >
--- a/Makefile +++ b/Makefile @@ -42,7 +42,7 @@ endif MAKEOPTS = --no-print-directory Q=$(Q) -progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol \ +progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol btrfsck \ btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \ btrfs-find-root btrfstune btrfs-show-super @@ -125,6 +125,11 @@ btrfs-show: $(objects) $(libs) btrfs-show.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-show btrfs-show.o $(objects) $(LDFLAGS) $(LIBS) +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck' +btrfsck: btrfs + @echo " [CP] $@" + $(Q)cp btrfs btrfsck + mkfs.btrfs: $(objects) $(libs) mkfs.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) -lblkid @@ -186,7 +191,7 @@ install-man: clean : @echo "Cleaning" $(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image btrfs-select-super \ - btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static \ + btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static btrfsck \ version.h \ $(libs) libbtrfs.so libbtrfs.so.0 libbtrfs.so.0.1 $(Q)$(MAKE) $(MAKEOPTS) -C man $@