-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
WIP: Export IFW (Beta) #1734
WIP: Export IFW (Beta) #1734
Conversation
Add Kinect driver to OpenNI2 port. OpenNI2 port generate Kinect driver when Kinect SDK has been installed in your system.
Dynamic path convert of Kinect SDK directory in patch file.
Hello everyone interested! |
Hello @alexkaratarakis and @ras0219-msft, |
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.
It looks like a great addition to me...
I did not review this code, I just noticed this typo from the screenshots.
toolsrc/src/commands_export.cpp
Outdated
line += "<Package>"; | ||
lines.push_back(line); line = skip; | ||
line += "<DisplayName>"; | ||
line += "Intergation"; |
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.
There is a typo here... Intergation -> Integration (same typo also a few lines later in the comment)
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.
Good catch. Thank you!
QtIFW CR203958/3 has updated. |
4e08ee5
to
0d3789f
Compare
@alexkaratarakis, ping :-) |
57b7956
to
1cc3222
Compare
49535dd
to
6f5bbdb
Compare
python2 version bump
Sorry for the delay! This is really cool and we would love to merge it in with some more polish! For the time being, we want to avoid external library dependencies to avoid the horrible bootstrapping problem. If there aren't too many fields, it may be clean enough to synthesize with Given the significant size of the XML generation code, I think it would be best to break it into a separate function or even another file. [1] https://github.com/Microsoft/vcpkg/blob/c2a735f6bee792d2419b0616eaa84abef91536e6/toolsrc/src/commands_integrate.cpp#L19-L25 |
Hello @ras0219-msft, thanks for approving my ideas. I hope that these changes will be useful for many developers. |
6f5bbdb
to
3239409
Compare
Add Kinect for Windows SDK v2.x port.
3239409
to
c454e4b
Compare
@ras0219-msft, today I have move my code to another files. If somewere is wrong, please correct me. |
toolsrc/src/commands_export.cpp
Outdated
std::string export_id = create_export_id(); | ||
if (ifw) | ||
{ | ||
export_id = "vcpkg-export"; // TODO: Remove after debugging |
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.
TODO: Remove after debugging
toolsrc/src/commands_export.cpp
Outdated
if (ifw) | ||
{ | ||
// Export real package and return data dir for installation | ||
spec_exported_dir_path = IFW::export_real_package(raw_exported_dir_path, action, fs); |
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.
How does this handle multiple export formats at the same time (i.e. --ifw --nuget --raw
)? I think either the from_destination_root()
call needs to be duplicated or you could add a check that requires --ifw
to not be passed with any other export types.
toolsrc/src/commands_export.cpp
Outdated
fs.create_directories(destination.parent_path(), ec); | ||
Checks::check_exit(VCPKG_LINE_INFO, !ec); | ||
fs.copy_file(source, destination, fs::copy_options::overwrite_existing, ec); | ||
Checks::check_exit(VCPKG_LINE_INFO, !ec); | ||
} | ||
|
||
if (ifw) | ||
{ | ||
// Unigue packages |
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.
Typo
toolsrc/src/commands_export.cpp
Outdated
// Integration | ||
IFW::export_integration(raw_exported_dir_path, fs); | ||
// Configuration | ||
IFW::export_config(raw_exported_dir_path, maybe_ifw_repository_url.value_or(""), fs); |
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.
It looks like the IFW path really isn't able to reuse much or anything from the main loop -- it might make sense to completely extract it from the loop and just have
IFW::export(export_plan, raw_exported_dir_path, fs);
Then, we'd completely skip the main loop when only running --ifw
.
Note that I do think the functions should probably stay separated, but they'd be private to the IFW implementation resulting in a "cleaner" public interface.
toolsrc/src/commands_export.cpp
Outdated
@@ -417,7 +460,7 @@ With a project open, go to Tools->NuGet Package Manager->Package Manager Console | |||
print_next_step_info("[...]"); | |||
} | |||
|
|||
if (!raw) | |||
if (!raw && !ifw) |
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 assume this was for debugging?
I pushed a commit that switches to using |
Hello @ras0219-msft, thanks for your comments and commit. Now I will correct my mistakes. |
[libspatialite] initial port
[telnetpp] Initial port for 'Telnet++'
[geos] allow geos_c static library cretation (libgeos_c.lib)
…naming convention.
CppCon is done... Ping :-) |
Extract WiX installer using Dark. It will be standalone extract files from installer of Kinect SDK 1.x even if Kinect SDK 1.x is not installed in user system.
Fix according to changes of Kinect SDK 1.x port. Change to refer Kinect SDK 1.x that installed using vcpkg port. It will be always generate the Kinect SDK 1.x driver, even when Kinect SDK 1.x is not installed in user system.
[geos] remove build_cmake
Add Kinect SDK v2.x port
Extract Kinect SDK 1.x Installer using Dark
Change to refer Kinect SDK 1.x that installed using vcpkg port
…podsvirov-export-ifw
Thanks again for all this work, it's really incredible! I've merged for now (with a few refactorings/housekeepings of my own); I know you have some follow-up improvements and I'd be happy to handle them through separate PRs! |
@ras0219-msft, thank you for merging my changes. It was really unexpected for me now, but I'm very glad that it happened. Of course I have plans for further improvement and I will do this work consistently on. |
@ras0219-msft, the problem of the first export is still relevant to me:
Unexpected symbols |
Hmm, we had some issues with byte-order marks before and were able to solve them with [1]. Could you put a breakpoint there, repro the issue, and let us know what the exact byte values are at the beginning of the response? |
Thanks, that's really helpful! It appears you're getting two byte order marks (because your bytes weren't sufficiently ordered, I guess? 😋). I've pushed 9b0c2cb to master which changes that function to loop until we no longer find any BOMs; I think this will solve your issue. |
@ras0219-msft thanks, it seems that now everything works as it should. |
Add export to binary crossplatform repository/installer
with GUI based on QtIFW:
http://doc.qt.io/qtinstallerframework/ifw-overview.html
For correct operation of these changes,
you must use the corrected QtIFW:
https://codereview.qt-project.org/#/c/203958