From 1414aafb74a5a033e5c7611d8ebd5410f05038ff Mon Sep 17 00:00:00 2001 From: Manfred Wallner Date: Thu, 20 Apr 2017 09:47:41 +0200 Subject: [PATCH 01/26] (GH-1258) Global mutex for config Previously, multiple Chocolatey processes running could affect or corrupt the chocolatey.config file depending on how they accessed the file. Implement a single global mutex (process-wide) on serialization and deserialization of the xml configuration file to ensure the file stays consistent. --- src/chocolatey/chocolatey.csproj | 1 + .../infrastructure/services/XmlService.cs | 150 ++++++++++-------- .../synchronization/GlobalMutex.cs | 97 +++++++++++ 3 files changed, 179 insertions(+), 69 deletions(-) create mode 100644 src/chocolatey/infrastructure/synchronization/GlobalMutex.cs diff --git a/src/chocolatey/chocolatey.csproj b/src/chocolatey/chocolatey.csproj index 579ed49788..ee2e352d48 100644 --- a/src/chocolatey/chocolatey.csproj +++ b/src/chocolatey/chocolatey.csproj @@ -110,6 +110,7 @@ + diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 3be9ca0ff4..9615f61f0b 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -24,6 +24,7 @@ namespace chocolatey.infrastructure.services using cryptography; using filesystem; using tolerance; + using chocolatey.infrastructure.synchronization; /// /// XML interaction @@ -32,6 +33,7 @@ public sealed class XmlService : IXmlService { private readonly IFileSystem _fileSystem; private readonly IHashProvider _hashProvider; + private const int MUTEX_TIMEOUT = 1000; public XmlService(IFileSystem fileSystem, IHashProvider hashProvider) { @@ -41,110 +43,120 @@ public XmlService(IFileSystem fileSystem, IHashProvider hashProvider) public XmlType deserialize(string xmlFilePath) { - return FaultTolerance.try_catch_with_logging_exception( + return FaultTolerance.retry(3, () => GlobalMutex.enter( () => { - var xmlSerializer = new XmlSerializer(typeof(XmlType)); - using (var fileStream = _fileSystem.open_file_readonly(xmlFilePath)) - using (var fileReader = new StreamReader(fileStream)) - using (var xmlReader = XmlReader.Create(fileReader)) + return FaultTolerance.try_catch_with_logging_exception( + () => { - if (!xmlSerializer.CanDeserialize(xmlReader)) + var xmlSerializer = new XmlSerializer(typeof(XmlType)); + using (var fileStream = _fileSystem.open_file_readonly(xmlFilePath)) + using (var fileReader = new StreamReader(fileStream)) + using (var xmlReader = XmlReader.Create(fileReader)) { - this.Log().Warn("Cannot deserialize response of type {0}", typeof(XmlType)); - return default(XmlType); - } + if (!xmlSerializer.CanDeserialize(xmlReader)) + { + this.Log().Warn("Cannot deserialize response of type {0}", typeof(XmlType)); + return default(XmlType); + } - try - { - return (XmlType)xmlSerializer.Deserialize(xmlReader); - } - catch(InvalidOperationException ex) - { - // Check if its just a malformed document. - if (ex.Message.Contains("There is an error in XML document")) + try + { + return (XmlType)xmlSerializer.Deserialize(xmlReader); + } + catch (InvalidOperationException ex) { - // If so, check for a backup file and try an parse that. - if (_fileSystem.file_exists(xmlFilePath + ".backup")) + // Check if its just a malformed document. + if (ex.Message.Contains("There is an error in XML document")) { - using (var backupStream = _fileSystem.open_file_readonly(xmlFilePath + ".backup")) - using (var backupReader = new StreamReader(backupStream)) - using (var backupXmlReader = XmlReader.Create(backupReader)) + // If so, check for a backup file and try an parse that. + if (_fileSystem.file_exists(xmlFilePath + ".backup")) { - var validConfig = (XmlType)xmlSerializer.Deserialize(backupXmlReader); - - // If there's no errors and it's valid, go ahead and replace the bad file with the backup. - if(validConfig != null) + using (var backupStream = _fileSystem.open_file_readonly(xmlFilePath + ".backup")) + using (var backupReader = new StreamReader(backupStream)) + using (var backupXmlReader = XmlReader.Create(backupReader)) { - _fileSystem.copy_file(xmlFilePath + ".backup", xmlFilePath, overwriteExisting: true); - } + var validConfig = (XmlType)xmlSerializer.Deserialize(backupXmlReader); - return validConfig; + // If there's no errors and it's valid, go ahead and replace the bad file with the backup. + if (validConfig != null) + { + _fileSystem.copy_file(xmlFilePath + ".backup", xmlFilePath, overwriteExisting: true); + } + + return validConfig; + } } } + + throw; } - - throw; } - } - }, - "Error deserializing response of type {0}".format_with(typeof(XmlType)), - throwError: true); + }, + "Error deserializing response of type {0}".format_with(typeof(XmlType)), + throwError: true); + + }, MUTEX_TIMEOUT)); } public void serialize(XmlType xmlType, string xmlFilePath) { - serialize(xmlType,xmlFilePath, isSilent: false); + serialize(xmlType, xmlFilePath, isSilent: false); } public void serialize(XmlType xmlType, string xmlFilePath, bool isSilent) { _fileSystem.create_directory_if_not_exists(_fileSystem.get_directory_name(xmlFilePath)); - FaultTolerance.try_catch_with_logging_exception( + FaultTolerance.retry(3, () => GlobalMutex.enter( () => { - var xmlSerializer = new XmlSerializer(typeof(XmlType)); - - // Write the updated file to memory - using(var memoryStream = new MemoryStream()) - using(var streamWriter = new StreamWriter(memoryStream, encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: true))) + FaultTolerance.try_catch_with_logging_exception( + () => { - xmlSerializer.Serialize(streamWriter, xmlType); - streamWriter.Flush(); - - memoryStream.Position = 0; - - // Grab the hash of both files and compare them. - var originalFileHash = _hashProvider.hash_file(xmlFilePath); - if (!originalFileHash.is_equal_to(_hashProvider.hash_stream(memoryStream))) + var xmlSerializer = new XmlSerializer(typeof(XmlType)); + + // Write the updated file to memory + using (var memoryStream = new MemoryStream()) + using (var streamWriter = new StreamWriter(memoryStream, encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: true))) { - // If there wasn't a file there in the first place, just write the new one out directly. - if(string.IsNullOrEmpty(originalFileHash)) + xmlSerializer.Serialize(streamWriter, xmlType); + streamWriter.Flush(); + + memoryStream.Position = 0; + + // Grab the hash of both files and compare them. + var originalFileHash = _hashProvider.hash_file(xmlFilePath); + if (!originalFileHash.is_equal_to(_hashProvider.hash_stream(memoryStream))) { - using(var updateFileStream = _fileSystem.create_file(xmlFilePath)) + // If there wasn't a file there in the first place, just write the new one out directly. + if (string.IsNullOrEmpty(originalFileHash)) { - memoryStream.Position = 0; - memoryStream.CopyTo(updateFileStream); + using (var updateFileStream = _fileSystem.create_file(xmlFilePath)) + { + memoryStream.Position = 0; + memoryStream.CopyTo(updateFileStream); + _fileSystem.write_file(xmlFilePath, () => memoryStream); - return; + return; + } } - } - // Otherwise, create an update file, and resiliently move it into place. - var tempUpdateFile = xmlFilePath + ".update"; - using(var updateFileStream = _fileSystem.create_file(tempUpdateFile)) - { - memoryStream.Position = 0; - memoryStream.CopyTo(updateFileStream); + // Otherwise, create an update file, and resiliently move it into place. + var tempUpdateFile = xmlFilePath + ".update"; + using (var updateFileStream = _fileSystem.create_file(tempUpdateFile)) + { + memoryStream.Position = 0; + memoryStream.CopyTo(updateFileStream); + } + _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup"); } - _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup"); } - } - }, - "Error serializing type {0}".format_with(typeof(XmlType)), - throwError: true, - isSilent: isSilent); + }, + "Error serializing type {0}".format_with(typeof(XmlType)), + throwError: true, + isSilent: isSilent); + }, MUTEX_TIMEOUT)); } } } diff --git a/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs b/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs new file mode 100644 index 0000000000..4b6321b704 --- /dev/null +++ b/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs @@ -0,0 +1,97 @@ +// Copyright © 2017 Chocolatey Software, Inc +// Copyright © 2011 - 2017 RealDimensions Software, LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace chocolatey.infrastructure.synchronization +{ + using System; + using System.Security.AccessControl; + using System.Security.Principal; + using System.Threading; + + /// + /// global mutex used for synchronizing multiple processes based on appguid + /// + /// + /// Based on http://stackoverflow.com/a/7810107/2279385 + /// + public class GlobalMutex : IDisposable + { + private readonly bool _hasHandle = false; + private const string APP_GUID = "bd59231e-97d1-4fc0-a975-80c3fed498b7"; // matches the one in Assembly + private Mutex _mutex; + + private void init_mutex() + { + var mutexId = "Global\\{{{0}}}".format_with(APP_GUID); + _mutex = new Mutex(initiallyOwned: false, name: mutexId); + + var allowEveryoneRule = new MutexAccessRule(new SecurityIdentifier(WellKnownSidType.WorldSid, null), MutexRights.FullControl, AccessControlType.Allow); + var securitySettings = new MutexSecurity(); + securitySettings.AddAccessRule(allowEveryoneRule); + _mutex.SetAccessControl(securitySettings); + } + + private GlobalMutex(int timeOut) + { + init_mutex(); + try + { + _hasHandle = _mutex.WaitOne(timeOut < 0 ? Timeout.Infinite : timeOut, exitContext: false); + + if (_hasHandle == false) + { + throw new TimeoutException("Timeout waiting for exclusive access to value."); + } + } + catch (AbandonedMutexException) + { + _hasHandle = true; + } + } + + public static void enter(Action action, int timeout) + { + using (new GlobalMutex(timeout)) + { + if (action != null) action.Invoke(); + } + } + + public static T enter(Func func, int timeout) + { + var returnValue = default(T); + + using (new GlobalMutex(timeout)) + { + if (func != null) returnValue = func.Invoke(); + } + + return returnValue; + } + + public void Dispose() + { + if (_mutex != null) + { + if (_hasHandle) + { + _mutex.ReleaseMutex(); + } + _mutex.Dispose(); + } + } + } +} From 7854b0da28dbff678b06db2d9c0ce1fc1269f4b1 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:28:55 -0500 Subject: [PATCH 02/26] (GH-1258) xml docs and logs Add logging and xml docs to public items --- .../synchronization/GlobalMutex.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs b/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs index 4b6321b704..89dd1574a5 100644 --- a/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs +++ b/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs @@ -20,6 +20,8 @@ namespace chocolatey.infrastructure.synchronization using System.Security.AccessControl; using System.Security.Principal; using System.Threading; + using logging; + using platforms; /// /// global mutex used for synchronizing multiple processes based on appguid @@ -35,6 +37,7 @@ public class GlobalMutex : IDisposable private void init_mutex() { + this.Log().Debug(ChocolateyLoggers.Trace, "Initializing global mutex"); var mutexId = "Global\\{{{0}}}".format_with(APP_GUID); _mutex = new Mutex(initiallyOwned: false, name: mutexId); @@ -44,11 +47,17 @@ private void init_mutex() _mutex.SetAccessControl(securitySettings); } + /// + /// Initializes a new instance of the class. + /// + /// The time out in milliseconds. + /// Timeout waiting for exclusive access to value. private GlobalMutex(int timeOut) { init_mutex(); try { + this.Log().Debug(ChocolateyLoggers.Trace, "Waiting on the mutext handle for {0} milliseconds".format_with(timeOut)); _hasHandle = _mutex.WaitOne(timeOut < 0 ? Timeout.Infinite : timeOut, exitContext: false); if (_hasHandle == false) @@ -62,6 +71,11 @@ private GlobalMutex(int timeOut) } } + /// + /// Enters the Global Mutex + /// + /// The action to perform. + /// The timeout in seconds. public static void enter(Action action, int timeout) { using (new GlobalMutex(timeout)) @@ -70,6 +84,14 @@ public static void enter(Action action, int timeout) } } + + /// + /// Enters the Global Mutext + /// + /// The return type + /// The function to perform. + /// The timeout in seconds. + /// public static T enter(Func func, int timeout) { var returnValue = default(T); From 985d81a83add384fa59c7f89fe6663dcb936bea5 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:30:13 -0500 Subject: [PATCH 03/26] (GH-1258) skip mutex if non-windows A named mutex cannot be used in mono, it causes errors. If choco is not being run on Windows, skip the named mutex and just allow the function / action to execute. --- .../synchronization/GlobalMutex.cs | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs b/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs index 89dd1574a5..d15abab5df 100644 --- a/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs +++ b/src/chocolatey/infrastructure/synchronization/GlobalMutex.cs @@ -78,12 +78,19 @@ private GlobalMutex(int timeOut) /// The timeout in seconds. public static void enter(Action action, int timeout) { - using (new GlobalMutex(timeout)) + + if (Platform.get_platform() == PlatformType.Windows) + { + using (new GlobalMutex(timeout)) + { + if (action != null) action.Invoke(); + } + } + else { if (action != null) action.Invoke(); } - } - + } /// /// Enters the Global Mutext @@ -96,11 +103,18 @@ public static T enter(Func func, int timeout) { var returnValue = default(T); - using (new GlobalMutex(timeout)) + if (Platform.get_platform() == PlatformType.Windows) + { + using (new GlobalMutex(timeout)) + { + if (func != null) returnValue = func.Invoke(); + } + } + else { if (func != null) returnValue = func.Invoke(); } - + return returnValue; } From 0b836c9d0f998dccd0183e87f99eee9bb727a26e Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:32:44 -0500 Subject: [PATCH 04/26] (GH-1258) Increase xml file timeout Give the MUTEX_TIMEOUT 2 seconds, but also increase the retries by waiting 200 milliseconds and increasing by 200 milliseconds between retries. --- src/chocolatey/infrastructure/services/XmlService.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 9615f61f0b..76a6d6fbec 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -33,7 +33,7 @@ public sealed class XmlService : IXmlService { private readonly IFileSystem _fileSystem; private readonly IHashProvider _hashProvider; - private const int MUTEX_TIMEOUT = 1000; + private const int MUTEX_TIMEOUT = 2000; public XmlService(IFileSystem fileSystem, IHashProvider hashProvider) { @@ -96,7 +96,9 @@ public XmlType deserialize(string xmlFilePath) "Error deserializing response of type {0}".format_with(typeof(XmlType)), throwError: true); - }, MUTEX_TIMEOUT)); + }, MUTEX_TIMEOUT), + waitDurationMilliseconds: 200, + increaseRetryByMilliseconds: 200); } public void serialize(XmlType xmlType, string xmlFilePath) @@ -156,7 +158,9 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen "Error serializing type {0}".format_with(typeof(XmlType)), throwError: true, isSilent: isSilent); - }, MUTEX_TIMEOUT)); + }, MUTEX_TIMEOUT), + waitDurationMilliseconds: 200, + increaseRetryByMilliseconds: 200); } } } From da5365cadd03c60135adb1f853362c656d40123f Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:34:10 -0500 Subject: [PATCH 05/26] (maint) stream write_file opens file allowing share When writing a file, allow shared read access when using a stream. This is similar to the other write_file operations (string). --- src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index c56f29ba6c..faf8893b2b 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -502,7 +502,7 @@ public void write_file(string filePath, string fileText, Encoding encoding) public void write_file(string filePath, Func getStream) { using (Stream incomingStream = getStream()) - using (Stream fileStream = File.Create(filePath)) + using (Stream fileStream = File.Open(filePath, FileMode.OpenOrCreate, FileAccess.Write, FileShare.Read)) { incomingStream.CopyTo(fileStream); fileStream.Close(); From 48e668dc1e8ddc75d6135fb6721b00f7326af9bc Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:34:31 -0500 Subject: [PATCH 06/26] (maint) formatting --- src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index faf8893b2b..ae2da315d1 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -390,7 +390,6 @@ public bool copy_file_unsafe(string sourceFilePath, string destinationFilePath, //} return success != 0; } - public void replace_file(string sourceFilePath, string destinationFilePath, string backupFilePath) { From db91b2a252eba15798daa7f2379bdcf5ac6533bd Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:35:52 -0500 Subject: [PATCH 07/26] (GH-1241) Use write_file for stream writing When writing a file with streams, use the built in methods in IFileSystem for doing so. --- .../infrastructure/services/XmlService.cs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 76a6d6fbec..5d3729f316 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -131,26 +131,19 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen var originalFileHash = _hashProvider.hash_file(xmlFilePath); if (!originalFileHash.is_equal_to(_hashProvider.hash_stream(memoryStream))) { + memoryStream.Position = 0; + // If there wasn't a file there in the first place, just write the new one out directly. if (string.IsNullOrEmpty(originalFileHash)) { - using (var updateFileStream = _fileSystem.create_file(xmlFilePath)) - { - memoryStream.Position = 0; - memoryStream.CopyTo(updateFileStream); - _fileSystem.write_file(xmlFilePath, () => memoryStream); + _fileSystem.write_file(xmlFilePath, () => memoryStream); - return; - } + return; } // Otherwise, create an update file, and resiliently move it into place. var tempUpdateFile = xmlFilePath + ".update"; - using (var updateFileStream = _fileSystem.create_file(tempUpdateFile)) - { - memoryStream.Position = 0; - memoryStream.CopyTo(updateFileStream); - } + _fileSystem.write_file(tempUpdateFile, () => memoryStream); _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup"); } } From f9d4298043d5ef354c5b1dc77e90a7dd8e670394 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 22 May 2017 14:44:18 -0500 Subject: [PATCH 08/26] (GH-1241) More Robust File.Replace Using File.Replace can have some access violation concerns. It demands full access to a file, which can cause issues if the file is locked. If a file is open in another process, it has issues renaming the file, which causes the rest to not work appropriately. Implement file replace methods in a move step that will work with files that are open and then copy the original file to the destination and delete the original file. If the file locked is the backup file, it should log that it was not able to produce the backup and continue without failure. If it has issues removing the source file, it should log that it was not able to delete the file and continue without failure. --- .../filesystem/DotNetFileSystem.cs | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index ae2da315d1..593bb0882b 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -400,7 +400,46 @@ public void replace_file(string sourceFilePath, string destinationFilePath, stri { try { - File.Replace(sourceFilePath, destinationFilePath, backupFilePath); + // File.Replace is very sensitive to issues with file access + // the docs mention that using a backup fixes this, but this + // has not been the case with choco - the backup file has been + // the most sensitive to issues with file locking + //File.Replace(sourceFilePath, destinationFilePath, backupFilePath); + + // move existing file to backup location + if (!string.IsNullOrEmpty(backupFilePath) && file_exists(destinationFilePath)) + { + try + { + this.Log().Debug(ChocolateyLoggers.Trace, "Backing up '{0}' to '{1}'.".format_with(destinationFilePath, backupFilePath)); + + if (file_exists(backupFilePath)) + { + this.Log().Debug(ChocolateyLoggers.Trace, "Deleting existing backup file at '{0}'.".format_with(backupFilePath)); + delete_file(backupFilePath); + } + this.Log().Debug(ChocolateyLoggers.Trace, "Moving '{0}' to '{1}'.".format_with(destinationFilePath, backupFilePath)); + move_file(destinationFilePath, backupFilePath); + } + catch (Exception ex) + { + this.Log().Debug("Error capturing backup of '{0}':{1} {2}".format_with(destinationFilePath, Environment.NewLine, ex.Message)); + } + } + // copy source file to destination + this.Log().Debug(ChocolateyLoggers.Trace, "Copying '{0}' to '{1}'.".format_with(sourceFilePath, destinationFilePath)); + copy_file(sourceFilePath, destinationFilePath, overwriteExisting: true); + + // delete source file + try + { + this.Log().Debug(ChocolateyLoggers.Trace, "Removing '{0}'".format_with(sourceFilePath)); + delete_file(sourceFilePath); + } + catch (Exception ex) + { + this.Log().Debug("Error removing '{0}':{1} {2}".format_with(sourceFilePath, Environment.NewLine, ex.Message)); + } } catch (IOException) { From 58cd65130103f0b0fa0874e32f29c04825cc99de Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Tue, 23 May 2017 12:30:20 -0500 Subject: [PATCH 09/26] (GH-1309) Restrict trace output Trace logging should only occur when tracing is enabled. It can provide a pretty significant amount of logging, so ensure it only happens if someone has enabled `--trace`. --- src/chocolatey.console/Program.cs | 10 +++-- src/chocolatey/GetChocolatey.cs | 2 +- .../runners/ConsoleApplication.cs | 2 +- .../logging/Log4NetAppenderConfiguration.cs | 39 +++++++++++++++---- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/chocolatey.console/Program.cs b/src/chocolatey.console/Program.cs index dbd1f8060f..d7f8f66e68 100644 --- a/src/chocolatey.console/Program.cs +++ b/src/chocolatey.console/Program.cs @@ -51,7 +51,7 @@ private static void Main(string[] args) //no file system at this point if (!Directory.Exists(loggingLocation)) Directory.CreateDirectory(loggingLocation); - Log4NetAppenderConfiguration.configure(loggingLocation); + Log4NetAppenderConfiguration.configure(loggingLocation, excludeLoggerNames: ChocolateyLoggers.Trace.to_string()); Bootstrap.initialize(); Bootstrap.startup(); var license = License.validate_license(); @@ -112,9 +112,11 @@ private static void Main(string[] args) Environment.Exit(config.UnsuccessfulParsing? 1 : 0); } - Log4NetAppenderConfiguration.set_logging_level_debug_when_debug(config.Debug, excludeLoggerName: "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Verbose.to_string())); - Log4NetAppenderConfiguration.set_verbose_logger_when_verbose(config.Verbose, config.Debug, "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Verbose.to_string())); - Log4NetAppenderConfiguration.set_trace_logger_when_trace(config.Trace, "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Trace.to_string())); + var verboseAppenderName = "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Verbose.to_string()); + var traceAppenderName = "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Trace.to_string()); + Log4NetAppenderConfiguration.set_logging_level_debug_when_debug(config.Debug, verboseAppenderName, traceAppenderName); + Log4NetAppenderConfiguration.set_verbose_logger_when_verbose(config.Verbose, config.Debug, verboseAppenderName); + Log4NetAppenderConfiguration.set_trace_logger_when_trace(config.Trace, traceAppenderName); "chocolatey".Log().Debug(() => "{0} is running on {1} v {2}".format_with(ApplicationParameters.Name, config.Information.PlatformType, config.Information.PlatformVersion.to_string())); //"chocolatey".Log().Debug(() => "Command Line: {0}".format_with(Environment.CommandLine)); diff --git a/src/chocolatey/GetChocolatey.cs b/src/chocolatey/GetChocolatey.cs index a0685934a8..c5d6f506a9 100644 --- a/src/chocolatey/GetChocolatey.cs +++ b/src/chocolatey/GetChocolatey.cs @@ -95,7 +95,7 @@ public class GetChocolatey /// public GetChocolatey() { - Log4NetAppenderConfiguration.configure(); + Log4NetAppenderConfiguration.configure(null, excludeLoggerNames: ChocolateyLoggers.Trace.to_string()); Bootstrap.initialize(); _license = License.validate_license(); _container = SimpleInjectorContainer.Container; diff --git a/src/chocolatey/infrastructure.app/runners/ConsoleApplication.cs b/src/chocolatey/infrastructure.app/runners/ConsoleApplication.cs index 229531125a..43ac69a8fd 100644 --- a/src/chocolatey/infrastructure.app/runners/ConsoleApplication.cs +++ b/src/chocolatey/infrastructure.app/runners/ConsoleApplication.cs @@ -67,7 +67,7 @@ public void run(string[] args, ChocolateyConfiguration config, Container contain // if debug is bundled with local options, it may not get picked up when global // options are parsed. Attempt to set it again once local options are set. // This does mean some output from debug will be missed (but not much) - if (config.Debug) Log4NetAppenderConfiguration.set_logging_level_debug_when_debug(config.Debug, excludeLoggerName: "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Verbose.to_string())); + if (config.Debug) Log4NetAppenderConfiguration.set_logging_level_debug_when_debug(config.Debug, "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Verbose.to_string()), "{0}LoggingColoredConsoleAppender".format_with(ChocolateyLoggers.Trace.to_string())); command.handle_additional_argument_parsing(unparsedArgs, config); diff --git a/src/chocolatey/infrastructure/logging/Log4NetAppenderConfiguration.cs b/src/chocolatey/infrastructure/logging/Log4NetAppenderConfiguration.cs index e9dd20727d..d320c04a5d 100644 --- a/src/chocolatey/infrastructure/logging/Log4NetAppenderConfiguration.cs +++ b/src/chocolatey/infrastructure/logging/Log4NetAppenderConfiguration.cs @@ -16,6 +16,7 @@ namespace chocolatey.infrastructure.logging { + using System.Collections.Generic; using System.IO; using System.Linq; using adapters; @@ -35,14 +36,15 @@ public sealed class Log4NetAppenderConfiguration private static readonly log4net.ILog _logger = LogManager.GetLogger(typeof(Log4NetAppenderConfiguration)); private static bool _alreadyConfiguredFileAppender; - private static string _summaryLogAppenderName = "{0}.summary.log.appender".format_with(ApplicationParameters.Name); + private static readonly string _summaryLogAppenderName = "{0}.summary.log.appender".format_with(ApplicationParameters.Name); /// /// Pulls xml configuration from embedded location and applies it. /// Then it configures a file appender to the specified output directory if one is provided. /// /// The output directory. - public static void configure(string outputDirectory = null) + /// Loggers, such as a verbose logger, to exclude from this. + public static void configure(string outputDirectory = null, params string[] excludeLoggerNames) { GlobalContext.Properties["pid"] = System.Diagnostics.Process.GetCurrentProcess().Id; @@ -59,7 +61,7 @@ public static void configure(string outputDirectory = null) if (!string.IsNullOrWhiteSpace(outputDirectory)) { - set_file_appender(outputDirectory); + set_file_appender(outputDirectory, excludeLoggerNames); } _logger.DebugFormat("Configured {0} from assembly {1}", resource, assembly.FullName); @@ -69,8 +71,11 @@ public static void configure(string outputDirectory = null) /// Adds a file appender to all current loggers. Only runs one time. /// /// The output directory. - private static void set_file_appender(string outputDirectory) + /// Loggers, such as a trace logger, to exclude from file appender. + private static void set_file_appender(string outputDirectory, params string[] excludeLoggerNames) { + if (excludeLoggerNames == null) excludeLoggerNames = new string[] {}; + if (!_alreadyConfiguredFileAppender) { _alreadyConfiguredFileAppender = true; @@ -111,7 +116,7 @@ private static void set_file_appender(string outputDirectory) infoOnlyAppender.ActivateOptions(); ILoggerRepository logRepository = LogManager.GetRepository(Assembly.GetCallingAssembly().UnderlyingType); - foreach (ILogger log in logRepository.GetCurrentLoggers()) + foreach (ILogger log in logRepository.GetCurrentLoggers().Where(l => excludeLoggerNames.All(name => !l.Name.is_equal_to(name))).or_empty_list_if_null()) { var logger = log as Logger; if (logger != null) @@ -129,9 +134,11 @@ private static void set_file_appender(string outputDirectory) /// /// if set to true [enable debug]. /// - /// Logger, such as a verbose logger, to exclude from this. - public static void set_logging_level_debug_when_debug(bool enableDebug, string excludeLoggerName) + /// Appenders, such as a verbose console appender, to exclude from debug. + public static void set_logging_level_debug_when_debug(bool enableDebug, params string[] excludeAppenderNames) { + if (excludeAppenderNames == null) excludeAppenderNames = new string[] { }; + if (enableDebug) { ILoggerRepository logRepository = LogManager.GetRepository(Assembly.GetCallingAssembly().UnderlyingType); @@ -145,7 +152,7 @@ public static void set_logging_level_debug_when_debug(bool enableDebug, string e } } - foreach (var append in logRepository.GetAppenders().Where(a => !a.Name.is_equal_to(excludeLoggerName.to_string())).or_empty_list_if_null()) + foreach (var append in logRepository.GetAppenders().Where(a => excludeAppenderNames.All(name => !a.Name.is_equal_to(name))).or_empty_list_if_null()) { var appender = append as AppenderSkeleton; if (appender != null && !appender.Name.is_equal_to(_summaryLogAppenderName)) @@ -196,6 +203,8 @@ public static void set_trace_logger_when_trace(bool enableTrace, string traceLog System.Diagnostics.Trace.AutoFlush = true; System.Diagnostics.Trace.Listeners.Add(new TraceLog()); + var fileAppenders = new List(); + ILoggerRepository logRepository = LogManager.GetRepository(Assembly.GetCallingAssembly().UnderlyingType); foreach (var append in logRepository.GetAppenders()) { @@ -206,6 +215,20 @@ public static void set_trace_logger_when_trace(bool enableTrace, string traceLog var minLevel = Level.Debug; appender.AddFilter(new log4net.Filter.LevelRangeFilter { LevelMin = minLevel, LevelMax = Level.Fatal }); } + + if (appender != null && appender.GetType() == typeof(RollingFileAppender)) fileAppenders.Add(appender); + } + + foreach (ILogger log in logRepository.GetCurrentLoggers().Where(l => l.Name.is_equal_to("Trace")).or_empty_list_if_null()) + { + var logger = log as Logger; + if (logger != null) + { + foreach (var appender in fileAppenders.or_empty_list_if_null()) + { + logger.AddAppender(appender); + } + } } } From 1992f8dda99475718a3236c356b70283179a653a Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Tue, 23 May 2017 12:31:18 -0500 Subject: [PATCH 10/26] (maint) restrict list start/end messages to log file No need to show start/end of list in console unless verbose is also selected. --- src/chocolatey/infrastructure.app/services/NugetService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index cdc4b904ad..70806902ee 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -125,7 +125,7 @@ public virtual IEnumerable list_run(ChocolateyConfiguration confi } if (config.RegularOutput) this.Log().Debug(() => "Running list with the following filter = '{0}'".format_with(config.Input)); - if (config.RegularOutput) this.Log().Debug(() => "--- Start of List ---"); + if (config.RegularOutput) this.Log().Debug(ChocolateyLoggers.Verbose, () => "--- Start of List ---"); foreach (var pkg in NugetList.GetPackages(config, _nugetLogger)) { var package = pkg; // for lamda access @@ -217,7 +217,7 @@ Package url yield return new PackageResult(package, null, config.Sources); } - if (config.RegularOutput) this.Log().Debug(() => "--- End of List ---"); + if (config.RegularOutput) this.Log().Debug(ChocolateyLoggers.Verbose, () => "--- End of List ---"); if (config.RegularOutput && !config.QuietOutput) { this.Log().Warn(() => @"{0} packages {1}.".format_with(count, config.ListCommand.LocalOnly ? "installed" : "found")); From a1b7d88ac6a1364e92603d85fbfe21bf108549e8 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Tue, 23 May 2017 17:25:52 -0500 Subject: [PATCH 11/26] (GH-1241) Use process id with update file To really limit the possibility of something holding a lock on a file, use process id in files. Also attempt to clean up any locked update files when getting the configuration. --- .../infrastructure/services/XmlService.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 5d3729f316..2e6e3113b6 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -17,6 +17,7 @@ namespace chocolatey.infrastructure.services { using System; + using System.Diagnostics; using System.IO; using System.Text; using System.Xml; @@ -24,7 +25,7 @@ namespace chocolatey.infrastructure.services using cryptography; using filesystem; using tolerance; - using chocolatey.infrastructure.synchronization; + using synchronization; /// /// XML interaction @@ -91,6 +92,18 @@ public XmlType deserialize(string xmlFilePath) throw; } + finally + { + foreach (var updateFile in _fileSystem.get_files(_fileSystem.get_directory_name(xmlFilePath), "*.update").or_empty_list_if_null()) + { + FaultTolerance.try_catch_with_logging_exception( + () => _fileSystem.delete_file(updateFile), + errorMessage: "Unable to remove update file", + logDebugInsteadOfError: true, + isSilent: true + ); + } + } } }, "Error deserializing response of type {0}".format_with(typeof(XmlType)), @@ -142,13 +155,13 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen } // Otherwise, create an update file, and resiliently move it into place. - var tempUpdateFile = xmlFilePath + ".update"; + var tempUpdateFile = xmlFilePath + "." + Process.GetCurrentProcess().Id + ".update"; _fileSystem.write_file(tempUpdateFile, () => memoryStream); _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup"); } } }, - "Error serializing type {0}".format_with(typeof(XmlType)), + errorMessage: "Error serializing type {0}".format_with(typeof(XmlType)), throwError: true, isSilent: isSilent); }, MUTEX_TIMEOUT), From def61a6504a1a6e4ac979462c4fcc714ae83d6d1 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 12:39:41 -0500 Subject: [PATCH 12/26] (maint) ReadMe updates - Fix the badges (shields) - Remove the issue stats badges - Add release announcements mailing list - Remove broken method of donations - Add facebook and github links --- README.md | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index fe52a649fd..71386314e4 100644 --- a/README.md +++ b/README.md @@ -3,11 +3,8 @@ You can just call me choco. ![Chocolatey Logo](https://cdn.rawgit.com/chocolatey/choco/14a627932c78c8baaba6bef5f749ebfa1957d28d/docs/logo/chocolateyicon.gif "Chocolatey") -[![](http://img.shields.io/chocolatey/dt/chocolatey.svg)](https://chocolatey.org/packages/chocolatey) [![](http://img.shields.io/chocolatey/v/chocolatey.svg)](https://chocolatey.org/packages/chocolatey) [![](http://img.shields.io/gittip/Chocolatey.svg)](https://www.gittip.com/Chocolatey/) +[![](https://img.shields.io/chocolatey/dt/chocolatey.svg)](https://chocolatey.org/packages/chocolatey) [![](https://img.shields.io/chocolatey/v/chocolatey.svg)](https://chocolatey.org/packages/chocolatey) [![](https://img.shields.io/gratipay/Chocolatey.svg)](https://www.gratipay.com/Chocolatey/) [![Project Stats](https://www.openhub.net/p/chocolatey/widgets/project_thin_badge.gif)](https://www.openhub.net/p/chocolatey) -[![Issue Stats](http://issuestats.com/github/Chocolatey/choco/badge/pr)](http://issuestats.com/github/Chocolatey/choco) - -[![Issue Stats](http://issuestats.com/github/Chocolatey/choco/badge/issue)](http://issuestats.com/github/Chocolatey/choco) ## Build Status @@ -27,8 +24,8 @@ Please make sure you've read over and agree with the [etiquette regarding commun ## Support Chocolatey! - * Purchase [Chocolatey Pro / Chocolatey for Business](https://chocolatey.org/compare) - * [Donate](https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=E8ZPVL5PNTABW) + * Purchase [Chocolatey Pro / Chocolatey for Business](https://chocolatey.org/pricing#compare) + * Donate [![](https://img.shields.io/gratipay/Chocolatey.svg)](https://www.gratipay.com/Chocolatey/) ## See Chocolatey In Action @@ -46,17 +43,16 @@ When requesting support, try to remember that we are all volunteers that have li ## Information - * [Chocolatey site](https://chocolatey.org) - * [Community Package Repository aka Chocolatey.org](https://chocolatey.org/packages) - * [Mailing List/Forum](http://groups.google.com/group/chocolatey) - * [Twitter](https://twitter.com/chocolateynuget) - * [Build Status Email List](http://groups.google.com/group/chocolatey-build-status) - * Join the [newsletter](https://chocolatey.us8.list-manage1.com/subscribe?u=86a6d80146a0da7f2223712e4&id=73b018498d) and stay up to date with the latest happenings! + * [Chocolatey Website and Community Package Repository](https://chocolatey.org) + * [Mailing List](http://groups.google.com/group/chocolatey) / [Release Announcements Only Mailing List](https://groups.google.com/group/chocolatey-announce) / [Build Status Mailing List](http://groups.google.com/group/chocolatey-build-status) + * [Twitter](https://twitter.com/chocolateynuget) / [Facebook](https://www.facebook.com/ChocolateySoftware) / [Github](https://github.com/chocolatey) + * [Blog](https://chocolatey.org/blog) / [Newsletter](https://chocolatey.us8.list-manage1.com/subscribe?u=86a6d80146a0da7f2223712e4&id=73b018498d) + * [Documentation](https://chocolatey.org/docs) / [Support](https://chocolatey.org/support) ### Documentation Please see the [docs](https://chocolatey.org/docs) -Give `choco.exe /?` a shot (or `choco.exe -h`). For specific commands, add the command and then the help switch e.g. `choco.exe install -h`. +Give `choco.exe -?` a shot (or `choco.exe -h`). For specific commands, add the command and then the help switch e.g. `choco.exe install -h`. ### Requirements * .NET Framework 4.0 From 6dd5408fe853eeb00338c7209e0f9ff8b81748e5 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 14:51:41 -0500 Subject: [PATCH 13/26] (GH-1284) ensure file/filefullpath on functions With `Install-ChocolateyVsixPackage` and `Install-Chocolatey-ZipPackage`, ensure that file / filefullpath are aliases for urls (and the 64bit variants). --- .../functions/Install-ChocolateyPackage.ps1 | 5 +++-- .../Install-ChocolateyVsixPackage.ps1 | 9 ++++++++- .../functions/Install-ChocolateyZipPackage.ps1 | 18 ++++++++++++++++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/chocolatey.resources/helpers/functions/Install-ChocolateyPackage.ps1 b/src/chocolatey.resources/helpers/functions/Install-ChocolateyPackage.ps1 index 294dcf2ca1..6065fe0c51 100644 --- a/src/chocolatey.resources/helpers/functions/Install-ChocolateyPackage.ps1 +++ b/src/chocolatey.resources/helpers/functions/Install-ChocolateyPackage.ps1 @@ -95,7 +95,8 @@ In 0.10.6+, `File` and `FileFullPath` are aliases for Url. These aliases, if used in earlier versions of Chocolatey, produce `ERROR: Cannot bind parameter because parameter 'fileType' is specified more than once.` See https://github.com/chocolatey/choco/issues/1284. Do not -use these aliases with the community package repository until 2018. +use these aliases with the community package repository until January +2018. .PARAMETER Url64bit OPTIONAL - If there is a 64 bit resource available, use this @@ -113,7 +114,7 @@ These aliases, if used in earlier versions of Chocolatey, produce `ERROR: Cannot bind parameter because parameter 'fileType' is specified more than once.` See https://github.com/chocolatey/choco/issues/1284. Do not use these aliases with the community package repository until -2018. +January 2018. .PARAMETER ValidExitCodes Array of exit codes indicating success. Defaults to `@(0)`. diff --git a/src/chocolatey.resources/helpers/functions/Install-ChocolateyVsixPackage.ps1 b/src/chocolatey.resources/helpers/functions/Install-ChocolateyVsixPackage.ps1 index 9c74ac57b9..2e1f7bc239 100644 --- a/src/chocolatey.resources/helpers/functions/Install-ChocolateyVsixPackage.ps1 +++ b/src/chocolatey.resources/helpers/functions/Install-ChocolateyVsixPackage.ps1 @@ -54,6 +54,13 @@ Prefer HTTPS when available. Can be HTTP, FTP, or File URIs. In 0.10.4+, `Url` is an alias for VsixUrl. +In 0.10.6+, `File` and `FileFullPath` are aliases for VsixUrl. These +aliases, if used in earlier versions of Chocolatey, may produce `ERROR: +Cannot bind parameter because parameter 'fileType' is specified more +than once.` See https://github.com/chocolatey/choco/issues/1284. Do not +use these aliases with the community package repository until January +2018. + .PARAMETER VsVersion The major version number of Visual Studio where the package should be installed. This is optional. If not @@ -132,7 +139,7 @@ Install-ChocolateyZipPackage #> param( [alias("name")][parameter(Mandatory=$true, Position=0)][string] $packageName, - [alias("url")][parameter(Mandatory=$false, Position=1)][string] $vsixUrl, + [alias("url","file","fileFullPath")][parameter(Mandatory=$false, Position=1)][string] $vsixUrl, [alias("visualStudioVersion")][parameter(Mandatory=$false, Position=2)][int] $vsVersion = 0, [parameter(Mandatory=$false)][string] $checksum = '', [parameter(Mandatory=$false)][string] $checksumType = '', diff --git a/src/chocolatey.resources/helpers/functions/Install-ChocolateyZipPackage.ps1 b/src/chocolatey.resources/helpers/functions/Install-ChocolateyZipPackage.ps1 index cf2cede758..0845edd205 100644 --- a/src/chocolatey.resources/helpers/functions/Install-ChocolateyZipPackage.ps1 +++ b/src/chocolatey.resources/helpers/functions/Install-ChocolateyZipPackage.ps1 @@ -54,6 +54,13 @@ a 32 bit installation on a 64 bit system. Prefer HTTPS when available. Can be HTTP, FTP, or File URIs. +In 0.10.6+, `File` and `FileFullPath` are aliases for Url. These +aliases, if used in earlier versions of Chocolatey, may produce `ERROR: +Cannot bind parameter because parameter 'fileType' is specified more +than once.` See https://github.com/chocolatey/choco/issues/1284. Do not +use these aliases with the community package repository until January +2018. + .PARAMETER Url64bit OPTIONAL - If there is a 64 bit resource available, use this parameter. Chocolatey will automatically determine if the user is @@ -65,6 +72,13 @@ this parameter. Prefer HTTPS when available. Can be HTTP, FTP, or File URIs. +In 0.10.6+, `File64` and `FileFullPath64` are aliases for Url64Bit. +These aliases, if used in earlier versions of Chocolatey, may produce +`ERROR: Cannot bind parameter because parameter 'fileType' is specified +more than once.` See https://github.com/chocolatey/choco/issues/1284. +Do not use these aliases with the community package repository until +January 2018. + .PARAMETER UnzipLocation This is the full path to a location to unzip the contents to, most likely your script folder. If unzipping to your package folder, the path @@ -162,11 +176,11 @@ Get-ChocolateyUnzip #> param( [parameter(Mandatory=$true, Position=0)][string] $packageName, - [parameter(Mandatory=$false, Position=1)][string] $url = '', + [alias("file","fileFullPath")][parameter(Mandatory=$false, Position=1)][string] $url = '', [parameter(Mandatory=$true, Position=2)] [alias("destination")][string] $unzipLocation, [parameter(Mandatory=$false, Position=3)] - [alias("url64")][string] $url64bit = '', + [alias("url64","file64","fileFullPath64")][string] $url64bit = '', [parameter(Mandatory=$false)][string] $specificFolder ='', [parameter(Mandatory=$false)][string] $checksum = '', [parameter(Mandatory=$false)][string] $checksumType = '', From 9b87e99b715ebc8c4cc079226b87e7f05a3b836a Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 14:56:47 -0500 Subject: [PATCH 14/26] (GH-1311) Install-ChocolateyPowershellCommand File/FileFullPath aliases Allow using File / FileFullPath aliases with PsFileFullPath as they are well-known aliases and it brings consistency across different commands. --- .../functions/Install-ChocolateyPowershellCommand.ps1 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/chocolatey.resources/helpers/functions/Install-ChocolateyPowershellCommand.ps1 b/src/chocolatey.resources/helpers/functions/Install-ChocolateyPowershellCommand.ps1 index 4ff8e616b4..796184245a 100644 --- a/src/chocolatey.resources/helpers/functions/Install-ChocolateyPowershellCommand.ps1 +++ b/src/chocolatey.resources/helpers/functions/Install-ChocolateyPowershellCommand.ps1 @@ -49,11 +49,13 @@ Full file path to PowerShell file to turn into a command. If embedding it in the package next to the install script, the path will be like `"$(Split-Path -parent $MyInvocation.MyCommand.Definition)\\Script.ps1"` +In 0.10.6+, `File` and `FileFullPath` are aliases for PsFileFullPath. + .PARAMETER Url This is the 32 bit url to download the resource from. This resource can be used on 64 bit systems when a package has both a Url and Url64bit specified if a user passes `--forceX86`. If there is only a 64 bit url -available, please remove do not use the parameter (only use Url64bit). +available, please remove do not use this parameter (only use Url64bit). Will fail on 32bit systems if missing or if a user attempts to force a 32 bit installation on a 64 bit system. @@ -176,7 +178,7 @@ Install-ChocolateyZipPackage #> param( [parameter(Mandatory=$false, Position=0)][string] $packageName, - [parameter(Mandatory=$true, Position=1)][string] $psFileFullPath, + [alias("file","fileFullPath")][parameter(Mandatory=$true, Position=1)][string] $psFileFullPath, [parameter(Mandatory=$false, Position=2)][string] $url ='', [parameter(Mandatory=$false, Position=3)] [alias("url64")][string] $url64bit = '', From 80392529bad738a1316c52f6662277ad0fc04b70 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 15:46:58 -0500 Subject: [PATCH 15/26] (maint) clean up lib nuspec Fix up chocolatey.lib nuspec to modernize the information and locations of the information. Update copyright to reflect the new company name as well. --- nuget/chocolatey.lib/chocolatey.lib.nuspec | 27 ++++++++++++---------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/nuget/chocolatey.lib/chocolatey.lib.nuspec b/nuget/chocolatey.lib/chocolatey.lib.nuspec index 4e4515ae8b..99859232cb 100644 --- a/nuget/chocolatey.lib/chocolatey.lib.nuspec +++ b/nuget/chocolatey.lib/chocolatey.lib.nuspec @@ -2,25 +2,28 @@ chocolatey.lib - Chocolatey Core [PREVIEW] 0.9.9 - Chocolatey Software, Inc Rob Reynolds - Chocolatey is your machine level NuGet repository. Think apt-get for Windows (executables/application packages), not library packages. + Chocolatey Core [PREVIEW] + Chocolatey Software, Inc + https://github.com/chocolatey/choco + https://raw.githubusercontent.com/chocolatey/choco/master/docs/logo/chocolateyicon.gif + https://raw.githubusercontent.com/chocolatey/choco/master/LICENSE + false + 2017 Chocolatey Software, Inc, 2011- 2017 RealDimensions Software, LLC + apt-get machine repository chocolatey + Chocolatey is the package manager for Windows (like apt-get but for Windows) Chocolatey is a package manager for Windows (like apt-get but for Windows). It was designed to be a decentralized framework for quickly installing applications and tools that you need. It is built on the NuGet infrastructure currently using PowerShell as its focus for delivering packages from the distros to your door, err computer. -Chocolatey is brought to you by the work and inspiration of the community, the work and thankless nights of the Chocolatey Team (https://github.com/orgs/chocolatey/people), and Rob heading up the direction. +Chocolatey is brought to you by the work and inspiration of the community, the work and thankless nights of the Chocolatey Team (https://github.com/orgs/chocolatey/people), with Rob heading up the direction. + +You can host your own sources and add them to Chocolatey, you can extend Chocolatey's capabilities, and folks, it's only going to get better. + +This is the Chocolatey Library (API / DLL) package which allows Chocolatey to be embedded in your application. -This is the core package which allows Chocolatey to be embedded in your application. - https://github.com/chocolatey/choco/blob/master/CHANGELOG.md - https://github.com/chocolatey/choco - https://raw.githubusercontent.com/chocolatey/choco/master/LICENSE - false - RealDimensions Software, LLC - 2011 - Present - apt-get yum machine repository chocolatey - https://raw.githubusercontent.com/chocolatey/choco/master/docs/logo/chocolateyicon.gif + See https://github.com/chocolatey/choco/blob/stable/CHANGELOG.md From 00ea7a510a65142a13d1b99b5c3c6601272eb8d6 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 15:47:44 -0500 Subject: [PATCH 16/26] (maint) nuspec - add info links to description --- nuget/chocolatey.lib/chocolatey.lib.nuspec | 8 ++++++++ nuget/chocolatey/chocolatey.nuspec | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/nuget/chocolatey.lib/chocolatey.lib.nuspec b/nuget/chocolatey.lib/chocolatey.lib.nuspec index 99859232cb..fa0ab43d9a 100644 --- a/nuget/chocolatey.lib/chocolatey.lib.nuspec +++ b/nuget/chocolatey.lib/chocolatey.lib.nuspec @@ -22,6 +22,14 @@ You can host your own sources and add them to Chocolatey, you can extend Chocola This is the Chocolatey Library (API / DLL) package which allows Chocolatey to be embedded in your application. +### Information + + * [Chocolatey Website and Community Package Repository](https://chocolatey.org) + * [Mailing List](http://groups.google.com/group/chocolatey) / [Release Announcements Only Mailing List](https://groups.google.com/group/chocolatey-announce) / [Build Status Mailing List](http://groups.google.com/group/chocolatey-build-status) + * [Twitter](https://twitter.com/chocolateynuget) / [Facebook](https://www.facebook.com/ChocolateySoftware) / [Github](https://github.com/chocolatey) + * [Blog](https://chocolatey.org/blog) / [Newsletter](https://chocolatey.us8.list-manage1.com/subscribe?u=86a6d80146a0da7f2223712e4&id=73b018498d) + * [Documentation](https://chocolatey.org/docs) / [Support](https://chocolatey.org/support) + See https://github.com/chocolatey/choco/blob/stable/CHANGELOG.md diff --git a/nuget/chocolatey/chocolatey.nuspec b/nuget/chocolatey/chocolatey.nuspec index be05077dac..5012a613a6 100644 --- a/nuget/chocolatey/chocolatey.nuspec +++ b/nuget/chocolatey/chocolatey.nuspec @@ -25,6 +25,14 @@ Chocolatey is brought to you by the work and inspiration of the community, the w You can host your own sources and add them to Chocolatey, you can extend Chocolatey's capabilities, and folks, it's only going to get better. +### Information + + * [Chocolatey Website and Community Package Repository](https://chocolatey.org) + * [Mailing List](http://groups.google.com/group/chocolatey) / [Release Announcements Only Mailing List](https://groups.google.com/group/chocolatey-announce) / [Build Status Mailing List](http://groups.google.com/group/chocolatey-build-status) + * [Twitter](https://twitter.com/chocolateynuget) / [Facebook](https://www.facebook.com/ChocolateySoftware) / [Github](https://github.com/chocolatey) + * [Blog](https://chocolatey.org/blog) / [Newsletter](https://chocolatey.us8.list-manage1.com/subscribe?u=86a6d80146a0da7f2223712e4&id=73b018498d) + * [Documentation](https://chocolatey.org/docs) / [Support](https://chocolatey.org/support) + ### Commands There are quite a few commands you can call - you should check out the [command reference](https://chocolatey.org/docs/commands-reference). Here are the most common: From 83c8c5c3bbb33a7e16ead5d7d588e6bfc5f64f9d Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 15:48:26 -0500 Subject: [PATCH 17/26] (maint) update CHANGELOG / nuspec Add release notes for 0.10.6 --- CHANGELOG.md | 40 +++++++++++++++++++++++++++++- nuget/chocolatey/chocolatey.nuspec | 40 +++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 987cf2cda5..d92595b7cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,44 @@ This covers changes for the "chocolatey" and "chocolatey.lib" packages, which ar **NOTE**: If you have a licensed edition of Chocolatey ("chocolatey.extension"), refer to this in tandem with [Chocolatey Licensed CHANGELOG](https://github.com/chocolatey/choco/blob/master/CHANGELOG_LICENSED.md). +## [0.10.6](https://github.com/chocolatey/choco/issues?q=milestone%3A0.10.6+is%3Aclosed) (unreleased) + +### BUG FIXES + + * Fix - Chocolatey config changes in 0.10.4+ - The process cannot access the file because it is being used by another process - see [#1241](https://github.com/chocolatey/choco/issues/1241) + * Fix - PowerShell sees authenticode hash as changed in scripts that are UTF8 (w/out BOM) that contain unicode characters - see [#1225](https://github.com/chocolatey/choco/issues/1225) + * Fix - Chocolatey timed out immediately when execution timeout was infinite (0) - see [#1224](https://github.com/chocolatey/choco/issues/1224) + * Fix - choco list / search / info - Some packages can't be found - see [#1004](https://github.com/chocolatey/choco/issues/1004) + * Fix - chocolatey.config gets corrupted when multiple processes access simultaneously - see [#1258](https://github.com/chocolatey/choco/issues/1258) + * Fix - Update ShimGen to 0.8.x to address some issues - see [#1243](https://github.com/chocolatey/choco/issues/1243) + * Fix - Trace logging should only occur on when trace is enabled - see [#1309](https://github.com/chocolatey/choco/issues/1309) + * Fix - RefreshEnv.cmd doesn't set the correct PATH - see [#1227](https://github.com/chocolatey/choco/issues/1227) + * [API] Fix- chocolatey.lib nuget package has incorrect documentation xml - see [#1247](https://github.com/chocolatey/choco/issues/1247) + * [API] Fix - Chocolatey file cache still adds a 'chocolatey' directory on each install - see [#1231](https://github.com/chocolatey/choco/issues/1231) + * [API] Fix - List and Count should implement similar functionality as run - see [#1298](https://github.com/chocolatey/choco/issues/1298) + * Pro/Business - [API] Fix - Ensure DLL can work with licensed code - see [#1287](https://github.com/chocolatey/choco/issues/1287) + +### IMPROVEMENTS + + * Default package push url now uses push subdomain - see [#1285](https://github.com/chocolatey/choco/issues/1285) + * Report process id in the log files - see [#1239](https://github.com/chocolatey/choco/issues/1239) + * choco info / list / search - Include summary on detailed view - see [#1253](https://github.com/chocolatey/choco/issues/1253) + * choco info / list /search - Include release notes on detailed view - see [#1263](https://github.com/chocolatey/choco/issues/1263) + * choco list / search - Option to list packages only by name - see [#1237](https://github.com/chocolatey/choco/issues/1237) + * choco list / search - Allow sorting package results by relevance - see [#1101](https://github.com/chocolatey/choco/issues/1101) + * choco list / search - Search by tags only - see [#1033](https://github.com/chocolatey/choco/issues/1033) + * choco outdated - Option to leave out pinned packages - see [#994](https://github.com/chocolatey/choco/issues/994) + * Install-ChocolateyPackage and other functions should alias File/File64 - see [#1284](https://github.com/chocolatey/choco/issues/1284) + * Install-ChocolateyPowerShellCommand should alias File/FileFullPath for PsFileFullPath - see [#1311](https://github.com/chocolatey/choco/issues/1311) + * Logging - capture more information about a user (user name, domain, remote?, system?) - see [#615](https://github.com/chocolatey/choco/issues/615) + * Stop saying "0 packages failed." - see [#1259](https://github.com/chocolatey/choco/issues/1259) + * [API] provide a way to see ChocolateyConfiguration - see [#1267](https://github.com/chocolatey/choco/issues/1267) + * [API] Attempt to get ChocolateyInstall environment variable prior to extraction - see [#1297](https://github.com/chocolatey/choco/issues/1297) + * [API] Expose Container directly - see [#1294](https://github.com/chocolatey/choco/issues/1294) + * Pro/Business - Support for Package Audit (who installed packages) - see [#1238](https://github.com/chocolatey/choco/issues/1238) + * Pro/Business - [API] Ensure configuration retains base info between uses - see [#1296](https://github.com/chocolatey/choco/issues/1296) + + ## [0.10.5](https://github.com/chocolatey/choco/issues?q=milestone%3A0.10.5+is%3Aclosed) (March 30, 2017) ### BUG FIXES @@ -12,7 +50,7 @@ This covers changes for the "chocolatey" and "chocolatey.lib" packages, which ar ### IMPROVEMENTS -* Show machine readable output with `choco outdated -r` - see [#1222](https://github.com/chocolatey/choco/issues/1222) + * Show machine readable output with `choco outdated -r` - see [#1222](https://github.com/chocolatey/choco/issues/1222) ## [0.10.4](https://github.com/chocolatey/choco/issues?q=milestone%3A0.10.4+is%3Aclosed) (March 30, 2017) diff --git a/nuget/chocolatey/chocolatey.nuspec b/nuget/chocolatey/chocolatey.nuspec index 5012a613a6..45cd312cfe 100644 --- a/nuget/chocolatey/chocolatey.nuspec +++ b/nuget/chocolatey/chocolatey.nuspec @@ -63,6 +63,44 @@ In that mess there is a link to the [PowerShell Chocolatey module reference](htt See all - https://github.com/chocolatey/choco/blob/stable/CHANGELOG.md +## 0.10.6 + +### BUG FIXES + + * Fix - Chocolatey config changes in 0.10.4+ - The process cannot access the file because it is being used by another process - see [#1241](https://github.com/chocolatey/choco/issues/1241) + * Fix - PowerShell sees authenticode hash as changed in scripts that are UTF8 (w/out BOM) that contain unicode characters - see [#1225](https://github.com/chocolatey/choco/issues/1225) + * Fix - Chocolatey timed out immediately when execution timeout was infinite (0) - see [#1224](https://github.com/chocolatey/choco/issues/1224) + * Fix - choco list / search / info - Some packages can't be found - see [#1004](https://github.com/chocolatey/choco/issues/1004) + * Fix - chocolatey.config gets corrupted when multiple processes access simultaneously - see [#1258](https://github.com/chocolatey/choco/issues/1258) + * Fix - Update ShimGen to 0.8.x to address some issues - see [#1243](https://github.com/chocolatey/choco/issues/1243) + * Fix - Trace logging should only occur on when trace is enabled - see [#1309](https://github.com/chocolatey/choco/issues/1309) + * Fix - RefreshEnv.cmd doesn't set the correct PATH - see [#1227](https://github.com/chocolatey/choco/issues/1227) + * [API] Fix- chocolatey.lib nuget package has incorrect documentation xml - see [#1247](https://github.com/chocolatey/choco/issues/1247) + * [API] Fix - Chocolatey file cache still adds a 'chocolatey' directory on each install - see [#1231](https://github.com/chocolatey/choco/issues/1231) + * [API] Fix - List and Count should implement similar functionality as run - see [#1298](https://github.com/chocolatey/choco/issues/1298) + * Pro/Business - [API] Fix - Ensure DLL can work with licensed code - see [#1287](https://github.com/chocolatey/choco/issues/1287) + +### IMPROVEMENTS + + * Default package push url now uses push subdomain - see [#1285](https://github.com/chocolatey/choco/issues/1285) + * Report process id in the log files - see [#1239](https://github.com/chocolatey/choco/issues/1239) + * choco info / list / search - Include summary on detailed view - see [#1253](https://github.com/chocolatey/choco/issues/1253) + * choco info / list /search - Include release notes on detailed view - see [#1263](https://github.com/chocolatey/choco/issues/1263) + * choco list / search - Option to list packages only by name - see [#1237](https://github.com/chocolatey/choco/issues/1237) + * choco list / search - Allow sorting package results by relevance - see [#1101](https://github.com/chocolatey/choco/issues/1101) + * choco list / search - Search by tags only - see [#1033](https://github.com/chocolatey/choco/issues/1033) + * choco outdated - Option to leave out pinned packages - see [#994](https://github.com/chocolatey/choco/issues/994) + * Install-ChocolateyPackage and other functions should alias File/File64 - see [#1284](https://github.com/chocolatey/choco/issues/1284) + * Install-ChocolateyPowerShellCommand should alias File/FileFullPath for PsFileFullPath - see [#1311](https://github.com/chocolatey/choco/issues/1311) + * Logging - capture more information about a user (user name, domain, remote?, system?) - see [#615](https://github.com/chocolatey/choco/issues/615) + * Stop saying "0 packages failed." - see [#1259](https://github.com/chocolatey/choco/issues/1259) + * [API] provide a way to see ChocolateyConfiguration - see [#1267](https://github.com/chocolatey/choco/issues/1267) + * [API] Attempt to get ChocolateyInstall environment variable prior to extraction - see [#1297](https://github.com/chocolatey/choco/issues/1297) + * [API] Expose Container directly - see [#1294](https://github.com/chocolatey/choco/issues/1294) + * Pro/Business - Support for Package Audit (who installed packages) - see [#1238](https://github.com/chocolatey/choco/issues/1238) + * Pro/Business - [API] Ensure configuration retains base info between uses - see [#1296](https://github.com/chocolatey/choco/issues/1296) + + ## 0.10.5 ### BUG FIXES @@ -71,7 +109,7 @@ See all - https://github.com/chocolatey/choco/blob/stable/CHANGELOG.md ### IMPROVEMENTS -* Show machine readable output with `choco outdated -r` - see [#1222](https://github.com/chocolatey/choco/issues/1222) + * Show machine readable output with `choco outdated -r` - see [#1222](https://github.com/chocolatey/choco/issues/1222) ## 0.10.4 From 0b0f3c824906e1ed1958b05cf9d708833c8b400d Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Wed, 24 May 2017 18:04:27 -0500 Subject: [PATCH 18/26] (log) replace_file use new lines for log --- src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index 593bb0882b..7276d5462e 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -393,7 +393,7 @@ public bool copy_file_unsafe(string sourceFilePath, string destinationFilePath, public void replace_file(string sourceFilePath, string destinationFilePath, string backupFilePath) { - this.Log().Debug(ChocolateyLoggers.Verbose, () => "Attempting to replace \"{0}\"{1} with \"{2}\". Backup placed at \"{3}\".".format_with(destinationFilePath, Environment.NewLine, sourceFilePath, backupFilePath)); + this.Log().Debug(ChocolateyLoggers.Verbose, () => "Attempting to replace \"{0}\"{1} with \"{2}\".{1} Backup placed at \"{3}\".".format_with(destinationFilePath, Environment.NewLine, sourceFilePath, backupFilePath)); allow_retries( () => From 75346e652122e4ac509a6b896b1343f3bb5ae467 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Thu, 25 May 2017 17:18:34 -0500 Subject: [PATCH 19/26] (GH-1292) Ensure manifest extracted on unpackself If the `choco.exe.manifest` file is not present on first use of choco.exe, it extracts the file. However Windows will have already cached that choco.exe doesn't have a manifest and will not recheck until the choco.exe changes (last modified time) as it caches that it doesn't have a manifest. To force a recheck, someone would need to "touch" the file to change the last modified date. The issue comes down to the fact that a manifest file needs to be in place when choco is installed, before the first time that `choco.exe` gets called. Ensure that the manifest file is packaged up and it will be unpacked automatically next to choco at the right time. --- .build.custom/nugetPrepare.post.step | 1 + .../commands/ChocolateyUnpackSelfCommand.cs | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.build.custom/nugetPrepare.post.step b/.build.custom/nugetPrepare.post.step index 9862c8ad86..3234ab8c4f 100644 --- a/.build.custom/nugetPrepare.post.step +++ b/.build.custom/nugetPrepare.post.step @@ -39,6 +39,7 @@ + diff --git a/src/chocolatey/infrastructure.app/commands/ChocolateyUnpackSelfCommand.cs b/src/chocolatey/infrastructure.app/commands/ChocolateyUnpackSelfCommand.cs index 15001ad5c7..a761da8441 100644 --- a/src/chocolatey/infrastructure.app/commands/ChocolateyUnpackSelfCommand.cs +++ b/src/chocolatey/infrastructure.app/commands/ChocolateyUnpackSelfCommand.cs @@ -86,7 +86,11 @@ public virtual void noop(ChocolateyConfiguration configuration) public virtual void run(ChocolateyConfiguration configuration) { this.Log().Info("{0} is unpacking required files for use. Overwriting? {1}".format_with(ApplicationParameters.Name, configuration.Force)); - //refactor - thank goodness this is temporary, cuz manifest resource streams are dumb + // refactor - thank goodness this is temporary, cuz manifest resource streams are dumb + + // unpack the manifest file as well + AssemblyFileExtractor.extract_all_resources_to_relative_directory(_fileSystem, Assembly.GetAssembly(typeof(ChocolateyUnpackSelfCommand)), _fileSystem.get_directory_name(_fileSystem.get_current_assembly_path()), new List(), "chocolatey.console"); + IList folders = new List { "helpers", From 260c872954782cc987ee9cbfa3f55ce7e809e500 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Thu, 25 May 2017 17:18:55 -0500 Subject: [PATCH 20/26] (maint) formatting --- .build.custom/nugetPrepare.post.step | 2 +- nuget/chocolatey/tools/chocolateysetup.psm1 | 2 +- .../infrastructure/extractors/AssemblyFileExtractor.cs | 2 +- src/chocolatey/infrastructure/services/XmlService.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.build.custom/nugetPrepare.post.step b/.build.custom/nugetPrepare.post.step index 3234ab8c4f..ba59af543a 100644 --- a/.build.custom/nugetPrepare.post.step +++ b/.build.custom/nugetPrepare.post.step @@ -41,7 +41,7 @@ commandline="${args.choco}" if="${platform::is-windows()}" /> - + diff --git a/nuget/chocolatey/tools/chocolateysetup.psm1 b/nuget/chocolatey/tools/chocolateysetup.psm1 index 3717ed14aa..9472eb4957 100644 --- a/nuget/chocolatey/tools/chocolateysetup.psm1 +++ b/nuget/chocolatey/tools/chocolateysetup.psm1 @@ -399,7 +399,7 @@ param( Copy-Item $chocoExe $chocoExeDest -force Write-Debug "Copying the contents of `'$chocInstallFolder`' to `'$chocolateyPath`'." - Copy-Item $chocInstallFolder\* $chocolateyPath -recurse -force + Copy-Item $chocInstallFolder\* $chocolateyPath -Recurse -Force } function Ensure-ChocolateyLibFiles { diff --git a/src/chocolatey/infrastructure/extractors/AssemblyFileExtractor.cs b/src/chocolatey/infrastructure/extractors/AssemblyFileExtractor.cs index 32363a12db..a49b82da91 100644 --- a/src/chocolatey/infrastructure/extractors/AssemblyFileExtractor.cs +++ b/src/chocolatey/infrastructure/extractors/AssemblyFileExtractor.cs @@ -101,7 +101,7 @@ public static void extract_all_resources_to_relative_directory(IFileSystem fileS //var fileLocation = fileSystem.combine_paths("", resourceString.ToString().Split('.')) + resourceName.Substring(fileExtensionLocation); var filePath = fileSystem.combine_paths(directoryPath, fileLocation); - if (logOutput) "chocolatey".Log().Debug("Unpacking {0} to '{1}'".format_with(fileLocation,filePath)); + if (logOutput) "chocolatey".Log().Debug("Unpacking {0} to '{1}'".format_with(fileLocation, filePath)); extract_binary_file_from_assembly(fileSystem, assembly, resourceName, filePath, overwriteExisting); } } diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 2e6e3113b6..a47e36230a 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -97,7 +97,7 @@ public XmlType deserialize(string xmlFilePath) foreach (var updateFile in _fileSystem.get_files(_fileSystem.get_directory_name(xmlFilePath), "*.update").or_empty_list_if_null()) { FaultTolerance.try_catch_with_logging_exception( - () => _fileSystem.delete_file(updateFile), + () => _fileSystem.delete_file(updateFile), errorMessage: "Unable to remove update file", logDebugInsteadOfError: true, isSilent: true From 46637aecb111292c319aed6bda8df05dd5f00efe Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 29 May 2017 13:11:04 -0500 Subject: [PATCH 21/26] (GH-1241) Explicitly handle closing For resources that need closed and disposed in XmlService, explicitly handle closing of those resources. This allows being extremely explicit about when resources are cleaned up and subsequently released. --- .../infrastructure/services/XmlService.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index a47e36230a..8a564ac6d5 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -133,30 +133,39 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen // Write the updated file to memory using (var memoryStream = new MemoryStream()) - using (var streamWriter = new StreamWriter(memoryStream, encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: true))) + using (var streamWriter = new StreamWriter(memoryStream, encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)) { + AutoFlush = true + } + ){ xmlSerializer.Serialize(streamWriter, xmlType); streamWriter.Flush(); - memoryStream.Position = 0; - // Grab the hash of both files and compare them. var originalFileHash = _hashProvider.hash_file(xmlFilePath); + memoryStream.Position = 0; if (!originalFileHash.is_equal_to(_hashProvider.hash_stream(memoryStream))) { - memoryStream.Position = 0; - // If there wasn't a file there in the first place, just write the new one out directly. if (string.IsNullOrEmpty(originalFileHash)) { + memoryStream.Position = 0; _fileSystem.write_file(xmlFilePath, () => memoryStream); + memoryStream.Close(); + streamWriter.Close(); + return; } // Otherwise, create an update file, and resiliently move it into place. var tempUpdateFile = xmlFilePath + "." + Process.GetCurrentProcess().Id + ".update"; + memoryStream.Position = 0; _fileSystem.write_file(tempUpdateFile, () => memoryStream); + + memoryStream.Close(); + streamWriter.Close(); + _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup"); } } From baf0c749074da111c2ff1bc637b43b11b027230f Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 29 May 2017 13:21:03 -0500 Subject: [PATCH 22/26] (GH-1241) FileSystem - move file creates directory When moving a file to a new location, create the directory for the new file if it doesn't already exist. --- src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index 7276d5462e..4474a75fce 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -336,6 +336,8 @@ public string get_file_date(dynamic file) public void move_file(string filePath, string newFilePath) { + create_directory_if_not_exists(get_directory_name(newFilePath), ignoreError: true); + allow_retries( () => { From 3da47609824464780f0707868a28c521dc9efdba Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 29 May 2017 13:22:04 -0500 Subject: [PATCH 23/26] (maint) formatting --- .../infrastructure/filesystem/DotNetFileSystem.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index 4474a75fce..fa9819776d 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -392,7 +392,7 @@ public bool copy_file_unsafe(string sourceFilePath, string destinationFilePath, //} return success != 0; } - + public void replace_file(string sourceFilePath, string destinationFilePath, string backupFilePath) { this.Log().Debug(ChocolateyLoggers.Verbose, () => "Attempting to replace \"{0}\"{1} with \"{2}\".{1} Backup placed at \"{3}\".".format_with(destinationFilePath, Environment.NewLine, sourceFilePath, backupFilePath)); @@ -407,7 +407,7 @@ public void replace_file(string sourceFilePath, string destinationFilePath, stri // has not been the case with choco - the backup file has been // the most sensitive to issues with file locking //File.Replace(sourceFilePath, destinationFilePath, backupFilePath); - + // move existing file to backup location if (!string.IsNullOrEmpty(backupFilePath) && file_exists(destinationFilePath)) { @@ -428,6 +428,7 @@ public void replace_file(string sourceFilePath, string destinationFilePath, stri this.Log().Debug("Error capturing backup of '{0}':{1} {2}".format_with(destinationFilePath, Environment.NewLine, ex.Message)); } } + // copy source file to destination this.Log().Debug(ChocolateyLoggers.Trace, "Copying '{0}' to '{1}'.".format_with(sourceFilePath, destinationFilePath)); copy_file(sourceFilePath, destinationFilePath, overwriteExisting: true); @@ -583,7 +584,7 @@ public string get_directory_name(string filePath) { filePath = filePath.Replace('\\', '/'); } - + try { return Path.GetDirectoryName(filePath); From c16b909aadbbfd651f6f07733a61618c266094bc Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 29 May 2017 13:24:27 -0500 Subject: [PATCH 24/26] (GH-1241) XmlService - trace logging information Log tracing output surrounding working with xml files. --- .../infrastructure/services/XmlService.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 8a564ac6d5..f2a405bff0 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -24,6 +24,7 @@ namespace chocolatey.infrastructure.services using System.Xml.Serialization; using cryptography; using filesystem; + using logging; using tolerance; using synchronization; @@ -47,6 +48,8 @@ public XmlType deserialize(string xmlFilePath) return FaultTolerance.retry(3, () => GlobalMutex.enter( () => { + this.Log().Debug(ChocolateyLoggers.Trace, "Entered mutex to deserialize '{0}'".format_with(xmlFilePath)); + return FaultTolerance.try_catch_with_logging_exception( () => { @@ -96,6 +99,7 @@ public XmlType deserialize(string xmlFilePath) { foreach (var updateFile in _fileSystem.get_files(_fileSystem.get_directory_name(xmlFilePath), "*.update").or_empty_list_if_null()) { + this.Log().Debug("Removing '{0}'".format_with(updateFile)); FaultTolerance.try_catch_with_logging_exception( () => _fileSystem.delete_file(updateFile), errorMessage: "Unable to remove update file", @@ -126,12 +130,13 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen FaultTolerance.retry(3, () => GlobalMutex.enter( () => { + this.Log().Debug(ChocolateyLoggers.Trace, "Entered mutex to serialize '{0}'".format_with(xmlFilePath)); FaultTolerance.try_catch_with_logging_exception( () => { var xmlSerializer = new XmlSerializer(typeof(XmlType)); - // Write the updated file to memory + this.Log().Debug(ChocolateyLoggers.Trace, "Opening memory stream for xml file creation."); using (var memoryStream = new MemoryStream()) using (var streamWriter = new StreamWriter(memoryStream, encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)) { @@ -142,16 +147,20 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen streamWriter.Flush(); // Grab the hash of both files and compare them. + this.Log().Debug(ChocolateyLoggers.Trace, "Hashing original file at '{0}'".format_with(xmlFilePath)); var originalFileHash = _hashProvider.hash_file(xmlFilePath); memoryStream.Position = 0; if (!originalFileHash.is_equal_to(_hashProvider.hash_stream(memoryStream))) { + this.Log().Debug(ChocolateyLoggers.Trace, "The hashes were different."); // If there wasn't a file there in the first place, just write the new one out directly. if (string.IsNullOrEmpty(originalFileHash)) { + this.Log().Debug("There was no original file at '{0}'".format_with(xmlFilePath)); memoryStream.Position = 0; _fileSystem.write_file(xmlFilePath, () => memoryStream); + this.Log().Debug(ChocolateyLoggers.Trace, "Closing xml memory stream."); memoryStream.Close(); streamWriter.Close(); @@ -160,12 +169,15 @@ public void serialize(XmlType xmlType, string xmlFilePath, bool isSilen // Otherwise, create an update file, and resiliently move it into place. var tempUpdateFile = xmlFilePath + "." + Process.GetCurrentProcess().Id + ".update"; + this.Log().Debug(ChocolateyLoggers.Trace, "Creating a temp file at '{0}'".format_with(tempUpdateFile)); memoryStream.Position = 0; + this.Log().Debug(ChocolateyLoggers.Trace, "Writing file '{0}'".format_with(tempUpdateFile)); _fileSystem.write_file(tempUpdateFile, () => memoryStream); memoryStream.Close(); streamWriter.Close(); + this.Log().Debug(ChocolateyLoggers.Trace, "Replacing file '{0}' with '{1}'.".format_with(xmlFilePath, tempUpdateFile)); _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup"); } } From 9779c4984659cd49741f195c4b30c650e8a145c4 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 29 May 2017 13:54:08 -0500 Subject: [PATCH 25/26] (doc) update CONTRIBUTING/COMMITTERS - Add more information related to contributing, specifically around testing and code design/formatting - Add Table of Contents to README / CONTRIBUTING / COMMITTERS. - Submitting issues step by step that adds FAQs and Troubleshooting sections for folks - Add why a CLA section - More about setup, including with git - Building and testing choco itself - committers and accepting changes --- COMMITTERS.md | 50 +++++++++------ CONTRIBUTING.md | 140 ++++++++++++++++++++++++++++++++++++------ README.md | 44 ++++++++----- docs/legal/CREDITS.md | 12 +--- 4 files changed, 184 insertions(+), 62 deletions(-) diff --git a/COMMITTERS.md b/COMMITTERS.md index 38352058d7..cdd7058b5f 100644 --- a/COMMITTERS.md +++ b/COMMITTERS.md @@ -1,15 +1,30 @@ Committers ============ + + +- [Branch Targeting](#branch-targeting) +- [Summary](#summary) +- [Terminology](#terminology) +- [Review Process](#review-process) + - [Receive new PR (pull request)](#receive-new-pr-pull-request) + - [Initial PR Review](#initial-pr-review) + - [Review the Code](#review-the-code) + - [Accepting a PR](#accepting-a-pr) +- [Merging](#merging) + - [Merge Retargeting to Stable](#merge-retargeting-to-stable) + + + ## Branch Targeting +* Stable currently targeting 0.10.x +* Master currently targeting 0.11.x -* Stable currently targeting 0.9.9.x -* Master currently targeting 0.9.10.x +**NOTE:** The above may be out of date. The best way to determine what branch is being targeted for stable is to run `git describe`. Stable and master are typically different on minor version (`x.y.z` - the `y`). Depending on where your fix/enhancement goes, please target the proper branch. Community members always target master, but committers should know where the fix they are presenting goes. It makes it much easier to push the shiny green button on a pull request. If you are not sure where to target, ask. ## Summary - We like to see folks contributing to Chocolatey. If you are a committer, we'd like to see you help from time to time with triage and the pull request process. In all cases politeness goes a long way. Please thank folks for contributions - they are going out of their way to help make the code base better, or adding something they may personally feel is necessary for the code base. @@ -17,22 +32,18 @@ In all cases politeness goes a long way. Please thank folks for contributions - Please be VERY familiar with [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and follow the process as well. ## Terminology +**Contributor** - A person who makes a change to the code base and submits a change set in the form of a pull request. -**contributor** - A person who makes a change to the code base and submits a change set in the form of a pull request. - -**change set** - A set of discrete commits which combined together form a contribution. A change set takes the form of git commits and is submitted in the form of a pull request. Used interchangeably with "pull request". +**Change Set** - A set of discrete commits which combined together form a contribution. A change set takes the form of git commits and is submitted in the form of a pull request. Used interchangeably with "pull request". -**committer** - A person responsible for reviewing a pull request and then making the decision what base branch to merge the change set into. +**Committer** - A person responsible for reviewing a pull request and then making the decision what base branch to merge the change set into. ## Review Process - #### Receive new PR (pull request) - * A contributor sends a pull request (usually against master). * A committer typically reviews it within a week or less to determine the feasibility of the changes. #### Initial PR Review - * Has the user signed the Contributor License Agreement (CLA)? * Did the user create a branch with these changes? If it is on their master, please ask them to review [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md). * Did the user reformat files and they should not have? Was is just white-space? You can try adding [?w=1](https://github.com/blog/967-github-secrets) to the URL on GitHub. @@ -43,32 +54,33 @@ Please be VERY familiar with [CONTRIBUTING](https://github.com/chocolatey/choco/ #### Review the Code * Does the code meet the naming conventions and formatting (need link)? * Is the code sound? Does it read well? Can you understand what it is doing without having to execute it? Principal of no clever hacks (need link). - * Does the code do what the purpose of the pull request is for? + * Does the code do what the purpose of the pull request is for (and only that)? #### Accepting a PR - Once you have reviewed the initial items, and are not waiting for additional feedback or work by the contributor, give the thumbs up that it is ready for the next part of the process (merging). Unless there is something wrong with the code, we don't ask contributors to rebase against master. They did the work to create the patch in the first place, asking them to unnecessarily come back and try to keep their code synced up with master is not an acceptable process. ## Merging - Once you have reviewed the change set and determined it is ready for merge, the next steps are to bring it local and evaluate the code further by actually working with it, running the tests locally and adding any additional commits or fix-ups that are necessary in a local branch. When merging the user's contribution, it should be done with `git merge --log --no-ff` to create a merge commit so that in case there is an issue it becomes easier to revert later, and so that we can see where the code came from should we ever need to go find it later (more information on this can be found [here](https://www.kernel.org/pub/software/scm/git/docs/git-merge.html) and also a discussion on why this is a good idea [here](http://differential.io/blog/best-way-to-merge-a-github-pull-request)). ### Merge Retargeting to Stable - Because we ask contributors to target master, sometimes a fix/enhancement may need to be retargeted to stable. This process is somewhat easy thanks to git. In most cases you won't even need to ask the user to do this for you. - * Add the user's remote. - * `git fetch user` - * `git checkout branch_from_user` + * `git fetch upstream pull//head:pr` - `upstream` is `git@github.com:chocolatey/choco.git` + * `git checkout pr` * `git rebase --onto stable master` - this uses the local branch, starts with latest stable and reapplies the commits from the branch to it, removing all commits that were only on the master. * `build.bat` - build and test * `git checkout stable` - * `git merge branch_from_user --log --no-ff` - * `git branch -d branch_from_user` + * Any addtional changes or testing here. + * `git merge pr --log --no-ff` + * `git branch -d pr` + * `git checkout master` + * `git merge stable` + * Make any last checks to ensure the git logs look good. The next step sets the commits in stone and unable to be changed. + * `git push upstream` References diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3cf94d410e..09da37f5c1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,29 +1,64 @@ -# Reporting an Issue/Bug? +# Contributing +The Chocolatey team has very explicit information here regarding the process for contributions, and we will be sticklers about the way you write your commit messages (yes, really), so to save yourself some rework, please make sure you read over this entire document prior to contributing. + + + +- [Are You In the Right Place?](#are-you-in-the-right-place) + - [Reporting an Issue/Bug?](#reporting-an-issuebug) + - [SolutionVersion.cs](#solutionversioncs) + - [Package Issue?](#package-issue) + - [Package Request? Package Missing?](#package-request-package-missing) + - [Submitting an Enhancement / Feature Request?](#submitting-an-enhancement--feature-request) + - [Submitting an Enhancement For Choco](#submitting-an-enhancement-for-choco) +- [Contributing](#contributing) + - [Prerequisites](#prerequisites) + - [Definition of Trivial Contributions](#definition-of-trivial-contributions) + - [Is the CLA Really Required?](#is-the-cla-really-required) +- [Contributing Process](#contributing-process) + - [Get Buyoff Or Find Open Community Issues/Features](#get-buyoff-or-find-open-community-issuesfeatures) + - [Set Up Your Environment](#set-up-your-environment) + - [Code Format / Design](#code-format--design) + - [C](#c) + - [PowerShell](#powershell) + - [Debugging / Testing](#debugging--testing) + - [Visual Studio](#visual-studio) + - [Chocolatey Build](#chocolatey-build) + - [Prepare Commits](#prepare-commits) + - [Submit Pull Request (PR)](#submit-pull-request-pr) + - [Respond to Feedback on Pull Request](#respond-to-feedback-on-pull-request) +- [Other General Information](#other-general-information) + + + +## Are You In the Right Place? +Chocolatey is a large ecosystem and each component has their own location for submitting issues and enhancement requests. While the website (the community package repository) may be all your know for packages, it represents only a tiny fraction of existing packages (organizations typically maintain and host their own packages internally). This is the repository for choco.exe (the client CLI tool) for Chocolatey, which spans multiple types of environments. + +Please follow this decision criteria to see if you are in the right location or if you should head to a different location to submit your request. + +### Reporting an Issue/Bug? ![submitting isseus](https://cloud.githubusercontent.com/assets/63502/12534440/fc223b74-c21e-11e5-9a41-1ffc1c9af48f.png) Submitting an Issue (or a Bug)? See the **[Submitting Issues](https://github.com/chocolatey/choco#submitting-issues) section** in the [README](https://github.com/chocolatey/choco/blob/master/README.md#submitting-issues). -**NOTE**: Do not submit issues for missing `SolutionVersion.cs`. Please see [Compiling / Building Source](https://github.com/chocolatey/choco#compiling--building-source). +#### SolutionVersion.cs +Do not submit issues for missing `SolutionVersion.cs`. Please see [Compiling / Building Source](https://github.com/chocolatey/choco#compiling--building-source). -# Submitting an Enhancement / Feature Request? - -This is the right place. See below. - -## Package Issue? +### Package Issue? Please see [Request Package Fixes or Updates / Become a maintainer of an existing package](https://chocolatey.org/docs/package-triage-process). -## Package Request? Package Missing? +### Package Request? Package Missing? If you are looking for packages to be added to the community feed (aka https://chocolatey.org/packages), please see [Package Requests](https://chocolatey.org/docs/package-triage-process#package-request-package-missing). -# Submitting an Enhancement +### Submitting an Enhancement / Feature Request? +If this is for choco (the CLI tool), this is the right place. See below. Otherwise see [Submitting Issues](https://github.com/chocolatey/choco#submitting-issues) for enhancements to the website, enhancements to the ChocolateyGUI, etc. +#### Submitting an Enhancement For Choco Log a github issue. There are less constraints on this versus reporting issues. -# Contributors +## Contributing The process for contributions is roughly as follows: -## Prerequisites - +### Prerequisites * Submit the Enhancement ticket. You will need the issue id for your commits. * Ensure you have signed the Contributor License Agreement (CLA) - without this we are not able to take contributions that are not trivial. * [Sign the Contributor License Agreement](https://www.clahub.com/agreements/chocolatey/choco). @@ -31,7 +66,7 @@ The process for contributions is roughly as follows: * If you are curious why we would require a CLA, we agree with Julien Ponge - take a look at his [post](https://julien.ponge.org/blog/in-defense-of-contributor-license-agreements/). * You agree to follow the [etiquette regarding communication](https://github.com/chocolatey/choco#etiquette-regarding-communication). -### Definition of Trivial Contributions +#### Definition of Trivial Contributions It's hard to define what is a trivial contribution. Sometimes even a 1 character change can be considered significant. Unfortunately because it can be subjective, the decision on what is trivial comes from the committers of the project and not from folks contributing to the project. It is generally safe to assume that you may be subject to signing the [CLA](https://www.clahub.com/agreements/chocolatey/choco) and be prepared to do so. Ask in advance if you are not sure and for reasons are not able to sign the [CLA](https://www.clahub.com/agreements/chocolatey/choco). What is generally considered trivial: @@ -44,26 +79,94 @@ What is generally not considered trivial: * Changes to any code that would be delivered as part of the final product. This includes any scripts that are delivered, such as PowerShell scripts. Yes, even 1 character changes could be considered non-trivial. +#### Is the CLA Really Required? + +Yes, and this aspect is not up for discussion. If you would like more resources on understanding CLAs, please see the following articles: + +* [What is a CLA and why do I care?](https://www.clahub.com/pages/why_cla) +* [In defense of Contributor License Agreements](https://julien.ponge.org/blog/in-defense-of-contributor-license-agreements/) +* [Contributor License Agreements](http://oss-watch.ac.uk/resources/cla) +* Dissenting opinion - [Why your project doesn't need a Contributor License Agreement](https://sfconservancy.org/blog/2014/jun/09/do-not-need-cla/) + +Overall, the flexibility and legal protections provided by a CLA make it necessary to require a CLA. As there is a company and a licensed version behind Chocolatey, those protections must be afforded. We understand this means some folks won't be able to contribute and that's completely fine. We prefer you to know up front this is required so you can make the best decision about contributing. + +If you work for an organization that does not allow you to contribute without attempting to own the rights to your work, please do not sign the CLA. + ## Contributing Process -### Get Buyoff Or Find Open Community Issues/Features +Start with [Prerequisites](#prerequisites) and make sure you can sign the Contributor License Agreement (CLA). + +### Get Buyoff Or Find Open Community Issues/Features * Through GitHub, or through the [mailing list](https://groups.google.com/forum/#!forum/chocolatey) (preferred), you talk about a feature you would like to see (or a bug), and why it should be in Chocolatey. * If approved through the mailing list, ensure the accompanying GitHub issue is created with information and a link back to the mailing list discussion. * Once you get a nod from one of the [Chocolatey Team](https://github.com/chocolatey?tab=members), you can start on the feature. * Alternatively, if a feature is on the issues list with the [Up For Grabs](https://github.com/chocolatey/choco/issues?q=is%3Aopen+is%3Aissue+label%3A%22Up+For+Grabs%22) label, it is open for a community member to patch. You should comment that you are signing up for it on the issue so someone else doesn't also sign up for the work. ### Set Up Your Environment - - * You create, or update, a fork of chocolatey/choco under your GitHub account. - * From there you create a branch named specific to the feature. - * In the branch you do work specific to the feature. + * Visual Studio 2010+ is recommended for code contributions. + * For git specific information: + 1. Create a fork of chocolatey/choco under your GitHub account. See [forks](https://help.github.com/articles/working-with-forks/) for more information. + 1. [Clone your fork](https://help.github.com/articles/cloning-a-repository/) locally. + 1. Open a command line and navigate to that directory. + 1. Add the upstream fork - `git remote add upstream git@github.com:chocolatey/choco.git` + 1. Run `git fetch upstream` + 1. Ensure you have user name and email set appropriately to attribute your contributions - see [Name](https://help.github.com/articles/setting-your-username-in-git/) / [Email](https://help.github.com/articles/setting-your-email-in-git/). + 1. Ensure that the local repository has the following settings (without `--global`, these only apply to the current repository): + * `git config core.autocrlf false` + * `git config core.symlinks false` + * `git config merge.ff false` + * `git config merge.log true` + * `git config fetch.prune true` + 1. From there you create a branch named specific to the feature. + 1. In the branch you do work specific to the feature. + 1. For committing the code, please see [Prepare Commits](#prepare-commits). + 1. See [Submit Pull Request (PR)](#submit-pull-request-pr). * Please also observe the following: * No reformatting * No changing files that are not specific to the feature * More covered below in the **Prepare commits** section. * Test your changes and please help us out by updating and implementing some automated tests. It is recommended that all contributors spend some time looking over the tests in the source code. You can't go wrong emulating one of the existing tests and then changing it specific to the behavior you are testing. + * While not an absolute requirement, automated tests will help reviewers feel comfortable about your changes, which gets your contributions accepted faster. * Please do not update your branch from the master unless we ask you to. See the responding to feedback section below. +### Code Format / Design + +#### C# + + * Class names are `PascalCase` - this is nearly the only time you start with uppercase. + * Namespaces (and their representative folders) are lowercase. + * Methods and functions are lowercase. Breaks between words in functions are typically met with an underscore (`_`, e.g. `run_actual()`). + * Variables and parameters are `camelCase`. + * Constants are `UPPER_CASE`. + * There are some adapters over the .NET Framework to ensure some additional functionality works and is consistent. Sometimes this is completely seamless that you are using these (e.g. `Console`). + +#### PowerShell + * PowerShell must be CRLF and UTF-8. Git attributes are not used, so Git will not ensure this for you. + * If you add a new file, also ensure you add it to the Visual Studio project and ensure it becomes an embedded resource. + +### Debugging / Testing + +When you want to manually verify your changes and run Choco, you have some options. + +**NOTE:** Chocolatey behaves differently when built with `Debug` and `Release` configurations. Release is always going to seek out the machine installation (`$env:ChocolateyInstall`), where Debug just runs right next to wherever the choco.exe file is. + +#### Visual Studio +When you are using Visual Studio, ensure the following: + + * Use `Debug` configuration - debug configuration keeps your local changes separate from the machine installed Chocolatey. + * `chocolatey.console` is the project you are looking to run. + * If you make changes to anything that is in `chocolatey.resources`, delete the folder in `chocolatey.console\bin\Debug` that corresponds to where you've made changes as Chocolatey does not automatically detect changes in the files it is extracting from resource manifests. + +#### Chocolatey Build + +**NOTE:** When you are doing this, we almost always recommend you take the output of the build to another machine to do the testing, like the [Chocolatey Test Environment](https://github.com/chocolatey/chocolatey-test-environment). + + * Run `build.bat`. + * There are two folders created - `build_output` and `code_drop`. You are looking for `code_drop\chocolatey\console` or `code_drop\nuget`. The `choco.exe` file contains everything it needs, but it does unpack the manifest on first use, so you could run into [#1292](https://github.com/chocolatey/choco/issues/1292). + * You will need to pass `--allow-unofficial-build` for it to work when built with release mode. + * You can also try `build.debug.bat` - note that it is newer and it may have an issue or two. It doesn't require `--allow-unofficial-build` as the binaries are built for debugging. + * Use `.\choco.exe` to point to the local file. By default in PowerShell.exe, if you have Chocolatey installed, when you call `choco`, that is using the installed `choco` and not the one in the folder you are currently in. You must be explicit. This catches nearly everyone. + ### Prepare Commits This section serves to help you understand what makes a good commit. @@ -115,7 +218,6 @@ Submitting PR: * One of the Chocolatey Team members, or one of the committers, will evaluate it within a reasonable time period (which is to say usually within 2-4 weeks). Some things get evaluated faster or fast tracked. We are human and we have active lives outside of open source so don't fret if you haven't seen any activity on your pull request within a month or two. We don't have a Service Level Agreement (SLA) for pull requests. Just know that we will evaluate your pull request. ### Respond to Feedback on Pull Request - We may have feedback for you to fix or change some things. We generally like to see that pushed against the same topic branch (it will automatically update the Pull Request). You can also fix/squash/rebase commits and push the same topic branch with `--force` (it's generally acceptable to do this on topic branches not in the main repository, it is generally unacceptable and should be avoided at all costs against the main repository). If we have comments or questions when we do evaluate it and receive no response, it will probably lessen the chance of getting accepted. Eventually this means it will be closed if it is not accepted. Please know this doesn't mean we don't value your contribution, just that things go stale. If in the future you want to pick it back up, feel free to address our concerns/questions/feedback and reopen the issue/open a new PR (referencing old one). diff --git a/README.md b/README.md index 71386314e4..e9a677ee99 100644 --- a/README.md +++ b/README.md @@ -5,15 +5,35 @@ You can just call me choco. [![](https://img.shields.io/chocolatey/dt/chocolatey.svg)](https://chocolatey.org/packages/chocolatey) [![](https://img.shields.io/chocolatey/v/chocolatey.svg)](https://chocolatey.org/packages/chocolatey) [![](https://img.shields.io/gratipay/Chocolatey.svg)](https://www.gratipay.com/Chocolatey/) [![Project Stats](https://www.openhub.net/p/chocolatey/widgets/project_thin_badge.gif)](https://www.openhub.net/p/chocolatey) + + +- [Build Status](#build-status) +- [Chat Room](#chat-room) +- [Support Chocolatey!](#support-chocolatey) +- [See Chocolatey In Action](#see-chocolatey-in-action) +- [Etiquette Regarding Communication](#etiquette-regarding-communication) +- [Information](#information) + - [Documentation](#documentation) + - [Requirements](#requirements) + - [License / Credits](#license--credits) +- [Submitting Issues](#submitting-issues) +- [Contributing](#contributing) +- [Committers](#committers) + - [Compiling / Building Source](#compiling--building-source) + - [Windows](#windows) + - [Other Platforms](#other-platforms) + - [Prerequisites:](#prerequisites) + - [Build Process:](#build-process) +- [Credits](#credits) + + ## Build Status - TeamCity | AppVeyor | Travis ------------- | ------------- | ------------- [![TeamCity Build Status](http://img.shields.io/teamcity/codebetter/bt429.svg)](http://teamcity.codebetter.com/viewType.html?buildTypeId=bt429) | [![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/jfxywa3xuwowt20w/branch/master?svg=true)](https://ci.appveyor.com/project/ferventcoder/choco/branch/master) | [![Travis Build Status](https://travis-ci.org/chocolatey/choco.svg?branch=master)](https://travis-ci.org/chocolatey/choco) ## Chat Room - Come join in the conversation about Chocolatey in our Gitter Chat Room [![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/chocolatey/choco?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) @@ -23,12 +43,10 @@ Or, you can find us in IRC at #chocolatey on freenode. IRC is not as often check Please make sure you've read over and agree with the [etiquette regarding communication](#etiquette-regarding-communication). ## Support Chocolatey! - * Purchase [Chocolatey Pro / Chocolatey for Business](https://chocolatey.org/pricing#compare) * Donate [![](https://img.shields.io/gratipay/Chocolatey.svg)](https://www.gratipay.com/Chocolatey/) ## See Chocolatey In Action - Chocolatey FOSS install showing tab completion and `refreshenv` (a way to update environment variables without restarting your shell): ![install](https://raw.githubusercontent.com/wiki/chocolatey/choco/images/gifs/choco_install.gif "Wat? Tab completion and updating environment variables!") @@ -38,11 +56,9 @@ Chocolatey FOSS install showing tab completion and `refreshenv` (a way to update ![install w/pro](https://raw.githubusercontent.com/wiki/chocolatey/choco/images/gifs/chocopro_install_stopped.gif "Chocolatey Pro availability now! A great option for individuals looking for that community PLUS option.") ## Etiquette Regarding Communication - When requesting support, try to remember that we are all volunteers that have lives outside of open source and none of us are paid to ensure things work for you, so please be considerate of others' time when you are asking for things. Many of us have families that also need time as well and only have so much time to give on a daily basis. In the future we hope that some of us are paid to do this full time and can provide better support when folks are running into issues, but until then a little consideration and patience can go a long way. After all, you are using a pretty good tool without cost. It may not be perfect (yet), and we know that. ## Information - * [Chocolatey Website and Community Package Repository](https://chocolatey.org) * [Mailing List](http://groups.google.com/group/chocolatey) / [Release Announcements Only Mailing List](https://groups.google.com/group/chocolatey-announce) / [Build Status Mailing List](http://groups.google.com/group/chocolatey-build-status) * [Twitter](https://twitter.com/chocolateynuget) / [Facebook](https://www.facebook.com/ChocolateySoftware) / [Github](https://github.com/chocolatey) @@ -67,9 +83,13 @@ Apache 2.0 - see [LICENSE](https://github.com/chocolatey/choco/blob/master/LICEN * If you are having issue with a package, please see [Request Package Fixes or Updates / Become a maintainer of an existing package](https://chocolatey.org/docs/package-triage-process). * If you are looking for packages to be added to the community feed (aka https://chocolatey.org/packages), please see [Package Requests](https://chocolatey.org/docs/package-triage-process#package-request-package-missing). - * If it is an issue with the website (the community feed aka https://chocolatey.org), please submit the issue to the [Chocolatey.org repo](https://github.com/chocolatey/chocolatey.org). - * If you have found an issue with the GUI (Chocolatey GUI), please see [the ChocolateyGUI repository](https://github.com/chocolatey/ChocolateyGUI#submitting-issues). - * If you have found an issue with the client (choco.exe), you are in the right place. Keep reading below. + + 1. Start with [Troubleshooting](https://github.com/chocolatey/choco/wiki/Troubleshooting) and the [FAQ](https://github.com/chocolatey/choco/wiki/ChocolateyFAQs) to see if your question or issue already has an answer. + 1. If not found or resolved, please follow one of the following avenues: + * If you are a licensed customer, please see [support](https://chocolatey.org/support). You can also log an issue to [Licensed Issues](https://github.com/chocolatey/chocolatey-licensed-issues) and we will submit issues to all other places on your behalf. Another avenue is to use email support to have us submit tickets and other avenues on your behalf (allowing you to maintain privacy). + * If it is an enhancement request or issue with the website (the community package repository aka [https://chocolatey.org](https://chocolatey.org)), please submit the issue to the [Chocolatey.org repo](https://github.com/chocolatey/chocolatey.org). + * If you have found an issue with the GUI (Chocolatey GUI) or you want to submit an enhancement, please see [the ChocolateyGUI repository](https://github.com/chocolatey/ChocolateyGUI#submitting-issues). + * If you have found an issue with the client (choco.exe), you are in the right place. Keep reading below. Observe the following help for submitting an issue: @@ -92,19 +112,15 @@ Submitting a ticket: * Include screenshots and/or animated gifs whenever possible, they help show us exactly what the problem is. ## Contributing - If you would like to contribute code or help squash a bug or two, that's awesome. Please familiarize yourself with [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md). ## Committers - Committers, you should be very familiar with [COMMITTERS](https://github.com/chocolatey/choco/blob/master/COMMITTERS.md). ### Compiling / Building Source - There is a `build.bat`/`build.sh` file that creates a necessary generated file named `SolutionVersion.cs`. It must be run at least once before Visual Studio will build. #### Windows - Prerequisites: * .NET Framework 4+ @@ -118,7 +134,6 @@ Build Process: Running the build on Windows should produce an artifact that is tested and ready to be used. #### Other Platforms - ##### Prerequisites: * Install and configure Mono 3.12.0 (3.8.0 should also work). @@ -184,5 +199,4 @@ chmod +x zip.sh Running the build on Mono produces an artifact similar to Windows but may have more rough edges. You may get a failure or two in the build script that can be safely ignored. ## Credits - Chocolatey is brought to you by quite a few people and frameworks. See [CREDITS](https://github.com/chocolatey/choco/blob/master/docs/legal/CREDITS.md) (just LEGAL/Credits.md in the zip folder) diff --git a/docs/legal/CREDITS.md b/docs/legal/CREDITS.md index 112e8795bc..86a932656f 100644 --- a/docs/legal/CREDITS.md +++ b/docs/legal/CREDITS.md @@ -25,7 +25,6 @@ Chocolatey has been the the thoughts, ideas, and work of a large community. Whil ### Committers - These are the committers to Chocolatey/Choco repositories: * [Core Development Team](https://github.com/orgs/chocolatey/teams/developers) @@ -36,21 +35,18 @@ These are the committers to Chocolatey/Choco repositories: * [Richard Simpson](https://github.com/RichiCoder1) - created and maintains the new Chocolatey GUI ### Chocolatey Community Team - The Chocolatey Community Team includes the committers and adds these fine folks: * [Community Package Repository Moderation Team](https://github.com/orgs/chocolatey/teams/community-moderators) * [Chocolatey Core Community Maintainers Team](https://github.com/orgs/chocolatey/teams/community-maintainers) ### Contributors - * [choco.exe](https://github.com/chocolatey/choco/graphs/contributors) * [Original Chocolatey - POSH choco](https://github.com/chocolatey/chocolatey/graphs/contributors) - * [Community Feed / Chocolatey.org](https://github.com/chocolatey/chocolatey.org/graphs/contributors) + * [Community Package Repository / Chocolatey.org](https://github.com/chocolatey/chocolatey.org/graphs/contributors) ### Other Contributors - -**NEEDS UPDATED** +**NOTE: NEEDS UPDATED** * Nekresh (https://github.com/nekresh) - Contributing code and ideas on direction * Chris Ortman (https://github.com/chrisortman) - package contributions and thoughts on where to take it @@ -59,7 +55,6 @@ The Chocolatey Community Team includes the committers and adds these fine folks: * Jason Jarrett (https://github.com/staxmanade) - contributing code and ideas ## Third Party Licenses - Development - Choco is built, tested and analyzed with the following fantastic frameworks (in no particular order): * ILMerge @@ -83,8 +78,7 @@ We would like to credit other super sweet tools/frameworks that aid in the devel * NuGet Framework ## Third Party Licenses - Runtime - -Chocolatey open source uses a number of 3rd party components. Their details are below. +Chocolatey open source uses a number of 3rd party components. Their details are below (order is alphabetical). From 1634247dcd94c5af707902f5a31b668262b34e27 Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Mon, 29 May 2017 13:54:47 -0500 Subject: [PATCH 26/26] (maint) COMMITTERS - EOL change to CRLF --- COMMITTERS.md | 176 +++++++++++++++++++++++++------------------------- 1 file changed, 88 insertions(+), 88 deletions(-) diff --git a/COMMITTERS.md b/COMMITTERS.md index cdd7058b5f..dd20e119f5 100644 --- a/COMMITTERS.md +++ b/COMMITTERS.md @@ -1,88 +1,88 @@ -Committers -============ - - - -- [Branch Targeting](#branch-targeting) -- [Summary](#summary) -- [Terminology](#terminology) -- [Review Process](#review-process) - - [Receive new PR (pull request)](#receive-new-pr-pull-request) - - [Initial PR Review](#initial-pr-review) - - [Review the Code](#review-the-code) - - [Accepting a PR](#accepting-a-pr) -- [Merging](#merging) - - [Merge Retargeting to Stable](#merge-retargeting-to-stable) - - - -## Branch Targeting -* Stable currently targeting 0.10.x -* Master currently targeting 0.11.x - -**NOTE:** The above may be out of date. The best way to determine what branch is being targeted for stable is to run `git describe`. Stable and master are typically different on minor version (`x.y.z` - the `y`). - -Depending on where your fix/enhancement goes, please target the proper branch. Community members always target master, but committers should know where the fix they are presenting goes. It makes it much easier to push the shiny green button on a pull request. If you are not sure where to target, ask. - -## Summary -We like to see folks contributing to Chocolatey. If you are a committer, we'd like to see you help from time to time with triage and the pull request process. - -In all cases politeness goes a long way. Please thank folks for contributions - they are going out of their way to help make the code base better, or adding something they may personally feel is necessary for the code base. - -Please be VERY familiar with [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and follow the process as well. - -## Terminology -**Contributor** - A person who makes a change to the code base and submits a change set in the form of a pull request. - -**Change Set** - A set of discrete commits which combined together form a contribution. A change set takes the form of git commits and is submitted in the form of a pull request. Used interchangeably with "pull request". - -**Committer** - A person responsible for reviewing a pull request and then making the decision what base branch to merge the change set into. - -## Review Process -#### Receive new PR (pull request) - * A contributor sends a pull request (usually against master). - * A committer typically reviews it within a week or less to determine the feasibility of the changes. - -#### Initial PR Review - * Has the user signed the Contributor License Agreement (CLA)? - * Did the user create a branch with these changes? If it is on their master, please ask them to review [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md). - * Did the user reformat files and they should not have? Was is just white-space? You can try adding [?w=1](https://github.com/blog/967-github-secrets) to the URL on GitHub. - * Are there tests? We really want any new contributions to contain tests so unless the committer believes this code really needs to be in the code base and is willing to write the tests, then we need to ask the contributor to make a good faith effort in adding test cases. Ask them to review the [contributing document](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and provide tests. **Note:** Some commits may be refactoring which wouldn't necessarily add additional test sets. - * Is the code documented properly? Does this additional set of changes require changes to the [wiki](https://github.com/chocolatey/choco/wiki)? - * Was this code warranted? Did the contributor follow the process of gaining approval for big change sets? If not please have them review the [contributing document](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and ask that they follow up with a case for putting the code into the code base on the mailing list. - -#### Review the Code - * Does the code meet the naming conventions and formatting (need link)? - * Is the code sound? Does it read well? Can you understand what it is doing without having to execute it? Principal of no clever hacks (need link). - * Does the code do what the purpose of the pull request is for (and only that)? - -#### Accepting a PR -Once you have reviewed the initial items, and are not waiting for additional feedback or work by the contributor, give the thumbs up that it is ready for the next part of the process (merging). - -Unless there is something wrong with the code, we don't ask contributors to rebase against master. They did the work to create the patch in the first place, asking them to unnecessarily come back and try to keep their code synced up with master is not an acceptable process. - -## Merging -Once you have reviewed the change set and determined it is ready for merge, the next steps are to bring it local and evaluate the code further by actually working with it, running the tests locally and adding any additional commits or fix-ups that are necessary in a local branch. - -When merging the user's contribution, it should be done with `git merge --log --no-ff` to create a merge commit so that in case there is an issue it becomes easier to revert later, and so that we can see where the code came from should we ever need to go find it later (more information on this can be found [here](https://www.kernel.org/pub/software/scm/git/docs/git-merge.html) and also a discussion on why this is a good idea [here](http://differential.io/blog/best-way-to-merge-a-github-pull-request)). - -### Merge Retargeting to Stable -Because we ask contributors to target master, sometimes a fix/enhancement may need to be retargeted to stable. This process is somewhat easy thanks to git. In most cases you won't even need to ask the user to do this for you. - - * `git fetch upstream pull//head:pr` - `upstream` is `git@github.com:chocolatey/choco.git` - * `git checkout pr` - * `git rebase --onto stable master` - this uses the local branch, starts with latest stable and reapplies the commits from the branch to it, removing all commits that were only on the master. - * `build.bat` - build and test - * `git checkout stable` - * Any addtional changes or testing here. - * `git merge pr --log --no-ff` - * `git branch -d pr` - * `git checkout master` - * `git merge stable` - * Make any last checks to ensure the git logs look good. The next step sets the commits in stone and unable to be changed. - * `git push upstream` - -References - - * http://pivotallabs.com/git-rebase-onto/ - * http://git-scm.com/book/ch3-6.html +Committers +============ + + + +- [Branch Targeting](#branch-targeting) +- [Summary](#summary) +- [Terminology](#terminology) +- [Review Process](#review-process) + - [Receive new PR (pull request)](#receive-new-pr-pull-request) + - [Initial PR Review](#initial-pr-review) + - [Review the Code](#review-the-code) + - [Accepting a PR](#accepting-a-pr) +- [Merging](#merging) + - [Merge Retargeting to Stable](#merge-retargeting-to-stable) + + + +## Branch Targeting +* Stable currently targeting 0.10.x +* Master currently targeting 0.11.x + +**NOTE:** The above may be out of date. The best way to determine what branch is being targeted for stable is to run `git describe`. Stable and master are typically different on minor version (`x.y.z` - the `y`). + +Depending on where your fix/enhancement goes, please target the proper branch. Community members always target master, but committers should know where the fix they are presenting goes. It makes it much easier to push the shiny green button on a pull request. If you are not sure where to target, ask. + +## Summary +We like to see folks contributing to Chocolatey. If you are a committer, we'd like to see you help from time to time with triage and the pull request process. + +In all cases politeness goes a long way. Please thank folks for contributions - they are going out of their way to help make the code base better, or adding something they may personally feel is necessary for the code base. + +Please be VERY familiar with [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and follow the process as well. + +## Terminology +**Contributor** - A person who makes a change to the code base and submits a change set in the form of a pull request. + +**Change Set** - A set of discrete commits which combined together form a contribution. A change set takes the form of git commits and is submitted in the form of a pull request. Used interchangeably with "pull request". + +**Committer** - A person responsible for reviewing a pull request and then making the decision what base branch to merge the change set into. + +## Review Process +#### Receive new PR (pull request) + * A contributor sends a pull request (usually against master). + * A committer typically reviews it within a week or less to determine the feasibility of the changes. + +#### Initial PR Review + * Has the user signed the Contributor License Agreement (CLA)? + * Did the user create a branch with these changes? If it is on their master, please ask them to review [CONTRIBUTING](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md). + * Did the user reformat files and they should not have? Was is just white-space? You can try adding [?w=1](https://github.com/blog/967-github-secrets) to the URL on GitHub. + * Are there tests? We really want any new contributions to contain tests so unless the committer believes this code really needs to be in the code base and is willing to write the tests, then we need to ask the contributor to make a good faith effort in adding test cases. Ask them to review the [contributing document](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and provide tests. **Note:** Some commits may be refactoring which wouldn't necessarily add additional test sets. + * Is the code documented properly? Does this additional set of changes require changes to the [wiki](https://github.com/chocolatey/choco/wiki)? + * Was this code warranted? Did the contributor follow the process of gaining approval for big change sets? If not please have them review the [contributing document](https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md) and ask that they follow up with a case for putting the code into the code base on the mailing list. + +#### Review the Code + * Does the code meet the naming conventions and formatting (need link)? + * Is the code sound? Does it read well? Can you understand what it is doing without having to execute it? Principal of no clever hacks (need link). + * Does the code do what the purpose of the pull request is for (and only that)? + +#### Accepting a PR +Once you have reviewed the initial items, and are not waiting for additional feedback or work by the contributor, give the thumbs up that it is ready for the next part of the process (merging). + +Unless there is something wrong with the code, we don't ask contributors to rebase against master. They did the work to create the patch in the first place, asking them to unnecessarily come back and try to keep their code synced up with master is not an acceptable process. + +## Merging +Once you have reviewed the change set and determined it is ready for merge, the next steps are to bring it local and evaluate the code further by actually working with it, running the tests locally and adding any additional commits or fix-ups that are necessary in a local branch. + +When merging the user's contribution, it should be done with `git merge --log --no-ff` to create a merge commit so that in case there is an issue it becomes easier to revert later, and so that we can see where the code came from should we ever need to go find it later (more information on this can be found [here](https://www.kernel.org/pub/software/scm/git/docs/git-merge.html) and also a discussion on why this is a good idea [here](http://differential.io/blog/best-way-to-merge-a-github-pull-request)). + +### Merge Retargeting to Stable +Because we ask contributors to target master, sometimes a fix/enhancement may need to be retargeted to stable. This process is somewhat easy thanks to git. In most cases you won't even need to ask the user to do this for you. + + * `git fetch upstream pull//head:pr` - `upstream` is `git@github.com:chocolatey/choco.git` + * `git checkout pr` + * `git rebase --onto stable master` - this uses the local branch, starts with latest stable and reapplies the commits from the branch to it, removing all commits that were only on the master. + * `build.bat` - build and test + * `git checkout stable` + * Any addtional changes or testing here. + * `git merge pr --log --no-ff` + * `git branch -d pr` + * `git checkout master` + * `git merge stable` + * Make any last checks to ensure the git logs look good. The next step sets the commits in stone and unable to be changed. + * `git push upstream` + +References + + * http://pivotallabs.com/git-rebase-onto/ + * http://git-scm.com/book/ch3-6.html