Add support for over-long filepaths via GNU extension and fix hardlinks

[hasufell]

Fixes #49 And #51

UTF8 filenames are still broken, but this is not in the scope of this PR. I tested this with the GHC bindist (packing and unpacking).

[hasufell]

There seems to be no interest of clc or anyone to maintain this package? I've since then forked it and will probably abandon that fork somtime soon in favor of libarchive.

[gbaz]

I'm going to reopen because I sincerly hope this package will get more active maintenance and this should be fixed.

[emilypi]

@hasufell My only critique don't think the addition of these brings anything to the table that we can't monomorphize and write directly in Types.hs. Otherwise, I'm happy to merge this when that's done and tests have been added.

[hasufell]

Updated and backported patch from hasufell@8e7b1d5

[hasufell]

Added a fix for #35

[hasufell]

Added fix for #63

[Bodigrim]

I do not particularly mind depending on These, but the change of signatures is a serious breakage for clients. Could we estimate how disruptive it would be in practice?

[hasufell]

Could we estimate how disruptive it would be in practice?

I don't know what you mean by that. You'll have to be more specific. That's what PVP is for.

[hasufell]

I don't know what you mean by that. You'll have to be more specific. That's what PVP is for.

I just realized the version is bumped to 0.6.0.0 anyway, so I don't see the point in worrying about public API breakage.

I removed These from most of the internal API, because it's cleaner. But it doesn't make any difference, since the public API changed in toTarPath and it must stay that way.

[Bodigrim]

I don't know what you mean by that. You'll have to be more specific. That's what PVP is for.

Well, this is certainly a major bump according to PVP, but that's not my point. How disruptive is this change for clients (like, cabal-install, stack, hackage-server)? Can they adapt to it in a backward compatible way with a moderate amount of CPP?

[hasufell]

How disruptive is this change for clients (like, cabal-install, stack, hackage-server)? Can they adapt to it in a backward compatible way with a moderate amount of CPP?

There's nothing disruptive about this. Instead of Either, we're dealing with These. That's pretty straight forward. But the exposed API is a bit brittle anyway.

When These (the constructor) is returned, the caller has to know that they're dealing with long filepaths and need to construct two Entry's. If we improve that, we will break the API even further. The way it is now, the caller can decide to just "ignore" this case and treat it like an error, like before (which is sorta backwards-compatible).

[hasufell]

I just built cabal-install with this branch. All it took was:

--- a/cabal-install/src/Distribution/Client/SrcDist.hs
+++ b/cabal-install/src/Distribution/Client/SrcDist.hs
@@ -62,8 +62,9 @@ packageDirToSdist verbosity gpd dir = do
             let prefix = prettyShow (packageId gpd)
             modify (Set.insert prefix)
             case Tar.toTarPath True prefix of
-                Left err -> liftIO $ die' verbosity ("Error packing sdist: " ++ err)
-                Right path -> tell [Tar.directoryEntry path]
+                Tar.This err -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                Tar.These err _ -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                Tar.That path -> tell [Tar.directoryEntry path]
 
             for_ files $ \file -> do
                 let fileDir = takeDirectory (prefix </> file)
@@ -72,13 +73,15 @@ packageDirToSdist verbosity gpd dir = do
                 when needsEntry $ do
                     modify (Set.insert fileDir)
                     case Tar.toTarPath True fileDir of
-                        Left err -> liftIO $ die' verbosity ("Error packing sdist: " ++ err)
-                        Right path -> tell [Tar.directoryEntry path]
+                        Tar.This err -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                        Tar.These err _ -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                        Tar.That path -> tell [Tar.directoryEntry path]
 
                 contents <- liftIO . fmap BSL.fromStrict . BS.readFile $ dir </> file
                 case Tar.toTarPath False (prefix </> file) of
-                    Left err -> liftIO $ die' verbosity ("Error packing sdist: " ++ err)
-                    Right path -> tell [(Tar.fileEntry path contents) { Tar.entryPermissions = Tar.ordinaryFilePermissions }]
+                    Tar.This err -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                    Tar.These err _ -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                    Tar.That path -> tell [(Tar.fileEntry path contents) { Tar.entryPermissions = Tar.ordinaryFilePermissions }]
 
     entries <- execWriterT (evalStateT entriesM mempty)
     let -- Pretend our GZip file is made on Unix.

---

To support over-long filepaths, the patch is as follows:

--- a/cabal-install/src/Distribution/Client/SrcDist.hs
+++ b/cabal-install/src/Distribution/Client/SrcDist.hs
@@ -62,8 +62,9 @@ packageDirToSdist verbosity gpd dir = do
             let prefix = prettyShow (packageId gpd)
             modify (Set.insert prefix)
             case Tar.toTarPath True prefix of
-                Left err -> liftIO $ die' verbosity ("Error packing sdist: " ++ err)
-                Right path -> tell [Tar.directoryEntry path]
+                Tar.This err -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                Tar.These _ path -> tell [Tar.longLinkEntry prefix, Tar.directoryEntry path]
+                Tar.That path -> tell [Tar.directoryEntry path]
 
             for_ files $ \file -> do
                 let fileDir = takeDirectory (prefix </> file)
@@ -72,13 +73,15 @@ packageDirToSdist verbosity gpd dir = do
                 when needsEntry $ do
                     modify (Set.insert fileDir)
                     case Tar.toTarPath True fileDir of
-                        Left err -> liftIO $ die' verbosity ("Error packing sdist: " ++ err)
-                        Right path -> tell [Tar.directoryEntry path]
+                        Tar.This err -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                        Tar.These _ path -> tell [Tar.longLinkEntry fileDir, Tar.directoryEntry path]
+                        Tar.That path -> tell [Tar.directoryEntry path]
 
                 contents <- liftIO . fmap BSL.fromStrict . BS.readFile $ dir </> file
                 case Tar.toTarPath False (prefix </> file) of
-                    Left err -> liftIO $ die' verbosity ("Error packing sdist: " ++ err)
-                    Right path -> tell [(Tar.fileEntry path contents) { Tar.entryPermissions = Tar.ordinaryFilePermissions }]
+                    Tar.This err -> liftIO $ die' verbosity ("Error packing sdist: " ++ show err)
+                    Tar.These _ path -> tell [Tar.longLinkEntry (prefix </> file), Tar.directoryEntry path]
+                    Tar.That path -> tell [(Tar.fileEntry path contents) { Tar.entryPermissions = Tar.ordinaryFilePermissions }]
 
     entries <- execWriterT (evalStateT entriesM mempty)
     let -- Pretend our GZip file is made on Unix.

[cartazio]

What are the current action items needed to get this patch over the last few ready to merge hump? I’m happy to help however is needed