as we purge all of the externals-handling via cmake (and therefore loose the drake-distro/externals directory), I think it would be appropriate to make that change? Bazel's handling of externals feels less like a superbuild than the cmake externals version did.
If yes, it would be a big change but the time is probably now (before we post our first monthly release).
Is it worth it?
++1!
That would be so nice!!
I'm not sure why we should rush it before a monthly release? The installed binary copy of Drake would change relatively little (I think only the resource searching would change); downstream include paths like #include <drake/systems/framework/system.h> would be unchanged.
As to the proposal itself, I would support changing e.g. drake-distro/drake/common in a source checkout to lose the duplication of the drake word, but I'm not sure why drake/src/comon is better? Why not just move drake-distro/drake/* up one level, so in git we have drake/common and drake/doc and drake/setup and etc?
i think of the monthly release as being both a binary release and a tag for people to build from source.
i don't have a strong preference about moving it up a level or not. i thought it would be more standard to separate the src from the e.g. build/ bazel-bin/ etc with an explicit src subdirectory.
I agree with @jwnimmer-tri that we should move everything up one level, though I think I agree with @RussTedrake that it would be nice to do so before we tag a release.
(if we are doing filesystem housekeeping, I would also like the matlab under the bindings directory)
+1 for moving it up one level.
I think that if we use src/, it may be a tad odd to use it externally via Bazel, such as referencing @drake//src/manipulation:robot_plan_interpolator.
(Then again, with how tensorflow is structured, it looks like you'd have to do @tensorflow//tensorflow:core, like what we presently have with something like @drake//drake:common.)
Potential Bike-Shedding:
Minor question, but just to check, would we simply change the resource locations as well (e.g. FindResource("manipulation/models/...") rather than FindResource("drake/manipulation/models/...")), such that (a) FindResource does not have to do special prefix-stripping operations and (b) is not dependent on the repository's name?
Also, given that the paths themselves don't involve drake/..., should we lock drake::FindResource down to be drake-specific (which is what the documentation presently implies) and used only for drake examples / testing, possibly discouraging its use externally (unless these are drake-centric externals)?
\cc @stonier regarding the role of FindResource in drake for installation, external usage, etc.
+1 for moving it up a level too and +1 before a release.
I expect there will be a few changes around the installation and FindResource. Right now the installed resources sit inside share/drake/drake/... which in itself is a little odd.
drake-distro/externals are gone. Thanks @jamiesnape !
i think it's time to schedule this (or decide it's not worth it). it probably needs a flag day, and some accompanying instructions for folks that will have a potentially confusing merge from their dev branches.
I am going to examine how to go about this.
I think it would be best to delay until I have stripped the rest of the CMake, but no harm in planning.
Right. I'm preparing patchsets that assume most or all CMakeLists.txt are gone (and so the patchsets don't touch the CMake code, and wouldn't pass CI right now), and I'll wait to PR them until other work lands first (CMake removal, Trusty removal, etc.) -- though a few tidy-ups might PR before then.
@EricCousineau-TRI
Minor question, but just to check, would we simply change the resource locations as well (e.g. FindResource("manipulation/models/...") rather than FindResource("drake/manipulation/models/...")), such that (a) FindResource does not have to do special prefix-stripping operations and (b) is not dependent on the repository's name?
No, my intention is for the relative_path arguments to FindResource that worked before to keep working. Otherwise we have a flag day on our hands. So, the strings would still all start with "drake/". If we want different (drake-less) strings to work, we can create a new method that offers the new semantics.
Also, given that the paths themselves don't involve drake/..., should we lock drake::FindResource down to be drake-specific (which is what the documentation presently implies) and used only for drake examples / testing, possibly discouraging its use externally (unless these are drake-centric externals)?
Nope, the opposite is true. FindResource (and its various sugars) are the only supported way for downstream projects to access Drake's published data, such as model packages, meshes, etc. Issue #2174 had a long discussion related to this topic.
PR #7450 begins some work on this front. I plan to ask Eric to review once its ready.
The folder name transition is nearly merge-ready. Right now, the plan is to merge the rename PR on the morning of Thursday, December ~4th~ _7th_. (That morning, we will block any other PRs from landing while we work on the transition.)
In the meantime, over the next couple days I suggest that all Drake Developers update all of their feature branches to be based off of a recent Drake master. We believe that the rename here will be relatively painless _if_ you are up to date.
The rename PR will _only_ be file renames (and removals), aside from one line of code. So it should merge into feature branches trivially _if_ they are up-to-date. We will offer help on slack for anyone struggling with git problems after the rename lands.
You mean Thursday December 7, right?
Evan Drumwright
Senior Research Scientist
http://positronicslab.github.io
Toyota Research Institute
Palo Alto, CA
On Mon, Dec 4, 2017 at 9:38 AM, Jeremy Nimmer notifications@github.com
wrote:
The folder name transition is nearly merge-ready. Right now, the plan is
to merge the rename PR on the morning of Thursday, December 4th. (That
morning, we will block any other PRs from landing while we work on the
transition.)In the meantime, over the next couple days I suggest that all Drake
Developers update all of their feature branches to be based off of a recent
Drake master. We believe that the rename here will be relatively painless
if you are up to date.The rename PR will only be file renames (and removals), aside from one
line of code. So it should merge into feature branches trivially if
they are up-to-date. We will offer help on slack for anyone struggling with
git problems after the rename lands.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/RobotLocomotion/drake/issues/6996#issuecomment-349041066,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACHwz74cDE0B-YgtffgGHMyNV_CweJtFks5s9C4ogaJpZM4PSUAN
.
Right-o. Fixed.
Jeremy, would you mind broadcasting on drakedevelopers #general when you start the PR block? That would seem to be a good time for all of us with active development branches to update.
The main rename has been merged. If you have rebase or merge trouble, the first suggestion is:
The first merge should go as usual -- any merge conflicts should be as expected with normal workflow, and resolved as normal. The second merge is just renames, so should do git magic and occur without trouble.
The only remaining problems was if your feature branch contained files that were not yet on master. You will probably have to git mv these into their new home manually (removing "drake" from their top-level folder name).
N.B. - the installation instructions on sphinx (http://drake.mit.edu/from_source.html) still refer to drake-distro, but should now just be drake. hoorah!
So far it looks like we might be able to survive with the new layout. Next up, I'll remove some of the transition shims left over on master, as well as som documentation updates.
Most code and docs are revised now, so I'm lowering the priority. There are still BUILD file cleanups pending.