[PATCH] D41393: Allow to apply cherry-picks when building Docker images.

Eric Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 20 06:27:01 PST 2017


ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lg after code comment is added.

Do we want to mention this change in the doc?



================
Comment at: utils/docker/scripts/build_install_llvm.sh:185
+# Sort cherrypicks and remove duplicates.
+CHERRYPICKS="$(echo "$CHERRYPICKS" | xargs -n1 | sort | uniq | xargs)"
+
----------------
ilya-biryukov wrote:
> ioeric wrote:
> > I'd do the sorting in `apply_cherrypicks`. 
> Why not here? `apply_cherrypicks` is called multiple times, sorting outside of it seems natural.
> Maybe we should define `apply_cherrypicks` after we do the sorting or add a comment to make it clear that it always applies cherrypicks in a sorted order instead?
Sorting inside `apply_cherrypicks`, you wouldn't need to worry about the state of `CHERRYPICKS`. The sorting here should also be really cheap. But up to you. This is shell script after all :) 

A comment in `apply_cherrypicks` requiring `CHERRYPICKS` to be sorted would still be nice.


https://reviews.llvm.org/D41393





More information about the llvm-commits mailing list