As you wrote in the task, is .download extension enough to decide if it's a download in progress ?

Maybe all macOS packages need to shown/treated in a special way?

We disabled directory view for .apps as they're really common and modifying them invalidates codesign. rB370ed6025f45: File Browser/macOS: Don't treat .app as directory

If any more bundles are added to such a list, extension based identification would be uniform.

Typos fixed.

As you wrote in the task, is .download extension enough to decide if it's a download in progress ?

That's would also be a good solution. I've checked the .app extension's exclusion code. We can easily add .download (and maybe even Chrome's .crdownload, though those don't seem to be folders/packages) and be done with it. No need to do the file attribute juggling.

So what do you think? Should I revert the storage_apple.mm changes and only do the filelist.c change?

I personally don't like pretending this is FILE_TYPE_APPLICATIONBUNDLE, which has some fairly specific behavior that doesn't apply to this.

Does it not do what you want if you just give it FILE_ATTR_HIDDEN and FILE_ATTR_TEMPORARY if the extension is ".download" ? Seems like a one-liner in BLI_file_attributes()

In D11613#298101, @Harley Acheson (harley) wrote:

I personally don't like pretending this is FILE_TYPE_APPLICATIONBUNDLE, which has some fairly specific behavior that doesn't apply to this.

I'm not sure we are pretending here. The macOS Finder treats those .download files as bundles/packages. Finder lets them resemble a single file and there's a "Show package contents" menu entry that allows you to open it and see inside it. I was able to manually create one by creating a folder and adding a .download extension to its filename.
So I think technically it is a bundle.

Does it not do what you want if you just give it FILE_ATTR_HIDDEN and FILE_ATTR_TEMPORARY if the extension is ".download" ? Seems like a one-liner in BLI_file_attributes()

That's more or less the first version I submitted, albeit I recognized it by file attribute instead of extension. It worked, yes. But @Ankit Meel (ankitm) suggested treating it as a bundle and that also makes sense.

I don't think Blender ever has to work with those partially downloaded files. And certainly shouldn't add contents inside of them. So from that point of view I'd say it's better to treat them as bundles and always hide them completely. That was also the reasoning behind D9162.

@Leon Zandman (lzandman) - But @Ankit Meel (ankitm) suggested treating it as a bundle and that also makes sense.

Makes sense. But isn't that done entirely by the small change in filelist.c only?

In D11613#298125, @Harley Acheson (harley) wrote:

But isn't that done entirely by the small change in filelist.c only?

Yes. In fact, the current revision of this diff effectively only runs that filelist.c code. The other code is skipped. I just kept it in, because it isn't clear to me which method I should choose.

@Leon Zandman (lzandman) - I just kept it in, because it isn't clear to me which method I should choose.

Ah, no worries. If you choose to keep the filelist.c change then I would only suggest adding a brief comment to the right of each of those two file types. So /* Application bundle. */ and /* In-process or paused internet download. */ or similar.

Should files with extension ".framework" also be in that list?

Should files with extension ".framework" also be in that list?

They are present mostly either

• in system directories like /Library or /System/Library (which people don't visit that often I think..)
• or inside .app bundles (which is already treated as a file)

So I'd say no.

In D11613#298249, @Ankit Meel (ankitm) wrote:

Should files with extension ".framework" also be in that list?

So I'd say no.

I agree. There are many more bundle file types. Even files that regularly reside in user space. Then we should also exclude those. Where does it stop? This only makes sense if we have some other method of recognizing bundle types (i.e. not by extension).

I've removed the extended attribute checking code. Now only relying on .download file extension. However, I did keep some of my refactored code in storage_apple.mm. Should I keep that or fully revert that file?

I've removed the extended attribute checking code. Now only relying on .download file extension. However, I did keep some of my refactored code in storage_apple.mm. Should I keep that or fully revert that file?

It's a totally separate change from the goal of this patch. I would've suggested to post it separately, but without the safari xattr, the improvement from the refactor is not significant.

In D11613#298297, @Leon Zandman (lzandman) wrote:

This only makes sense if we have some other method of recognizing bundle types (i.e. not by extension).

So, I dug into this some more. And it looks like it's kind of a mess. Some conclusions based on this StackOverflow article:

1. Apparently on newer macOS versions the fact that a file is a Package or a Bundle (just names for the same thing?) is stored in metadata. You can use mdls to retrieve the kMDItemContentTypeTree key and if it contains com.apple.folder it's a Package. However, my Safari download doesn't contain that property:

leon@Leons-MacBook-Air Downloads % mdls -name kMDItemContentTypeTree kali-linux-2021.2-installer-amd64.iso.download

kMDItemContentTypeTree = (
"public.item",
"dyn.ah62d4qmuhk2x43dts71g255bqu",
"public.data"
)

2. On older versions there's a central registry, which is still in use on Big Sur. And it seems the "Safari download" package type is registered here. You can dump the registry using this command: /System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister -dump
   
   Here is the "Safari download" record and you'll notice that in the flags section the package flag is set:

claim id:                   Safari download (0x1750)
localizedNames:             "ar" = "تنزيل Safari", "Base" = ?, "ca" = "Descàrrega del Safari", "cs" = "Stahování Safari", "da" = "Safari-overførsel", "de" = "Safari-Download", "el" = "Λήψη Safari", "en" = "Safari download", "en_AU" = "Safari download", "en_GB" = "Safari download", "es" = "Descarga de Safari", "es_419" = "Descarga de Safari", "fi" = "Safarin tiedostolataus", "fr" = "Téléchargement de Safari", "fr_CA" = "Téléchargement de Safari", "he" = "הורדת Safari ", "hi" = "Safari डाउनलोड", "hr" = "Safari preuzimanje", "hu" = "Safari letöltés", "id" = "Unduhan Safari", "it" = "Download Safari", "ja" = "Safariダウンロード", "ko" = "Safari 다운로드", "LSDefaultLocalizedValue" = "Safari download", "ms" = "Muat turun Safari", "nl" = "Safari-download", "no" = "Safari-nedlasting", "pl" = "pobrane w Safari", "pt" = "Download do Safari", "pt_PT" = "Descarga do Safari", "ro" = "Descărcare Safari", "ru" = "Загрузка Safari", "sk" = "Súbor stiahnutý v Safari", "sv" = "Safari-hämtning", "th" = "ดาวน์โหลด Safari", "tr" = "Safari ile indirilen", "uk" = "Викачування Safari", "vi" = "Bản tải về từ Safari", "zh_CN" = "Safari浏览器下载", "zh_HK" = "Safari下載", "zh_TW" = "Safari下載"
rank:                       Default
bundle:                     Safari (0x4c4)
flags:                      apple-internal  package  doc-type  resolves-icloud-conflicts (000000000000022a)
roles:                      Editor (0000000000000004)
bindings:                   .download

Anyway, this is all terminal scripting code. I haven't yet sorted out how to do this programmatically. But while filtering by extensions .app and .download works, it certainly doesn't seem to be a real solid solution and doesn't catch all packages/bundles.

Also, does it also filter out those files on Windows? Because it doesn't make any sense there, right?

In D11613#298304, @Ankit Meel (ankitm) wrote:

I've removed the extended attribute checking code. Now only relying on .download file extension. However, I did keep some of my refactored code in storage_apple.mm. Should I keep that or fully revert that file?

It's a totally separate change from the goal of this patch. I would've suggested to post it separately, but without the safari xattr, the improvement from the refactor is not significant.

Yes, you're right. I'll revert that file.

In D11613#299119, @Bastien Montagne (mont29) wrote:

and why would .download files appear as directories, and not just regular unknown file type...).

Because these .download files are directories. The macOS Finder uses some heuristics and external registries to determine if such a directory is a bundle/package. Blender currently only does some additional extension checking to recognize bundles/packages. It's this code that's extended in this diff.

Any reason why this code is not OSX-specific then? with appropriate #ifdef?

In D11613#299206, @Bastien Montagne (mont29) wrote:

Any reason why this code is not OSX-specific then? with appropriate #ifdef?

I asked that same question in one of my comments above. My code isn't, because the existing code isn't. This is only a very minor addition. Wouldn't make sense to make just my code macOS-specific and leave the existing code as is, IMHO.

@Leon Zandman (lzandman) yes I was indeed thinking about both cases, not only the new one added in this patch.