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

Jump to bottom

genymotion: refactor src from requireFile to fetchurl #62109

Merged
merged 1 commit into from May 29, 2019
Merged

genymotion: refactor src from requireFile to fetchurl #62109

merged 1 commit into from May 29, 2019

Conversation

aakropotkin
Copy link
Contributor

The old src url was updated by the host, making the required file inaccessible. The new url links directly to the required file.

Motivation for this change

The old src URL was updated by the host, making the required file inaccessible.

Things done

The old URL was to an html page which shows the most recent version of genymotion.
There was recently a major version update (2.8.0 -> 3.0.2) which now occupies the old URL.
I tried updating our nix expression to the latest package, but ran into issues linking xcb.
Next I tried finding the old file but could not locate it through genymotion's webpage, or various search engines.
Next I looked at the download URL associated with the newest version. I swapped out the version numbers of the URL, and luckily the file was still available at it's direct path.
I am updating the Nix expression to include this direct path so that it will provide a more helpful message to users.
I am hoping to save future users from the trouble of tracking the file down.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

teto
teto requested changes May 27, 2019
View reviewed changes
Copy link
Member

@teto teto left a comment

Choose a reason for hiding this comment

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

thanks for looking into it. I've made a few comments.

pkgs/development/mobile/genymotion/default.nix Outdated Show resolved Hide resolved
pkgs/development/mobile/genymotion/default.nix Outdated Show resolved Hide resolved
@etu
Copy link
Contributor

etu commented May 27, 2019 •
edited

@GrahamcOfBorg build genymotion

Ah, it's unfree. Scratch that attempt.

@etu
Copy link
Contributor

etu commented May 28, 2019

@BadDecisionsAlex Do you want to squash your commits to a single one (which I feel is logical for this change) or should someone who merges this do it for you? :)

@aakropotkin
Copy link
Contributor Author

I won’t lie I’ve never done a squash but I’ll do a bit of reading right quick.

@teto
Copy link
Member

teto commented May 28, 2019

or we can squash from the github UI.

@infinisil
Copy link
Member

I won’t lie I’ve never done a squash but I’ll do a bit of reading right quick.

I think it would be a good idea to learn it then:

  • git rebase -i HEAD~4 (because you want to make a modification to the last 4 commits)
  • Replace pick with squash for the last three commits, they'll get squashed in the commit before. That page has some more info on what this mode can do
  • git push --force to push the changes here

@aakropotkin
Copy link
Contributor Author

I won’t lie I’ve never done a squash but I’ll do a bit of reading right quick.

I think it would be a good idea to learn it then:

  • git rebase -i HEAD~4 (because you want to make a modification to the last 4 commits)
  • Replace pick with squash for the last three commits, they'll get squashed in the commit before. That page has some more info on what this mode can do
  • git push --force to push the changes here

I really appreciate you taking the time to help me out with this. Thank you!

@aakropotkin
genymotion: refactor src from requireFile to fetchurl
4b46acd 
The old src url was updated by the host, making the required file inaccessible. The new url links directly to the required file. This direct link allows us to use fetchurl rather than requireFile.

genymotion: refactor src url

Fixed revisions:
1) Uses `pname`.
2) URL uses version variable in path. Data type for `url` was changed from path to string.

genymotion: refactor src url

Removed redundant `name` definition.

fixed typo in URL

changed requireFile to fetchurl
@aakropotkin aakropotkin changed the title genymotion: refactor src url genymotion: refactor src from requireFile to fetchurl May 28, 2019
@etu etu merged commit 4318c3c into NixOS:master May 29, 2019
@etu
Copy link
Contributor

etu commented May 29, 2019

@BadDecisionsAlex Thanks! :)

@eoli3n eoli3n mentioned this pull request Feb 13, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etu etu etu left review comments

@infinisil infinisil infinisil left review comments

@teto teto teto approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aakropotkin @etu @teto @infinisil