The pijul commands for file operations (pijul add, pijul mv, and pijul remove) seem to not be working properly on Windows.

Before going into the details on each, it should be mentioned that pijul can tell if the files operations are to be preformed on exist or not. When targeting a file name that does not exist, all three commands produce the error Error: The system cannot find the file specified. (os error 2).

pijul remove

When targeting a file that exists in the working directory, this command will silently do nothing. If the file is being tracked by pijul before running the command, it will still be tracked. If it was not tracked, it still is not. In other words, it is effectively a no-op if the specified file exists.

If a tracked file is removed from the working directory (whether be deletion, moving, or renaming) pijul will correctly detect it as a deletion when running `pijul diff’.

pijul mv

This command does move / rename the target file to the specified name. However, it does not update the tracked files to reflect the move. It fails with the error message Error: prefix not found.

It can be run for both files that are tracked and those that are not with the same results in terms for moving / renaming the file and failing to update what files are tracked.

There should probably be a command line flag for this command that tells it that it is being run for a file that has already been moved.

pijul add

Silently fails to add the specified files to the set of tracked files.

pijul add -r

The recursive version does add files from the target directory. However, on Windows it considers src/extension.ts and src\extension.ts as distinct paths. As a result if src/extension.ts is tracked (from a Unix system), it will be tracked again under the name src\extension.ts. Changes to the file will be picked up as changes to both src/extension.ts and src\extension.ts.

As a note the recursive version does not pick up files with names starting with ‘.’ (a period). While this is a reasonable default, there should probably be a flag to force it to pick up files starting with ‘.’.

Thanks! I do have a Windows VM, but I rarely use it since it was only recently that I learned how to compile Pijul on it (thanks to awesome contributors ;-).

@pmeunier

I suspect the error message from the mv command likely occurs due to how Rust std::fs::canonicalize works on Windows. On Windows it converts the path to extended length path syntax. Which means it will be an absolute path prefixed with \\?\ and the use of / as an interchangeable alternative to \ is disallowed. This way of writing paths allows for bypassing the 260 max path limit Windows inherited from DOS.

Alright, I could finally compile and run Pijul on Windows today. Thanks a million for reporting this, I’ll try to fix it very soon.

@pmeunier

Since I have time today and there has not been any news, here is some help on finding the issues:

From pijul/src/commands/file_operations.rs in impl Add::run() (lines 115 to 121):

                let path = path.canonicalize()?;
                let meta = std::fs::metadata(&path)?;
                let path = if let Ok(path) = path.strip_prefix(&repo.path) {
                    path
                } else {
                    continue;
                };

On Windows, if the file being added is U:/devel/vscode-pijul/foo.txt, then path.canonicalize()? returns "\\?\U:\devel\vscode-pijul\foo.txt".

This causes an error at path.strip_prefix(&repo.path) as the prefix does not match match as repo.path is something like "U:\devel\vscode-pijul\". So on Windows repo.path would need to be canonicalized() as well. And then the resulting path would need to have the '\' separators replaced with '/' before checking if the file is tracked.

Since there are similar cases in a number of places, it is likely best to have a sanitize_filename function somewhere that is imported and used to prepare file names. Such a function would then hide the difference in steps between Unix-like systems and Windows.

In pijul/src/commands/file_operations.rs (lines 41 to 53):

        for p in self.paths {
            debug!("p = {:?}", p);
            let source = std::fs::canonicalize(&path(&self.repo_path, p.clone()))?;
            let target = if is_dir { to.join(p) } else { to.clone() };
            debug!("target = {:?}", target);
            std::fs::rename(&source, &target)?;
            let target = std::fs::canonicalize(&target)?;

            let source = source.strip_prefix(&repo.path)?;
            let target = target.strip_prefix(&repo.path)?;
            debug!("moving {:?} -> {:?}", source, target);
            txn.move_file(&source.to_string_lossy(), &target.to_string_lossy())?
        }

The result of std::fs::canonicalize() is the best file name to use for file operations on Windows (it avoids legacy API issues that could come up otherwise), so std::fs::rename(&source, &target)?; works fine.

However, let source = source.strip_prefix(&repo.path)?; fails. Unlike in the add command implementation, the error is forwarded allowing the user to know an error occurred.

The rename should probably be undone if there is an error that causes the move to not be committed to the repository.

In pijul/src/commands/file_operations.rs (lines 157 to 162):

            let path = path.canonicalize()?;
            let path = if let Ok(path) = path.strip_prefix(&repo.path) {
                path
            } else {
                continue;
            };

Same as in the implementation for pijul add.

In pijul/src/commands/credit.rs (lines 42 to 52):

        let (pos, _ambiguous) = if has_repo_path {
            let root = std::fs::canonicalize(repo.path.join(&self.file))?;
            let path = root.strip_prefix(&repo.path)?.to_str().unwrap();
            txn.follow_oldest_path(&repo.changes, &channel, &path)?
        } else {
            let mut root = crate::current_dir()?;
            root.push(&self.file);
            let root = std::fs::canonicalize(&root)?;
            let path = root.strip_prefix(&repo.path)?.to_str().unwrap();
            txn.follow_oldest_path(&repo.changes, &channel, &path)?
        };

Has the same problems with stripping the prefix from a canonicalize path. The fact that it is in a different file means a centralized function to prepare file paths from input would need to be located in a places available to both source files. Possibly as a member of Repository type from pijul/src/repository.rs.

As to the implementation for pijul add -r:

            if self.recursive {
                use libpijul::working_copy::filesystem::*;
                if let Ok((full, prefix)) = get_prefix(Some(&repo.path), path) {
                    repo.working_copy
                        .add_prefix_rec(&mut txn, &repo.path, &full, &prefix, threads)?
                }
            }

The actual meat of what is happening is in add_prefix_rec(). On line 131 of libpijul/src/working_copy/filesystem.rs there is let repo_path_ = std::fs::canonicalize(repo_path)?;. The variable repo_path_ is then used for striping prefixes, so there are no errors from that unlike in the other highlighted sections. However, the '\' path separators are not replaced with '/', which causes the path to not match the ones from Unix-like systems.

I got the commands working properly on Windows except for pijul add -r. The exact changes are not the best way to do it though since it would change how the commands work on Unix-like systems as well. The reason I do not work out a full proper fix is I am not familiar with Rust, its libraries, or the tools so even simple changes take far more time / work than they really should.

Here is the changes I made to the pijul remove implementation as an example:

(Original code in pijul/src/commands/file_operations.rs on lines 157 to 166):

       let path = path.canonicalize()?;
       let path = if let Ok(path) = path.strip_prefix(&repo.path) {
           path
       } else {
           continue;
       };
       let path_str = path.to_str().unwrap();
       if txn.is_tracked(&path_str) {
           txn.remove_file(&path_str)?;
       }

Changing the above to:

           let path = path.canonicalize()?;
           let meta = std::fs::metadata(&path)?;
           let path = if let Ok(path) = path.strip_prefix(repo.path.canonicalize()?) {
               path
           } else {
               continue;
           };
           let path_str = path.to_str().unwrap().replace("\\", "/");
           if !txn.is_tracked(&path_str) {
               txn.add(&path_str, meta.is_dir())?
           }

Makes the pijul remove command work properly on Windows.

Again, thanks a lot for your work! This really is one of my top priorities, now that a basic CI system for the Nest is ready to deploy.

Alright, I’ve fixed this by using the canonical-path crate, which forces paths to be canonicalized when needed. This is change #ZHABNS3S6FSINO74FOI5KHYXYDTBPO4FQTTYTUS7NNKEVVNLYC4AC, going through CI now.

@pmeunier

In pijul 1.0.0-alpha.24, there is still more work to do on this issue. I did not bring it up on on the 22nd alpha as there were more related changes on the Nest that were not in that release so I was waiting to see if they fixed thing (which they did fix some more of the issues).

The path prefix is now fixed for the pijul add command. However, it still exists for the pijul mv, pijul remove, and pijul credit commands. I will try my hand a bit later at replicating the changes that were done to pijul add in those commands and pushing them to this discussion.

There is also still the problem with Windows using a different path separator than Unix-like systems.

Here is a demonstration with the vscode-pijul repository using pijul 1.0.0-alpha.24:

PS U:\devel\vscode-pijul> pijul ls

default.nix

package.json

src

src/extension.ts

tsconfig.json

PS U:\devel\vscode-pijul> pijul add src/extension.ts

PS U:\devel\vscode-pijul> pijul ls

default.nix

package.json

src

src/extension.ts

src\extension.ts

tsconfig.json

The file src/extension.ts is now in the repository twice. The same happens with pijul add -r src. The issue of path separators also affects the functionality of the other commands.

Diff for fixing remaining issues with striping path prefixes on Windows:

message = 'Fix path prefix striping on Windows.'
timestamp = '2020-12-20T17:13:06.340461Z'
authors = [{ name: "Skia", full_name: "James Adam Armstrong", email: "gwenio@live.com" }]

# Dependencies
[2] L4JXJHWXYNCL4QGJXNKKTOKKTAXKKXBJUUY7HFZGEUZ5A2V5H34QC
[3] GJNJ75U5ADCWNMLPBRM4FMYV6KJ366YLN5C7PWPZX3C5CWE5HZZQC
[4]+SXEYMYF7P4RZMZ46WPL4IZUTSQ2ATBWYZX7QNVMS3SGOYXYOHAGQC
[5]+NLGQAH4H35XC5XTH26BRXVFWGPPAMA4MDN3MHMGCOYE6ZZQMQ4AAC
[*] SXEYMYF7P4RZMZ46WPL4IZUTSQ2ATBWYZX7QNVMS3SGOYXYOHAGQC
[*] NLGQAH4H35XC5XTH26BRXVFWGPPAMA4MDN3MHMGCOYE6ZZQMQ4AAC
[*] AEPEFS7O3YT7CRRFYQVJWUXUUSRGJ6K6XZQVK62B6N74UXOIFWYAC
[*] 4H2XTVJ2BNXDNHQ3RQTMOG3I4NRGZT7JDLC2GRINS56TIYTYTO4QC
[*] KWAMD2KR5UYRHHPZWL7GY2KQKNXNVS4BYBVK3FXDI23NQMWA3U4QC
[*] BZSC7VMYSFRXDHDDAMCDR6X67FN5VWIBOSE76BQLX7OCVOJFUA3AC

# Changes

1. Edit in pijul/src/commands/file_operations.rs:42 4.169058
  up 4.169908, new 0:69, down 4.169908
+         let repo_path = CanonicalPathBuf::canonicalize(&repo.path)?;

2. Replacement in pijul/src/commands/file_operations.rs:51 4.169058
B:BD 4.170280 -> 4.170280:170398/4
  up 4.170280, new 70:188, down 4.170398
-             let source = source.strip_prefix(&repo.path)?;
-             let target = target.strip_prefix(&repo.path)?;
+             let source = source.strip_prefix(&repo_path)?;
+             let target = target.strip_prefix(&repo_path)?;

3. Edit in pijul/src/commands/file_operations.rs:164 4.169058
  up 4.172786, new 189:258, down 4.172786
+         let repo_path = CanonicalPathBuf::canonicalize(&repo.path)?;

4. Replacement in pijul/src/commands/file_operations.rs:177 4.169058
B:BD 4.173175 -> 4.173175:173248/4
  up 4.173175, new 259:342, down 4.173248
-             let path = if let Ok(path) = path.strip_prefix(&repo.path) {
+             let path = if let Ok(path) = path.strip_prefix(&repo_path.as_path()) {

5. Edit in pijul/src/commands/credit.rs:4 4.178684
  up 2.2811, new 343:381, down 2.2811
+ use canonical_path::CanonicalPathBuf;

6. Edit in pijul/src/commands/credit.rs:43 4.178684
  up 4.179828, new 382:451, down 4.179828
+         let repo_path = CanonicalPathBuf::canonicalize(&repo.path)?;

7. Replacement in pijul/src/commands/credit.rs:46 4.178684
B:BD 4.179954 -> 4.179954:180027/4
  up 4.179954, new 452:535, down 4.180027
-             let path = root.strip_prefix(&repo.path)?.to_str().unwrap();
+             let path = root.strip_prefix(&repo_path.as_path())?.to_str().unwrap();

8. Replacement in pijul/src/commands/credit.rs:52 4.178684
:D 3.113 -> 5.260:333/3, B:BD 5.260 -> 5.260:333/5
  up 3.113, new 536:619, down 4.180164
-             let path = root.strip_prefix(&repo.path)?.to_str().unwrap();
+             let path = root.strip_prefix(&repo_path.as_path())?.to_str().unwrap();

The pijul record command seems to have developed a new issue. I will look into that more tomorrow. Not sure if I formatted the author part correctly since I was adding that in myself.

The formatting of the author field is right. Is there a specific reason why you can’t/don’t want to push to the discussion?

pijul push Skia@nest.pijul.com:pijul/pijul --to-channel :188

@pmeunier

Because pijul record -m "message" was failing with an OS error about a file not being found and I did not have time too look into why.

It turns out that if I remove the format hook from the config file then pijul record works. I have found where the problem with hooks is coming from and will be opening a separate issue for it.

Cool, thanks! Do you want me to copy and apply your text patch, or do you prefer to push it?

@pmeunier

Any thoughts on how to handle replacing \ with / in paths on Windows?

Or more specifically, should in be in the code for the commands or should it be handled in libpijul?

And I know ho to do the replacement, but not how to do it in a way that would make it only happen in Windows builds.

Thanks for the patch!

Any thoughts on how to handle replacing \ with / in paths on Windows?

Yes. There are things about that in libpijul, so I’ll take care of it (hopefully today).

@pmeunier

I found another place that needed fixing. It was in the reset command file. I have pushed the fix to this discussion. Did not notice it before as I had not tried just resetting specific files.

A full text search on all the files shows that this is indeed the last place that needs fixing for the path striping. At least as far as the pijul command line source files go.

@pmeunier

The latest change I pushed to this discussion (Y7YAFMFFJY3SQ3GYN3SS4V3FZWMH3B5L65AXQBXOR5XARSMF5JJQC) is a ‘squashed’ version of the two previous changes.

The reason for it was my local clone broke (I think it was during a pull where the connection randomly dropped, but the problem did not manifest until later when I was trying things with channels) and I was having trouble applying the changes to a fresh clone. The new change is in case my local repo had a problem before I made the other changes.

The others might be fine though, it occurred to me after pushing the new change that pushing/pulling from a repository that is having issues is likely why the changes not applying correctly to a fresh clone.

Oh, thanks! I was actually going through that and trying to figure out where the conflicts came from ;-)

The reason for it was my local clone broke (I think it was during a pull where the connection randomly dropped, but the problem did not manifest until later when I was trying things with channels) and I was having trouble applying the changes to a fresh clone

I really want to know all about that. What did you do? Where were the conflicts? Do you still have the repo? If so, would you mind sending a tarball to pe@pijul.org?

I can’t tell if this is a bug or just poor documentation, and in both cases I want to fix that.

Sent to CI, should be applied within a few minutes, see https://nest.pijul.com/pijul/pijul/ci

@pmeunier

I will email the tar file in a bit.

I think there is something unstable with pulling and cloning over HTTPS, at least on Windows. When trying to get a fresh clone setup after things broke, when using HTTPS the connection would often break down and when it did complete it would have some of the files in the working directory with incorrect contents (such as some files being empty or only having part of the contents). Using SSH does not seem to have such problems so far.

Then thing with the broken repository was that when ever I switched back to the ‘main’ branch, half the pijul/src/config.rs file would be missing. When I would try moving the changes via pushing and pulling between local repos the log would show the changes were applied, but the contents of the files would not have the changes as though they were being applied and then overwritten by a different change.

I was using a build of pijul that had my changes applied to it for the path prefix thing and my changes to how the record hook works from discussion #234. I pulled the ‘main’ channel from the Nest to work on a set of changes to how remotes are handled in isolation from the others. It was after switching channels that problems started appearing.