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

gitlab: 12.1.6 -> 12.3.5, bundler: 1.17.2 -> 1.17.3 #70216

Merged
merged 6 commits into from Oct 8, 2019

Conversation

talyz
Copy link
Contributor

@talyz talyz commented Oct 1, 2019

Motivation for this change
  • Update GitLab to 12.3.2
  • Update update.py to cope with the new upstream repository structure
  • Refactor gitlab-shell to use buildGoPackage and bundlerEnv for
    dependencies
  • Make update.py able to update gitlab-shell dependencies
  • Various fixes necessary for update to work

This would replace #69325

Things done

Manually built and tested both ce and ee on fresh nixops VirtualBox VMs. I would like to test it more thoroughly, but it seems to be working well in my quick tests.

  • 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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @flokli @globin @bgamari @krav

Copy link
Contributor

@bgamari bgamari left a comment

Choose a reason for hiding this comment

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

This looks great. I'll try testing it tonight.

@bgamari
Copy link
Contributor

bgamari commented Oct 2, 2019

I suspect that gitlab-shell still isn't quite right. Trying to push to a repository via SSH appears to fail with:

$ git push
/nix/store/wdr7r9vzshv61x0mw00i1gbnggy23vpi-gitlab-shell-7.1.2/lib/gitlab_shell.rb:270:in `repo_path=': Repository path not provided. Please make sure you're using GitLab v8.10 or later. (ArgumentError)
	from /nix/store/wdr7r9vzshv61x0mw00i1gbnggy23vpi-gitlab-shell-7.1.2/lib/gitlab_shell.rb:108:in `verify_access'
	from /nix/store/wdr7r9vzshv61x0mw00i1gbnggy23vpi-gitlab-shell-7.1.2/lib/gitlab_shell.rb:42:in `block in exec'
	from /nix/store/wdr7r9vzshv61x0mw00i1gbnggy23vpi-gitlab-shell-7.1.2/lib/gitlab_metrics.rb:50:in `measure'
	from /nix/store/wdr7r9vzshv61x0mw00i1gbnggy23vpi-gitlab-shell-7.1.2/lib/gitlab_shell.rb:42:in `exec'
	from /nix/store/wdr7r9vzshv61x0mw00i1gbnggy23vpi-gitlab-shell-7.1.2/bin/gitlab-shell:20:in `<main>'
fatal: Could not read from remote repository.

@talyz
Copy link
Contributor Author

talyz commented Oct 2, 2019

@bgamari Ah, I'll see if I get the same result..

@bgamari
Copy link
Contributor

bgamari commented Oct 2, 2019

Actually, looking at my GitLab user's authorized keys file it refers to gitlab-shell-7.2.1 (my test box was quite out-of-date). I suspect authorized_keys just needs to be rebuilt.

@bgamari
Copy link
Contributor

bgamari commented Oct 2, 2019

Indeed, false alarm. Simply running

sudo -u git gitlab-rake gitlab:shell:setup

resolved the issue. Things appear to be just peachy.

@talyz
Copy link
Contributor Author

talyz commented Oct 2, 2019

Yeah, I wasn't able to reproduce it on my end, so that makes sense. :)

"owner": "gitlab-org",
"repo": "gitlab-ce",
"rev": "v12.1.6",
"repo": "gitlab-foss",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to keep separate attrsets? It seems like gitlab-foss is just a poorly-synced version of the gitlab repo these days...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe not? It doesn't seem to be too poorly synced to me, but from what I've gathered, the only difference between them is that the gitlab repo has an ee directory which is removed from the tree when it's synced to gitlab-foss...

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, at least the git repo history is a bit less informative.

Obviously, Nix doesn't care much. However, after reading up on https://about.gitlab.com/2019/02/21/merging-ce-and-ee-codebases/, it should be safe to just remove the /ee folder if we're not building the enterprise edition, so I'd advocate for having a conditional step in postUnpack depending on the gitlabEnterprise flag, and hardcode everything else to https://gitlab.com/gitlab-org/gitlab.

This would greatly simplify data.nix and the updater.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that states pretty clearly that all we have to do is get rid of the ee folder, so I'll look into it. :)

alyssais pushed a commit that referenced this pull request Oct 2, 2019
@alyssais
Copy link
Member

alyssais commented Oct 2, 2019 via email

@talyz
Copy link
Contributor Author

talyz commented Oct 2, 2019

I just pushed some minor changes:

@bgamari
Copy link
Contributor

bgamari commented Oct 2, 2019

Whoops, I accidentally commented on the old PR.

In short, currently the NixOS systemd unit does not clear /run/gitlab/config/initializers which can cause issues with this upgrade.

Also, there are some very strange migration issues which I had to work around.

@bgamari bgamari mentioned this pull request Oct 2, 2019
10 tasks
@flokli
Copy link
Contributor

flokli commented Oct 2, 2019

Can we bump this to 12.3.3? There has been another security release…

@ghost
Copy link

ghost commented Oct 2, 2019

I have tested the gitlab commit and it works nicely 👍

@jslight90 jslight90 mentioned this pull request Oct 3, 2019
10 tasks
@talyz
Copy link
Contributor Author

talyz commented Oct 3, 2019

@flokli ..and now they've released 12.3.4! I'll update to it once the deb has been built.

@bgamari Hm, yeah, that should definitely be purged on startup. I feel like it fits better into #68721, which already tries to address similar migration issues concerning the state directory. That PR also needs a bit of a push. :)

@bgamari
Copy link
Contributor

bgamari commented Oct 3, 2019

For what it's worth I have tested this and it works well.

- Update GitLab to 12.3.4

- Update update.py to cope with the new upstream repository structure

- Refactor gitlab-shell to use buildGoPackage and bundlerEnv for
  dependencies

- Refactor gitlab-workhorse to use buildGoPackage for dependencies

- Make update.py able to update gitlab-shell and gitlab-workhorse
  dependencies

- Various fixes necessary for update to work
Split the remove-hardcoded-locations patch into two separate patches,
one for the ruby package and one for the go package. This is clearer
and results in fewer rebuilds.
@talyz
Copy link
Contributor Author

talyz commented Oct 4, 2019

I just pushed some updates:

  • Updated to 12.3.4
  • Added support for updating the go dependencies of gitlab-workhorse in update.py

@talyz talyz changed the title gitlab: 12.1.6 -> 12.3.2, bundler: 1.17.2 -> 1.17.3 gitlab: 12.1.6 -> 12.3.4, bundler: 1.17.2 -> 1.17.3 Oct 4, 2019
@flokli
Copy link
Contributor

flokli commented Oct 4, 2019

@talyz can you address #70216 (comment) too?

@talyz
Copy link
Contributor Author

talyz commented Oct 4, 2019

@flokli Oh, sorry, I completely missed that. Will do.

@flokli
Copy link
Contributor

flokli commented Oct 4, 2019

Thanks!

@ghost
Copy link

ghost commented Oct 8, 2019

Here goes 12.3.5..

Instead of extracting prebuilt assets from the debian build, build
them from the source. This should give faster package updates and
reduces the amount of data needed to be downloaded by more than 500MB.
@flokli
Copy link
Contributor

flokli commented Oct 8, 2019 via email

@talyz
Copy link
Contributor Author

talyz commented Oct 8, 2019

@flokli Yeah, I'm on it! :)

GitLab recently restructured their repos; whereas previously they had
one gitlab-ce and one gitlab-ee repo, they're now one and the
same. All proprietary components are put into the ee subdirectory -
removing it gives us the foss / community version of GitLab. For more
info, see
https://about.gitlab.com/2019/02/21/merging-ce-and-ee-codebases/

This gives us the opportunity to simplify things quite a bit, since we
don't have to keep track of two separate versions of either the base
data or rubyEnv.
@talyz
Copy link
Contributor Author

talyz commented Oct 8, 2019

Just pushed some changes - two major ones:

  • Rewrote the base package and updater to only use the gitlab-org/gitlab repo and simply delete the ee directory for the community edition
  • Build the frontend assets instead of extracting them from the debian build - this means we don't have to rely on upstream builds, which sometimes take quite a while to finish. As an added bonus it also saves more than 500MB on downloads, but at the expense of added build time.

... and two smaller ones:

  • Updated to 12.3.5
  • Added myself to list of maintainers for gitlab, gitlab-shell and gitlab-workhorse. I don't mind removing this if anyone is opposed. :)

@talyz talyz changed the title gitlab: 12.1.6 -> 12.3.4, bundler: 1.17.2 -> 1.17.3 gitlab: 12.1.6 -> 12.3.5, bundler: 1.17.2 -> 1.17.3 Oct 8, 2019
@globin globin merged commit eadeca9 into NixOS:master Oct 8, 2019
@talyz talyz deleted the gitlab_12_3_2 branch October 9, 2019 06:53
@lheckemann
Copy link
Member

This isn't building, because apparently the hash is wrong:

trying https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-workhorse/repository/archive.tar.gz?sha=v8.10.0
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  196k  100  196k    0     0   912k      0 --:--:-- --:--:-- --:--:--  912k
unpacking source archive /private/tmp/nix-build-source.drv-0/archive.tar.gz?sha=v8.10.0
hash mismatch in fixed-output derivation '/nix/store/yr8w0isia86rg96wl4f2ikbg4h8ahw2h-source':
  wanted: sha256:11cfhh48dga5ghfcijb68gbx0nfr5bs3vvp2j1gam9ac37fpvk0x
  got:    sha256:07hv2kl2w1gw9613mvz844vy2dc4my4d8n9b36j25hyaw564zj82

(from hydra: https://hydra.nixos.org/build/102785558/nixlog/18 )

Could you have a look at that?

@talyz
Copy link
Contributor Author

talyz commented Oct 9, 2019

@lheckemann Hm, strange. I'll look into it right away.

@talyz
Copy link
Contributor Author

talyz commented Oct 9, 2019

@lheckemann Hm, it looks like the hash is different on macOS, for some reason? I can't reproduce this error on NixOS, at least. I also don't have any mac with just the right version of macOS at hand, so I can't verify that this is what happens. If this is the case, I think the reasonable thing to do is to limit gitlab-workhorse, and possibly the others, to linux; this is the restriction put on gitlab already and I don't see why you would want to host the other components on a different platform... That said, if fetchFromGitLab sometimes returns different hashes for different platforms, this should be investigated.

@zimbatm
Copy link
Member

zimbatm commented Oct 9, 2019

@talyz try changing the hash to something different and re-run the build. If your /nix/store already has a fixed-output derivation with the given hash it won't try and re-download the dependency and you won't see the issue.

@talyz
Copy link
Contributor Author

talyz commented Oct 9, 2019

@zimbatm Yeah, I know - I ran nix-collect-garbage to get rid of any build in my store, but it still built without any problem. Then, just to be sure, I changed the hash to a hash that is probably wrong, which caused a hash mismatch that suggests that the right hash is the one that was already specified in the file. I've also tried it on one additional NixOS machine and I managed to find a mac to test it on - it builds fine there too. Additionally, the aarch64-linux build on hydra works - https://hydra.nixos.org/build/102785354, and it uses the same source. So yeah, I don't really know how to take this any further - someone with access to hydra probably has to look into it.

@ghost
Copy link

ghost commented Oct 9, 2019

@talyz try changing the hash to something different and re-run the build. If your /nix/store already has a fixed-output derivation with the given hash it won't try and re-download the dependency and you won't see the issue.

I can not reproduce the issue either even after cleaning the store.

talyz added a commit to talyz/nixpkgs that referenced this pull request Oct 9, 2019
Extracted from NixOS#70216.

(cherry picked from commit 96a1dba)
@talyz talyz mentioned this pull request Oct 9, 2019
10 tasks
alyssais pushed a commit that referenced this pull request Oct 9, 2019
Extracted from #70216.

(cherry picked from commit 96a1dba)
@talyz
Copy link
Contributor Author

talyz commented Oct 10, 2019

@lheckemann @zimbatm Well, I guess this solved itself, or someone poked around in hydra :)

@zimbatm
Copy link
Member

zimbatm commented Oct 10, 2019

hehe, oh well :)

alyssais pushed a commit that referenced this pull request Nov 11, 2019
Extracted from #70216.

(cherry picked from commit 96a1dba)
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