Code Quality: Refactor code and make it more modern

[Lamparter]

Hello (again again)!
In this PR I've done the following:

• Run Code Cleanup on the project - remove unnecessary using directives, format documents, etc.
  • Run special advanced code cleanup
  • Sort out .editorconfig
• Update namespaces of files to be consistent
• Change folder structure to match a modern project
• Fix NuGet package sources

...and various other changes that I will add here soon

[Muny]

• Fix NuGet package sources

Can you explain what the purpose of this change is?

[Lamparter]

Can you explain what the purpose of this change is?

There was a duplicate entry for the NuGet Gallery server, and the name for MyGet was wrong.

[Muny]

Can you explain what the purpose of this change is?

There was a duplicate entry for the NuGet Gallery server, and the name for MyGet was wrong.

I guess I'm wondering why MyGet was kept in lieu of NuGet

[Lamparter]

I guess I'm wondering why MyGet was kept in lieu of NuGet

The NuGet Gallery server is automatically enabled as a global package source in Visual Studio. Having it inside the nuget.config file would cause it to be duplicated.

[Lamparter]

Essentially I think it would be best for IronPython to follow a similar structure as the .NET Runtime and Files:

Top-level folder	2nd-level folder	Comment
src	DLR IronPython.Analyzer IronPython.Modules etc.	Should also contain a README.md file
docs		This wouldn't exist because of #1812
tests		The Tests directory
build	Packaging	where [Pp]ackaging is the current top-level Package directory
utils		The Util directory

In all .NET repos, it's common practice to have build files inside the eng directory. I wonder whether that might be a better name than build

[Lamparter]

This is why I hate submodules..
about the 5th time I've had to reclone 🫤

[BCSharp]

What's gained in running without the standard library?

1. Reduced deployment footprint (relevant in resource constrained cases, like e.g. embedded).
2. Reduced "attack surface" (removed unused code that may contain exploitable bugs; 3.4 is an old library, EOL upstream).

NB IronPython already supports it since the interpreter engine and the standard library are distributed in separate NuGet packages.

[Lamparter]

I don't understand why the tests are failing 🫤

---

[net462 cs]    Error Message:
[net462 cs]     Microsoft.Scripting.SyntaxErrorException : Could not find a part of the path 'E:\Repositories\Lamparter\IronPython\Tests\Inputs\raise.py'.
[net462 cs]    Stack Trace:
[net462 cs]       at Microsoft.Scripting.ErrorSink.Add(SourceUnit source, String message, SourceSpan span, Int32 errorCode, Severity severity) in E:\Repositories\Lamparter\IronPython\src\runtime\Src\Microsoft.Scripting\ErrorSink.cs:line 22
[net462 cs]     at IronPython.Compiler.Parser.CreateParser(CompilerContext context, PythonOptions options) in E:\Repositories\Lamparter\IronPython\src\core\IronPython\Compiler\Parser.cs:line 106
[net462 cs]     at IronPython.Runtime.PythonContext.ParseAndBindAst(CompilerContext context) in E:\Repositories\Lamparter\IronPython\src\core\IronPython\Runtime\PythonContext.cs:line 752
[net462 cs]     at IronPython.Runtime.PythonContext.CompilePythonCode(SourceUnit sourceUnit, CompilerOptions options, ErrorSink errorSink) in E:\Repositories\Lamparter\IronPython\src\core\IronPython\Runtime\PythonContext.cs:line 807
[net462 cs]     at IronPython.Runtime.PythonContext.CompileSourceCode(SourceUnit sourceUnit, CompilerOptions options, ErrorSink errorSink) in E:\Repositories\Lamparter\IronPython\src\core\IronPython\Runtime\PythonContext.cs:line 816
[net462 cs]     at Microsoft.Scripting.SourceUnit.Execute(Scope scope, ErrorSink errorSink) in E:\Repositories\Lamparter\IronPython\src\runtime\Src\Microsoft.Scripting\SourceUnit.cs:line 217
[net462 cs]     at Microsoft.Scripting.Hosting.ScriptSource.Execute(ScriptScope scope) in E:\Repositories\Lamparter\IronPython\src\runtime\Src\Microsoft.Scripting\Hosting\ScriptSource.cs:line 107
[net462 cs]     at Microsoft.Scripting.Hosting.ScriptEngine.ExecuteFile(String path, ScriptScope scope) in E:\Repositories\Lamparter\IronPython\src\runtime\Src\Microsoft.Scripting\Hosting\ScriptEngine.cs:line 150
[net462 cs]     at IronPythonTest.EngineTest.TestLineInfo(ScriptScope scope, String lineNumber) in E:\Repositories\Lamparter\IronPython\tests\ironpython\EngineTest.cs:line 2370
[net462 cs]     at IronPythonTest.EngineTest.ScenarioStackFrameLineInfo() in E:\Repositories\Lamparter\IronPython\tests\ironpython\EngineTest.cs:line 2355

[Lamparter]

Could not find a part of the path 'E:\Repositories\Lamparter\IronPython\Tests\Inputs\raise.py'

This path isn't specified anywhere, and the source code literally directs the tests to tests\python\Inputs and not Tests - I have no idea where it's getting this path from.

[slozier]

Could not find a part of the path 'E:\Repositories\Lamparter\IronPython\Tests\Inputs\raise.py'

This path isn't specified anywhere, and the source code literally directs the tests to tests\python\Inputs and not Tests - I have no idea where it's getting this path from.

This one maybe?
https://github.com/Lamparter/IronPython/blob/refactor/tests/ironpython/Cases/IronPythonCases.cs

[slozier]

A few comments:

Test issue:

         public TestManifest(Type parent) {
-            var file = parent.Assembly.GetManifestResourceStream($"IronPythonTest.Cases.{parent.Name}Manifest.ini");
+            var file = parent.Assembly.GetManifestResourceStream($"IronPython.Tests.Cases.{parent.Name}Manifest.ini");

There's a bunch of files which had their perms altered from 100755 → 100644 (like ipy3.4, dos2unix.sh, etc.). Having proper permissions on these scripts is important.

.gitignore: I definitely do not like the generic catch all of every conceivable generated file types. Please remove the additions.

Not a fan of the tests folder structure. Maybe something like:

• _ctypes_test
• IronPython.Tests
• suite (not sure I really like this one, but better than python)... or cases?

Otherwise it feels like it's almost there. Unfortunately I still can't review with GH because of the 3k file limit. Maybe we could split it up into smaller PRs, for example one with the non-src changes and then src? Guess I need to start looking at merging main into 3.6 before the refactoring...

[Lamparter]

I've been gone from work on this project for a while to focus on other things. Back soon!

[GeorgeS2019]

Will there be jumpstart for Ironclad3?

[Lamparter]

Will there be jumpstart for Ironclad3?

Not in this PR

[Lamparter]

There's a bunch of files which had their perms altered from 100755 → 100644 (like ipy3.4, dos2unix.sh, etc.). Having proper permissions on these scripts is important.

I have no idea how to fix this issue

• suite (not sure I really like this one, but better than python)... or cases?

I'll do suite for now but let me know if you change your mind

[Lamparter]

@slozier

[slozier]

I have no idea how to fix this issue

git update-index --chmod=+x script.sh

Any objections to my trying to do some of the refactoring changes outside of this PR?

[Lamparter]

Sure!
Probably would have been better if I'd split this into multiple PRs in the first place 😄
Also make sure to mention me when you do, I am watching this repo but tend to just bulk clear all my notifications so some get lost

[slozier]

I created a branch where I moved folders (using git mv so hopefully attributes and everything should be correct) around to roughly match the structure that the PR proposes: https://github.com/IronLanguages/ironpython3/tree/refactor

Here are the moves I performed (on Linux so I could change the casing):

git mv Documentation docs

git mv Src src
mkdir src/core
git mv src/IronPython src/core
git mv src/IronPython.Modules src/core
git mv src/StdLib src/core/IronPython.StdLib
git mv src/core/IronPython.StdLib/Lib src/core/IronPython.StdLib/lib
mkdir src/extensions
git mv src/IronPython.Wpf src/extensions
git mv src/IronPython.SQLite src/extensions
git mv src/DLR src/dlr
mkdir src/roslyn
git mv IronPythonAnalyzer src/roslyn/IronPython.Analyzer
mkdir src/executables
git mv src/IronPythonCompiler src/executables/IronPythonCompiler
git mv src/IronPythonConsole src/executables/IronPythonConsole
git mv src/IronPythonConsole32 src/executables/IronPythonConsole32
git mv src/IronPythonWindow src/executables/IronPythonWindow
git mv src/IronPythonWindow32 src/executables/IronPythonWindow32

git mv Build eng
git mv Package eng/package
git mv Util eng/utils
git mv src/Scripts eng/scripts

mkdir tests
git mv Tests tests/suite
git mv src/ctypes_test tests/ctypes_test
git mv src/IronPythonTest tests/IronPython.Tests

Which results in this top level directory tree:

.
├── docs
├── eng
│   ├── package
│   │   ├── choco
│   │   ├── deb
│   │   ├── dotnettool
│   │   ├── msi
│   │   ├── nuget
│   │   ├── pkg
│   │   └── zip
│   ├── scripts
│   └── utils
├── src
│   ├── core
│   │   ├── IronPython
│   │   ├── IronPython.Modules
│   │   └── IronPython.StdLib
│   │       └── lib
│   ├── dlr
│   ├── executables
│   │   ├── IronPythonCompiler
│   │   ├── IronPythonConsole
│   │   ├── IronPythonConsole32
│   │   ├── IronPythonWindow
│   │   └── IronPythonWindow32
│   ├── extensions
│   │   ├── IronPython.SQLite
│   │   └── IronPython.Wpf
│   └── roslyn
│       └── IronPython.Analyzer
└── tests
    ├── IronPython.Tests
    ├── ctypes_test
    └── suite

I did not rename the executables since I do not really like the proposed names. The two options that come to mind would be:

1. Add a . to match what we've done elsewhere (e.g. IronPython.Console).
2. Match the names to the produced executables:
   • IronPythonCompiler → ipyc
   • IronPythonConsole → ipy
   • IronPythonConsole32 → ipy32
   • IronPythonWindow → ipyw
   • IronPythonWindow32 → ipyw32

I'll do the fixing up of stuff to get things building as a PR on top of the refactor branch just so it's clear what's being modified.

@Lamparter @BCSharp If you have any thoughts or objections to the proposed structure please let me know and I can tweak the branch. Once we're happy with the core restructuring I'll merge and additional changes that this PR proposes (such as adding sub-READMEs) can hopefully be done then.

[Lamparter]

Wow, that looks amazing

I did not rename the executables since I do not really like the proposed names. The two options that come to mind would be [...]

The root namespace IronPython.* should definitely be enforced, and matching the project name to that is also standard. (the product assembly can be whatever though)
I understand what you mean by the names I proposed not being good (in the future I intend to minimise the need for these projects and infrastructure such as the Console(32)/window(32)), maybe for now just appending IronPython.{0} to the start would be alright:

• IronPythonCompiler: IronPython.Compiler
• IronPythonConsole: IronPython.Console
• IronPythonWindow32: IronPython.Window32
  For now that would be easier than any more complex rename.

[BCSharp]

I like the proposed structure. About the executables projects: I prefer the convention of adding the dot to separate the IronPython. prefix, like in IronPython.Console. Not only it is consistent with other naming, but also matches the name of the dotnet tool available on NuGet.

I am a little bit confused about src/roslyn/IronPython.Analyzer/IronPythonAnalyzer. Do we need that extra level or can it be collapsed to simply src/roslyn/IronPython.Analyzer (with the appriopiate project name change)?

[slozier]

I am a little bit confused about src/roslyn/IronPython.Analyzer/IronPythonAnalyzer. Do we need that extra level or can it be collapsed to simply src/roslyn/IronPython.Analyzer (with the appriopiate project name change)?

Good catch, I did not intend to have the extra level. Will fix it right now.

[slozier]

(with the appriopiate project name change)?

Just a quick note. I will look at project name changes once we're happy with the final folder structure and things are building properly in #1880.