dotnet publish fails on macOS on ARM

Hi,
I tried to publish an image from a fresh web app, but hit an ObjectDisposedException:
❯ dotnet new web -n test-container
The template "ASP.NET Core Empty" was created successfully.

Processing post-creation actions...
Restoring /Users/dkBjMaMa/test-container/test-container.csproj:
  Determining projects to restore...
  Restored /Users/dkBjMaMa/test-container/test-container.csproj (in 30 ms).
Restore succeeded.

❯ cd test-container

❯ dotnet add package Microsoft.NET.Build.Containers
...

❯ dotnet publish --os linux --arch x64 -c Release -p:PublishProfile=DefaultContainer
MSBuild version 17.4.0+18d5aef85 for .NET
  Determining projects to restore...
  Restored /Users/User/test-container/test-container.csproj (in 83 ms).
  test-container -> /Users/User/test-container/bin/Release/net7.0/linux-x64/test-container.dll
  test-container -> /Users/User/test-container/bin/Release/net7.0/linux-x64/publish/
  Building image 'test-container' with tags 1.0.0 on top of base image mcr.microsoft.com/dotnet/aspnet:7.0
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018: The "CreateNewImage" task failed unexpectedly. [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018: System.ObjectDisposedException: Cannot access a disposed object. [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018: Object name: 'System.Security.Cryptography.SHA256+Implementation'. [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at System.Security.Cryptography.HashAlgorithm.TransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount) [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at System.Security.Cryptography.CryptoStream.FlushFinalBlockAsync(Boolean useAsync, CancellationToken cancellationToken) [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at System.Security.Cryptography.CryptoStream.FlushFinalBlock() [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at System.Security.Cryptography.CryptoStream.Dispose(Boolean disposing) [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at System.IO.Stream.Close() [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Layer.HashDigestGZipStream.Dispose(Boolean disposing) in D:\a\_work\1\s\Microsoft.NET.Build.Containers\Layer.cs:line 143 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Layer.FromFiles(IEnumerable`1 fileList) in D:\a\_work\1\s\Microsoft.NET.Build.Containers\Layer.cs:line 59 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Layer.FromDirectory(String directory, String containerPath) in D:\a\_work\1\s\Microsoft.NET.Build.Containers\Layer.cs:line 24 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Tasks.CreateNewImage.Execute() in D:\a\_work\1\s\Microsoft.NET.Build.Containers\CreateNewImage.cs:line 212 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.2.7/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask) [/Users/User/test-container/test-container.csproj]

My system:
System Version: macOS 13.0 (22A380)
Model Identifier: MacBookPro18,1
Chip: Apple M1 Pro

❯ dotnet --info
.NET SDK:
 Version:   7.0.100
 Commit:    e12b7af219

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  13.0
 OS Platform: Darwin
 RID:         osx.13-arm64
 Base Path:   /usr/local/share/dotnet/sdk/7.0.100/

Host:
  Version:      7.0.0
  Architecture: arm64
  Commit:       d099f075e4

.NET SDKs installed:
  6.0.402 [/usr/local/share/dotnet/sdk]
  7.0.100 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.10 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.10 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

This is really interesting - The code looks like what I'd expect to see here (no direct invocation of a disposed object):
    {
        long fileSize;
        Span<byte> hash = stackalloc byte[SHA256.HashSizeInBytes];
        byte[] uncompressedHash;

        string tempTarballPath = ContentStore.GetTempFile();
        using (FileStream fs = File.Create(tempTarballPath))
        {
            using (HashDigestGZipStream gz = new(fs, leaveOpen: true))
            {
                using (TarWriter writer = new(gz, TarEntryFormat.Gnu, leaveOpen: true))
                {
                    foreach (var item in fileList)
                    {
                        // Docker treats a COPY instruction that copies to a path like `/app` by
                        // including `app/` as a directory, with no leading slash. Emulate that here.
                        string containerPath = item.containerPath.TrimStart(PathSeparators);

                        writer.WriteEntry(item.path, containerPath);
                    }
                } // Dispose of the TarWriter before getting the hash so the final data get written to the tar stream

                uncompressedHash = gz.GetHash();
            }

            fileSize = fs.Length;

            fs.Position = 0;

            SHA256.HashData(fs, hash);
        }

and we're explicitly not closing any of the streams until the outer filestream exits the scope.  @rainersigwald any thoughts on this one?  Wondering if tere might be some kind of ARM64 interaction on hashing on macOS?

Yeah, that tracks quite well with what I've figured out too. @eerhardt any hope this immediately rings a bell for you?

Hmm, I tried to repro this on my macOS Ventura M1, but it didn't repro.
@aeons - does it repro for you every time? Can you pass /bl to the command and attach a binlog?

I wonder if an exception is happening inside
https://github.com/dotnet/sdk-container-builds/blob/4ed9915125c422af4927ee6092f550b44928619b/Microsoft.NET.Build.Containers/Layer.cs#L44-L58
but before gz.GetHash() is being called. GetHash() will call
https://github.com/dotnet/sdk-container-builds/blob/4ed9915125c422af4927ee6092f550b44928619b/Microsoft.NET.Build.Containers/Layer.cs#L134
Which will set the _finalBlockTransformed flag on CryptoStream. If GetHash() is never called (because an exception occurred), Dispose() will still be called, which tries to call FlushFinalBlock() here:
https://github.com/dotnet/runtime/blob/264d7391ec9f6e698051db0621c5e090d0ae4710/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CryptoStream.cs#L750-L753
But in our Dispose method, we are disposing the hashAlgorithm first, and then calling sha256Stream.Dispose():
https://github.com/dotnet/sdk-container-builds/blob/4ed9915125c422af4927ee6092f550b44928619b/Microsoft.NET.Build.Containers/Layer.cs#L138-L143
Since the hashAlgorithm is already disposed, it is throwing ODE.
I think the fix here is to flip around the ordering of calling Dispose
https://github.com/dotnet/sdk-container-builds/blob/4ed9915125c422af4927ee6092f550b44928619b/Microsoft.NET.Build.Containers/Layer.cs#L138-L143
Dispose the hash256Stream first, then dispose the hashAlgorithem.
cc @bartonjs  - to see check to make sure what I'm saying makes sense. CryptoStream takes a ICryptoTransform, but doesn't dispose of it. It is up to the caller to dispose of it, but it must dispose of the ICryptoTransform after it disposes the CryptoStream instance.

I think a better fix would be to drop CryptoStream altogether.  And SHA256 while you're at it.  (IncrementalHash.CreateForHash(HashAlgorithmName.SHA256) is designed for this scenario).

@aeons - does it repro for you every time? Can you pass /bl to the command and attach a binlog?

I encounted the same issue on a real project I tried to use container builds in, and so far it happens every time.
I have a binlog. From my looking at it, it doesn't contain any more information on the exception. I can email it to you if you are interested, but I would prefer not to share it here.

@aeons I just merged the PR that would fix this - if you wouldn't mind, could you try the auto-generated package from GitHub Packages (instructions here) and see if that resolves your issue?

It still fails, but now it seems we get the real exception:
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018: The "CreateNewImage" task failed unexpectedly. [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018: System.IO.IOException: Undefined error: 0 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Interop.Sys.GetGroupName(UInt32 gid) [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at System.Formats.Tar.TarWriter.ConstructEntryForWriting(String fullPath, String entryName, FileOptions fileOptions) [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Layer.FromFiles(IEnumerable`1 fileList) in /home/runner/work/sdk-container-builds/sdk-container-builds/Microsoft.NET.Build.Containers/Layer.cs:line 54 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Layer.FromDirectory(String directory, String containerPath) in /home/runner/work/sdk-container-builds/sdk-container-builds/Microsoft.NET.Build.Containers/Layer.cs:line 24 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.NET.Build.Containers.Tasks.CreateNewImage.Execute() in /home/runner/work/sdk-container-builds/sdk-container-builds/Microsoft.NET.Build.Containers/CreateNewImage.cs:line 100 [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() [/Users/User/test-container/test-container.csproj]
/Users/User/.nuget/packages/microsoft.net.build.containers/0.3.0-alpha.12/build/Microsoft.NET.Build.Containers.targets(124,9): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask) [/Users/User/test-container/test-container.csproj]

Ok, this I expect is because we're using the 'convenience' wrappers for constructing Tar Entry objects, and those wrappers do user/group lookups. We've got an item logged to move away from this and do more explicit creation of the file and directory entries (partially because of this, partially to support explicit user/group mapping for rootless container support), but we haven't done that work quite yet. See https://github.com/dotnet/sdk-container-builds/issues/160 for the discussion around that.

Interesting.  I wonder why that fails on macOS.

FYI - @carlossanlop
Should we re-open this issue? Or open a new one? (Or move it to dotnet/runtime since the exception is coming from the TarWriter?)

Makes sense to reopen - @aeons' still blocked from the end result, even if you did address a blocker they found.

Any updates on this? Also seeing errors when trying to use this with macOS on ARM.

I am also experiencing this issue on a Mac (but this one's Intel)
error MSB4018: The "CreateNewImage" task failed unexpectedly.
error MSB4018: System.IO.IOException: No such file or directory
error MSB4018:    at Interop.Sys.GetGroupName(UInt32 gid)
error MSB4018:    at System.Formats.Tar.TarWriter.ConstructEntryForWriting(String fullPath, String entryName, FileOptions fileOptions)
error MSB4018:    at Microsoft.NET.Build.Containers.Layer.FromFiles(IEnumerable`1 fileList) in D:\a\_work\1\s\Microsoft.NET.Build.Containers\Layer.cs:line 55
error MSB4018:    at Microsoft.NET.Build.Containers.Layer.FromDirectory(String directory, String containerPath) in D:\a\_work\1\s\Microsoft.NET.Build.Containers\Layer.cs:line 25
error MSB4018:    at Microsoft.NET.Build.Containers.Tasks.CreateNewImage.Execute() in D:\a\_work\1\s\Microsoft.NET.Build.Containers\CreateNewImage.cs:line 106
error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)

Sorry folks, no update on this one. The root cause is this line - instead of letting the TAR APIs construct a TarEntry for us, we'd need to manually create the group/user - it seems the metadata on the existing files isn't correct somehow on macOS. Neither Rainer nor I have easy access to a macOS device, though, so it will likely be slow going.

@carlossanlop - why doesn't TarWriter work "correctly" in this case?

@baronfel Thanks for pointing out the crashing line! I can repro in a simple example and created an issue (which automatically got linked above) with the report.
Additionally, it does look like explicitly creating TarEntry instances and calling the overload of TarWriter.WriteEntry that takes a TarEntry works for me. But as I noted in the bug report, there's a lot of convenience code in the crashing overload, so depending on the types of entries you need to create for the container archive, this might be a pain.

Luckily we should only need to make Directory and RegularFile entries (at least the last time I took a look at this). I think this works on Windows (as you noted) because on Windows the group/user data is mostly elided. Thanks for writing up the issue on Runtime - you wouldn't by chance be interested in trying to fix it here would you? :fishing_pole_and_fish:

The problem is reported in the callstack to be coming from this line specifically: https://github.com/dotnet/runtime/blob/0ca364767715fee7bc9d0273eec4f30a3c6cc8f0/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs#L85
It seems we are getting an ERRNO 0. According to the docs, it is a possible value for a failure to retrieve a groupname or a groupid: https://linux.die.net/man/3/getgrgid_r
I assume we're using getgrgid_r, the thread-safe version of that method. It is supported by MacOSX: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/getgrgid_r.3.html
Since the method that captures the ERRNO is returning null (L85 above), we are throwing here: https://github.com/dotnet/runtime/blob/9d44b9bd4eddd5e523a44056c722bcc93bccca0e/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetGroupName.cs#L20
So my question is for @baronfel: what groupname/groupid is expected here? It seems it does not exist. Either we get the group created before executing all this tar code, or as you suggested, manually construct the entry.

The User and group information should come from the User metadata of the base image being build upon - this may be empty! When the container is run, the container runtime is responsible for mounting the layer contents as the user specified by the --user option if one is specified, otherwise the metadata from the image takes precedence.
I wonder if we're been wrong all along in our tooling - perhaps there should almost never be an inferred user or group assignment!

manually construct the entry.

I'd really like to avoid this plan to address this issue, if we can. I think it may just be masking a bug in the TarWriter itself. Anyone else using TarWriter.WriteEntry on macOS will hit the same problem.

you wouldn't by chance be interested in trying to fix it here would you?

Heh; I locally made the requisite changes to Layer.cs and LocalDocker.cs (plus some additional changes for the DOTNET_ROOT environment variable, which isn't set when dotnet is installed to the default location), but dotnet test reports 5 failures in Test.Microsoft.NET.Build.Containers.Filesystem.dll.
The output is a little hard to read; it seems like there's some interleaving of Docker output with test output... but there are two main types of errors:

Unexpected return codes from Docker—I don't really know Docker and haven't done much more than install it, so perhaps I've missed some expected system configuration?
The same tar-creation crash happening in what appears to be a cached nuget version of the Build.Containers code... 🫤

Here's a gist with the full test output, if it's helpful:
https://gist.github.com/jacobcarpenter/0b105dc62030a6f3af8bd88b1258d6b2

Anyone else using TarWriter.WriteEntry on macOS will hit the same problem.

Yes; correct. Should this Issue title be edited to remove the "on ARM" qualifier?

manually construct the entry.

I'd really like to avoid this plan to address this issue, if we can. I think it may just be masking a bug in the TarWriter itself. Anyone else using TarWriter.WriteEntry on macOS will hit the same problem.

I can submit a fix for WriteEntry to not fail to create the entry if the group isn't found and the return value is 0, and I can request backporting it to .NET 7 too. Sounds good?

I think that solution would be ok - from what I can see the name is really what matters. If the group or user name is present in the entry, then tar will attempt to use the matching user/group from the destination system during extraction. Only if it can't find the name does it then defer to the numeric identifier, with a final fallback to the currently-executing group/user if neither is found.

I can submit a fix for WriteEntry to not fail to create the entry if the group isn't found and the return value is 0, and I can request backporting it to .NET 7 too.

Fixing the underlying issue so others don't run into it sounds good to me.
I'm not sure if it needs to be backported to .NET 7 or not. But if you think so, I wouldn't object.

Thanks both.

Only if it can't find the name does it then defer to the numeric identifier

Is this necessary? The tar entry header has a standard field gid. That field would already have the numeric value stored. I am of the opinion that we should just leave the gname field empty if it wasn't possible to find a matching group name for that gid.

with a final fallback to the currently-executing group/user if neither is found.

Do we want this to happen? This WriteEntry method is writing an entry from an existing file in the filesystem, which already has a user owner and a group owner. I am of the opinion that we should not tamper with the information that we tried to retrieve from the filesystem. Instead, we should just leave empty what we couldn't fully collect.

I'm not sure if it needs to be backported to .NET 7 or not

@baronfel I think you can answer the question since sdk-container-builds is the customer - Would this repo benefit from backporting such fix to .NET 7, or should it just stay in main for .NET 8?

defaults for naming

I agree that leaving it unmodified is the best course of action for the .NET API - I was just trying to relate the behavior of tar during extraction as some context that I thought was interesting. Sorry to muddy the water :)

need for backport

We would benefit a ton from a fix here - it seems like many (all?) of our users on macOS are hitting this wall and only able to succeed using Windows or Linux CI machines. For 7.0.2xx this feature is easily lit up for WebSDK projects, and for 7.0.3xx we're targeting full, automatic support for all project types via shipping in the .NET SDK, so I'd hope that usage of the tool (and therefore errors from macOS users) would also go up.

This fix is in the runtime and has been merged and backported, so I'll close this issue for now. Thank you all for the report, discussion, and thank you @carlossanlop for the fix!


Combine create_lignin.py and plot_bond_formation.py

This will be complete when the latest pull request is approved


Report out-of-memory and any other malloc failures better already within CRYPTO_malloc and friends

As partly discussed already in #5781 and #6217:
Currently there are some 900 places within crypto/ and ssl/ where in case CRYPTO_malloc() and friends returned NULL an error report with the reason ERR_R_MALLOC_FAILURE is pushed on the ERR queue (rather than the allocation function doing this itself). This approach is not only pretty redundant and cumbersome but also more or less inaccurate and error-prone:

Assuming that the callers of the allocation function(s) are responsible for flagging memory allocation failure, it easily happens that callers simply forget (or do not care) to do so. There are (or at least were) many occurrences of such false negatives.
Indirect callers of functions that involve memory allocation can also report false positives regarding ERR_R_MALLOC_FAILURE where they misinterpret the reason for obtaining a NULL pointer in case this was not actually due to an malloc failure but due to some other issue (see, e.g.., two instances fixed in #6217).
It also happens that indirect callers of functions that involve memory allocation re-report ERR_R_MALLOC_FAILURE after obtaining a NULL pointer though the malloc failure has already been reported by a function in between (see, e.g.., two instances fixed in #6217 caused by the same calls PKCS12err(PKCS12_F_PKCS12_SAFEBAG_CREATE_PKCS8_ENCRYPT, ERR_R_MALLOC_FAILURE); as in the item before).
If a function like CRYPTO_malloc() does the error reporting itself this relieves all its callers from doing so.
A function like CRYPTO_malloc() can do the failure/error reporting more accurately than its callers because it knows, in contrast to them, whether the failure is due to memory shortage or due to other issues like unsuitable arguments.

When I asked why CRYPTO_malloc() does not do the error reporting itself I learned about the following reasons.

An malloc failure could either be because the system is running low on memory, or the call to CRYPTO_malloc() was erroneous (e.g. because it incorrectly attempts to allocate a vast amount of memory). In the second case this kind of problem would be difficult to find if all you have is the error code in CRYPTO_malloc().
In the same direction goes the desire to preserve the file/line numbers (and if possible also the function name) of the caller(s) for being able to trace.
A entirely different issue is that the ERR reporting may in turn need memory allocation (in case its internal queue allocated so far has become full), which may cause a loop in case memory is exhausted.

Here are some thoughts on these issues and suggestions how to deal with them.

In case of an actual out-of-memory situation it would not help to obtain information about the caller because this is a low-level issue related to the execution environment (rather than to the caller).
In case of other reasons for malloc failures the malloc functions should give specific error reasons (which the callers cannot do), such as an overly large amount of memory requested or any internal allocation function issue. In case the caller gave an unsuitable argument it may be helpful to know which caller it was, but this is a very general issue that would better be solved more generally (such as, by providing a stack trace as far as possible). When specific value is seen in reporting malloc failures within some calling functions of course this can be done in addition to a more low-level reporting by the malloc function itself.
In order to remove the obstacle of a potential loop with the ERR queue, it should be possible to adapt its implementation such that there is always an element pre-allocated and reserved for use by malloc functions in case there is actually shortage of memory, such that in case the malloc function adds an ERR_R_OUT_OF_MEMORY entry there is no need to (recursively) allocate it. When an out-of-memory error has already been reported (and thus the reserved cell has been filled) any further requests to add elements to the ERR queue should be ignored in case the queue buffer is full and would need re-allocation (supposedly this would be acceptable because the entry regarding the out-of-memory error is already in the queue and at least from the library's point of view an out-of-memory condition is fatal anyway).

In case the caller gave an unsuitable argument it may be helpful to know which caller it was,

As pointed out in #6217, CRYPTO_malloc() actually knows who the caller is because it gets passed the file and line number of the caller in its arguments. So it seems we could record information about the caller from inside the CRYPTO_malloc() implementation.

It is often the case that malfunction starts at a single malloc failure, and it is good to
have the complete call stack when that happens, becase you cannot tell where exactly
the error handling is broken, it is usually not the direct caller of CRYPTO_malloc.

Maybe, but as I said, if you only know that you have a malfunction that started at a specific
malloc call, then you have not enough data to track down the bug.  There is no replacement
for proper error handling at all.  That includes recoding file names / line numbers along
the whole call stack until the error condition is resolved.

Hmmm... but pushing all these file names and line numbers on the error stack after a malloc failure will most certainly fail. So unless you keep track of the call stacks before the error occurs (you would have to do it all the time, which is a waste) or rely on os methods to walk the stack, the best you can get is probably a single malloc error message (which is much better than no error message at all). And in that case, I don't see any reason why this could not have been done by CRYPTO_malloc() itself.

It is often the case that malfunction starts at a single malloc failure, and it is good to
have the complete call stack when that happens, becase you cannot tell where exactly
the error handling is broken, it is usually not the direct caller of CRYPTO_malloc.

I see error reporting (and in particular the place where malloc failure reporting is done, which is the topic of this issue) and error handling as different things.
Of course, if a (direct or indirect) caller of an malloc function fails to check the obtained pointer for NULL or does some other related mistake,, this is a major issue, but not one for which the malloc function would be responsible. The malloc function might include the file name and line number of its (direct) caller in its error reports, but I see this only as a secondary extra service.

In other words, my suggestion is basically just to remove the need to have all those
XYZerr(fn, ERR_R_MALLOC_FAILURE) invocations, essentially by adding
    if (ret == NULL)
        CRYPTOerr(CRYPTO_F_CRYPTO_MALLOC, ERR_R_MALLOC_FAILURE);

to CRYPTO_malloc() and friends.
Of course, the checks for NULL need to stay at the callers' side, such that a typical invocation would simplify from
    ptr = CRYPTO_malloc(size);
    if (ptr == NULL) {
        XYZerr(fn, ERR_R_MALLOC_FAILURE);
        goto err;
    }

to
    ptr = CRYPTO_malloc(size);
    if (ptr == NULL)
        goto err;

Or is there any real drawback or unsolvable obstacle for this?
Of course, a callers would still be free to keep their XYZerr(fn, ERR_R_MALLOC_FAILURE); if that is felt to be of any help.

Use ERR_put_error inside the allocation routines (look at how CRYPTOerr is defined).
That still has to address the error-in-error-handler loop issue.

Use ERR_put_error inside the allocation routines (look at how CRYPTOerr is defined).
That still has to address the error-in-error-handler loop issue.

Of course.
I just checked and found that ERR_put_error(), as already mentioned by @bernd-edlinger https://github.com/openssl/openssl/issues/5781#issuecomment-377124586, invokes ERR_get_state() which in turn calls OPENSSL_zalloc() when called for the first time. After that, in contrast to what I expected, the queue is never re-allocated. This actually simplifies the solution to the loop issue. I see two approaches:

make sure, as already suggested by @FdaSilvaYY https://github.com/openssl/openssl/issues/5781#issuecomment-377389199, that the dynamic allocation always happens during statup (where memory shortage does not need to be expected)
or make the whole ERR_STATE structure (rather than just the state pointer to it) a static element of the memory of the CRYPTO_THREAD, such that it is automatically allocated already on loading the library.

Yes, the interesting question is, how you want to make sure that the error state
is always allocated whenever a new thread starts and how to prevent the new thread
from running into an undefined state, when that allocation fails.

Additional question: how to allocate the error state for threads that are already running
before OPENSSL_init_crypto is called for the first time.

Is there maybe a possibility to statically assign a memory area (of moderate size, in this case sizeof(ERR_STATE)) to each new thread of an application that uses OpenSSL?

How many static areas should we create?  Do folks think the API doesn't malloc internally?
Move on.

There are technically much simpler solutions to the ERR/malloc loop problem.
How about replacing in ERR_get_state() the use of OPENSSL_zalloc() by malloc() and memset()
(and consequently replacing in ERR_STATE_free() the OPENSSL_free() call by free())?
Or even more straightforward and certainly acceptable:
Replace in ERR_get_state() the use of OPENSSL_zalloc() by a variant of OPENSSL_malloc() that does not invoke CRYPTOerr() (while the default variant would do the error reporting as sketched above) and  then, as above, use memset().

This issue came up again on openssl-users:
https://mta.openssl.org/pipermail/openssl-users/2021-August/014207.html


Invalid buffers list after change session

After load another session FzfxBuffers shows only one buffer. For example, Telescope buffers show all session buffers.
Used session maganer: Possession

hi @damanis , it's late in night (in my timezone), I will look into it later.

@linrongbin16 ,
I just opened issue. Maybe I'll se it on weekend.

hi @damanis, if this issue didn't happen before, just show up in recent days, then it could be introduced in #526.
To prove this thought, could you help try the previous tag 'v4.1.2'? it's the latest version before that PR.

@linrongbin16
Confirm, tagv4.1.2 works properly, show all buffers after load new session.

hi @damanis , then it must be the PR's change, I will create one more data source, one is the old one (include the unload buffers), one is the PR's behavior (exclude the unload buffers).

@linrongbin16

one is the old one (include the unload buffers), one is the PR's behavior (exclude the unload buffers).

If I understand correctly, you will create two tags with different behavior? Why just not get buffers list in every call of FzfxBuffers?

recommend users to specify version/tag when install this plugin

But in this case the features added after the tag will be unavailable.

recommend users to specify version/tag when install this plugin

But in this case the features added after the tag will be unavailable.

I will upgrade the major version number (for example v4 - v5) when we have a break change.
only lazy.nvim support the semver, you can use v4.* to accept following updates in v4. and upgrade to v5 when you  want.

4.1.2 has no buffers problem, 4.1.3 has one. If select 4.1.2, so all changes in 4.1.{4-*} will be unavailable, isn't it?

4.1.2 has no buffers problem, 4.1.3 has one. If select 4.1.2, so all changes in 4.1.{4-*} will be unavailable, isn't it?

I think it's yes.

So, again, why just not get buffers list in every call of FzfxBuffers? It will solve both problem.
Or, maybe, nvim provides some additional event (fzf-lua and telescope buffers show proper list).

hi @damanis, the fix pr is merged, please pull and try again.

@linrongbin16
Confirmed.
Happy New Year!

hi @savchenko, sorry previous #522 #526 is been reverted in #536, but I will create another variant buffers.


[Godot4 a2]Base class can't recognize Sub class parameter

Godot version
v4.0.alpha2.official [79077e6c1]
System information
Window2 10
Issue description

Steps to reproduce
Please run the reproduction project.
Minimal reproduction project
SubClsTest.zip

I can't reproduce this in current master, I assume it has already been fixed.


WindowManager.ShowDialog never returns nulled bool

From a window requesting close after having been opened with ShowDialog on WindowManager, the returned result from ShowDialog will never be null. I tried RequestClose(); and RequestClose(null); and both instead return false.

WindowManager.ShowDialog just wraps WPF's Window.ShowDialog(), and returns the same value. Calling RequestClose(x) sets the window's DialogResult to x.
From the docs, it looks like Window.ShowDialog() will never return null, which means that WindowManager.ShowDialog will never return null. I don't think there's anything I can do about this: changing the signature of that method would be a breaking change.

Closing as no response.


In C2W2 Optional Labs, there aren't any labs for the "Back Propagation" section in Andrew NG's course

Describe the Error
In "C2W2 Optional labs" folder, there aren't any labs for the "Back Propagation" section in Andrew NG's Coursera course
To Reproduce
Steps to reproduce the behavior:

Go to https://github.com/greyhatguy007/Machine-Learning-Specialization-Coursera/tree/main/C2%20-%20Advanced%20Learning%20Algorithms/week2/optional-labs
Scroll down to the list of ipynb files
There aren't any labs for "Back Propagation"

Expected behavior
I expected two more optional labs, one about "Derivatives" and another about "Back propagation"
Screenshots
This is what was shown Andrew NG's Machine Learning Specialization course on Coursera

Desktop (please complete the following information):

OS: Windows 10
Browser: Chrome
Version: 112.0.5615.138 (Official Build) (64-bit)

Additional context
I'm just wondering if you forgot to add these "Derivatives" and "Back Propagation" optional labs. If this was intended, I apologize.

Will add it soon.
Contributions are welcome.

Actually, the course was updated a few months ago, and the back propagation was added in the update. This repository has all the contents before that update.

resolved in PR #30


sync terminal buffer's cwd with shell's pwd

In gnome-terminal/xfce4-terminal, it's handy to create a new terminal tab with pwd set to the current terminal's pwd. To let vim's terminal enumerator have this feature, we should set a buffer's cwd to the shell's pwd.
In current version of vim, when a new terminal instance is started, the cwd will be set to ~.  On Linux, when there is the change of current working directory, the shell will notify the terminal by the OSC 7; command. Use this feature to chdir buffer's cwd to the shell's working directory.

I do not see a change to the help files - what will the user experience?
Changing directories can have side effects.  Perhaps it should be possible to disable this feature?
Or it should be disabled by default and only used when the user wants it?

I do not see a change to the help files - what will the user experience?
Changing directories can have side effects. Perhaps it should be possible to disable this feature?
Or it should be disabled by default and only used when the user wants it?

Thank you for the suggestion! Change applied.

@brammool glib provides g_filename_from_uri to convert a file URI to a path which is exactly what I need. Since vim (vim-tiny) isn't supposed to depend on glib, now I have two choices,

borrow the code of g_filename_from_uri (~100 lines) from glib
only enable this feature for glib

Which one do you prefer?

@brammool glib provides g_filename_from_uri to convert a file URI to a path which is exactly what I need. Since vim (vim-tiny) isn't supposed to depend on glib, now I have two choices,

borrow the code of g_filename_from_uri (~100 lines) from glib
only enable this feature for glib

Which one do you prefer?

In the patch I see syn_cwd() already doing this.  Is something missing
here and you want to replace it?
I believe glib is GPL licensed, thus we can't just copy that code.  You
can look at the code and rewrite it (leave out parts we don't need,
etc.).
There are many places where glib won't be available, disabling this
feature there would be disappointing.
The uri_decode() I see in the patch mentions it's coming from github.
Did you check the license on that code?  It's also possible to rewrite
that code.  You need to use "char_u" instead of "unsigned char", for
example.  And we don't use "const" much in Vim code.
--
Eight Megabytes And Continually Swapping.
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

On Wed, Feb 03, 2021 at 07:29:45AM -0800, Bram Moolenaar wrote:

@brammool glib provides g_filename_from_uri to convert a file URI to a path which is exactly what I need. Since vim (vim-tiny) isn't supposed to depend on glib, now I have two choices,

borrow the code of g_filename_from_uri (~100 lines) from glib
only enable this feature for glib

Which one do you prefer?

In the patch I see syn_cwd() already doing this.  Is something missing
here and you want to replace it?

It now can only deal with one case, i.e., file://path. It can't deal
with g_filename_from_uri file://host/path or other corner cases as shown
in the test suite for g_filename_from_uri [1].
[1] http://web.mit.edu/barnowl/src/glib/glib-2.16.3/tests/uri-test.c

I believe glib is GPL licensed, thus we can't just copy that code.  You
can look at the code and rewrite it (leave out parts we don't need,
etc.).

Thanks for reminding me about the license issue.

There are many places where glib won't be available, disabling this
feature there would be disappointing.

I'll re-write uri_decode based on glib's uri_decode then:)

The uri_decode() I see in the patch mentions it's coming from github.
Did you check the license on that code?  It's also possible to rewrite
that code.  You need to use "char_u" instead of "unsigned char", for
example.  And we don't use "const" much in Vim code.

Thanks again for reminding me about the license issue and also the
programming tips for Vim.

--
Eight Megabytes And Continually Swapping.
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/vim/vim/pull/6290#issuecomment-772595015

--
Best regards,
Coiby

@brammool It turns out we needn't to deal with all the corner cases listed in http://web.mit.edu/barnowl/src/glib/glib-2.16.3/tests/uri-test.c since we can assume the emitted OSC 7 escape sequence is correctly uri-encoded. If there's something wrong, whoever emits the OSC 7 escape sequence should take the responsibility. So the license issues have been resolved.
I've also fixed the test failure for FreeBSD. But there is one failed test on macos (huge, clang). Unfortunately, I don't have a Mac system to test on.

The name 'autochdirsh' is hard to read.  How about making it 'autoshelldir' ?
Please use the Vim usual C code formatting.  E.g. "} else {" is written in three lines.
"char buffer[3] = {src[i+1], src[i+2], 0};"  I'm not sure all C90 compilers accept this.
You can use the hexhex2nr() function.
syn_cwd() should be renamed to sync_shell_dir().
The test should probably CheckNotBSD now.

On Tue, Mar 02, 2021 at 11:25:58AM -0800, Bram Moolenaar wrote:

The name 'autochdirsh' is hard to read.  How about making it 'autoshelldir' ?
Please use the Vim usual C code formatting.  E.g. "} else {" is written in three lines.
"char buffer[3] = {src[i+1], src[i+2], 0};"  I'm not sure all C90 compilers accept this.
You can use the hexhex2nr() function.
syn_cwd() should be renamed to sync_shell_dir().

Thanks for the suggestions all of which have been applied.
hexhex2nr also reminds me previously the test case didn't cover
part of code in url_decode. So I've added some percent-code
characters (Chinese "中文") to the path in the test.

The test should probably CheckNotBSD now.

Yes, the reason is FreeBSD's sh doesn't play well with OSC 7 escape
sequence. If we use bash for the test in FreeBSD, the test could
pass. Since vim use sh for testing, the test has to be disabled for
FreeBSD.

--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/vim/vim/pull/6290#issuecomment-789154518

--
Best regards,
Coiby

For the change in parse_osc(), we should perhaps always recognize the code, but ignore it when p_asd is not set.
Or is the code dropped anyway when we don't recognize it?

You need to adjust the #ifdef for hexhex2nr(). See test failures.

Thanks. I have used FEAT_AUTOSHELLDIR to enable hexhex2nr to fix this kind of test failures.
commit<PHONE_NUMBER>355c185e7810e67d52802453576e81 ("patch 8.2.2627: no need to check for BSD after checking for not root") removed CheckNotBSD. What can I do to skip the tests for BSD (CheckNotRoot doesn't work)?

You need to adjust the #ifdef for hexhex2nr(). See test failures.

Thanks. I have used FEAT_AUTOSHELLDIR to enable hexhex2nr to fix this kind of test failures.
commit<PHONE_NUMBER>355c185e7810e67d52802453576e81 ("patch 8.2.2627: no
need to check for BSD after checking for not root") removed
CheckNotBSD. What can I do to skip the tests for BSD (CheckNotRoot
doesn't work)?

You can restore that check if you need to use it.
I see a test failure on Linux, looks like there is still something to
fix.
--
"Space is big.  Really big.    I mean, you may think it's a long way down the
road to the chemist, but that's just peanuts to space."
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///                                                                      \
\\        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Looks OK now.  We don't need a feature for this, just check that the 'autoshelldir' option works.

https://github.com/vim/vim/commit/8b9abfd86c736ed3f298dfa8b50a962c118b3983#r48888218
// should not recover to default value later?

not sure (or not understand) the user case of this change too much,
but the doc of this change mentioned to source '/etc/profile.d/vte.sh' (ubuntu20.04 is vte-2.91.sh), BTW: be careful to do this, this looks had bug or flaw, PROMPT_COMMAND="__vte_prompt_command" may overwrite your previous 'PROMOT_COMMAND', and if it was not login shell then maynot find '__vte_prompt_command', note the order or timing of/to 'source'......

8b9abfd#r48888218
// should not recover to default value later?

Do you mean there should be an "set noacd" after finishing the test?

not sure (or not understand) the user case of this change too much,
but the doc of this change mentioned to source '/etc/profile.d/vte.sh' (ubuntu20.04 is vte-2.91.sh), BTW: be careful to do this, this looks had bug or flaw, PROMPT_COMMAND="__vte_prompt_command" may overwrite your previous 'PROMOT_COMMAND', and if it was not login shell then maynot find '__vte_prompt_command', note the order or timing of/to 'source'......

Thanks! It seems I should have mentioned this issue in the doc which according to https://gitlab.gnome.org/GNOME/vte/-/issues/37 is still not resolved.

Looks OK now. We don't need a feature for this, just check that the 'autoshelldir' option works.

Thanks! I'm not sure I understand you correctly. Do you mean FEAT_AUTOSHELLDIR is not needed?


Split render from editor instantiation

Drop the element argument from editor instantiation.
Should this perhaps be editor.append?
Fixes https://github.com/bustlelabs/content-kit-editor/issues/88

Was adding an option like renderImmediately: false considered? Using an extra call to render seems like it may come as a mild surprise to an implementer; the 80% use case seems like it would be to instantiate an editor and immediately render it.
Otherwise :+1:

I'd prefer one flow over making immediate render the default case. IMO Having a default render behavior encourages us to be lazy about what things look like before rendering has occurred. Several APIs should fail if you have not yet rendered.
Having the editor pre-render allows the user to call configuration methods like disableInput without us also needing to translate those into options.

@bantic let us continue to discuss. I'm going to merge and cut a release to move forward though.


Release 0.18.0

Description & motivation

Release v0.18.0 changes from next into current
Includes changes from: #302, #314, #315, #319, #323, #329, #339, #328, #344, #286, #363
Update v0.18.0 migration guide

@clrcrl @drewbanin Thanks for the review! No need to make any changes re: the Algolia values or the prerelease banner. Those are conditional in Netlify based on whether it is building the next branch, so they won't affect docs.getdbt.com.


[1.9.9] Cannot resize splitted windows with linux

Prerequisites

[X] Can you reproduce the problem in safe mode?
[X] Are you running the latest version of Atom?
[X] Did you check the debugging guide?
[X] Did you check the FAQs on Discuss?
[X] Are you reporting to the correct repository?
[X] Did you perform a cursory search to see if your bug or enhancement is already reported?

Description
On Linux you cannot resize the splitted window size.
Steps to Reproduce

Open Atom
Open two files
Drag one file to the right
Try to resize the splitted windows and make the right smaller/larger
You do not see any "resize curser".

Versions
Atom    : 1.9.9
Electron: 1.3.4
Chrome  : 52.0.2743.82
Node    : 6.3.0
Running archlinux:
atom 1.9.9-2
Linux arch 4.4.19-1-lts #1 SMP Sat Aug 20 21:58:36 CEST 2016 x86_64 GNU/Linux
xfce4
A side note: I can resize the directory panel on the left, so Its not a bug of my desktop environment. It happens vertically and horizontally. I've also tried this on windows lately and this works. This came with some version of 1.9.x, 1.8.x worked before.

Your Atom is packaged with an unsupported electron newer version. If your check out the electron upgrade PR you can get some clue on how to fix this.

Versions
Atom    : 1.9.9
Electron: 0.37.8
Chrome  : 49.0.2623.75
Node    : 5.10.0
I just tested this with my Fedora 24 installation and managed to resize the window with no issues.

Electron: 1.3.4

As @Ben3eeE said, that is an unsupported Electron version.  Please uninstall your version of Atom and reinstall from https://atom.io.
https://github.com/atom/design-decisions/blob/master/unofficial-atom-distributions.md

The element <atom-pane-resize-handle> is misplaced by Chromium 52 (Electron 1.3) due to the change explained here. I fixed it with this patch, but the fix is not compatible with Electron 0.37.


Bulk Implementation

Description
This PR implements the use of bulk operations for Azure Spring Data CosmosDB SaveAll and DeleteAll functions. Tests were added where gaps exist in testing functionality.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
All SDK Contribution checklist:

[x] The pull request does not introduce [breaking changes]
[x] CHANGELOG is updated for new features, bug fixes or other significant changes.
[x] I have read the contribution guidelines.

General Guidelines and Best Practices

[x] Title of the pull request is clear and informative.
[x] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

[x] Pull request includes test coverage for the included changes.

API change check
APIView has identified API level changes in this PR and created following API reviews.
azure-spring-data-cosmos
azure-spring-data-cosmos

/azp run java - spring - tests

/azp run java - spring - tests

/azp run java - spring - tests

/azp run java - spring - tests

/azp run java - spring - tests


[Refactor] check base table in MV

Why I'm doing:

Most code doesn't check the existence of base table, which would throw NPE if it doesn't exist

What I'm doing:

mayGetTable return an Optional to enforce check
getTableChecked throw SemanticException which contains table name
unify materialized view exception and error message to MaterializedViewExceptions
refactor all referencing code

Fixes #issue
What type of PR is this:

[ ] BugFix
[ ] Feature
[ ] Enhancement
[x] Refactor
[ ] UT
[ ] Doc
[ ] Tool

Does this PR entail a change in behavior?

[ ] Yes, this PR will result in a change in behavior.
[x] No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

[ ] Interface/UI changes: syntax, type conversion, expression evaluation, display information
[ ] Parameter changes: default values, similar parameters but with different default values
[ ] Policy changes: use new policy to replace old one, functionality automatically enabled
[ ] Feature removed
[ ] Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

[ ] I have added test cases for my bug fix or my new feature
[ ] This pr needs user documentation (for new or modified features or behaviors)

[ ] I have added documentation for my new feature or new function

[ ] This is a backport pr

Bugfix cherry-pick branch check:

[x] I have checked the version labels which the pr will be auto-backported to the target branch

[x] 3.2
[x] 3.1
[x] 3.0
[x] 2.5

@Mergifyio backport branch-2.5 branch-3.0 branch-3.1 branch-3.2


Add Sublime Text logo

Please? :)

I couldn't find this in vector/svg yet. I'll try again soon.

The home page actually has the .svg
https://www.sublimetext.com/
https://www.sublimetext.com/images/logo.svg

Added in c122ccfcfdb15d9958a85696ff2460ac3b01f8ca
Thanks


Submission URL error handling fix

Fixes #1553
Simple fix raising the correct error in case of invalid submission URL

Can you please rebase and resolve the conflicts?

Can you please rebase and resolve the conflicts?

Done

Can you please also squash your commits?

Can you please also squash your commits?

My bad, I should've done this in the first place. It's done now!

Thanks for the PR!

Thanks for the PR!

Happy to help!


Eliminate the redundant control flow

The optimizer module has several unnecessary if statements inside. We can simplify such control flows by eliminating them.
It relates to https://github.com/prestosql/presto/pull/4300.

@Lewuathe Please rebase

@losipiuk Thanks. I rebased.

merged, thanks!


DebugConfigurationProvider: provide snippets for launch.json

VSCode Version: 1.16.1
OS Version: mac

Steps to Reproduce:
https://github.com/Microsoft/vscode/issues/12735#issuecomment-329381584
@weinand I'm writing a extension to provide debug  configurations.
as mentioned at docs "initialConfigurations": "extension.eggjs.provideInitialConfigurations"
my question is how to link a command to configurationSnippets?
had try "configurationSnippets": ["${command.extension.eggjs.provideConfigurationSnippets}"] and "configurationSnippets": ["extension.eggjs.provideConfigurationSnippets"] without luck.
the command is not trigger.

We are moving away from this model.
The new way of doing things is using a ConfigurationProvider. More about this can be found here
Your feature request could be solved by extending the ConfigurationProvider to also provide snippets, thus renaming the issue

Since the new introduce DebugConfigurationProvider addresses this need, we can close this request.

Reopening since the DebugConfigurationProvider does not (yet) support to provide snippets.

@weinand additional 2 request:

DebugConfigurationProvider should support compounds
allow hidden or sort order at debug configuration drop list.

We only want to show user: Egg Debug and Egg Debug with brk(which is compounds)
so feature request is either provide a hidden property for "Egg Master", "Egg Agent", "Egg Worker", or compounds should show at top.
{
  // Use IntelliSense to learn about possible Node.js debug attributes.
  // Hover to view descriptions of existing attributes.
  // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
  "version": "0.2.0",
  "configurations": [
    {
      "type": "node",
      "request": "launch",
      "name": "Egg Debug",
      "runtimeExecutable": "npm",
      "runtimeArgs": [ "run", "debug" ],
      "console": "integratedTerminal",
      "restart": true,
      "protocol": "auto",
      "port": 9999
    },
    {
      "type": "node",
      "request": "launch",
      "name": "Egg Master",
      "runtimeExecutable": "npm",
      "runtimeArgs": [ "run", "debug", "--", "--inspect-brk" ],
      "console": "integratedTerminal",
      "restart": true,
      "protocol": "inspector",
      "port": 9229
    },
    {
      "type": "node",
      "request": "attach",
      "name": "Egg Agent",
      "restart": true,
      "protocol": "inspector",
      "port": 5800
    },
    {
      "type": "node",
      "request": "attach",
      "name": "Egg Worker",
      "restart": true,
      "protocol": "inspector",
      "port": 9999
    }
  ],
  "compounds": [
    {
      "name": "Egg Debug with brk",
      "configurations": [ "Egg Master", "Egg Agent", "Egg Worker" ]
    }
  ]
}

@atian25 please don't add new feature requests onto existing but unrelated issues. They will be ignored. Create new issues.

I believe this can be done using the CompletionItemProvider API.
Simply register a CompletionItemProvider only for launch.json file using this API. And your CompletionItemProvider can returtn CompletionItems which inserts a SnippetString as an insertText.


Feature request: add voice control to PC use demo

Could we add a voice control to the PC use demo ?

You could try to point Claude to: https://github.com/ufal/whisper_streaming and ask it whether it sees a way to integrate it with streamlit.py


Copy pubspec.lock file

Opening to track a codereview comment by @jonasfj:
We probably should copy in both pubspec.yaml and pubspec.lock.
Assuming the idea is to make a layer that can be cached and reuse when rebuilding the image.
@athomas pointed out that the old image did: ONBUILD ADD pubspec.* /app/

We need to handle the cases where there is a pubspec.lock (checked-in or generated from a local run of pub) and where there isn't (e.g. on build system that just checks out the code and immediately runs docker build).

@jonasfj's comment referred to the example for the Dart Functions Framework. I think we chose originally not to copy pubspec.lock since you write a library function lib/functions.dart and then we generate the app with a builder. For the canonical structure of a function app, see the examples as well as the templates for the dartfn command. However, since ultimately we are building an app, I think @jonasfj is probably right and should revisit this decision -- building an app should be deterministic for the user who likely does want to pin dependencies to what has been tested; what do you think, @kevmoo?
Regarding using ONBUILD, we want to avoid this for an official image. The official images maintainers provide a rationale for this and suggest instead providing an example Dockerfile in the published docs. In the case of the Dart Functions Framework, we generate this Dockerfile when the function app is generated using dartfn. We might want to also update stagehand to do this automatically (or with a --dockerfile option).

Re ONBUILD: we're all in agreement on this. Nobody is arguing for ONBUILD it just happens that the old images used this.
I think production apps would use lockfiles. Otherwise, your deployment may pick up a different dependency resolution from what passed testing just because someone published a new, potentially broken, version of one of your dependencies.

Yes, for apps the recommendation is to checkin .lock files:
https://dart.dev/guides/libraries/private-files#pubspeclock

This is probably what we want:
COPY pubspec.* /app/
RUN dart pub get
COPY . .

Sounds like we all agree that apps should have the .lock file copied to the docker build context. Again, will just note the original reason for not doing that for the functions framework example was because the user was only supposed to write a function under in lib. Once we moved to the builder model for generating a full app (generating bin/server.dart) that is bundled together and copied into the Dockerfile, we should have also ensured that ensured that the generated .lock file was also copied. I'll open an issue in the Functions Framework repo so we can fix this.

COPY pubspec.* /app/

Because app is the workdir, is that the same as COPY pubspec.* ./?

Yes. 👍

The examples for the official image use COPY pubspec.* ./. So as far as this repo is concerned, the issue can be closed. Please re-open if you disagree.


Graph every prometheus metric we check (depends on metrics names clear)

git-grepping for Coda_metrics might be useful to this end.


Look up custom type cast operators by name instead of full toString

Summary:
Today we look up custom type cast operator in CastExpr using the full toString instead of just the type name.
Looking at the implementation of getCustomTypeCastOperator, it looks up the CustomTypeFactories for the
type, and this mapping is based on type name, so passing in the return value of toString is technically
incorrect, although in practice toString() normally just returns name() unless it's a complex type.
The larger problem comes with https://github.com/facebookincubator/velox/pull/7256 toString is called on
every batch at every level of  the type tree on both the to and from types.  This problem existed prior that PR,
but with that change it becomes a significant performance issue for large row types in particular.
============================================================================
[...]pression/benchmarks/CastBenchmark.cpp     relative  time/iter   iters/s
============================================================================
scalar                                                     13.69ns    73.06M
renameSmallStruct                                         231.17ns     4.33M
renameLargeStruct                                          76.32us    13.10K
smallStructNested                                         275.48ns     3.63M
largeStructNested                                          90.98us    10.99K

after this change
============================================================================
[...]pression/benchmarks/CastBenchmark.cpp     relative  time/iter   iters/s
============================================================================
scalar                                                     14.85ns    67.32M
renameSmallStruct                                           8.03ns   124.50M
renameLargeStruct                                         215.92ns     4.63M
smallStructNested                                          44.93ns    22.26M
largeStructNested                                          13.99us    71.46K

note renameLargeStruct is several hundred times faster
Differential Revision: D51206341

presto fuzzer failure unrelated:
IN list must not be empty


Fixed theme bug

Hello. I noticed an issue with the theme. When you open the website for the first time or you open it with the light mode saved from local storage, then the cards have this appearance:

Clicking on the sun fixes the appearance of the cards, but it does not switch to dark mode as expected.
I made a few changes that I think solves this issue. Let me know if there is anything I still need to change. Thanks.

Hello. I noticed an issue with the theme. When you open the website for the first time or you open it with the light mode saved from local storage, then the cards have this appearance:
Clicking on the sun fixes the appearance of the cards, but it does not switch to dark mode as expected.
I made a few changes that I think solves this issue. Let me know if there is anything I still need to change. Thanks.

Looks good, thank you for fixing this issue!


Vulnerable shared libraries might make kivy vulnerable. Can you help upgrade to patch versions?

Hi, @tito , @Zen-CODE , I'd like to report a vulnerability issue in kivy_2.1.0.
Dependency Graph between Python and Shared Libraries

Issue Description
As shown in the above dependency graph, kivy_2.1.0 directly or transitively depends on 22 C libraries (.so). However, I noticed that some C libraries are vulnerable, containing the following CVEs:
libjpeg-43436dd5.so.62.1.0 from C project libjpeg-turbo(version:1.3.0) exposed 1 vulnerabilities:
CVE-2018-14498
libtiff-b6eb22fb.so.5.2.0 from C project tiff(version:<=4.0.6) exposed 11 vulnerabilities:
CVE-2020-35523 , CVE-2019-7663, CVE-2019-14973, CVE-2018-10963, CVE-2017-9935, CVE-2016-9537, CVE-2016-9538,  CVE-2016-9534, CVE-2016-9536, CVE-2016-9540, CVE-2016-9535
libwebp-2877905a.so.4.0.2 from C project libwebp(version:<=0.4.0) exposed 11 vulnerabilities:
CVE-2018-25009, CVE-2018-25014, CVE-2018-25013, CVE-2018-25012, CVE-2018-25011, CVE-2018-25010, CVE-2020-36331, CVE-2020-36330, CVE-2020-36328, CVE-2020-36329, CVE-2020-36332
Furthermore, the vulnerable methods in these vulnerable shared libraries can be actually invoked by Python code.
For instance, the following C methods are actually invoked by python code in kivy, which declared in lib\sdl2.pxi:
...
cdef extern from "SDL_image.h":
    ...
    cdef int IMG_Init(IMG_InitFlags flags)
    cdef char *IMG_GetError()
    cdef SDL_Surface *IMG_Load(char *file)
    cdef SDL_Surface *IMG_Load_RW(SDL_RWops *src, int freesrc)
...

The following call chain starting from int IMG_Init(IMG_InitFlags flags) can reach the vulnerable methods:

int gtTileContig (TIFFRGBAImage *img, uint32 *raster, uint32 w, uint32 h) in tif_getimage.c reported by  CVE-2020-35523 :

call chain -----
IMG_Init() -> IMG_InitTIF() -> TIFFReadRGBAImage() -> TIFFReadRGBAImageOriented() -> TIFFRGBAImageBegin() -> PickContigCase() -> gtTileContig()

int TIFFWriteDirectoryTagTransferfunction (TIFF *tif, uint32 *ndir, TIFFDirEntry *dir) in tif_dirwrite.c reported by  CVE-2019-7663:

call chain -----
IMG_Init() -> IMG_InitTIF() -> TIFFClientOpen() -> TIFFCleanup() -> TIFFFlush() -> TIFFRewriteDirectory() -> TIFFWriteDirectory() -> TIFFWriteDirectorySec() -> TIFFWriteDirectoryTagTransferfunction()

int gtStripSeparate (TIFFRGBAImage *img, uint32 *raster, uint32 w, uint32 h) in tif_getimage.c reported by  CVE-2019-14973 :

call chain -----
IMG_Init() -> IMG_InitTIF() -> TIFFReadRGBAImage() -> TIFFReadRGBAImageOriented() -> TIFFRGBAImageBegin() -> PickSeparateCase() -> gtStripSeparate()

Suggested Vulnerability Patch Versions
libjpeg-turbo has fixed the vulnerabilities in versions >=2.0.0
tiff has fixed the vulnerabilities in versions >=4.1.0
libwebp has fixed the vulnerabilities in versions >=1.0.1
Python build tools cannot report vulnerable C libraries, which may induce potential security issues to many downstream Python projects.
As a popular python package (kivy has 150,107 downloads per month), could you please upgrade the above shared libraries to their patch versions?
Thanks for your help~
Best regards,
Andy

We get these libraries automatically from the OS (centos 7) on which kivy is compiled and ourselves don't actually compile these so we can't really provide updated versions of these libraries if the OS hasn't updated them.
More importantly, I don't think this is part of our threat model. I think it's understood, especially for python packages, that only you are supposed to control what gets passed to these libraries. Meaning that since you're fully in control of the app you make and its inputs you shouldn't be in a position where you e.g. unknowingly can load a bad image that would cause these issues. But, if you make an app using Kivy that is distributed and your users could load random images and be exposed to an attack, then you should probably compile kivy yourself and ensure that it's updated as Kivy itself is not kept updated with any security issues, beyond yearly releases.

Thank you very much for your kindly feedback. @matham

To corroborate @matham, the only security issue vis-a-vis Kivy would be if Kivy or subcomponents thereof (e.g., Buildozer, python-for-android) contained a narrow dependency on a maximum version of a vulnerable C library and all such versions were vulnerable. So, something like
libtiff <= 4.0.6 in a setup.py installer or Buildozer recipe. That probably isn't the case; this can probably be safely closed.
It's also pertinent to note that @matham appears to be a Main Coon (source: have two Maine Coons) communicating with the feisty kitten that is @Andy201709. Joy is cat avatars on GitHub.

Closing as this seems to be out of Kivy's scope.
{I really appreciate the quality of this analysis though!]


[Module Proposal]: Role assignments

Have you checked this module does not already exist in the registry?
Yes
Why is the module needed?
It would be great to have a module to simplify role assignments.
Module path
identity/role-assignment
Describe the module
The module contains a role assignment resource. It takes an array of role definition IDs as a parameter to enable batch role assignments.

Not sure why this was closed but it is needed.
Modules being published are not consistent on whether they include assigning RBAC or not. Key Vault does, but the Storage Account module does not.
If you publish modules that include the RBAC/IAM baked in that is more to maintain on the module (in my view). Publishing a Role Assignment module puts the ownership on the user consuming it for assigning access to resources being deployed. (We are doing this in our private registry.)
If this is re-opened, please allow the user-friendly name of the role and do not make users lookup the IDs.

I think the issue is that we need to finish https://github.com/azure/bicep/issues/2246 in order to make a module like this realistic.

If this is re-opened, please allow the user-friendly name of the role and do not make users lookup the IDs.

Agreed we should get this done. That is tracked here: https://github.com/Azure/bicep/issues/1895


Update privacy documentation

Minor additions to our privacy statements, and switching to an aka.ms link for instructions to disable telemetry reporting (so it's easier to update later if we decide to rename it again)

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
6 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.008%) to 41.805%

Files with Coverage Reduction
New Missed Lines
%

extensions/notebook/src/dialog/configurePython/configurePathPage.ts
1
57.02%

extensions/sql-bindings/src/common/azureFunctionsUtils.ts
1
80.65%

extensions/notebook/src/jupyter/serverInstance.ts
4
75.5%

Totals

Change from base Build<PHONE_NUMBER>:
-0.008%

Covered Lines:
30269

Relevant Lines:
67046

💛  - Coveralls


Extract range function to fc.util.range

It's turning up in almost every file now -
        // ordinal axes have a rangeExtent function, this adds any padding that
        // was applied to the range. This functions returns the rangeExtent
        // if present, or range otherwise
        function range(scale) {
            return scale.rangeExtent ? scale.rangeExtent() : scale.range();
        }

Looks like d3 has the same problem. Might be worth pulling over their scaleExtent function as well (not sure when it's useful though)?

Good point, I was considering doing that myself ... as I cut and paste the three-line comment that accompanied it ...

This was implemented a while back as fc.util.scale.range.


Check other apps versions before reinstalling them

Usually third party apps only need to be reinstalled if there are new versions of them, which speeds up session initialization.

Lgtm

Good timing on this. I'll publish this and get it into 1.8.0


make /timeline-dashboard consistent between build and paint profiles

The simplified timeline currently only works with debugProfilePaintsEnabled but not with debugProfileBuildsEnabled.


refactor(doc site): audit home page for a11y issues

Fixes #720
Description
Run an accessibility audit through the home page of the documentation site.
Testing in sage-lib

Navigate to locahost
Verify that the home page has no regressions
Important - confirm the page passes accessibilty audits

Testing in kajabi-products

N/A - sage-lib only

@pixelflips - good catch. I got caught up in the organization of main and slapped the id in the wrong spot! I'll adjust it and move it up to the correct section (to contain the full page contents). Thanks for taking a look!

@joedakroub  - Yup! 😬 I am in the process of refactoring this a bit! Thank you for the review!

@pixelflips - implementing grid here is beautiful and painstaking all at the same time. I am going to revert some layout changes in an effort to address the more straightforward a11y changes. Thank you for continuing to dig in and find this issues!

@pixelflips - implementing grid here is beautiful and painstaking all at the same time. I am going to revert some layout changes in an effort to address the more straightforward a11y changes. Thank you for continuing to dig in and find this issues!

@monicawheeler happy to help and it definitely seems less straightforward than it should be. Let me know if I can help and feel free to request another review once it's ready.


build chart with app-build-suite

2021-03-02 14:58:15,905 app_build_suite.build_steps.helm ERROR: kube-linter run failed with exit code 1
2021-03-02 14:58:15,905 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/nmi-daemonset.yaml: (object: default/azure-ad-pod-identity-app-nmi apps/v1, Kind=DaemonSet) container "nmi" has DROP capabilities: [], but does not drop capability "NET_RAW" which is required (check: drop-net-raw-capability, remediation: NET_RAW grants an application within the container the ability to craft raw packets, use raw sockets, and it also allows an application to bind to any address. Please specify to drop this capability in the containers under containers security contexts.)
2021-03-02 14:58:15,905 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/nmi-daemonset.yaml: (object: default/azure-ad-pod-identity-app-nmi apps/v1, Kind=DaemonSet) container "nmi" does not have a read-only root file system (check: no-read-only-root-fs, remediation: Set readOnlyRootFilesystem to true in your container's securityContext.)
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/nmi-daemonset.yaml: (object: default/azure-ad-pod-identity-app-nmi apps/v1, Kind=DaemonSet) container "nmi" is privileged (check: privileged-container, remediation: Don't run your container as privileged unless required.)
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/nmi-daemonset.yaml: (object: default/azure-ad-pod-identity-app-nmi apps/v1, Kind=DaemonSet) container "nmi" is not set to runAsNonRoot (check: run-as-non-root, remediation: Set runAsUser to a non-zero number, and runAsNonRoot to true, in your pod or container securityContext. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ for more details.)
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/mic-deployment.yaml: (object: default/azure-ad-pod-identity-app-mic apps/v1, Kind=Deployment) object has 2 replicas but does not specify inter pod anti-affinity (check: no-anti-affinity, remediation: Specify anti-affinity in your pod spec to ensure that the orchestrator attempts to schedule replicas on different nodes. You can do this by using podAntiAffinity, specifying a labelSelector that matches pods of this deployment, and setting the topologyKey to kubernetes.io/hostname. See https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#inter-pod-affinity-and-anti-affinity for more details.)
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/mic-deployment.yaml: (object: default/azure-ad-pod-identity-app-mic apps/v1, Kind=Deployment) container "mic" does not have a read-only root file system (check: no-read-only-root-fs, remediation: Set readOnlyRootFilesystem to true in your container's securityContext.)
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: helm/azure-ad-pod-identity-app/templates/mic-deployment.yaml: (object: default/azure-ad-pod-identity-app-mic apps/v1, Kind=Deployment) container "mic" is not set to runAsNonRoot (check: run-as-non-root, remediation: Set runAsUser to a non-zero number, and runAsNonRoot to true, in your pod or container securityContext. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ for more details.)
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR:
2021-03-02 14:58:15,906 app_build_suite.build_steps.helm ERROR: Error: found 7 lint errors

From what I heard, we could turn this off if we want it, @piontec, right?

From what I heard, we could turn this off if we want it, @piontec, right?
Put a file like this one in chart's dir and name the tests you want to exclude:
https://github.com/giantswarm/app-build-suite/blob/master/examples/apps/hello-world-app/.kube-linter.yaml

Do we need to set catalog URL in main.yaml or will architect-orb do it?

architect-orb specifies the catalog url through flag, so the value in the yaml file will be overwritten


api.add_route() give 200 OK without proper method when a class is used for endpoint.

Here is the code.
import responder

api = responder.API()

class some_empty_api:
    pass

api.add_route('/route/{anything}', endpoint=some_empty_api)

if __name__ == '__main__':
    api.run()

This code returns 200 OK to any incoming request like http://root_url/route/some_string.
Is this a default behavior when a class is used for the endpoint or an unintended one?
If is a default one, is there any way to avoid this and make it more "strict" so that the api returns 200 OK only when proper method exists?

Thanks for all your attention payed to my question!


Pessing q in the GPT buffer exits vim

Hi, with several buffers open and one of them being the GPT Log, when pressing q in the GPT buffer, vim exits cleanly. Expectation would be that only the GPT buffer is closed.
Any idea what could be the problem?
NVIM v0.9.0
Build type: Release
LuaJIT 2.1.0-beta3

should be fixed now :)

you can repen the issue if the issue persists.


update dev dependencies

:warning: Please install the  to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.76%. Comparing base (a0f5143) to head (4358a2c).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@           Coverage Diff           @@
##           master      #15   +/-   ##
=======================================
  Coverage   94.76%   94.76%
=======================================
  Files          12       12
  Lines         802      802
=======================================
  Hits          760      760
  Misses         42       42

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.