Recursive & subpath pinning: update and add test #4876
Conversation
|
### opam pin
pin1.~dev (uninstalled) git git+file://${BASEDIR}/pinnes#master
pin1sub1.~dev (uninstalled) rsync file://${BASEDIR}/pinnes (a)
-pin1sub1sub1.~dev (uninstalled) git git+file://${BASEDIR}/pinnes#master (a/i)
-pin1sub1sub1sub1.~dev (uninstalled) git git+file://${BASEDIR}/pinnes#master (a/i/j)
-pin1sub1sub2.~dev (uninstalled) rsync file://${BASEDIR}/pinnes (a/k)
-pin1sub2.~dev (uninstalled) git git+file://${BASEDIR}/pinnes#master (b)
+pin1sub1sub1.~dev (uninstalled) rsync file://${BASEDIR}/pinnes (a\i)
+pin1sub1sub1sub1.~dev (uninstalled) rsync file://${BASEDIR}/pinnes (a\i\j)
+pin1sub1sub2.~dev (uninstalled) rsync file://${BASEDIR}/pinnes (a\k)
+pin1sub2.~dev (uninstalled) rsync file://${BASEDIR}/pinnes (b)
### opam unpin -n --recursive pinnes |
rjbou
added
PR: QUEUED
src/core/opamFilename.ml
Outdated
| module SubPath = struct | ||
|
|
||
| include OpamStd.AbstractString | ||
|
|
||
| let compare = String.compare | ||
| let equal = String.equal | ||
|
|
||
| let of_string s = | ||
| OpamSystem.back_to_forward s | ||
| |> OpamStd.String.remove_prefix ~prefix:"./" | ||
| |> of_string | ||
| let to_string = OpamSystem.forward_to_back | ||
| let to_pretty_string s = "("^s^")" | ||
| let to_normalised_string s = s | ||
|
|
||
| let (/) d s = d / to_string s | ||
| let (/?) d = function | ||
| | None -> d | ||
| | Some s -> d / to_string s | ||
|
|
||
| end |
There was a problem hiding this comment.
Added this module to help concatenation and path rewriting.
src/core/opamFilename.ml
Outdated
| |> OpamStd.String.remove_prefix ~prefix:"./" | ||
| |> of_string | ||
| let to_string = OpamSystem.forward_to_back | ||
| let to_pretty_string s = "("^s^")" |
There was a problem hiding this comment.
do we want that pretty string ? or [x/a] ? or other thing ?
src/core/opamFilename.mli
Outdated
| val (/): Dir.t -> t -> Dir.t | ||
| val (/?): Dir.t -> t option -> Dir.t |
There was a problem hiding this comment.
new operators for subpaths, to help using then as in most cases it is an option given to a function
| | url, Result None -> Done (Result (OpamUrl.to_string url)) | ||
| | url, Result None -> | ||
| let url = | ||
| Printf.sprintf "%s%s" |
There was a problem hiding this comment.
It might be wise to document this new behaviour (the string returned might not be the url anymore). Currently the return value isn’t documented in the .mli file
There was a problem hiding this comment.
In fact, I’m not sure where this value is used. It might not be used at all and it might be more interesting to simply return unit
There was a problem hiding this comment.
Ah I see
-> retrieved pin1sub1sub2.dev (file://${BASEDIR}/pinnes(a/k))
mmh, I’m not sure about the implementation of OpamFilename.SubPath.to_pretty_string, it’s a bit confusing. For instance, how can one see the difference between dir-with-parethesis and dir ?(subdir) ?
What about:
-> retrieved pin1sub1sub2.dev (file://${BASEDIR}/pinnes [subpath: a/k])
or something like that?
There was a problem hiding this comment.
We don't see the difference indeed. If we can find a less verbose way to notify about subpathes, it's better, it is used in several places
There was a problem hiding this comment.
proposal:
-> retrieved pin1sub2.dev (git+file://${BASEDIR}/pinnes[b]#master)
-> retrieved pin1sub1sub2.dev (file://${BASEDIR}/pinnes[a/k])
It will have more effect or url, as we need to insert the subpath in the path itself.
Same differentiation issue with [] than ().
rjbou
force-pushed
the
recpin
branch
2 times, most recently
from
f514e66 to
5ed6353
Compare
rjbou
removed
the
PR: QUEUED
Waiting # 5079 & # 5080rebased on top of them