-
Notifications
You must be signed in to change notification settings - Fork 89
[WIP] RPM packaging doesn't flag .NET Core DLLs as executables #64
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
dbf8861
ebc9045
cfc1ea5
f13682a
8cb6b26
1e758f1
fdc35b3
d80ddbd
693c8fa
fc8c949
b2b0de7
c19298e
333273c
85e3bb2
614bdbf
114f34c
bba3266
667f91d
5214cc2
85cc823
2f06734
34844e8
2a612e2
2030f61
7e9d088
4de2cf5
a0f9d7b
c27b07f
375b24d
11271fa
a0bef75
2d9e358
b2d9526
c73b8b2
a482d7e
76d933e
0bffc23
d74fdbb
c47ecd0
5e80a6f
03a32b1
15a0059
12207f6
825031a
92133ef
8ed5793
e32cbd4
d737c44
693e999
dfbd117
1c84878
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| { | ||
| // Use IntelliSense to find out which attributes exist for C# debugging | ||
| // Use hover for the description of the existing attributes | ||
| // For further information visit https://github.com/OmniSharp/omnisharp-vscode/blob/master/debugger-launchjson.md | ||
| "version": "0.2.0", | ||
| "configurations": [ | ||
| { | ||
| "name": ".NET Core Launch (console)", | ||
| "type": "coreclr", | ||
| "request": "launch", | ||
| "preLaunchTask": "build", | ||
| // If you have changed target frameworks, make sure to update the program path. | ||
| "program": "${workspaceFolder}/dotnet-deb/bin/Debug/netcoreapp1.0/dotnet-deb.dll", | ||
| "args": [], | ||
| "cwd": "${workspaceFolder}/dotnet-deb", | ||
| // For more information about the 'console' field, see https://github.com/OmniSharp/omnisharp-vscode/blob/master/debugger-launchjson.md#console-terminal-window | ||
| "console": "internalConsole", | ||
| "stopAtEntry": false, | ||
| "internalConsoleOptions": "openOnSessionStart" | ||
| }, | ||
| { | ||
| "name": ".NET Core Attach", | ||
| "type": "coreclr", | ||
| "request": "attach", | ||
| "processId": "${command:pickProcess}" | ||
| } | ||
| ,] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "version": "2.0.0", | ||
| "tasks": [ | ||
| { | ||
| "label": "build", | ||
| "command": "dotnet", | ||
| "type": "process", | ||
| "args": [ | ||
| "build", | ||
| "${workspaceFolder}/dotnet-deb/dotnet-deb.csproj" | ||
| ], | ||
| "problemMatcher": "$msCompile" | ||
| } | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,41 +2,21 @@ | |
|
|
||
| namespace Packaging.Targets.Rpm | ||
| { | ||
| /* | ||
| * from https://github.com/rpm-software-management/rpm/blob/master/build/rpmfc.h | ||
| */ | ||
|
|
||
| /// <summary> | ||
| /// Determines the type of the file. | ||
| /// </summary> | ||
| [Flags] | ||
| internal enum RpmFileColor | ||
| internal enum RpmFileColor : int | ||
| { | ||
| RPMFC_BLACK = 0, | ||
| RPMFC_ELF32 = 1 << 0, | ||
| RPMFC_ELF64 = 1 << 1, | ||
| RPMFC_ELFMIPSN32 = 1 << 2, | ||
| RPMFC_PKGCONFIG = 1 << 4, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we keep these enum values? They map to the values defined in the RPM API I would prefer to keep them in sync.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did some research before I pulled those values out. The values you have here got dropped from RPM in 2010 and RPM's classification model doesn't use them anymore but rather uses the text classifier dictionary. My edits align to the current rpmfc.h header.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, sounds good then! |
||
| RPMFC_LIBTOOL = 1 << 5, | ||
| RPMFC_BOURNE = 1 << 6, | ||
| RPMFC_MODULE = 1 << 7, | ||
| RPMFC_EXECUTABLE = 1 << 8, | ||
| RPMFC_SCRIPT = 1 << 9, | ||
| RPMFC_TEXT = 1 << 10, | ||
| RPMFC_DATA = 1 << 11, | ||
| RPMFC_DOCUMENT = 1 << 12, | ||
| RPMFC_STATIC = 1 << 13, | ||
| RPMFC_NOTSTRIPPED = 1 << 14, | ||
| RPMFC_COMPRESSED = 1 << 15, | ||
| RPMFC_DIRECTORY = 1 << 16, | ||
| RPMFC_SYMLINK = 1 << 17, | ||
| RPMFC_DEVICE = 1 << 18, | ||
| RPMFC_LIBRARY = 1 << 19, | ||
| RPMFC_ARCHIVE = 1 << 20, | ||
| RPMFC_FONT = 1 << 21, | ||
| RPMFC_IMAGE = 1 << 22, | ||
| RPMFC_MANPAGE = 1 << 23, | ||
| RPMFC_PERL = 1 << 24, | ||
| RPMFC_JAVA = 1 << 25, | ||
| RPMFC_PYTHON = 1 << 26, | ||
| RPMFC_PHP = 1 << 27, | ||
| RPMFC_TCL = 1 << 28, | ||
| RPMFC_ELF = RPMFC_ELF32 | RPMFC_ELF64 | RPMFC_ELFMIPSN32, | ||
| RPMFC_WHITE = 1 << 29, | ||
| RPMFC_INCLUDE = 1 << 30, | ||
| RPMFC_ERROR = 1 << 31 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This try/catch sounds very expensive, and we can probably apply some more logic to determine whether a file is a .NET executable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exceptions are worrisome if they're on the hot path for performance sensitive code that runs on a sustained basis, like inside an ASP.NET app. This here is done once for each file during packaging of an app, so I don't find that of concern. The alternative is to pull in PE file decoder code, which is far more code (I started with that approach and discarded it). We could safeguard the section with a simple probe for the 'MZ' magic word being the first two bytes of the file content which should catch most cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a good compromise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add that.