Manage `sources.list` and make bootstrap script purge lists before updating #520

choudhary001 · 2016-10-24T14:42:09Z

@aneeshusa
Hi,
As discussed I have created this pull request. Let know me what you think could be the issue.

One way to test this is manual verification inside a Vagrant VM,
I tried this as well after rebuilding the VMs, I checked in servo-master1 vm but could find the XXX string in the vm's sources.list file.

Perhaps I am missing to modify some other config files.

Fixes #417.

This change is

highfive · 2016-10-24T14:42:36Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @aneeshusa (or someone else) soon.

aneeshusa

It seems like Travis at least picks up on the updated sources.list file, so this is working to some degree. A few review nits so far, I will also check this PR inside Vagrant as well.

aneeshusa · 2016-10-30T07:49:25Z

ubuntu/apt/sources.list

+## your rights to use the software. Also, please note that software in 
+## multiverse WILL NOT receive any review or updates from the Ubuntu
+## security team.
+deb http://archive.ubuntu.com/ubuntu trusty multiverse


Since we're pre-enabling multiverse here, we'll need to remove the multiverse related states in servo-build-dependencies/init.sls.

servo-build-dependencies/init.sls has some if-else condition around multiverse related states. I am not sure what is the correct way to remove them. Should I remove multiverse links from source.list file?

Just remove the 2 multiverse related states in that file (but keep the ttf-mscorefonts-installer state), and fixup the requisites. You can ignore the if/else conditions for now.

aneeshusa · 2016-10-30T07:49:25Z

ubuntu/init.sls

+    - group: root
+    - mode: 755
+    - source: salt://{{ tpldir }}/apt/sources.list
+


nit: remove trailing empty line

will take care of it.

aneeshusa · 2016-10-30T07:49:25Z

ubuntu/init.sls

+    - user: root
+    - group: root
+    - mode: 755
+    - source: salt://{{ tpldir }}/apt/sources.list


nit: use salt://{{ tpldir }}/files/sources.list for consistency instead with other formulas

will take care of it.

aneeshusa

Looks like your commit history is a bit messed up - please rebase your changes on top of the latest master. Ask here or on IRC if you need help.

aneeshusa · 2016-10-31T20:15:52Z

servo-build-dependencies/init.sls

@@ -57,20 +58,6 @@ servo-darwin-install-autoconf213-and-fix-links:
    - runas: {{ homebrew.user }}
    - require:
      - pkg: servo-dependencies
-{% else %}


You need to keep this {% else %} line to avoid breaking the Jinja syntax.

aneeshusa · 2016-10-31T20:15:52Z

ubuntu/files/sources.list

@@ -0,0 +1,54 @@
+## Note that this sources.list file is picked from saltfs repository


Note the original source instead (I'm assuming an Ubuntu VM).
It's also probably a good idea to add a note/warning that this file is managed by Salt.

bors-servo · 2016-11-01T18:55:19Z

☔ The latest upstream changes (presumably #531) made this pull request unmergeable. Please resolve the merge conflicts.

choudhary001 · 2016-11-04T13:01:53Z

@aneeshusa Hey, I squashed all my commits in one. Let me know if everything is fine, it still shows the label 'needs-rebase'.

aneeshusa

Sorry about the delay, this is almost ready to merge!

aneeshusa · 2016-12-08T21:26:43Z

ubuntu/files/sources.list

@@ -0,0 +1,55 @@
+# Note that this sources.list file is copied from a Ubuntu vm


grammar nit: vm -> VM

aneeshusa · 2016-12-08T21:26:43Z

ubuntu/files/sources.list

+deb-src http://security.ubuntu.com/ubuntu trusty-security main
+deb http://security.ubuntu.com/ubuntu trusty-security universe
+deb-src http://security.ubuntu.com/ubuntu trusty-security universe
+# deb http://security.ubuntu.com/ubuntu trusty-security multiverse


We probably ought to uncomment these next two lines to enable security fixes for multiverse.

aneeshusa · 2016-12-08T21:26:43Z

ubuntu/init.sls

+  file.managed:
+    - user: root
+    - group: root
+    - mode: 755


This file should not be executable, use 644 permissions.

aneeshusa · 2017-01-01T23:21:07Z

Interesting - everything seems to work fine in Vagrant, but the tests are failing on Travis, only on the builds from scratch. I'm not really sure what the problem is, so we'll have to do some research/try some different things out. Off the top of my head I'm guessing the issue may be related to ordering of the Salt states, which we can fix by adding requisites, or something apt related, e.g. maybe we need to run apt-get update after changing the sources.list file.

choudhary001 · 2017-06-06T19:46:54Z

@aneeshusa
The travis build failed with below error, I am not sure why:

[ERROR ] Data passed to highstate outputter is not a valid highstate return: {'local': ["No function declared in state 'onchanges' in SLS 'ubuntu'"]}

aneeshusa

This should fix the Travis build hopefully; I also recommend testing in Vagrant if you can.
I will try to test this manually in the next few days.

aneeshusa · 2017-06-06T20:13:21Z

ubuntu/init.sls

+refresh_pkg_db:
+  module.run:
+    - name: pkg.refresh_db
+  onchanges:


Oh, this needs to be an argument to module.run:

refresh_pkg_db: module.run: - name: ... - onchanges: - file: /etc/apt/sources.list


        Avoid picking sources.list file from EC2 mirrors

- sources.list file is now managed by Salt. - sources.list file is copied from a Ubuntu vm - auto refresh ubuntu package databases - use jinja templating create appropriate sources.list file depending on Ubuntu version

aneeshusa · 2017-09-20T02:58:38Z

Sorry about the many delays, this LGTM and is r=me. I'll merge this after #719, which fixes our build.

aneeshusa · 2017-09-22T07:01:38Z

@bors-servo r+

bors-servo · 2017-09-22T07:01:39Z

📋 Looks like this PR is still in progress, ignoring approval

aneeshusa · 2017-09-22T07:02:07Z

@bors-servo r+

bors-servo · 2017-09-22T07:02:08Z

📌 Commit 15c1068 has been approved by aneeshusa

bors-servo · 2017-09-22T07:15:40Z

⌛ Testing commit 15c1068 with merge 229ea9e...


        Auto merge of #520 - choudhary001:master, r=aneeshusa

@aneeshusa

Manage `sources.list` and make bootstrap script purge lists before updating @aneeshusa Hi, As discussed I have created this pull request. Let know me what you think could be the issue. > > One way to test this is manual verification inside a Vagrant VM, > > I tried this as well after rebuilding the VMs, I checked in servo-master1 vm but could find the XXX string in the vm's sources.list file. Perhaps I am missing to modify some other config files. Fixes #417.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/520)

bors-servo · 2017-09-22T07:34:02Z

☀️ Test successful - status-travis
Approved by: aneeshusa
Pushing 229ea9e to master...

highfive assigned aneeshusa Oct 24, 2016

highfive added the S-awaiting-review label Oct 24, 2016

choudhary001 mentioned this pull request Oct 27, 2016

Manage `sources.list` and make bootstrap script purge lists before updating #417

Closed

aneeshusa suggested changes Oct 30, 2016

View changes

aneeshusa added S-needs-code-changes and removed S-awaiting-review labels Oct 30, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 31, 2016

aneeshusa suggested changes Oct 31, 2016

View changes

aneeshusa added S-needs-code-changes and removed S-awaiting-review labels Oct 31, 2016

highfive added the S-needs-rebase label Nov 1, 2016

choudhary001 force-pushed the choudhary001:master branch from eaa6d1f to 74e0af8 Nov 3, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 3, 2016

choudhary001 force-pushed the choudhary001:master branch from 74e0af8 to 480a9fc Nov 3, 2016

KiChjang removed the S-needs-rebase label Nov 4, 2016

aneeshusa suggested changes Dec 8, 2016

View changes

aneeshusa added S-needs-code-changes and removed S-awaiting-review labels Dec 8, 2016

choudhary001 force-pushed the choudhary001:master branch from 480a9fc to 5783df3 Dec 13, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Dec 13, 2016

aneeshusa added S-tests-failed S-needs-code-changes and removed S-awaiting-review labels Jan 1, 2017

aneeshusa removed the S-awaiting-review label Jun 5, 2017

choudhary001 force-pushed the choudhary001:master branch from a77f081 to f11060a Jun 5, 2017

highfive added S-awaiting-review and removed S-needs-code-changes labels Jun 5, 2017

choudhary001 force-pushed the choudhary001:master branch from f11060a to af5f984 Jun 5, 2017

aneeshusa suggested changes Jun 6, 2017

View changes

aneeshusa added S-needs-code-changes and removed S-awaiting-review S-needs-rebase S-needs-squash labels Jun 6, 2017

choudhary001 force-pushed the choudhary001:master branch from af5f984 to 15c1068 Jun 6, 2017

highfive added S-awaiting-review and removed S-needs-code-changes labels Jun 6, 2017

aneeshusa changed the title ~~WIP: issue#417 Manage `sources.list` and make bootstrap script purge lists before updating~~ Manage `sources.list` and make bootstrap script purge lists before updating Sep 22, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Sep 22, 2017

bors-servo merged commit 15c1068 into servo:master Sep 22, 2017
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive added S-needs-deploy and removed S-awaiting-merge labels Sep 22, 2017

aneeshusa removed the S-needs-deploy label Sep 26, 2017

servo / saltfs

Manage `sources.list` and make bootstrap script purge lists before updating #520

Manage `sources.list` and make bootstrap script purge lists before updating #520

choudhary001 commented Oct 24, 2016 •

edited by KiChjang

highfive commented Oct 24, 2016

aneeshusa left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

aneeshusa left a comment

This comment has been minimized.

This comment has been minimized.

bors-servo commented Nov 1, 2016

choudhary001 commented Nov 4, 2016

aneeshusa left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

aneeshusa commented Jan 1, 2017

choudhary001 commented Jun 6, 2017

aneeshusa left a comment

This comment has been minimized.

aneeshusa commented Sep 20, 2017

aneeshusa commented Sep 22, 2017

bors-servo commented Sep 22, 2017

aneeshusa commented Sep 22, 2017

bors-servo commented Sep 22, 2017

bors-servo commented Sep 22, 2017

bors-servo commented Sep 22, 2017

		@@ -0,0 +1,54 @@
		## Note that this sources.list file is picked from saltfs repository

		@@ -0,0 +1,55 @@
		# Note that this sources.list file is copied from a Ubuntu vm

servo / saltfs

Join GitHub today

Manage `sources.list` and make bootstrap script purge lists before updating #520

Manage `sources.list` and make bootstrap script purge lists before updating #520

Conversation

choudhary001 commented Oct 24, 2016 • edited by KiChjang

highfive commented Oct 24, 2016

aneeshusa left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

aneeshusa left a comment

This comment has been minimized.

This comment has been minimized.

bors-servo commented Nov 1, 2016

choudhary001 commented Nov 4, 2016

aneeshusa left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

aneeshusa commented Jan 1, 2017

choudhary001 commented Jun 6, 2017

aneeshusa left a comment

This comment has been minimized.

aneeshusa commented Sep 20, 2017

aneeshusa commented Sep 22, 2017

bors-servo commented Sep 22, 2017

aneeshusa commented Sep 22, 2017

bors-servo commented Sep 22, 2017

bors-servo commented Sep 22, 2017

bors-servo commented Sep 22, 2017

choudhary001 commented Oct 24, 2016 •

edited by KiChjang