{"action":"created","issue":{"url":"https://api.github.com/repos/macports/macports-ports/issues/26","repository_url":"https://api.github.com/repos/macports/macports-ports","labels_url":"https://api.github.com/repos/macports/macports-ports/issues/26/labels{/name}","comments_url":"https://api.github.com/repos/macports/macports-ports/issues/26/comments","events_url":"https://api.github.com/repos/macports/macports-ports/issues/26/events","html_url":"https://github.com/macports/macports-ports/pull/26","id":188236602,"number":26,"title":"cmake-1.1 portgroup: generator support.","user":{"login":"RJVB","id":1756884,"avatar_url":"https://avatars.githubusercontent.com/u/1756884?v=3","gravatar_id":"","url":"https://api.github.com/users/RJVB","html_url":"https://github.com/RJVB","followers_url":"https://api.github.com/users/RJVB/followers","following_url":"https://api.github.com/users/RJVB/following{/other_user}","gists_url":"https://api.github.com/users/RJVB/gists{/gist_id}","starred_url":"https://api.github.com/users/RJVB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RJVB/subscriptions","organizations_url":"https://api.github.com/users/RJVB/orgs","repos_url":"https://api.github.com/users/RJVB/repos","events_url":"https://api.github.com/users/RJVB/events{/privacy}","received_events_url":"https://api.github.com/users/RJVB/received_events","type":"User","site_admin":false},"labels":[{"id":457720722,"url":"https://api.github.com/repos/macports/macports-ports/labels/update","name":"update","color":"1d76db","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2016-11-09T12:31:45Z","updated_at":"2016-11-15T11:37:38Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/macports/macports-ports/pulls/26","html_url":"https://github.com/macports/macports-ports/pull/26","diff_url":"https://github.com/macports/macports-ports/pull/26.diff","patch_url":"https://github.com/macports/macports-ports/pull/26.patch"},"body":"This revision adds support for one more of CMake's generators, and\r\nallows ports to use the more efficient ninja tool. This is supposed to\r\nmake a significant difference for large projects like llvm and clang,\r\neven for once-off builds.\r\nAs an additional improvement the default destroot target for the Makefile\r\ngenerator is set to install/fast. This skips the implicit \"make\" step\r\nthat is otherwise always part of \"make install\", and this can reduce\r\nthe destroot time significantly.\r\n"},"comment":{"url":"https://api.github.com/repos/macports/macports-ports/issues/comments/260619041","html_url":"https://github.com/macports/macports-ports/pull/26#issuecomment-260619041","issue_url":"https://api.github.com/repos/macports/macports-ports/issues/26","id":260619041,"user":{"login":"RJVB","id":1756884,"avatar_url":"https://avatars.githubusercontent.com/u/1756884?v=3","gravatar_id":"","url":"https://api.github.com/users/RJVB","html_url":"https://github.com/RJVB","followers_url":"https://api.github.com/users/RJVB/followers","following_url":"https://api.github.com/users/RJVB/following{/other_user}","gists_url":"https://api.github.com/users/RJVB/gists{/gist_id}","starred_url":"https://api.github.com/users/RJVB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RJVB/subscriptions","organizations_url":"https://api.github.com/users/RJVB/orgs","repos_url":"https://api.github.com/users/RJVB/repos","events_url":"https://api.github.com/users/RJVB/events{/privacy}","received_events_url":"https://api.github.com/users/RJVB/received_events","type":"User","site_admin":false},"created_at":"2016-11-15T11:37:38Z","updated_at":"2016-11-15T11:37:38Z","body":"On Monday November 14 2016 15:02:19 Clemens Lang wrote:\n\nI'll do a grouped reply by email to limit the number of notifications.\n\n\nRe: a more general switch to ninja: if claims are correct it might be of some interest for the build bots due to reduced resource use. If that is something you take into consideration...\n\n> >  Which leads me to another thought: suppose we come to the conclusion that there is justification for a user-configurable preferred cmake generator\n> \n> I'd be interested in how you'd come to this conclusion. I don't think we should allow switching the generator, because certain projects might just not build with the ninja generator. I think our maintainers should make informed decisions on whether switching to ninja works for a port and provides enough of a speed benefit to warrant the extra dependency.\n\nI think we can ignore the extra dependency: ninja is really small (which is why it can be as fast as it claims it is).\nWhat I had in mind was for cmake-based projects only, and only for ports which do not provide a cmake.generator setting themselves, something like setting `default cmake.generator` to a value read from file. It was inspired by the fact that I would be wanting to keep a good number of my KF5 ports on the Makefiles generator because it fits my development workflow better, but that this wouldn't be a very courteous to (future) users building from source if indeed using ninja would come with performance benefits.\nI agree it's a bit of a far-fetched scenario, and I could implement it locally by some additional code around the `default cmake.generator` bit in cmake-1.1.tcl which I'd keep out of the published version.\n\n> Exactly, that won't work. You can try having a setting that can be set from the command line, e.g. `port install yourport cmake.generator=ninja`, although I'm not sure whether we allow setting arbitrary variables from the command line (probably not).\n\nNo, I already tried that for I forget which other variable, and it doesn't work.\n\n> Your stack trace is caused by not using `{\"Unix Makefiles\"}` as argument to `default cmake.generator`. Whenever you use `default` the default value needs double quoting.\n> That should stay `{\"Unix Makefiles\"}`\n\nOK, I must have misunderstood at least part of the whole \"list vs string\" thing but you did reflect on a weird initialisation somewhere, IIRC.\n\n> As @larryv pointed out, this is equal to `[llength $args] != 1`. Also not that my suggestion was doing this above the switch and then running the switch only on the first index in this list, i.e.\n\nYes, I noticed that. I implemented it differently for a few reasons (listed in chronological order, not order of importance):\n1- I don't know exactly what the `error` statement does.\n2- Catching an argument that's a list instead of a string is just another case of catching an invalid/unsupported argument and I like handling all related errors in a single location.\n3- With Marko's experience I prefer to keep the error checking to a single location where it'd be easier to restore the differential handling when running from the registry, should that again prove necessary.\n\nThe ultimate main reason is 2), evidently.\n\n> Otherwise, you'll still have the same list issues as before.\n\nThis is part of the things I apparently misunderstood: I'm not seeing any list issues anymore. I thought you just wanted me to drop the non-requirement of using double quotes, and since I did that (and restored the proper default syntax) everything works as I intend. So what list issue(s) could I be overlooking?\n\n> Also, please try to drop the extra merge commits on the branch.\n\nAh, I thought I had to keep the branch synced. The PR page still shows only a single file change, so this is just about the merge messages in the history (and thus a simple `git revert` will not do here)? \nIf that's undesirable I see another solution that's less work: once we're at the point where the pull can go through, I close this request and make another one with the exact same file. I'd prefer that, but you can also teach me how to drop commits so they disappear completely from the history.\n\n@larryv\n\n> Tangentially, I find the use of \"natural language\" values somewhat jarring.\n\nI considered doing things as you suggested while drafting the code because I somewhat agree, but in the end I decided it'd be better to use the actual values CMake uses. It shouldn't make much of a difference for people used to CMake; without this feature they'd be adding expressions like `-G \"Unix Makefiles\"` or `-G \"CodeBlocks - Unix Makefiles\"` to configure.args. The intention of the feature is more to streamline the associated changes (destroot'ing, build dependency) than to provide a shortened syntax which ultimately removes port maintainer control over the exact Make or Ninja generator they want to use. (The \"enriched\" versions just generate a few files for a selection of IDEs in addition to the exact same Make/Ninja files.)\n\n\nR.\n\n\n\n"}}
{"actor":{"display_login":"RJVB"}}