Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VFS URI reimplementation #2361

Closed
mc-butler opened this issue Sep 23, 2010 · 40 comments
Closed

VFS URI reimplementation #2361

mc-butler opened this issue Sep 23, 2010 · 40 comments
Assignees
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/2361
Reporter andrew_b (@aborodin)
Mentions zaytsev (@zyv)
Keywords ipv6, URL

VFS used in mc has some leaks in design. For instance, the VFS URI contains a VFS name: testcase.tar#utar. In such case VFS name looks like a part of file name. Such desision a) limits a usage of # and @ symbols in filenames and passwords and b) is a root of many problems in URI parsing (see tickets in Blocking: field).

The new VFS URI is proposed: create a special path element /#vfs:vfsname/ that contains only vfs name with #vfs: prefix (like path encoding /#enc:encodingname/). This scheme simplifies the VFS URI parsing and removes the limitation of @ and # symbols usage in file names and passwords.

Examples:

/some/path/#vfs:patchfs/foo.diff
/#vfs:ftp/user:password@host/path/file

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 23, 2010 at 9:44 UTC (comment 1)

  • Cc set to zaytsev

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 19, 2010 at 12:03 UTC

Replying to andrew_b (#2361):

Examples:

/some/path/#vfs:patchfs/foo.diff
/#vfs:ftp/user:password@host/path/file

Well, this is not good solution due to an extra token in path. That extra token makes the path parsing more complicated.

The better solutions are
/some/path/#vfs:patchfs:foo.diff
or
/some/path/#vfs:patchfs@foo.diff
or
/some/path/#vfs:patchfs$foo.diff
In any case, some defined symbol is used to split vfs name and file name. Vfs prefix, vfs name and file name are joined into sungle token.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 19, 2010 at 12:04 UTC

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Nov 7, 2010 at 12:15 UTC (comment 4)

oioi, deep waters ahead ... :D

first of all, let's get the terminology clear. mc's vfs paths are *not* URIs in an RFC sense - and it's good that way, as it's optimized for local file names which are by far the most common use case.
however, this intentionally creates an incompatibility with modern desktop environments which usually don't have much interest in user-editable complex paths, but like using (real) URIs for everything. or something completely bizarre like gio does. this topic needs to be re-visited at some point, but i think automatic transformation between mc's user-friendly format and something desktop-compatible would be feasible without much effort.

i don't think we actually need the "vfs:" prefix - it just adds verbosity. i think /# followed by the vfs name (and a colon if parameters follow) is a sufficiently distinct magic. for the rare case where this would cause a false hit, a /#literal:<original> pseudo-vfs could be introduced.

to determine appropriate quoting/escaping rules, try constructing a path which represents a file inside a bzip-compressed tar inside an encrypted rar which resides in a subdirectory of an ftp-server which needs authentication. afterwards you try something really weird. :D

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Nov 7, 2010 at 12:57 UTC (comment 4.5)

Replying to ossi:

first of all, let's get the terminology clear. mc's vfs paths are *not* URIs in an RFC sense

Yes, if you mean RFC2396. But it is Uniform Resource Identifier within MC VFS.

however, this intentionally creates an incompatibility with modern desktop environments

avfs created that incompatibility for years ago and I don't care about that now.

i don't think we actually need the "vfs:" prefix - it just adds verbosity. i think /# followed by the vfs name (and a colon if parameters follow) is a sufficiently distinct magic. for the rare case where this would cause a false hit, a /#literal:<original> pseudo-vfs could be introduced.

#2230? #a, #rpm can be a real file name. #rpms can be a real directoriy name. I want to decrease potential false hits and some verbosity like usage of #vfs: prefix will help us to do that.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Nov 7, 2010 at 16:39 UTC (comment 5.6)

Replying to andrew_b:

#2230?

the first problem is that the #a# is parsed as a vfs entry even though it clearly does not match the regexp /#(vfs1|vfs2|...)(:|/|$) (as implied by my first comment).
second, the real bug in mc is that it does not escape the path it produces (which may be impossible given the current situation - that's why we have this ticket, after all).
and lastly, this is an utter corner case. let's optimize for the legibility of the more common case.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Nov 7, 2010 at 16:50 UTC (comment 7)

fwiw, we should probably use a less verbose escape - simply doubling the hash would already work: /#a/ would become /##a/. whether the the escape should be applied to uris which don't strictly need it (because they don't match the above regexp, like the /#a# example) is a matter of taste, but _must_ be determined upfront - i think i'd enforce it for uniformity (and it makes the parser simpler, because it doesn't have to do look-ahead).

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 1, 2011 at 13:18 UTC (comment 8)

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 1, 2011 at 13:22 UTC (comment 9)

  • Milestone changed from 4.7 to 4.8.0-pre1
  • Status changed from new to accepted
  • Severity changed from no branch to on review
  • Owner set to andrew_b

Created 2361_vfs_uri branch. Parent branch is master.
Initial [c7105e59899019bb823bc7d571e07f097585f29c]

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jan 5, 2011 at 10:47 UTC (comment 10)

Create test environment:

$ tar cf arch.tar /etc/hosts
$ mkdir -p '#vfs:utar:arch.tar/test'

Testing in mc:
1) go to '#vfs:utar:arch.tar' directory and then go up again. Cursor will moved to arch.tar file
2) try to copy/rename/move/delete '#vfs:utar:arch.tar' directory. All these operations will applied to file 'arch.tar', not to directory

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 5, 2011 at 14:09 UTC (comment 10.11)

Replying to slavazanko:

All these operations will applied to file 'arch.tar', not to directory

Fixed.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Jan 5, 2011 at 17:16 UTC (comment 12)

why did you completely disregard my comments about proper escaping and optimizing the common case?
i actually use cd /#sh:somehost rather often, and i certainly won't like having to type some silly and completely unnecessary vfs: prefix now.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 5, 2011 at 17:55 UTC (comment 13)

You can use sh://somehost.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Jan 5, 2011 at 19:13 UTC (comment 14)

as far as i'm concerned, that doesn't exactly emphasize the U in URI.

also, the vfs: syntax is simply a gratuitous incompatible change of long-standing behavior. i've yet to see a compelling justification for it.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jan 19, 2011 at 8:51 UTC (comment 15)

  • Votes set to slavazanko

ossi, can you help with writing new URI VFS parser as you like?

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Jan 19, 2011 at 22:57 UTC (comment 16)

you mean, like, more than reviewing? maybe when i find a long enough free timeslice while my brain is not toasted from work yet. ;)

reviewing the existing branch is quite a nightmare due to a lot of mixed in coding style cleanups ... you should take some inspiration from http://qt.gitorious.org/qt/pages/CommitPolicy point 8.

anyway, on the theory of the necessary changes:

if you want to start from the existing branch, first you need to get rid of the vfx prefix changes, leaving only the refactoring in. i'd probably throw away the commits and extract interesting parts from the cumulative diff with git gui.

you need a single structure to represent a parsed URI, as a list of segments - plain path segments and vfs root segments. then you need a function which parses URI strings (using callbacks to registered vfs handlers) and unescapes literal hashmarks, and a function which assembles the parts back to an URI (again using callbacks), including escaping hashmarks. a single entry point per direction ensures consistent handling.

i wonder what to do about the file#utar syntax. i'd personally get rid of it and require file/#utar (which, btw, kinda works until you try to leave the archive), but this cannot be done without a deprecation phase of the old syntax. but this is non-critical anyway - when hashmarks are properly escaped, a single hashmark could actually still be used as hierarchy delimiter, only that it looks weird (imagine #ftp:foo:b##r@host/yo/my#utar/thefile#urar:passwd/holla/die/wald##fee.gz#gz vs. /#ftp:foo:b##r@host/yo/my/#utar/thefile/#urar:passwd/holla/die/wald##fee.gz/#gz - i find the latter way more readable).

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 4, 2011 at 8:22 UTC

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 28, 2011 at 11:16 UTC (comment 18)

  • Severity changed from on review to on rework

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 19, 2011 at 12:09 UTC (comment 19)

  • Owner changed from andrew_b to slavazanko
  • Votes slavazanko deleted
  • Severity changed from on rework to on review
  • Priority changed from major to critical

Branch [2361_vfs_uri] have been totally rewritten.

New initial [a89c7d252dd34af26473459e5ccf8d0bd2870821]

Review, please.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 19, 2011 at 12:18 UTC (comment 20)

Note: VFS paths now handled as

vfsprefix1://vfsdata/vfsprefix2://vfsdata

Also, 'bindings' user file was renamed to 'mc.ext', so you need search in this file all

 Open=file.ext#vfsprefix

and replace them to

 Open=file.ext/vfsprefix://

After this you should rename your 'bindings' file to 'mc.ext'

Old-style paths are handled too, but you couldn't mix URL-like and old style path elements in one path string.

Support of old-style paths will be removed in next major release (probably in 4.9, who knows...)

Be aware.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Jun 20, 2011 at 7:19 UTC (comment 21)

i'll do a proper review in the next days.

why did you introduce the new syntax? a) it's noisier (see my previous arguments) b) it pretends to be something which it isn't ("proper" URLs) c) it is an apparently gratuitous change (i didn't see the commit message explain an actual need to do it)

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 22, 2011 at 11:15 UTC (comment 22)

  • Votes set to andrew_b

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 22, 2011 at 11:19 UTC (comment 23)

why did you introduce the new syntax?

The new syntax more reliable to parse path string and easily for reading by humans :)

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Jun 22, 2011 at 15:09 UTC (comment 24)

care to back up that claim with some examples and counter-examples?

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Jun 22, 2011 at 21:42 UTC (comment 25)

  • Votes changed from andrew_b to andrew_b angel_il
  • Severity changed from on review to approved

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 23, 2011 at 11:50 UTC (comment 26)

  • Severity changed from approved to merged
  • Blocking #33, #81, #1605, #1687, #2016, #2073, #2220, #2251, #2360 deleted
  • Votes changed from andrew_b angel_il to commited-master
  • Resolution set to fixed
  • Status changed from accepted to testing

Merge [f2ebbd2]

For getting list of commits in branch type:

git log --pretty=oneline fcfa76b..ef676d3

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 23, 2011 at 11:59 UTC (comment 27)

  • Status changed from testing to closed

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Jun 26, 2011 at 9:48 UTC (comment 28)

$ cd
$ mkdir '#utar'
enter that directory
leave that directory. you are in /home now.

$ cd
$ mkdir 'foo#utar'
enter that directory
make directory "bar". you get "function not implemented".

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 27, 2011 at 7:30 UTC (comment 29)

well, do we need to drop old-style path parser?

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 27, 2011 at 7:41 UTC (comment 29.30)

Replying to slavazanko:

well, do we need to drop old-style path parser?

I think, yes.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 27, 2011 at 20:33 UTC (comment 31)

I think, need to leave old-style pathes just in one place: in "Directory hotlist".
But new Directory hotlist entries should be saved in URL-like format.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 29, 2011 at 7:38 UTC (comment 32)

  • Severity changed from merged to on rework
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Priority changed from critical to major

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 29, 2011 at 7:38 UTC (comment 33)

  • Status changed from reopened to accepted
  • Votes commited-master deleted

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 29, 2011 at 9:00 UTC (comment 34)

Support of relative symlinks was broken in new vfs.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 29, 2011 at 13:09 UTC (comment 35)

Created branch 2361_url_path

Initial [e6c25891e7b5cd8a0b8cae0d566e5c1c68a52f7a]

Review, please.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 29, 2011 at 13:10 UTC (comment 36)

  • Severity changed from on rework to on review

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 30, 2011 at 10:43 UTC (comment 37)

  • Votes set to andrew_b

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Jul 4, 2011 at 14:31 UTC (comment 38)

  • Severity changed from on review to approved
  • Votes changed from andrew_b to andrew_b angel_il

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jul 5, 2011 at 13:22 UTC (comment 39)

  • Severity changed from approved to merged
  • Votes changed from andrew_b angel_il to commited-master
  • Status changed from accepted to testing
  • Resolution set to fixed
  • Keywords set to ipv6, URL

Merge [906f688]

For getting list of commits type:

git log --pretty=oneline 92d2119..0f249bd

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jul 5, 2011 at 13:22 UTC (comment 40)

  • Status changed from testing to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress
Development

No branches or pull requests

2 participants