Skip to content

Commit

Permalink
Address PR feedback for #14243
Browse files Browse the repository at this point in the history
  • Loading branch information
anthony-c-martin committed Jun 7, 2024
1 parent 91b64d4 commit 84b8147
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 18 deletions.
13 changes: 3 additions & 10 deletions src/Bicep.Cli/Commands/PublishProviderCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,15 @@ public class PublishProviderCommand : ICommand
{
private readonly IModuleDispatcher moduleDispatcher;
private readonly IFileSystem fileSystem;
private readonly IFeatureProviderFactory featureProviderFactory;
private readonly IOContext ioContext;
private readonly ILogger logger;

public PublishProviderCommand(
IOContext ioContext,
ILogger logger,
IModuleDispatcher moduleDispatcher,
IFileSystem fileSystem,
IFeatureProviderFactory featureProviderFactory)
IFileSystem fileSystem)
{
this.moduleDispatcher = moduleDispatcher;
this.fileSystem = fileSystem;
this.featureProviderFactory = featureProviderFactory;
this.ioContext = ioContext;
this.logger = logger;
}

public async Task<int> RunAsync(PublishProviderArguments args)
Expand All @@ -49,8 +42,8 @@ public async Task<int> RunAsync(PublishProviderArguments args)
return null;
}

var data = BinaryData.FromStream(fileSystem.FileStream.New(PathHelper.ResolvePath(binaryPath), FileMode.Open, FileAccess.Read, FileShare.Read));
return new(architecture, data);
using var binaryStream = fileSystem.FileStream.New(PathHelper.ResolvePath(binaryPath), FileMode.Open, FileAccess.Read, FileShare.Read);
return new(architecture, BinaryData.FromStream(binaryStream));
}

await ioContext.Error.WriteLineAsync("The 'publish-provider' CLI command group is an experimental feature. Experimental features should be enabled for testing purposes only, as there are no guarantees about the quality or stability of these features. Do not enable these settings for any production usage, or your production environment may be subject to breaking.");
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Emit/TemplateWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,7 @@ private void EmitModuleForLocalDeploy(PositionTrackingJsonTextWriter jsonWriter,

this.EmitDependsOn(emitter, module.DependsOn);

// Since we don't want to be mutating the body of the original ObjectSyntax, we create an placeholder body in place
// Since we don't want to be mutating the body of the original ObjectSyntax, we create a placeholder body in place
// and emit its properties to merge decorator properties.
foreach (var property in ApplyDescription(module, ExpressionFactory.CreateObject(ImmutableArray<ObjectPropertyExpression>.Empty)).Properties)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Registry/LocalModuleRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ protected override void WriteArtifactContentToCache(LocalModuleReference referen
{
if (SupportedArchitectures.TryGetCurrent() is not {} architecture)
{
throw new InvalidOperationException($"Unsupported architecture: {RuntimeInformation.ProcessArchitecture}");
throw new InvalidOperationException($"Failed to determine the system OS or architecture to execute provider extension \"{reference}\".");
}

if (entity.Provider.Binaries.SingleOrDefault(x => x.Architecture.Name == architecture.Name) is not {} binary)
{
throw new InvalidOperationException($"Unsupported architecture: {RuntimeInformation.ProcessArchitecture}");
throw new InvalidOperationException($"The provider extension \"{reference}\" does not support architecture {architecture.Name}.");
}

var binaryUri = GetProviderBinUri(reference);
Expand Down
11 changes: 6 additions & 5 deletions src/Bicep.Core/Registry/OciArtifactRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -407,17 +407,18 @@ protected override void WriteArtifactContentToCache(OciArtifactReference referen
null;

// if the artifact supports local deployment, fetch the provider binary
if (config?.LocalDeployEnabled == true)
if (config?.LocalDeployEnabled == true &&
config?.SupportedArchitectures is {} binaryArchitectures)
{
if (SupportedArchitectures.TryGetCurrent() is not {} architecture)
{
throw new InvalidOperationException($"Unsupported architecture: {RuntimeInformation.ProcessArchitecture}");
throw new InvalidOperationException($"Failed to determine the system OS or architecture to execute provider extension \"{reference}\".");
}

var layerName = BicepMediaTypes.GetProviderArtifactLayerV1Binary(architecture);
if (result.TryGetSingleLayerByMediaType(layerName) is not {} sourceData)
if (binaryArchitectures.Contains(architecture.Name) ||
result.TryGetSingleLayerByMediaType(BicepMediaTypes.GetProviderArtifactLayerV1Binary(architecture)) is not {} sourceData)
{
throw new InvalidOperationException($"Unsupported architecture: {RuntimeInformation.ProcessArchitecture}");
throw new InvalidOperationException($"The provider extension \"{reference}\" does not support architecture {architecture.Name}.");
}

using var binaryStream = sourceData.ToStream();
Expand Down

0 comments on commit 84b8147

Please sign in to comment.