Skip to content
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

Fix #3410 - UnauthorizedAccessException writing to MyProject.paket.references.cached #3617

Merged
merged 7 commits into from
Sep 20, 2019

Conversation

yaakov-h
Copy link
Contributor

Root cause as discussed in #3410 - a source control system that marks paket.references as read-only.

I mostly made the integration test by copying it from the one above. It fails without the changes from the second commit, and passes with them. 😃

I made two changes:

  1. Before deleting obj\foo.csproj.paket.references.cached we clear the read-only flag, so that Paket can upgrade from a broken version without issue.
  2. After copying obj\foo.csproj.paket.references.cached we clear the read-only flag, so that this doesn't re-occur, and so none of the intermediary obj files are read-only.

@forki
Copy link
Member

forki commented Jul 22, 2019

Can you please take a look at the CI errors? Thx

@yaakov-h
Copy link
Contributor Author

Hmmm. I can't reproduce the Windows failure, nor the Linux failure on Mono 4.6.

I'll try grab latest Mono (5.x) and see if I can figure anything out.

There seems to be a bug of some kind in Mono 5 where set_IsReadOnly
throws an exception because the existing set of attributes is -1.

In any event, file-read-only is really just a Windows concept. .NET only
emulates it on *nix by adjusting the file write permission (chmod -w).
@yaakov-h
Copy link
Contributor Author

The core issue seems to be a bug in Mono where File.GetAttributes(...)/FileSystemInfo.Attributes returns -1, so FileSystemInfo.set_IsReadOnly attempts to set the attributes to -1 | ReadOnly which is an invalid combination, and throws.

I haven't managed to cleanly reproduce, and it seems odd to be messing with read-only on Linux, since read-only is actually a Windows thing. On Linux and macOS, Mono and .NET Core emulate the read-only bit by adjusting the file write bits.

As such, I've locked the file-read-only behavior to Windows only, which is probably the only place where this sort of bug would be legitimately encountered anyway.

@forki
Copy link
Member

forki commented Jul 29, 2019

It's still red on the appveyor.

@forki
Copy link
Member

forki commented Jul 29, 2019

  1. Error : Paket.IntegrationTests.RestoreSpec.Paket failed with UnauthorizedAccessException writing to MyProject.paket.references.cached #3410 Paket restore fails when obj files are readonly
    System.IO.DirectoryNotFoundException : Could not find a part of the path 'C:\projects\paket\integrationtests\scenarios\i003410-readonly-obj\temp\dotnet\obj\dotnet.csproj.paket.references.cached'.

@yaakov-h
Copy link
Contributor Author

Yeah, looking into it now.

Do you have any idea why another test is also failing with a readonly file error?

I'm not familiar with the internal test infrastructure or if there are dependencies between tests / dirty state / etc.

@forki
Copy link
Member

forki commented Jul 29, 2019

the other one may be unrelated

@yaakov-h
Copy link
Contributor Author

integrationtests/scenarios/*/before/**/obj is in .gitignore, so one of my test input files was never present. 😭

🤞 it'll be good now

@yaakov-h
Copy link
Contributor Author

yaakov-h commented Aug 5, 2019

@forki any idea what's with integration test i003012? I haven't touched it, and my new test isn't leaving anything behind...

@forki
Copy link
Member

forki commented Aug 10, 2019

It says:

UnauthorizedAccessException: Access to the path 'C:\projects\paket\integrationtests\scenarios\i003012\temp\dotnet\obj\dotnet.csproj.paket.references.cached' is denied.

@yaakov-h
Copy link
Contributor Author

I can see that, but I cannot for the life of me figure out why.

Are tests run in parallel, perhaps?

@yaakov-h
Copy link
Contributor Author

I can't even reproduce it locally anymore. 😢

@qiuhaotc
Copy link

Same problem and that project is also .net core project😫

@yaakov-h
Copy link
Contributor Author

@forki fixed appveyor 😄

@forki forki merged commit eed76d4 into fsprojects:master Sep 20, 2019
@forki
Copy link
Member

forki commented Sep 20, 2019

thanks!!!

@yaakov-h yaakov-h deleted the fix/3410-readonly-cached-file branch September 21, 2019 08:59
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

Successfully merging this pull request may close these issues.

3 participants