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

netcore #2088

Merged
merged 109 commits into from
Aug 13, 2024
Merged

netcore #2088

merged 109 commits into from
Aug 13, 2024

Conversation

dosymep
Copy link
Member

@dosymep dosymep commented Feb 13, 2024

No description provided.

@dosymep dosymep mentioned this pull request Feb 13, 2024
6 tasks
@jmcouffin jmcouffin added the pyRevit 5 pyRevit 5 coming release label Feb 13, 2024
@dosymep
Copy link
Member Author

dosymep commented Feb 14, 2024

@eirannejad, I don't have libs on .NET Core

  • FileFormatWavefront.dll
  • Ifc.NET.dll
  • Ifc.NET.XmlSerializers.dll
  • Microsoft.WindowsAPICodePack.dll
  • Microsoft.WindowsAPICodePack.Shell.dll
  • pyRevitLabs.MahAppsMetro.dll (available alpha version)
  • PythonStubsBuilder.dll
  • Rhino3dmIO.dll
  • rhino3dmio_native.dll
  • MadMilkman.Ini
  • Expression.Blend.Sdk

@dosymep
Copy link
Member Author

dosymep commented Feb 15, 2024

  • transferred pyRevit paths to targets
  • added revit 2025 dependensis
  • added revit 2026-2025 docs (xml) (removed)
  • added revit 2025 project to pyRevit.Runtime
  • added revits preprocessor variables like this
  • added multitargets net48, net8.0
  • remove $(RevitVersion) from version pyRevitLoader

@dosymep
Copy link
Member Author

dosymep commented Feb 15, 2024

I think we need to divide projects into those that use Revit, just libraries, applications.
Transfer all libraries to nuget, thereby greatly simplifying the assembly of the solution.
Remove destination paths from the solution and compile using dotnet build -c Release -o "pyRevit/bin".

@dosymep dosymep marked this pull request as ready for review February 15, 2024 11:14
@dosymep dosymep marked this pull request as draft February 15, 2024 11:26
# Conflicts:
#	dev/pyRevitLoader/pyRevitLoader.2710.csproj
#	dev/pyRevitLoader/pyRevitLoader.273.csproj
#	dev/pyRevitLoader/pyRevitLoader.277.csproj
#	dev/pyRevitLoader/pyRevitLoader.278.csproj
#	dev/pyRevitLoader/pyRevitLoader.279.csproj
#	dev/pyRevitLoader/pyRevitRunner.2710.csproj
#	dev/pyRevitLoader/pyRevitRunner.273.csproj
#	dev/pyRevitLoader/pyRevitRunner.277.csproj
#	dev/pyRevitLoader/pyRevitRunner.278.csproj
#	dev/pyRevitLoader/pyRevitRunner.279.csproj
@jmcouffin jmcouffin marked this pull request as ready for review February 21, 2024 14:58
@RTLflat
Copy link

RTLflat commented Aug 12, 2024

One thing I noticed when I attach the visual studio debugger: I got a ton of Exception thrown: 'IronPython.Runtime.UnboundNameException' in pyRevitLabs.IronPython.dll messages, and the loading takes ages.

@sanzoghenzo
This is your Visual Studio debug settings. Does this as well in pyRevit 4. Can't rembember which debug setting causes it, but I can show what my current debug settings are that don't have the issue if you want

@sanzoghenzo
Copy link
Contributor

Alright, Cpython tests are working now! I was overwriting the __builtins__ of the module with the pyrevit variables instead of updating the dict 😅

I also added a config parameter for the pyrevit build products command that gets passed to dotnet build, so that one can build everything (well, except the dependencies) in Debug mode (pyrevit build products debug) and attach the visual studio debugger.

The last commit is a refactor of the various Get.*Engine.* methods to use linq and simplify the netcore/netfx conditions; maybe it will solve @jmcouffin problem, but my expectations are very low...

@sanzoghenzo
Copy link
Contributor

sanzoghenzo commented Aug 12, 2024

@jmcouffin you were right, I've edited too many lines of code and introduced a bug 😅

In trying to reduce the code repetition, I wrongly set all the engines as netfx, even the netcore ones. It is now fixed.

The code in pyrevit cli throws an exception, but the program itself exits with a 0 exit code, that's why the installer doesn't complain.
I think this should be changed, and use an non-zero exit code if there's any error. this also helps when using scripts to automate things (I'm thinking about corporate setup and configuration)

EDIT: I went ahead and added the non-zero exit code. That's it for today, I consider it mission accomplished 😎

@jmcouffin
Copy link
Contributor

mission accomplished

Clearly

@jmcouffin
Copy link
Contributor

Checked this morning the latest WIP installer
It works in all versions of Revit!
Good Job @sanzoghenzo and @dosymep 🍾 🚀


A thing I noticed, and I am pretty sure this is not a new bug:

==> Running Revit Instances
Debug: Getting host product info for: 20240319_1700(x64)
Debug: Getting data source "C:\Users\Jean-Marc\AppData\Roaming\pyRevit\Cache\pyrevit-hosts.json"
Debug: Getting data source "C:\Users\Jean-Marc\AppData\Roaming\pyRevit-Master\bin\pyrevit-hosts.json"
Debug: Data source exists "C:\Users\Jean-Marc\AppData\Roaming\pyRevit-Master\bin\pyrevit-hosts.json"
Debug: Using already loaded data. Identical signatures "1205207726" = "1205207726"
Error: Object reference not set to an instance of an object. (System.NullReferenceException)
   at pyRevitCLI.PyRevitCLIRevitCmds.<>c.<PrintLocalRevits>b__1_0(RevitProcess x) in D:\a\pyRevit\pyRevit\dev\pyRevitLabs\pyRevitClI\PyRevitCLIRevitCmds.cs:line 31
   at System.Linq.EnumerableSorter`2.ComputeKeys(TElement[] elements, Int32 count)
   at System.Linq.EnumerableSorter`1.ComputeMap(TElement[] elements, Int32 count)
   at System.Linq.EnumerableSorter`1.Sort(TElement[] elements, Int32 count)
   at System.Linq.OrderedEnumerable`1.GetEnumerator()+MoveNext()
   at pyRevitCLI.PyRevitCLIRevitCmds.PrintLocalRevits(Boolean running) in D:\a\pyRevit\pyRevit\dev\pyRevitLabs\pyRevitClI\PyRevitCLIRevitCmds.cs:line 31
   at pyRevitCLI.PyRevitCLIAppCmds.MakeEnvReport(Boolean json) in D:\a\pyRevit\pyRevit\dev\pyRevitLabs\pyRevitClI\PyRevitCLIAppCmds.cs:line 144
   at pyRevitCLI.PyRevitCLI.ProcessArguments() in D:\a\pyRevit\pyRevit\dev\pyRevitLabs\pyRevitClI\PyRevitCLI.cs:line 189
   at pyRevitCLI.PyRevitCLI.Main(String[] args) in D:\a\pyRevit\pyRevit\dev\pyRevitLabs\pyRevitClI\PyRevitCLI.cs:line 150

the hosts file is located in all 4 locations:
image
which is a bit of a waste considering the CLI is looking for all these location apparently

and checked against in the CLI in the running instances and throws a NullReferenceException
Previously, the behaviour of the CLI was:

  • NullReferenceException when no identical version of Revit running was found in the pyrevit-hosts.json file
  • Listing properly running instances of Revit when the list pyrevit-hosts.json was up to date and running instances versions where found in the file

Like @dosymep suggested yesterday, I think we could merge this PR and making it a beta release and from now on, address the remaining issues in a new PR.
This might involve adding the GH workflow for publication from your branch @dosymep ?

Thoughts?

@sanzoghenzo
Copy link
Contributor

Yes, I think this is ready to be merged.
If you don't mind, I would rebase and squash all the commits into a single one, so that the git history remains clean.

@dosymep
Copy link
Member Author

dosymep commented Aug 13, 2024

We need to merge the current master into my branch, and then make a squash commit. Then we'll refine the beta and fix the problems in the new PR. I don't have the opportunity to do this at the moment, I'm far from home :)

good work @sanzoghenzo @jmcouffin :)

@jmcouffin
Copy link
Contributor

We need to merge the current master into my branch, and then make a squash commit. Then we'll refine the beta and fix the problems in the new PR. I don't have the opportunity to do this at the moment, I'm far from home :)

good work @sanzoghenzo @jmcouffin :)

You do it, or I do? @sanzoghenzo

@sanzoghenzo
Copy link
Contributor

You do it, or I do? @sanzoghenzo

My OCD will make me take way longer than needed, so you can go ahead, even without squash (the git history is a total mess anyway 🤣)

Only one thing: maybe we should merge the develop-4 branch onto dosymep's, so that we have also the latest fixes...

@jmcouffin
Copy link
Contributor

@dosymep I will need write access to your fork to make this happen
dosymep#2

@jmcouffin
Copy link
Contributor

Only one thing: maybe we should merge the develop-4 branch onto dosymep's, so that we have also the latest fixes...

exactly what I thought

@sanzoghenzo
Copy link
Contributor

@dosymep I will need write access to your fork to make this happen dosymep#2

You could have done it directly in your local git without the need for PRs, just add dosymeps repo as a remote im your working folder

@jmcouffin
Copy link
Contributor

I did my best to squash and merge it without messing the work previously done 🤞
I will merge this one so that we start clean with bug fixing and new items

@jmcouffin jmcouffin merged commit dd05ab3 into pyrevitlabs:develop Aug 13, 2024
@sanzoghenzo
Copy link
Contributor

sanzoghenzo commented Aug 13, 2024

The merge of develop-4 deleted the .cs files in the dev/pyRevitLabs.PyRevitRuntime directory!

@jmcouffin
Copy link
Contributor

The merge of develop-4 deleted the .cs files in the dev/pyRevitLabs.PyRevitRuntime directory!

It might be a mistake from my side, let me check

@jmcouffin
Copy link
Contributor

jmcouffin commented Aug 13, 2024

The merge of develop-4 deleted the .cs files in the dev/pyRevitLabs.PyRevitRuntime directory!

do we need those? with all the changes?

@sanzoghenzo
Copy link
Contributor

sanzoghenzo commented Aug 13, 2024

Yes, the .cs files are needed, otherwise the resulting pyRevitLabs.Pyrevit.Runtime.YYYY.dll would be empty!

The build and CI doesn't fail because the Directory.Build.props in the parent directory has <Compile Include="..\*.cs" />, so it doesn't look for specific files to be present

@jmcouffin
Copy link
Contributor

Yes, the .cs files are needed, otherwise the resulting pyRevitLabs.Pyrevit.Runtime.YYYY.dll would be empty!

The build and CI doesn't fail because the Directory.Build.props in the parent directory has <Compile Include="..\*.cs" />, so it doesn't look for specific files to be present

fixing it right now

@jmcouffin
Copy link
Contributor

@sanzoghenzo can you go on slack, that will make things easier?

@jmcouffin jmcouffin mentioned this pull request Aug 13, 2024
@eirannejad
Copy link
Collaborator

Y'all are rockstars!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pyRevit 5 pyRevit 5 coming release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants