-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
SetCreationTime on Mac #39132
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
We'd certainly like to make it work if there's an API. Can you figure out what API we could call? I assume there's something lower level. |
@danmosemsft It took me a while, but I have figured out working C# code that does what it should: using System;
using System.Runtime.InteropServices;
namespace dotnet_runtime_39132_poc
{
class Program
{
internal const string libobjc = "/usr/lib/libobjc.dylib";
[DllImport(libobjc)]
private static extern IntPtr objc_getClass(string className);
[DllImport(libobjc)]
private static extern IntPtr sel_registerName(string str);
[DllImport(libobjc)]
private static extern IntPtr objc_msgSend(IntPtr basePtr, IntPtr selector);
[DllImport(libobjc)]
private static extern IntPtr objc_msgSend(IntPtr basePtr, IntPtr selector, double secondsSince);
[DllImport(libobjc)]
private static extern IntPtr objc_msgSend(IntPtr basePtr, IntPtr selector, [MarshalAs(UnmanagedType.LPUTF8Str)] string arg1);
[DllImport(libobjc)]
private static extern IntPtr objc_msgSend(IntPtr basePtr, IntPtr selector, IntPtr arg1, IntPtr arg2);
[DllImport(libobjc)]
private static extern sbyte objc_msgSend(IntPtr basePtr, IntPtr selector, IntPtr arg1, IntPtr arg2, IntPtr arg3);
static void Main(string[] args)
{
SetTime("/Users/Hamish/Desktop/test file", true, new DateTime(2020, 1, 1, 10, 0, 57));
SetTime("/Users/Hamish/Desktop/test file", false, new DateTime(2020, 1, 2, 10, 0, 57));
}
static IntPtr NSDate;
static IntPtr NSDictionary;
static IntPtr NSFileManager;
static IntPtr NSString;
static IntPtr alloc;
static IntPtr init;
static IntPtr initWithUTF8String_;
static IntPtr initWithTimeIntervalSince1970_;
static IntPtr dictionaryWithObject_forKey_;
static IntPtr defaultManager;
static IntPtr setAttributes_ofItemAtPath_error_;
static IntPtr release;
static IntPtr NSFileCreationDate;
static IntPtr NSFileModificationDate;
static IntPtr _NSFileManager;
static Program()
{
NSDate = objc_getClass("NSDate");
NSDictionary = objc_getClass("NSDictionary");
NSFileManager = objc_getClass("NSFileManager");
NSString = objc_getClass("NSString");
alloc = sel_registerName("alloc");
init = sel_registerName("init");
initWithUTF8String_ = sel_registerName("initWithUTF8String:");
initWithTimeIntervalSince1970_ = sel_registerName("initWithTimeIntervalSince1970:");
dictionaryWithObject_forKey_ = sel_registerName("dictionaryWithObject:forKey:");
defaultManager = sel_registerName("defaultManager");
setAttributes_ofItemAtPath_error_ = sel_registerName("setAttributes:ofItemAtPath:error:");
release = sel_registerName("release");
NSFileCreationDate = objc_msgSend(NSString, alloc);
NSFileCreationDate = objc_msgSend(NSFileCreationDate, initWithUTF8String_, "NSFileCreationDate");
NSFileModificationDate = objc_msgSend(NSString, alloc);
NSFileModificationDate = objc_msgSend(NSFileModificationDate, initWithUTF8String_, "NSFileModificationDate");
_NSFileManager = objc_msgSend(NSFileManager, defaultManager);
}
static void SetTime(string path, bool isModificationDate, DateTime time)
{
var date = objc_msgSend(NSDate, alloc);
date = objc_msgSend(date, initWithTimeIntervalSince1970_, (time.ToUniversalTime() - DateTime.UnixEpoch).TotalSeconds);
var fileAttributes = objc_msgSend(NSDictionary, dictionaryWithObject_forKey_, date, isModificationDate ? NSFileModificationDate : NSFileCreationDate);
var native_filePath = objc_msgSend(NSString, alloc);
native_filePath = objc_msgSend(native_filePath, initWithUTF8String_, path);
var retV = objc_msgSend(_NSFileManager, setAttributes_ofItemAtPath_error_, fileAttributes, native_filePath, IntPtr.Zero);
objc_msgSend(date, release);
objc_msgSend(fileAttributes, release);
objc_msgSend(native_filePath, release);
}
}
} This code can set the creation date, and the modification date. var creationDate = File.GetCreationTimeUtc(path);
SetTime(path, true, newModDateUtc);
if (newModDateUtc < creationDate) SetTime(path, false, creationDate); Would this be an acceptable way to implement this? If so I'll create a PR with the necessary changes (I'll need to know how to make this code Mac only w/o using #ifs if that's required). |
Area owners for IO should answer whether they'd accept a PR. (@carlossanlop @jozkee) Most IO stuff is done with glibc calls - but we do have some libobjc use elsewhere: https://github.com/dotnet/runtime/blob/master/src/libraries/Common/src/Interop/OSX/Interop.libobjc.cs https://github.com/dotnet/runtime/blob/master/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs would have to be split into ...OSX.cs and OtherUnix.cs and System.IO.FileSystem.dll looks like it only has a Unix configuration today, it would need to have an OSX configuration broken out. |
@carlossanlop @jozkee, would you guys accept a PR for this? |
FWIW: This also fails on linux distros, I tried it on WSL and got the reported error so maybe we don't have to split the file and perhaps there is an unified way to solve the problem for Unix (Linux and OSX). |
@jozkee I do not believe that there is an easy way to do it on all Unix systems. It says that there is no API to update the CreationTime on Unix systems here, and it says that Linux does not even store it, so I think that we should either have it as a known issue on Linux (and document it, as it took me quite a while to debug my code when it was undocumented), or throw a |
But there could be an API that I do not know about. |
From what I have read recently and in the past, there is not really a way to do this on Linux. And that is why the Apple way is not using a POSIX API at all. |
Ah, I didn't see that part. OK, in that case I think is good that we could fix this for Mac, @hamarb123 a PR for it would be very welcome. |
@jozkee what TFMs should I break it up into? The current csproj file has: <TargetFrameworks>$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser</TargetFrameworks> |
@danmosemsft @jozkee I have a few questions.
|
@jozkee's area but my guesses
|
I don't think so since a code search finds the callers. Unfortunately I don't know the answer for the build failure. Bad sync point perhaps? Maybe git clean -fdx and git pull forward and try again. |
@hamarb123 are you able to perform Perhaps you may want to take a look at https://github.com/dotnet/runtime/blob/master/docs/workflow/README.md#workflow-guide and once your build suceeds you can go into the folder of one of the test projects and run it individually with |
Sorry, I haven't had a chance to work on this for a while, and probably won't for a while to come either. There is one caveat: if you set the modification date to before the creation date it will change the creation date to the new modification date, which differs in behaviour to Windows. This could easily happen when you are copying file (the modification date stays, but the creation date changes to the current date). So I recommend that we check the creation date when changing the modification date, and changing the creation date back afterwards if it was after the new modification date, which would look something like this: var creationDate = File.GetCreationTimeUtc(path);
SetTime(path, true, newModDateUtc);
if (newModDateUtc < creationDate) SetTime(path, false, creationDate); And it makes sense that it should set only the modification date (to me) when you set the modification date, otherwise I think we should have a note in the docs saying the current behaviour on Mac. |
OK, thanks for the effort @hamarb123 i will close this until you or someone wishes to take it further. |
Reopening this issue. @hamarb123 wrote:
@hamarb123 feel free to send a PR, I'm looking forward to reviewing it. |
Hmm I guess you already installed all the prereqs for OSX? https://github.com/dotnet/runtime/blob/main/docs/workflow/requirements/macos-requirements.md#toolchain-setup |
I've got them all, the only thing I can think of is that my XCode version is 12.3 instead of 12.4, does this matter?
|
@AaronRobinsonMSFT is this related to your corerun change? |
Oh - since that's not merged yet - no! |
@hamarb123 it would be helpful if you could share |
Hi @safern, I don't seem to have any files named |
Hmmm... could you try adding this to
Also, it might be helpful if you could share the version of cmake that you have? i.e running |
cmake version 3.19.1 |
Oddly enough, it builds on all machines and SDK's that I've tried. Even the command verbatim from your compile command list. Can you please try this:
|
I have no idea, but would it make sense to update it? |
For what is worth, I tried both 12.1 and 12.4 and CMake 3.16.5 and 3.19.7. For SDK I used macOS SDK 10.15 instead of 11.1 as you used, and so did @safern. Trying to update the SDK to test that out. |
@hoyosjs thanks for your command, I think it's found the issue.
I'm not sure where paxctl is supposed to be coming from, but it's coming from Parallels. |
My guess is that it's a completely different tool that happens to have the same name (something like 'parallels access' vs 'page access'). The command line here: https://kb.parallels.com/en/121134 does not look like the command line for the expected tool: http://man.he.net/man1/paxctl My guess is that this isn't expected to be found on macOS. @janvorli can you confirm? |
and if so, should it be wrapped with
or something else? |
Yes, as Dan says this is a collision in tool names. I believe only Linux supports paxctl as macOS uses other mechanisms for page protection, so it would just be wrapping the implementation of the function that's in runtime/eng/native/functions.cmake Line 400 in f7072ec
In a |
@hoyosjs thank you!
Edit: the ilasm executable had read/write permissions for system only (must've downloaded this one with sudo), this is probably the issue. |
I'm now able to reproduce the errors in #49555! |
My main issue seems to be that I missed the |
Good teamwork 🙂 |
* Fix setting creation date for OSX Fix for #39132 * Fix setting creation date for OSX - fix for setattrlist This commit adds a fallback for setattrlist failing. It can sometimes fail on specific volume types, so this is a workaround to use the old behaviour on unsupported filesystems. Fix for #39132 * Fix setting creation date for OSX - fix for caching when set time & possible stack overflow This commit adds _fileStatusInitialized = -1; which resets the cache for the file status after setting a time value to the file using the setattrlist api version so that it will not simply return the last cached time. It also fixes SetCreationTime_OtherUnix method so that it calls SetLastWriteTime_OtherUnix directly instead of SetLastWriteTime because otherwise you'll get stuck in an infinite loop on OSX when the fallback is theoretically needed. Fix for #39132 * Fix setting creation date for OSX - changes for PR 1 Fix the behaviour of SetLastWriteTime (OSX) when setting lastwritetime before creationtime. Use Invalidate() instead of _fileStatusInitialized = -1, and only when appropriate as per PR #49555. Add SettingUpdatesPropertiesAfterAnother test to test things such as setting lastwritetime to before creationtime. Rename the added _OtherUnix methods to _StandardUnixImpl, a more logical name given their purpose (#49555) Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - changes for PR 2 Added a new test to test the behaviour of the file time functions on symlinks as per standard Windows behaviour, and also what I use in my code today. It makes sure that the times are set on the symlink itself, not the target. It is likely to fail on many unix platforms, so these will be given the [ActiveIssue] attribute when we discover which ones. Make access time be set using setattrlist on OSX to fix following symlinks issue that would've failed the new test. Fix and update wording of comment in SetAccessOrWriteTime. Add T CreateSymlinkToItem(T item) to BaseGetSetTimes<T> (and implementation in inheritors) for the new test. Made the SettingUpdatesPropertiesAfterAnother test skip on browser since it only, in effect, has one date attribute. Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - changes for PR 3 Revert change in last commit: Make access time be set using setattrlist on OSX to fix following symlinks issue that would've failed the new test. It is now using the other API as otherwise some file watcher tests fail. Set AT_SYMLINK_NOFOLLOW in pal_time.c to not follow symlinks for access time on OSX, or any times on other unix systems. * Fix setting creation date for OSX - fix test for Windows and browser + update comments Fix the SettingUpdatesPropertiesOnSymlink test for Windows by setting FILE_FLAG_OPEN_REPARSE_POINT in the CreateFile used for setting times. Disable the SettingUpdatesPropertiesOnSymlink test for Browser as it can't use symlinks with the current API in the test project. Update comments. Add FILE_FLAG_OPEN_REPARSE_POINT. Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - make modification time use normal unix APIs Only implement creation time setting with setattrlist. Here is why: eg. the old code doesn't account for setting creation time to 2002, then setting modification time to 2001, and then setting access time (since access time didn't have the creation date check). I've split up SetAccessOrWriteTime into 3 parts - SetAccessOrWriteTime, SetAccessOrWriteTime_StandardUnixImpl, and SetAccessOrWriteTimeImpl (this last one contains the logic of the method without the things like refreshing the times so it's not called twice on OSX). In this process I replaced the _fileStatusInitialized = -1 calls with Invalidate() calls. And to make sure that if the creation time is needed to be set by SetAccessOrWriteTime, then we don't get in an infinite loop in case setattrlist fails, so we call SetAccessOrWriteTime_StandardUnixImpl. * Fix setting creation date for OSX - hotfix: Remove ATTR_CMN_MODTIME Adding this commit now to avoid 2x CI. It was meant to be in the last one, but I forgot to save the file. Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - revert windows changes & simplify code Revert the changes for Windows & disable the SettingUpdatesPropertiesOnSymlink test on Windows. Remove unnecessary setting to 0 on the attrList variable. Remove the unnecessary MarshalAs attributes in the AttrList type. Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - hotfix: change to CULong for marshalling with setattrlist Use CULong as per #49555 (comment) Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - hotfix: change to CULong for marshalling with setattrlist 2 Use CULong as per #49555 (comment) Forgot to save the other file's changes Fix for #39132 and issues in #49555. * Fix setting creation date for OSX - consolidate unix nanosecond calculation Consolidate unix nanosecond calculation into UnixTimeSecondsToNanoseconds as per #49555 (comment). Fix for #39132 and issues in #49555. * Use InvalidateCaches instead of Invalidate and EnsureCachesInitialized instead of EnsureStatInitialized * Fixes for reviews in PR #49555 Add an identifying field to the TimeFunction tuple and a function to get a TimeFunction from the name. This function is then used so that SettingUpdatesPropertiesAfterAnother can be written as a Theory, and is a lot more readable. Fix some comments that were incorrect. * Hotfix for missing parameter * Fix naming * Revert changes re SettingUpdatesPropertiesAfterAnother (mainly) and revert using a Theory Reversion as per #49555 (comment) This change can be unreverted if that's what's decided. * Fix errors of #49555 due to missing variables * Remove the parameters that were meant to be removed * Finish merge * Remove duplicate file * Add custom error handling for setattrlist Add custom error handling for setattrlist as per #49555 (comment). ENOTDIR means it is not a directory, so directory not found seems logical to me, I think the other errors can be dealt with by an IOException, happy to change this. * Remove symlink specific code from this PR Removes the symlink specific code as per #49555 (comment). This will be reverted (and modified) as per #49555 (comment) into #52639. * Remove custom ENOTDIR handling, enable tests for other OSX-like platforms + move items in project file • Removed ENOTDIR handling as per #49555 (comment) • Enabled tests for other OSX-like platforms since the code was included on them (IsOSXLike is presumably defined at src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Unix.cs). If it fails on any of these platforms, they'll be fully explicitly excluded everywhere for this PR or fixed. • Move item in project file (System.Private.CoreLib.Shared.projitems) as per #49555 (comment) * Rename files according to review Rename files according to #49555 (comment) (and nearby comments). * Change names of functions to improve readability and refactor code This commit changes and refactors the core functions of this PR to have clearer names and to create a more logical flow of the code. It also updates some comments and fixes some oversights. Changes are based on the discussion #49555 (comment) (read whole conversation). * Fix the wrongly named variable and add the missing 'unsafe' keyword Fix the wrongly named variable and add the missing 'unsafe' keyword * Better comments and minor improvements Changes according to #49555 (review) • Use explicit type `IEnumerable<TimeFunction>` instead of var • Use PlatformDetection rather than OperatingSystem • Move the InvalidateCaches code for creation time setting to avoid unnecessary checks • Update explanatory comments to be more useful * Add additional tests and improvements - Use tuple destruction to swap variables - Add SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAreSame test as per #49555 (comment) - Change TFM(/s) as per #49555 (comment) - Avoid unnecessary call to `UnixTimeToDateTimeOffset` as per #49555 (comment) - Use InvalidOperationException as per #49555 (comment) * Fix typos * Revert to <TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser</TargetFrameworks> Revert to using <TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser</TargetFrameworks> since <TargetFramework>$(NetCoreAppCurrent)</TargetFramework> failed; I wouldn't be surprised if this failed too. * Remove SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAreSame tests Remove SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAreSame tests since they don't work on windows because it is apparently an invalid time. Additionally, the function SettingOneTimeMaintainsOtherDefaultTimes wasn't working and needed to be fixed. * Fix: setattrlist can only return 0 or -1 As per #49555 (comment), use GetLastErrorInfo to get the error when setattrlist indicates that there is one. Co-authored-by: David Cantú <dacantu@microsoft.com> * Update code comment for SettingUpdatesPropertiesAfterAnother test The comment is now clearer and grammatically correct :) * Update code comment for SettingUpdatesPropertiesAfterAnother test Describe how utimensat change more dates than would be desired * Update comment for SettingUpdatesPropertiesAfterAnother for consistency Update for consistency with the comment in FileStatus.Unix.cs * Fixes for compilation and update to comment • Fix trailing spaces and incorrect capitalisation in FileStatus.SetTimes.OSX.cs • Add more info to the comment in the SettingUpdatesPropertiesAfterAnother test * Update FileStatus.SetTimes.OSX.cs Use the Error property * Move comments and add explicit types to SettingUpdatesPropertiesAfterAnother test Move comments and add explicit types to SettingUpdatesPropertiesAfterAnother test Co-authored-by: David Cantú <dacantu@microsoft.com>
Description
The method
System.IO.File.SetCreationTime[Utc](string, DateTime)
does not work correctly on mac, but theSystem.IO.File.GetCreationTime[Utc](string, DateTime)
does.The following code snippet creates different results on mac and windows:
I believe that this method contains the implementation for Unix platforms, but I think that Mac should have a different implementation for this function to Unix as there is an actual way to do this on Mac.
Swift code to change date:
This code, in theory, sets the creation date of the file; there must be a way to implement this in C# on the Mac version, it is also probably a good idea to implement the other methods (last write and access dates) like this, as it seems that setting the last write date changes the creation date sometimes (see this).
Configuration
I tested the code on macOS Catalina (x64) and Windows 10 (x64). It is specific to unix platforms.
Regression?
I do not know.
Other information
A workaround could be to set the creation date, and then set the modification date back again. This will work sometimes, but not always.
The text was updated successfully, but these errors were encountered: