buildbot: 0.9.4 -> 0.9.5 #24131

nand0p · 2017-03-20T16:58:29Z

adds distro dependency
buildbot nodaemon in service module
module test fixup
tested on darwin
tested on nixos

mention-bot · 2017-03-20T16:58:31Z

@nand0p, thanks for your PR! By analyzing the history of the files in this pull request, we identified @copumpkin, @joachifm and @FRidh to be potential reviewers.

copumpkin · 2017-03-21T16:33:09Z

nixos/modules/services/continuous-integration/buildbot/master.nix

        ${cfg.package}/bin/buildbot create-master ${cfg.buildbotDir}
+        echo "import sys" >> ${cfg.buildbotDir}/buildbot.tac


This might be a bit more maintainable as a combination of

let logExtension = pkgs.writeScript "stdout-logs.py" '' import sys from twisted.logger import textFileLogObserver, globalLogPublisher globalLogPublisher.addObserver(textFileLogObserver(sys.stdout)) ''; in { preStart = '' ln -sf ${logExtension} stdoutLogs.py # TODO: check that it isn't already there echo "import stdoutLogs" >> buildbot.tac ''; }

Or something like that. Otherwise maintaining the log extension gets kind of awkward. Or maybe there's a more native way to get it to spit logs to stdio?

I also think that with your current solution, every time you start the server you'll get a new round of logs getting dumped to stdout 😄

Or maybe there's a more native way to get it to spit logs to stdio?

Apparently launching through twistd is endorsed and supported. It seems like that might be best here:

twistd -n -l - -y ${cfg.buildbotDir}/buildbot.tac

instead of buildbot start should get us what we want with no .tac shenanigans!

Mic92 · 2017-03-21T16:46:05Z

nixos/modules/services/continuous-integration/buildbot/master.nix

-        default = [
-          "worker.Worker('example-worker', 'pass')"
-        ];
-        example = [ "worker.LocalWorker('example-worker')" ];


why did you drop the example here?

i thought it was redundant, as default serves as an example. This follows the pattern with other vars in which this is the case, such as "home", "user" and "port".

nand0p · 2017-03-31T19:48:00Z

@copumpkin This is now working calling twisted directly in module, but daemon standalone mode still has the PYTHONPATH inheritance issue. (should be fixed globally in makeWrapper?)

nand0p · 2017-04-04T19:47:05Z

@Mic92 @FRidh @copumpkin ready_for_review

FRidh · 2017-04-05T12:19:01Z

nixos/modules/services/continuous-integration/buildbot/master.nix

      serviceConfig = {
        Type = "simple";
        User = cfg.user;
        Group = cfg.group;
        WorkingDirectory = cfg.home;
-        ExecStart = "${cfg.package}/bin/buildbot start --nodaemon ${cfg.buildbotDir}";
-      };
+        Environment = "PYTHONPATH=${cfg.package}/lib/python2.7/site-packages:${pkgs.buildbot-plugins.www}/lib/python2.7/site-packages:${pkgs.buildbot-plugins.waterfall-view}/lib/python2.7/site-packages:${pkgs.buildbot-plugins.console-view}/lib/python2.7/site-packages:${pkgs.python27Packages.future}/lib/python2.7/site-packages:${pkgs.python27Packages.dateutil}/lib/python2.7/site-packages:${pkgs.python27Packages.six}/lib/python2.7/site-packages:${pkgs.python27Packages.sqlalchemy}/lib/python2.7/site-packages:${pkgs.python27Packages.jinja2}/lib/python2.7/site-packages:${pkgs.python27Packages.markupsafe}/lib/python2.7/site-packages:${pkgs.python27Packages.sqlalchemy_migrate}/lib/python2.7/site-packages:${pkgs.python27Packages.tempita}/lib/python2.7/site-packages:${pkgs.python27Packages.decorator}/lib/python2.7/site-packages:${pkgs.python27Packages.sqlparse}/lib/python2.7/site-packages:${pkgs.python27Packages.txaio}/lib/python2.7/site-packages:${pkgs.python27Packages.autobahn}/lib/python2.7/site-packages:${pkgs.python27Packages.pyjwt}/lib/python2.7/site-packages:${pkgs.python27Packages.distro}/lib/python2.7/site-packages:${pkgs.python27Packages.pbr}/lib/python2.7/site-packages:${pkgs.python27Packages.urllib3}/lib/python2.7/site-packages";


why are you setting PYTHONPATH here like this? Why don't you provide a working Python environment?

This is a workaround due to the python environment no longer being set correctly as of 49b20e5 and ca5a1d9 . As of these two commits, the package builds, but does not run correctly due to missing PYTHONPATH. I think @copumpkin mentioned the issue may be with makeWrapper and should be fixed separately globally. once that is fixed, this can be removed.

looks like the broken python environment when running buildbot start is actually a separate issue. the environment has to set PYTHONPATH here in this module because twistd is being called natively in ExecStart, and so needs to know about all the python libs used.

cc @copumpkin

FRidh · 2017-04-05T12:20:34Z

nixos/modules/services/continuous-integration/buildbot/worker.nix

@@ -115,7 +116,11 @@ in {
        User = cfg.user;
        Group = cfg.group;
        WorkingDirectory = cfg.home;
-        ExecStart = "${cfg.package}/bin/buildbot-worker start --nodaemon ${cfg.buildbotDir}";
+        Environment = "PYTHONPATH=${cfg.package}/lib/python2.7/site-packages:${pkgs.python27Packages.future}/lib/python2.7/site-packages";


same here, why not provide a Python on PATH that has everything set up already?

calling twistd natively requires setting PYTHONPATH

@FRidh does this make sense regarding calling twistd natively (as suggested above in comments... previously calling buildbot start --nodaemon)? is there another way to set PYTHONPATH other than the systemd's unit Environment? perhaps an example of provide a Python on PATH that has everything set up already?

nand0p · 2017-04-05T12:57:31Z

nixos/tests/buildbot.nix

+
+
+    # Test buildbot daemon mode
+    # NOTE: daemon mode tests disabled due to broken PYTHONPATH child inheritence


Buildbot standalone daemon mode broken as of 49b20e5 and ca5a1d9 due to broken PYTHONPATH in runCommand makeWrapper. Tests disabled.

- adds distro dependency - buildbot nodaemon in service module - fakerepo for module tests - service module parameter fixup - tested on nixos - tested on darwin

nand0p · 2017-04-21T14:38:10Z

@FRidh @Mic92 @copumpkin updated to remove the now merged distro commit. ready_for_review

nand0p · 2017-04-27T01:56:46Z

@copumpkin this PR fixes the buildbot module, which is currently broken. could we go ahead with merge?

copumpkin · 2017-04-27T15:08:52Z

@nand0p sorry, will review later today, but in general it looks good except for the big list o' python paths. I guess we could merge for now and make a follow-up commit that fixes those?

nand0p · 2017-04-27T18:03:13Z

@copumpkin yup, i can certainly do another PR to better fix this, as buildbot-0.9.6 just came out. Note that the big list of python paths in the module (the systemd unit environment), is due to calling twistd natively in ExecStart, and so im not sure exactly how else to let twisted know about these paths.

nand0p · 2017-05-02T22:00:03Z

@copumpkin do we want to go ahead with merge, so as to have a working module? I will follow-up in another PR as noted.

Changes: ``` 654ae8c1e4 base-filesystem.c: add trailing zero byte for s390x entry e4a19eef33 basic/missing_loop.h: fix missing lo_flags LO_FLAGS_DIRECT_IO 24238be484 mount-util: fix error code 1b1ad8c79f udev: certainly restart event for previously locked device 7dacfb3fb4 stub: Use EfiLoaderCode for kernel memory eaeaf4f6ef network: do not silently stop to process configuration on activation failure bb803856bc bus: use inline trace argument for ANONYMOUS auth 6349062326 Fix ObjectManager interface emitted for non-manager objects c90ab07fa0 test-bus-objects: Test interfaces added/removed signal interfaces e32fe1b457 Fix GetManagedObjects returning ObjectManager interface for non-manager objects efd8e39f4a test-bus-objects: Test GetManagedObjects interfaces are correct 344efd022a coredump: when parsing json, optionally copy the string first de08edca17 systemctl: color ignored exit status in yellow, not red 1531a496e3 manager: make clear internal Dump() logic is debugging only. c4fd38f7d2 man: document the Dump() calls of the PID 1 D-Bus interface, and what they are 140fee4627 resolve: do not cache mDNS goodbye packet 1a2d93a770 kbd-model-map: correct variants for cz-qwerty to include comma 9d1ebb2247 resolve: persist DNSOverTLS configuration in state file 3137ac6ef5 udev: support by-path devlink for multipath nvme block devices c948091cc5 run: make --working-directory= work for --scope too 7bb204620d kbd-model-map: add a mapping for switched czech qwerty/us e5157050d1 test: add more test cases for mkdir_p_safe() and mkdir_p_root() b3a9f7b5cb mkdir: chase_symlinks_and_stat() does not return 0 0bfdc91807 units: make sure that initrd-switch-root.service pulls in .target 45fb64c54b units: add dependency ordering for emergency.service conflicts 6535813084 units: add ordering dependencies on initrd-switch-root.target 09c90224f1 units/systemd-network-generator.service: add forgotten ordering for shutdown 1dd723a3b8 units: reorder/split unit dependency blocks 054cad0097 man: explicitly document that "reboot -f" is different from "systemctl reboot -f" c5b0ae86b1 watchdog: use /dev/watchdog0 only if it exists ac805eac15 journalctl: respect --quiet flag during file concistency verification c1d729795d xdg-autostart-service: expand tilde in Exec lines 35c5f5d688 unit: drop ProtectClock=yes from systemd-udevd.service 175ba30cf6 busctl: Fix warning about invaild introspection data 6c7b91372d udev/rules,hwdb: filter out mostly meaningless default strings 8b89e677e9 units: prolong the stop timeout for homed 202a79e7c5 homed: don't wait indefinitely for workers on exit 44660d2e12 man: fix static bridge example e0dde8a14f log: don't attempt to duplicate closed fd 254b77e73c condition: fix device-tree firmware path 96da39ddb1 udev-util: minor cleanups for on_ac_power() 3345520512 docs: fix incorrect env var name for credentials directory 49f9fa87b2 shell-completion: drop unused $mode 1e29d934de oomd: fix off-by-one when dumping kill candidates b00cb050c8 on-ac-power: ignore devices with scope==Device 9886011356 on-ac-power: rework logic 1fc74d251e sd-device: add helper to read a unsigned int attribute 6d4c138534 shared/udev-util: say "ignoring device", not "ignoring" cd2fad2300 virt: Support detection of Apple Virtualization.framework guests 6e47e75c86 virt: align tables 951e99231e check-os-release.py compatible with Python < 3.8 d572a74163 core/mount: adjust deserialized state based on /proc/self/mountinfo 2e372afc35 Allow uneven length BootXXXX variables 8ad143e684 gpt: fix native uuids for s390x 2bb9a0a29b udev: fix inversed inequality for timeout of retrying event cf67d5ed1b bash-completion: add systemd-sysext support ada437cfb1 sysext: add missing COMMAND to the help output and man synopsis 58bc1e8e04 hostname: make chassis type actually obtained from ACPI when nothing from DMI 4ffde70981 booctl: do not say uuids differ if one of the uuids is unset 5219a99ccb bash-completion: autocomplete cgroup names in systemd-cgtop 9f2f391153 sysusers: add fsync for passwd (NixOS#24324) c966377c51 dhcp6: do not append ORO option when no option requested 97474b03e7 dhcp6: gracefully handle NoBinding error c67a388aef udev/cdrom_id: check last track info 52c631b02e firstboot: fix can't overwrite timezone f279a6f4d1 cryptenroll: fix memory leak 66b060225d sd-device-enumerator: drop noisy log messages 6e1acfe818 sd-device-monitor: actually refuse to send invalid devices 81339c45e8 sd-device-monitor: fix inversed condition 1760559918 resolvctl: only remove protocol after last dot when mangling ifname for resolvconf a3348ba748 oom: drop invalid %m in the log message b3dd66f32b meson: Test correct efi linker for supported args f9d936b865 sysusers: properly process user entries with an explicit GID ec5a46ca34 sysusers: only check whether the requested GID is available 037b1a8acc dhcp: fix potential buffer overflow ed2955f8fe udev-util: assume system is running on AC power when no battery found 37b54927d3 Fix issue with system time set back (NixOS#24131) 4fdca1ab9e shared/generator: Ensure growfs unit runs after repart 32f9d70f8b manager: optionally, do a full preset on first boot ```

nand0p force-pushed the buildbot-0.9.5 branch from 696c25e to e0e03ea Compare March 20, 2017 18:10

Mic92 changed the title ~~[WIP] Buildbot 0.9.5~~ [WIP] buildbot: 0.9.4 -> 0.9.5 Mar 20, 2017

Mic92 added the 8.has: package (update) label Mar 20, 2017

FRidh added the 6.topic: python label Mar 21, 2017

copumpkin reviewed Mar 21, 2017

View reviewed changes

Mic92 reviewed Mar 21, 2017

View reviewed changes

nand0p force-pushed the buildbot-0.9.5 branch 3 times, most recently from f8cefc5 to 64f17d5 Compare March 21, 2017 19:52

nand0p force-pushed the buildbot-0.9.5 branch from 64f17d5 to 8d378d9 Compare March 31, 2017 19:02

nand0p changed the title ~~[WIP] buildbot: 0.9.4 -> 0.9.5~~ buildbot: 0.9.4 -> 0.9.5 Mar 31, 2017

nand0p force-pushed the buildbot-0.9.5 branch 4 times, most recently from 1ea730f to b7a16c5 Compare April 3, 2017 19:30

FRidh reviewed Apr 5, 2017

View reviewed changes

nand0p commented Apr 5, 2017

View reviewed changes

mucaho mentioned this pull request Apr 13, 2017

conan: init at 0.21.2 #24865

Merged

9 tasks

buildbot: 0.9.4 -> 0.9.5

4ac06ea

- adds distro dependency - buildbot nodaemon in service module - fakerepo for module tests - service module parameter fixup - tested on nixos - tested on darwin

nand0p force-pushed the buildbot-0.9.5 branch from b7a16c5 to 4ac06ea Compare April 21, 2017 14:36

FRidh merged commit 9e48fc3 into NixOS:master May 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buildbot: 0.9.4 -> 0.9.5 #24131

buildbot: 0.9.4 -> 0.9.5 #24131

nand0p commented Mar 20, 2017

mention-bot commented Mar 20, 2017

copumpkin Mar 21, 2017 •

edited

copumpkin Mar 21, 2017

Mic92 Mar 21, 2017

nand0p Mar 21, 2017

nand0p commented Mar 31, 2017

nand0p commented Apr 4, 2017

FRidh Apr 5, 2017

nand0p Apr 5, 2017 •

edited

nand0p Apr 5, 2017

FRidh Apr 10, 2017

FRidh Apr 5, 2017

nand0p Apr 5, 2017 •

edited

nand0p Apr 10, 2017

nand0p Apr 5, 2017

nand0p commented Apr 21, 2017

nand0p commented Apr 27, 2017

copumpkin commented Apr 27, 2017

nand0p commented Apr 27, 2017

nand0p commented May 2, 2017

		${cfg.package}/bin/buildbot create-master ${cfg.buildbotDir}
		echo "import sys" >> ${cfg.buildbotDir}/buildbot.tac



		# Test buildbot daemon mode
		# NOTE: daemon mode tests disabled due to broken PYTHONPATH child inheritence

buildbot: 0.9.4 -> 0.9.5 #24131

buildbot: 0.9.4 -> 0.9.5 #24131

Conversation

nand0p commented Mar 20, 2017

mention-bot commented Mar 20, 2017

copumpkin Mar 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nand0p commented Mar 31, 2017

nand0p commented Apr 4, 2017

Choose a reason for hiding this comment

nand0p Apr 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nand0p Apr 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nand0p commented Apr 21, 2017

nand0p commented Apr 27, 2017

copumpkin commented Apr 27, 2017

nand0p commented Apr 27, 2017

nand0p commented May 2, 2017

copumpkin Mar 21, 2017 •

edited

nand0p Apr 5, 2017 •

edited

nand0p Apr 5, 2017 •

edited