Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: manage access to pyrevit-config.ini at startup #2463

Open
5 tasks done
jmcouffin opened this issue Nov 22, 2024 · 6 comments
Open
5 tasks done

[Bug]: manage access to pyrevit-config.ini at startup #2463

jmcouffin opened this issue Nov 22, 2024 · 6 comments
Assignees
Labels
Bug Bug that stops user from using the tool or a major portion of pyRevit functionality [class] Investigating Waiting for information from contributor or other party Prioritize The issue is planned to be resolved in the next version

Comments

@jmcouffin
Copy link
Contributor

✈ Pre-Flight checks

  • I don't have SentinelOne antivirus installed (see above for the solution)
  • I have searched in the issues (open and closed) but couldn't find a similar issue
  • I have searched in the pyRevit Forum for similar issues
  • I already followed the installation troubleshooting guide thoroughly
  • I am using the latest pyRevit Version

🐞 Describe the bug

image

⌨ Error/Debug Message

'used by another process'

♻️ To Reproduce

Launch multiple sessions of Revit at once (within 10/30sec)

⏲️ Expected behavior

  • be able to launch as many revit sessions as possible at the same time
  • handling access to pyrevit-config.ini properly so that it does not get locked

🖥️ Hardware and Software Setup (please complete the following information)

NA

Additional context

old issue, never fixed

@jmcouffin jmcouffin added Bug Bug that stops user from using the tool or a major portion of pyRevit functionality [class] Prioritize The issue is planned to be resolved in the next version Investigating Waiting for information from contributor or other party labels Nov 22, 2024
@jmcouffin jmcouffin self-assigned this Nov 22, 2024
@jmcouffin
Copy link
Contributor Author

@sanzoghenzo
Copy link
Contributor

The problem is that the library that we use to parse the configuration always tries to open the file in write mode

https://github.com/MarioZ/MadMilkman.Ini/blob/64ac0180cc44afd72f8e78945d62ce10ed585f3d/MadMilkman.Ini/IniFile.cs#L106

but there's an override of that method (Line 144) that accepts a stream, so we could open the stream in read only mode and

One note, though: the library is abandoned, and doesn't support .net core, so we should replace it with something else...

@jmcouffin
Copy link
Contributor Author

The problem is that the library that we use to parse the configuration always tries to open the file in write mode

https://github.com/MarioZ/MadMilkman.Ini/blob/64ac0180cc44afd72f8e78945d62ce10ed585f3d/MadMilkman.Ini/IniFile.cs#L106

but there's an override of that method (Line 144) that accepts a stream, so we could open the stream in read only mode and

One note, though: the library is abandoned, and doesn't support .net core, so we should replace it with something else...

Thanks for the analysis.
Pardon my ignorance: there is no native way to access a stream in C#?
I am too illiterate in C# to get there
I ask my robot friend to get a native .NET solution

gpted answer

To address the issue in the pyRevit repository, here are the key steps and considerations:

  1. Immediate Fix Using Existing Library:
    The MadMilkman.Ini library's IniFile class has a method override that accepts a Stream. You can use this to open the INI file in read-only mode. Here’s how to adjust the code:

    using System.IO;
    
    // Assume 'filePath' is the path to the configuration file
    using (var fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read))
    {
        var iniFile = new MadMilkman.Ini.IniFile();
        iniFile.Load(fileStream); // Load from the stream in read-only mode
    }

    This will ensure the INI file is opened in a read-only manner, avoiding the issue with the library's default behavior of opening the file in write mode.

  2. Long-Term Solution: Replace MadMilkman.Ini:
    Since the MadMilkman.Ini library is abandoned and does not support .NET Core, consider replacing it with an actively maintained library. Some options include:

    • ini-parser: A lightweight, modern library for handling INI files.
    • Tomlyn: If you want to migrate to a more modern configuration format like TOML.

    Example of using ini-parser:

    using IniParser;
    using IniParser.Model;
    
    var parser = new FileIniDataParser();
    IniData data = parser.ReadFile(filePath); // No need for a specific mode as it respects file access
  3. Native Stream Handling in .NET:
    If you still need clarification about streams, in .NET, the FileStream class provides native ways to access files as streams. The following modes are particularly useful:

    • FileMode.Open: Open an existing file.
    • FileAccess.Read: Open the file in read-only mode.
    • FileShare.Read: Allow other processes to read the file while it's open.
  4. Plan for .NET Core Support:
    Since pyRevit is transitioning to .NET Core, ensure the replacement library supports .NET Core or .NET Standard. Test the solution thoroughly in a .NET Core environment.

  5. Propose a Fix for pyRevit:
    Create a pull request with the above adjustments for using Stream and communicate the long-term plan to replace MadMilkman.Ini. Include clear comments on why the change was made.

Would you like help drafting a pull request or implementing these changes in a development branch?

If you want to manage INI files natively in .NET without relying on external libraries, you can build a simple INI parser. While .NET doesn't have a built-in INI parser, it's straightforward to create one using the StreamReader and Dictionary classes.

Here’s how you can do it:

Step 1: Native INI Parsing Implementation

using System;
using System.Collections.Generic;
using System.IO;

public class IniParser
{
    private readonly Dictionary<string, Dictionary<string, string>> _data = new();

    public IniParser(string filePath)
    {
        Parse(File.OpenRead(filePath));
    }

    public IniParser(Stream iniStream)
    {
        Parse(iniStream);
    }

    private void Parse(Stream iniStream)
    {
        using var reader = new StreamReader(iniStream);
        string? currentSection = null;

        while (!reader.EndOfStream)
        {
            var line = reader.ReadLine()?.Trim();

            // Skip empty lines and comments
            if (string.IsNullOrWhiteSpace(line) || line.StartsWith(";") || line.StartsWith("#"))
                continue;

            // Detect section headers
            if (line.StartsWith("[") && line.EndsWith("]"))
            {
                currentSection = line[1..^1].Trim();
                if (!_data.ContainsKey(currentSection))
                {
                    _data[currentSection] = new Dictionary<string, string>();
                }
            }
            else if (currentSection != null)
            {
                // Parse key-value pairs
                var keyValue = line.Split('=', 2);
                if (keyValue.Length == 2)
                {
                    var key = keyValue[0].Trim();
                    var value = keyValue[1].Trim();
                    _data[currentSection][key] = value;
                }
            }
        }
    }

    public string? GetValue(string section, string key)
    {
        return _data.ContainsKey(section) && _data[section].ContainsKey(key)
            ? _data[section][key]
            : null;
    }

    public Dictionary<string, string>? GetSection(string section)
    {
        return _data.ContainsKey(section) ? _data[section] : null;
    }

    public override string ToString()
    {
        var result = string.Empty;
        foreach (var section in _data)
        {
            result += $"[{section.Key}]\n";
            foreach (var kvp in section.Value)
            {
                result += $"{kvp.Key} = {kvp.Value}\n";
            }
        }
        return result;
    }
}

Step 2: Example Usage

public class Program
{
    public static void Main()
    {
        var filePath = "config.ini";

        // Read INI file
        var iniParser = new IniParser(filePath);

        // Access specific values
        var value = iniParser.GetValue("Section1", "Key1");
        Console.WriteLine($"Value: {value}");

        // Get an entire section
        var section = iniParser.GetSection("Section2");
        if (section != null)
        {
            foreach (var kvp in section)
            {
                Console.WriteLine($"{kvp.Key} = {kvp.Value}");
            }
        }
    }
}

Explanation of Key Features

  1. Stream Support: You can pass a Stream to IniParser for flexibility, enabling scenarios like working with in-memory streams or read-only file streams.
  2. Dictionary-Based Storage: The parsed data is stored in a nested Dictionary structure for easy access.
  3. Section and Key Handling: You can retrieve values by section and key or entire sections as dictionaries.

Benefits

  • Fully native to .NET.
  • Lightweight and dependency-free.
  • Easily extendable for writing back to INI files if needed.

Enhancements (Optional)

  • Add support for escape characters or multi-line values.
  • Implement a method to write changes back to the file.
  • Add error handling for malformed INI files.

This approach ensures compatibility with .NET Core while avoiding reliance on third-party libraries. Let me know if you'd like to adapt this further!

@sanzoghenzo
Copy link
Contributor

Pardon my ignorance: there is no native way to access a stream in C#?

Yes there is. And it's the first answer that chatgpt gave you. It is the same thing that the Load metod that we currently use does, minus the FileShare.ReadWrite option.

As per the long term fix, I simply didn't have the time to check for alternative libraries yet. We can implement it ourselves, yes, but we have to make sure that the API are the same to avoid rewriting the code that accesses it (I didn't analyze the code yet, it might as well be that only the PyRevitConfig class uses it so we could just modify it)

@sanzoghenzo
Copy link
Contributor

I think we can go ahead with the short term solution since @dosymep already took care of finding the .net core version of the library;
Thinking long term and breaking-changes-v5-wise, we should definitely abandon ini files in favour of a more structured fornat like toml or json (which will reduce the dependencies to just newtonsoft)

@dosymep
Copy link
Member

dosymep commented Nov 23, 2024

@jmcouffin @sanzoghenzo
or we can rewrite configuration solution :)

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug that stops user from using the tool or a major portion of pyRevit functionality [class] Investigating Waiting for information from contributor or other party Prioritize The issue is planned to be resolved in the next version
Projects
None yet
Development

No branches or pull requests

3 participants