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.

Sign up for GitHub

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

Jump to bottom

(tortoisesvn) Fix finding download URL from SourceForge #2446

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

(tortoisesvn) Fix finding download URL from SourceForge #2446

wants to merge 3 commits into from
+6 −6

Conversation

WhereTheTimeWent
Copy link

@WhereTheTimeWent WhereTheTimeWent commented Mar 31, 2024 •
edited
Loading

The current attempt of finding the download URL on SourceForge for TortoiseSVN doesn't work anymore.
It now gets the URL by checking data-release-url for "downloads.sourceforge.net".

How Has this Been Tested?

I tested it with VSCode. It returned downloadable URLs. They were both downloadable with Firefox and System.Net.WebClient (PowerShell).
I also ran Update-Package and it completed without errors.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Migrated package (a package has been migrated from another repository)

Checklist:

  • My code follows the code style of this repository.
  • My change requires a change to documentation (this usually means the notes in the description of a package).
  • I have updated the documentation accordingly (this usually means the notes in the description of a package).
  • I have updated the package description and it is less than 4000 characters.
  • All files are up to date with the latest Contributing Guidelines
  • The added/modified package passed install/uninstall in the Chocolatey Test Environment(https://github.com/chocolatey-community/chocolatey-test-environment/). Note that we don't support the use of any other environment.
  • The changes only affect a single package (not including meta package).

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@WhereTheTimeWent WhereTheTimeWent marked this pull request as draft March 31, 2024 19:30
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@WhereTheTimeWent WhereTheTimeWent marked this pull request as ready for review March 31, 2024 19:41
@AdmiringWorm AdmiringWorm linked an issue Apr 4, 2024 that may be closed by this pull request
(tortoisesvn) outdated #2413
Open
4 tasks
@WhereTheTimeWent WhereTheTimeWent marked this pull request as draft April 4, 2024 18:09
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

1 similar comment
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@WhereTheTimeWent WhereTheTimeWent marked this pull request as ready for review April 4, 2024 19:24
@WhereTheTimeWent
Copy link
Author

Sorry for this mess. Not new to PowerShell, but new to AU and pull requests.
We should now have a reliable way of updating TortoiseSVN.

JPRuskin
JPRuskin approved these changes Jun 13, 2024
View reviewed changes
Copy link
Contributor

@JPRuskin JPRuskin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I'll just rebase it and add the commit heading we use to indicate that it affects a package.

Thank you for getting this sorted!

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@JPRuskin
Copy link
Contributor

image

Given that error, I'm going to trigger the build by pushing and removing a commit. Sorry for the noise.

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@WhereTheTimeWent @JPRuskin
(tortoisesvn) Fix finding download URL from SourceForge
05ecae1 
The current attempt of finding the download URL on SourceForge for TortoiseSVN doesn't work anymore.
@WhereTheTimeWent @JPRuskin
(tortoisesvn) Removing the part after ? so Get-RemoteFiles works
c132f52 
Get-RemoteFiles tried to add the ? and everything afterwards to the download's file name.
@WhereTheTimeWent @JPRuskin
(tortoisesvn) Using /download instead of data-release-url
3ed6af2 
Grabbing data-release-url wasn't reliable and unnecessarily complicated. Just putting /download at the end of the URL and returning FileType = "msi" did the trick.
@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@WhereTheTimeWent
Copy link
Author

Are there problems with my change? Update-Package and Test-Package run without issues on my end and download/install the latest version 1.14.7.29687.

@JPRuskin
Copy link
Contributor

Are there problems with my change?

Not at all.

All works fine on my end too - I'm just trying to chase down the reason we're getting that failure on the AppVeyor build. It's an odd one as a) it's not a package that's in the dependency chain from TortoiseSVN and b) it's not failing in other builds yesterday/today.

@pauby
Copy link
Member

pauby commented Jun 29, 2024

@JPRuskin did you find any more on this?

@sebvonhelsinki
Copy link

@JPRuskin Is there any update on the state of this PR? Would it be possible to just re-trigger the build and see if it is working now? Is there anything someone else could do to advance this PR?

@fbuehler-vector
Copy link

@JPRuskin can we please rerun the CI? i tested it localy an it works fine. Not sure why the CI failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JPRuskin JPRuskin JPRuskin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(tortoisesvn) outdated
6 participants
@WhereTheTimeWent @AppVeyorBot @JPRuskin @pauby @sebvonhelsinki @fbuehler-vector