Skip to content

Path issues on a Windows environment #45

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

Closed
jonasraoni opened this issue Jun 20, 2022 · 3 comments
Closed

Path issues on a Windows environment #45

jonasraoni opened this issue Jun 20, 2022 · 3 comments

Comments

@jonasraoni
Copy link
Contributor

Hi @whikloj!

We're using your package to preserve academic journals at @pkp, and some users had issues under Windows, which I've just confirmed by my own.

Are you open for contributions? For now I saw just two issues:

  • Path issues
    The package is failing to format the path properly in some cases (e.g. trying to open /c:/bag).

  • The repository cannot be even cloned in Windows without hacks, due to invalid file names (below I've left the offending entries):

tests/resources/TestBadFilePathsBag/data/this-file-will-be-fine-%-and-
.txt

tests/resources/TestEncodingBag/data/carriage
return-file.txt

tests/resources/TestEncodingBag/data/directory
line
break/carriage
return/Example-file-%-and-%25.txt

tests/resources/TestEncodingBag/data/directory
line
break/some-file.txt

p.s.: About the cloning problem, that's not a real issue for us, as composer is "fixing" the paths by itself.

@whikloj
Copy link
Owner

whikloj commented Jun 20, 2022

Hi @jonasraoni,

I'm more than happy to accept contributions if it helps. I have not yet, but have considered trying to setup Windows under Github Actions. If you have any experience with that I'd love it.

As for the filenames, those tests might be overkill. I'll have a look and see if I can replicate the test generated test objects.

jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
…t before splitting/joining paths and improved validation
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
… have an "internal" path is using always the "/" as directory separator
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
jonasraoni added a commit to jonasraoni/BagItTools that referenced this issue Jun 22, 2022
@jonasraoni
Copy link
Contributor Author

jonasraoni commented Jun 22, 2022

Hi @whikloj!

I've left a PR here:

I tried to organize the commits, and left some comments to make it easier to review 👀

About the problematic files, I've removed them and updated the tests, let me know if you're ok with it.

whikloj pushed a commit that referenced this issue Jun 23, 2022
* #45 Removed files which cannot be checked out on Windows

* #45 Fixed command validator

* #45 Ensured BagUtils::getAbsolute() is stripping the drive part before splitting/joining paths and improved validation

* #45 Ensured Bag::internalPath() and related code that needs to have an "internal" path is using always the "/" as directory separator

* #45 Fixed issues with line wrapping

* #45 Fixed issue with extension recognition (e.g. filename.EXTRA.tar.gz fails)

* #45 Ensured Bag::makeAbsolute() and related places return paths using DIRECTORY_SEPARATOR

* #45 Formatting/reduced nesting

* #45 Updated tests that relied on unfriendly Windows filenames

* #45 Fixed tests on Windows due to line-break differences/directory separators

* #45 Removed not needed call

* #45 Fixed code sniffing issues
@whikloj
Copy link
Owner

whikloj commented Jun 25, 2022

Resolved with f2496ce

@whikloj whikloj closed this as completed Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants