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

[Package Request]: Trippy 0.7.0 #100719

Closed
1 task done
fujiapple852 opened this issue Mar 26, 2023 · 20 comments · Fixed by #101363
Closed
1 task done

[Package Request]: Trippy 0.7.0 #100719

fujiapple852 opened this issue Mar 26, 2023 · 20 comments · Fixed by #101363
Labels
In-PR Package-Request This is a request for a package (new or updated version)
Milestone

Comments

@fujiapple852
Copy link
Contributor

How can we help?

I would like someone else to build the manifest.

Please read and ensure the following

  • The installer meets the above requirements

Please provide the following information

Download Page Url: https://github.com/fujiapple852/trippy/releases/tag/0.7.0
Publisher: https://github.com/fujiapple852
Package Name: Trippy (trip.exe)
Description: A network diagnostic tool
Package Version: 0.7.0
Installer URL: https://github.com/fujiapple852/trippy/releases/download/0.7.0/trippy-0.7.0-x86_64-pc-windows-msvc.zip

Note that I tried to create this myself using wingetcreate but was not able to get it to work (I understood that a zip'd exe should work?):

PS C:\Users\vboxuser> wingetcreate new https://github.com/fujiapple852/trippy/releases/download/0.7.0/trippy-0.7.0-x86_64-pc-windows-msvc.zip
Downloading and parsing: https://github.com/fujiapple852/trippy/releases/download/0.7.0/trippy-0.7.0-x86_64-pc-windows-msvc.zip...
Failed to parse the package from [https://github.com/fujiapple852/trippy/releases/download/0.7.0/trippy-0.7.0-x86_64-pc-windows-msvc.zip]
@fujiapple852 fujiapple852 added Help-Wanted This is a good candidate work item from the community. Package-Request This is a request for a package (new or updated version) labels Mar 26, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage This work item needs to be triaged by a member of the core team. label Mar 26, 2023
@BrandonWanHuanSheng
Copy link
Contributor

It also same for me. It doesn't work also.

@stephengillie stephengillie removed the Needs-Triage This work item needs to be triaged by a member of the core team. label Mar 27, 2023
@fujiapple852
Copy link
Contributor Author

Any thought on how we can diagnose the problem?

@fujiapple852
Copy link
Contributor Author

Ah the issue is that I didn't read the instructions properly, trippy on Windows is compressed with 7z which is not supported by winget.

I'll have to look at moving to another supported format, or perhaps an uncompressed package.

@fujiapple852
Copy link
Contributor Author

I tried using 7z a -tzip "$staging.zip" "$staging" but that didn't fix it

@BrandonWanHuanSheng
Copy link
Contributor

This is an examples of windows portable package
Screenshot (122)

@mdanish-kh
Copy link
Contributor

I've created the package PR, but converted it to draft because there is a known issue with nested portable packages where Path variable isn't updated correctly in non-developer mode / installation from non-admin shells in Windows.

The fix for the above issue is already merged in a PR microsoft/winget-cli#3002 and will likely be available in the next release of WinGet, so I would say we hold off adding this package until the fix appears in the next release for best user experience.

As for the issue with winget-create, I've create an issue (linked above) over at the winget-create repo.

@fujiapple852
Copy link
Contributor Author

Thank you very much @mdanish-kh!

Does the nested portable package issue you mention explain the CI failures in your PR? I don't really understand #101363 (comment) which makes me think it is expecting an installer?

In case it matters; whilst I now build the package as a real zip archive using 7z a -tzip "$staging.zip" "$staging" (see here), the 0.7.0 release archive was built with 7z a "$staging.zip" "$staging" so is not a real zip archive, despite the name. I only mention it as 7z archives are not supported by wingetcreate.

@BrandonWanHuanSheng
Copy link
Contributor

Hold Off first.

@mdanish-kh
Copy link
Contributor

mdanish-kh commented Apr 11, 2023

Does the nested portable package issue you mention explain the CI failures in your PR?

I don't think the two issues are related.

I don't really understand #101363 (comment) which makes me think it is expecting an installer?

@stephengillie would be able to clarify more, but I think the CI environment is expecting trip command to return a non-error code after installation to ascertain the installation was successful. Nevertheless the installation was manually verified by @stephengillie, and Validation-Completed Validation passed label was added. The PR is waiting on the PATH behavior fix from winget-cli to appear in the next release.

In case it matters; whilst I now build the package as a real zip archive using 7z a -tzip "$staging.zip" "$staging" (see here), the 0.7.0 release archive was built with 7z a "$staging.zip" "$staging" so is not a real zip archive, despite the name. I only mention it as 7z archives are not supported by wingetcreate.

I don't think it matters, the fix for the nested portable issue got merged in winget-create's codebase in PR microsoft/winget-create#356 (tested with your package and it works) and the fix will likely appear in the next release for winget-create. I'm not sure when the next release will ship out but until then you could try out other manifest authoring tools if you wish

@fujiapple852
Copy link
Contributor Author

That’s great, thanks @mdanish-kh

@fujiapple852
Copy link
Contributor Author

fujiapple852 commented May 2, 2023

I saw some recent activity on the PR @mdanish-kh, it says "Validation has completed" and it was approved but looks like it's still failing. Is it still just a case of waiting or is some action needed? Thanks!

@mdanish-kh
Copy link
Contributor

mdanish-kh commented May 2, 2023

@fujiapple852 The PR is good to go; if I convert it from draft to "Ready for review", it will get merged into the repo. The only catch is that the package will work correctly only on the latest preview build which is WinGet v1.5.1081-preview (and higher versions when they come out). Ideally, I would like to wait until 1.5 is a stable release before we merge the package to avoid issues with people on the current stable release but if you want we can merge the package now (just know it won't work for people on the current stable 1.4 release)

@fujiapple852
Copy link
Contributor Author

I’m fine to wait for the stable release, thanks for the update!

@BrandonWanHuanSheng
Copy link
Contributor

BrandonWanHuanSheng commented May 26, 2023

Current Version is 0.8.0 Due to the author released new update on 2023 May 15. I also have some pull-request still in draft. I am also waiting for winget to roll out the update.

@stephengillie
Copy link
Collaborator

Thank you very much @mdanish-kh!

Does the nested portable package issue you mention explain the CI failures in your PR? I don't really understand #101363 (comment) which makes me think it is expecting an installer?

In case it matters; whilst I now build the package as a real zip archive using 7z a -tzip "$staging.zip" "$staging" (see here), the 0.7.0 release archive was built with 7z a "$staging.zip" "$staging" so is not a real zip archive, despite the name. I only mention it as 7z archives are not supported by wingetcreate.

Sorry for not seeing this earlier. The label Internal-Error-Dynamic-Scan indicates an issue local with the Automated Validation pipeline, not an issue with the package itself, and is one reason why we have the Manual Validation pipeline. I was able to manually validate the package, and applied the Validation-Complete label when this became true.

For portable command-line applications, these still may be blocked by Automated Validation, which is somewhat built to expect a Windows Form. This is another reason for the Manual Validation pipeline, and both issues were resolved in the same manual validation.

@microsoft-github-policy-service microsoft-github-policy-service bot added In-PR and removed Help-Wanted This is a good candidate work item from the community. labels Jul 6, 2023
@fujiapple852
Copy link
Contributor Author

fujiapple852 commented Sep 29, 2023

Hi there @BrandonWanHuanSheng @mdanish-kh @stephengillie I wonder if you can help with something.

Trippy requires allowing some incoming ICMP traffic in Windows defender and so I’d like to explore the possibility of enabling this access at installation time. This has become something if a FAQ for Windows users of Trippy (recent example fujiapple852/trippy#693).

Is this something winget allows (both technically and as a policy)?

According to ChatGPT it can be done by adding something like the following (untested, so just to illustrate the idea):

{
  "Id": "YourPublisher.YourApp",
  "Name": "YourApp",
  "Version": "1.0",
  "Publisher": "YourPublisher",
  "Description": "YourApp description",
  "Homepage": "https://yourapp.com",
  "License": "MIT",
  "LicenseUrl": "https://yourapp.com/license",
  "Installers": [
    {
      "Architecture": "x64",
      "InstallerType": "custom",
      "InstallerUrl": "https://yourapp.com/download/yourapp.exe",
      "InstallerSha256": "<SHA256 hash>",
      "Custom": {
        "Silent": "/S /C \"netsh advfirewall firewall add rule name=\"ICMPv4\" protocol=icmpv4:8,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv4 Echo Reply\" protocol=icmpv4:0,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv4 Time Exceeded\" protocol=icmpv4:11,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv6\" protocol=icmpv6:8,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv6 Echo Reply\" protocol=icmpv6:0,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv6 Time Exceeded\" protocol=icmpv6:11,any dir=in action=allow\"",
        "SilentWithProgress": "/S /C \"netsh advfirewall firewall add rule name=\"ICMPv4\" protocol=icmpv4:8,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv4 Echo Reply\" protocol=icmpv4:0,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv4 Time Exceeded\" protocol=icmpv4:11,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv6\" protocol=icmpv6:8,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv6 Echo Reply\" protocol=icmpv6:0,any dir=in action=allow & netsh advfirewall firewall add rule name=\"ICMPv6 Time Exceeded\" protocol=icmpv6:11,any dir=in action=allow\""
      }
    }
  ]
}

@mdanish-kh
Copy link
Contributor

mdanish-kh commented Sep 30, 2023

@fujiapple852 I don't think that the snippet provided by ChatGPT will work. Since Trippy is a portable app, WinGet only takes the responsibility of extracting the zip archive into a directory, making that directory available in the PATH variable and its metadata available in the registry.

Modifying Windows Defender settings looks like a use case for the winget configure command provided there's a resource available for it (that I don't know of but maybe @denelon could help here)

At the very least though, we can add InstallationNotes: to the manifest informing the user post-installation that this package requires enabling ICMP traffic in Windows Defender. It would look something like:

zoomit

@fujiapple852
Copy link
Contributor Author

fujiapple852 commented Oct 1, 2023

Thanks @mdanish-kh, I agree that adding a message is likely the best option. What i'll do is create FAQ entry in README that gives details of how to do this which can then be linked from the InstallationNotes as you suggest.

I've created https://github.com/fujiapple852/trippy#windows-defender for this. Shall I raise a PR to update the InstallationNotes or does this need to wait for the next release?

@mdanish-kh
Copy link
Contributor

@fujiapple852 You can raise the PR anytime you'd like :)

@denelon denelon added this to the 1.7 Packages milestone Nov 1, 2023
@fujiapple852
Copy link
Contributor Author

@mdanish-kh I've updated the manifest to have:

InstallationNotes: You *must* configure the Windows Defender firewall to allow incoming ICMP traffic, see https://github.com/fujiapple852/trippy#windows-defender

I've also bumped the version to the new 0.9.0 release at the same time:

#129163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In-PR Package-Request This is a request for a package (new or updated version)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants