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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

rstudio: 2023.9.0+463 -> 2023.12.1+402 #289393

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

Kupac
Copy link
Contributor

@Kupac Kupac commented Feb 16, 2024

Update RStudio. Visual mode is still not functional, but otherwise usable.

Description of changes

  • Update and add patches
  • Fix boost to 1.83
  • Update to rsconnect 1.2.0

Things done

  • Built on platform(s)

    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:

  • Tested basic functionality of all binary files (usually in ./result/bin/)

  • Fits CONTRIBUTING.md.


Add a 馃憤 reaction to pull requests you find important.

Copy link
Contributor

@christoph-heiss christoph-heiss left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 289393 run on x86_64-linux 1

4 packages built:
  • rstudio
  • rstudio-server
  • rstudioServerWrapper
  • rstudioWrapper

Also tested the binary, LGTM.

Copy link
Member

@diogotcorreia diogotcorreia left a comment

Choose a reason for hiding this comment

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

Is there any reason we're using commit hashes here instead of tags?
This commit seems to correspond to tag v1.2.0.

Changing this to use a tag instead of a commit would make it easier to spot malicious activity on nixpkgs: see #20836 (comment)

@@ -62,8 +63,8 @@ let
rsconnectSrc = fetchFromGitHub {
owner = "rstudio";
repo = "rsconnect";
rev = "5175a927a41acfd9a21d9fdecb705ea3292109f2";
hash = "sha256-c1fFcN6KAfxXv8bv4WnIqQKg1wcNP2AywhEmIbyzaBA=";
rev = "872b53e3d39a43c543498f437587fd7fb7fdca59";
Copy link
Member

Choose a reason for hiding this comment

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

I meant to leave a comment here instead, sorry 馃槄

#289393 (review)

@Kupac
Copy link
Contributor Author

Kupac commented Feb 18, 2024

Thanks for pointing this out, really good to know! I switched to tags. Unfortunately, another dependency, quarto, doesn't use tags or releases :-/

@diogotcorreia
Copy link
Member

@Kupac That's fine for now, since there's no alternative 馃槄
It might make sense to open an issue on quarto's repository to ask for tags on releases.

@Kupac
Copy link
Contributor Author

Kupac commented Feb 19, 2024

I just realised that the lsb-release is not a dependency, so I removed it. It was a remainder of my attempt to fix the lack of /etc/os-release in the build sandbox.

@Kupac
Copy link
Contributor Author

Kupac commented Feb 25, 2024

Could someone please merge this if it's good to go? If we don't hurry up, we'll have a new version soon :)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1490

Update and add patches
Fix boost to 1.83
Update to rsconnect 1.2.0

Apply suggestions from code review

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>

Improve version breakdown
@Kupac
Copy link
Contributor Author

Kupac commented Mar 6, 2024

Thanks for the suggestion. recompiled it and it's still working for me. If Ofborg and everyone else is happy, please merge, so I can update again a few weeks, probably ;-)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1538

@SuperSandro2000 SuperSandro2000 merged commit 9b5ca6a into NixOS:master Mar 20, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants