On Fri, 16 Jul 2010 11:23:10 -0700, Eric Kow <ko...@darcs.net> wrote:
Note to Kevin Quick:
I'm CC'ing you to give you the heads up that the the --nolinks flag likely to
go away. See the second patch below. I'd like you to comment before I push
this particular patch, if you would be so kind
So the name of the game is to banish DarcsFlag from the Darcs.Patch and
Darcs.Repository layer (starting from the inside)
See http://bugs.darcs.net/issue1157
and the IRC discussion on
http://irclog.perlgeek.de/darcs/2010-07-14#i_2553121
I am really happy to see Petr start to chip away at this opts argument ::
[DarcsFlag] that we've been threading through all our functions. Future is
slightly uncertain (maybe this will get messy), but rah in principle.
In general I think this overall flags effort is a good direction to go in. I do remember
that there's a lot of spread and vagueness about DarcsFlags throughout the code, and that
nolink caused me to have to increase the "splatter" (as it was referred to
below). It also bothered me at the time I was doing this work that we were pushing
DarcsFlags explicitly AND that there were also flags implicitly saved in the passed
repository objects (beware of imprecision: these are 4 year old recollections I'm tossing
out). And on that same note I also vaguely recall that the flags in the repository
object weren't always the same as the global explicit flags (IIRC the flags attached to
the repository were a subset of the main flags). Anyhow, enough vague rambling about
ancient history...
I'm no longer using the --nolinks flag... unfortunately because another VCS was
chosen by the selection committee for the environment in question (although I
still personally use darcs extensively). As such, it's removal wouldn't
directly break things for me.
That said, I do think it still serves an important purpose. It's not common in the open-source world, but in the
corporate world where there's much more rigid and controlled access to code in the "QA" and
"Release" domains (especially ISO-9001 organizations) the issues of permissions and stand-alone completeness
overshadow issues like saving disk space. It becomes more important to "lock down" QA and Release
environments using permissions (and more) and hard-links are a back-door in that whole mechanism, and from the user
perspective, "--nolinks" seems like an easy solution.
But the basic argument in the chat above is that (A) --nolinks does
not work with hashed repositories
I hadn't known that... I would be much more alarmed if I was currently using
darcs in the environment that necessitated --nolinks. :-)
(B) old-fashioned repositories should
be treated as deprecated and features which are only used by a minority
of users for the deprecated format should be pruned away (sorry!)
The eternal stress between forward progress and backward compatibility.
There's no good answer to that and we don't want to be to cavalier about
abandoning previously released functionality too hastily. I'm trying to avoid
coming across as Mr. Corporate here, but I can say that if we were using darcs
in the originally targeted environment that even things such as abandoning old
fashioned repositories wouldn't be a trivial upgrade.
You folks are definitely DTRT: checking with known users before deprecating and
removing a feature, and in this case even I can't confess to still *requiring*
this. It does trigger concerns for me but if no one else raises a concern then
either (a) no one is using old-fashioned repos anymore or (b) no one who cares
is reading this... and both are equally likely.
I suspect that you are primarily removing --nolinks because it extended the
DarcsFlags tentacles and is therefore awkward to work with. Refactoring and
cleaning up code is one of my favorite things to do as well, but I would throw
out the caution that the nolink flag doesn't seem to be a very radical flag (in
either purpose or implementation requirements) so its very inconvenience in the
implementation might be illuminating a design that is still suboptimal. As
such, I can't argue very vociferously for its preservation because I no longer
currently require it but you may want to take advantage of its irritation
factor to create the best flags containment pearl in the refactored code.
-KQ
Remove --nolinks, since its scope and usefulness is very limited.
-----------------------------------------------------------------
Kevin: This patch is motivated by code-cleanup work to get rid of
[DarcsFlag] (opts) in the Darcs.Repository layer. Rather
than try to support NoLinks in this fashion, Petr decided
that it would make more sense to just remove it.
See http://irclog.perlgeek.de/darcs/2010-07-16#i_2560752
for discussion on --nolinks
Unfortunately, I have not yet gotten around to re-understanding
the motivation behind --nolinks (there are cases where you really
want to use a copy operation instead of a hard link, perhaps related to
permissions)
* http://lists.osuosl.org/pipermail/darcs-devel/2007-July/005891.html
* http://lists.osuosl.org/pipermail/darcs-devel/2007-July/005899.html
You might be able to make the case that --nolinks should simply be
made to work with hashed repositories. My natural inclination is,
in case of doubt, to prune and simplify but I think it would only be
fair if Kevin had a chance to weigh in first.
- copyFileOrUrl [NoLinks] (repodir </> prefsRelPath)
- prefsRelPath Uncachable `catchall` return ()
+ (fetchFilePS (repodir </> prefsRelPath) Uncachable >>= B.writeFile
prefsRelPath)
+ `catchall` return ()
Wouldn't some sort of cloneFileOrUrl be generally useful instead?
(following the convention that copy is maybe-link and clone is no-link?)
-copyLocal :: [DarcsFlag] -> String -> FilePath -> IO ()
-copyLocal opts fou out | NoLinks `elem` opts = cloneFile fou out
- | otherwise = createLink fou out `catchall` cloneFile
fou out
+copyLocal :: String -> FilePath -> IO ()
+copyLocal fou out = createLink fou out `catchall` cloneFile fou out
Just the general case
-copyLocals :: [DarcsFlag] -> String -> [String] -> FilePath -> IO ()
-copyLocals opts u ns d =
- doWithPatches (\n -> copyLocal opts (u++"/"++n) (d++"/"++n)) ns
+copyLocals :: String -> [String] -> FilePath -> IO ()
+copyLocals u ns d =
+ doWithPatches (\n -> copyLocal (u++"/"++n) (d++"/"++n)) ns
Since we no longer have the option to pass NoLinks down the chain, we remove
the extra argument.
When the darcs library becomes more stable, we'll no longer be able to be so
quick about ripping things apart like this.
Interestingly, in the 2007 threads I remarked that Kevin's work introduced a
lot of splatter (functions that had to be updated to thread opts through).
And in this patch, we only remove a very small amount of the splatter. Why?
It seems that we still need the threading of [DarcsFlag] so that we can keep
track of the new --remote-darcs option that we later introduced. Hmm!
--
-KQ
_______________________________________________
darcs-users mailing list
darcs-users@darcs.net
http://lists.osuosl.org/mailman/listinfo/darcs-users
