Rename build directory about arcade HOT 42 CLOSED

@mmitche I think that's only half the solution though. There are two directories that we need to define:

1. The directory that core engineering "owns". The contents of this directory are typically modified by central tooling, automation, etc ... Developers should rarely directly edit this directory.
2. The directory in which build tools, scripts, etc ... which are specific to that repository exist. These are often a tab bit specific to a repo + commonly edited by developers.

There is likely some overlap here in some cases.

Seems weird to have two top level directories per repo for build though. Think it would make more sense to have one top level directory and put both items under it. Example:

eng
  \core 
  \tools 

In this example eng\core represents (1) and eng\tools represents (2).

from arcade.

Another proposal for simpler structure:

• eng\common ... common infrastructure files
• eng ... repo specific files go directly to this directory (i.e. no under another subdir)

Example for symreader:

• eng\common\build.ps1
• eng\common\cibuild.cmd
• eng\common\cibuild.sh
• eng\SignToolData.json
• eng\Versions.props

from arcade.

What I think we should do and what I end up getting forced to do don't always agree with each other :)

from arcade.

I don't understand the completion issue. Typing b TAB ENTER completes to build and runs build.cmd.

from arcade.

Renaming this directory would be major PITA since the naming convention is used in 19 repositories and all their scripts and build definitions would need to be updated. In addition the build definitions would need to be versioned.

from arcade.

I work in powershell and so typing b TAB completes to \build\ which will not run the script, so I have to TAB again. Also the Build.cmd has a capital B instead of b which is going to be annoying on non-windows.

In general I think it is a good idea to not have clashing names like this to avoid ambiguity.

Renaming this directory would be major PITA since the naming convention is used in 19 repositories and all their scripts and build definitions would need to be updated. In addition the build definitions would need to be versioned.

That leads to the question had in the other issue we which is what is the strategy for updating these scripts? If it is manually go update all the repo's that is going to kill us (it is already killing us in similar situations).

from arcade.

I still want to continue this discussion as I don't believe just because you cannot change it in the other repo's easily doesn't mean we cannot change it here and other repo's going forward.

from arcade.

@weshaggard I don't think it's worth it. Tab twice.

from arcade.

There is another reason to consider renaming. GitHub hides files in folders named "build/", "log/", and "tmp/". It's mildly annoying that you can easily navigate or find files under build/, and it's not something you can configure on GitHub.

from arcade.

@weshaggard I don't think it's worth it. Tab twice.

I disagree as I do think it is worth it. The build script is our primary entry point for most developers and we shouldn't be having unnecessary conflicts with it.

However I'm curious what others think? @jaredpar @markwilkie @chcosta?

@natemcmaster what do you mean by github hides files? I can see the files in that folder when I browse github.

from arcade.

It's only hidden from file finder (when you press T). I think it may be hidden from some search functions too.

from arcade.

It's only hidden from file finder (when you press T).

Doesn't make it less annoying 😄

from arcade.

My 2 cents:

I find it only a minor annoyance which occasionally crops up if I type something like code b<tab> and VS Code ends up opening the build folder instead of the build.cmd script. If I'm doing a lot of work in a repo though, I learn the pattern of code b<tab><tab>. So I'm ok adjusting on a per repo basis to this type of scenario.

The more annoying thing (for me) is porting changes from a repo which has source in a build folder to another repo which has build in the .gitignore so that when I do my usual git add -A I don't realize that my changes won't get committed. This has caused me to lose changes before. So, the fact that it's named after a folder which has special default behavior in the majority of repo's (and on GitHub apparently), bothers me more than having to tab once or twice.

I think I tried to move BuildTools logic into a build folder at one point and time but there were enough special case nuances around that folder name that it wasn't worth the convenience of using such an obvious choice.

Only slightly related, it bothers me that build.sh calls into build\build.sh on repos which use RepoToolset to build on Linux. It's not really obvious that one version of build.sh is the "special" version that handles the same logic as build.ps1 on Windows and the other is just the wrapper like Build.cmd. Actually, it'd be great if both build.sh and Build.cmd were wrappers around the same build.ps1, but then you'd need to bootstrap Powershell Core somehow and things start getting complicated again. Perhaps Cli local toolset will solve this as well.

from arcade.

special default behavior in the majority of repo's

What special default behavior? Why is build special? Where is it documented that it's a reserved name? We have used build in Roslyn for many years and never had any problems. Why is it a problem now?

from arcade.

What special default behavior?

porting changes from a repo which has source in a build folder to another repo which has build in the .gitignore

I guess it's not "special default behavior", it's just very common practice (in my experience) to see build in the .gitignore file.

from arcade.

@tmat - I would imagine that the 19 repos wouldn't need to be updated until they took a dependency on arcade. While the PITA work is still there, it could be deferred/scheduled. Having said that, I do like the notion of looking carefully at changes/suggestions to make sure there's a good reason.

So far, there's majority consensus that we should go ahead and make the directory change, and significant rationale has been presented. (doesn't mean we all agree...grin)

@jaredpar / @livarcocc - this probably affects you guys the most. Thoughts?

from arcade.

@tmat

What special default behavior? Why is build special?

GitHub special cases several folders to be excluded from a variety of search functions based on their name: build, log, tmp and vendor.

Where is it documented that it's a reserved name?

To my understanding it's not. I only know the details due to conversations with their support team.

We have used build in Roslyn for many years and never had any problems.

This has been a problem in Roslyn for me and a few others. Our troubles are what caused me to reach out to GitHub in the first place and proactively file issues on other repositories. Example:

dotnet/sdk#1071

It's not a prominent issue because it really only affects the set of developers who are focused on build infrastructure + frequently use GitHub UI to navigate.

Why is it a problem now?

It's always been a problem and if we were starting from scratch I would have pushed for a different directory name. But when I dug into this previously virtually everyone used a directory named "build" and the problem is limited to a subset of developers. Didn't seem like up-ending all the repos to move to a new directory name.

I imagine that for this repo it's likely to have more visibility. Reason being that this only affects developers who care about infrastructure and this is a repository for developers who care about infrastructure.

Honestly I was hoping GitHub would just see the light here and fix the problem. Nope.

from arcade.

@markwilkie

So far, there's majority consensus that we should go ahead and make the directory change, and significant rationale has been presented.

My 2 cents here: I think consistency is more important here than the downsides of the "build" folder. Virtually every .NET repository has a folder called "build". It's the more prominent name even though it's less functional (WHY GITHUB WHY???). I'd keep it over up ending everything.

this probably affects you guys the most. Thoughts?

Our product construction V2 plans essentially mandate that there is a folder in every repository that core-eng can use. This is necessary to hold flow information and other declarative files. In our docs I believe we tentatively call it "core-eng" just to have a name on paper.

The only reason I'd change this folder is as a part of pushing this down. Otherwise we're asking them to take directory changes twice.

My 2cents.

from arcade.

Where is it documented that it's a reserved name?

They don't explicitly document but they do list this fact in a tip at https://help.github.com/articles/finding-files-on-github/.

it's not a prominent issue because it really only affects the set of developers who are focused on build infrastructure + frequently use GitHub UI to navigate.

I'm not sure I agree fully here as every developer will be calling build.cmd/sh and if they have to keep adding keystrokes it can become very annoying. While some folks might think that is minor I think we need to keep things simple for the common dev scenarios.

I think consistency is more important here than the downsides of the "build" folder.

I agree with consistency as that is one of primary reasons we are doing this work. However I will state there are a lot of repo's that do not have a build directory. As @chcosta stated we explicitly decided not to use that name when we setup all the core repos.

from arcade.

However I will state there are a lot of repo's that do not have a build directory. As @chcosta stated we explicitly decided not to use that name when we setup all the core repos.

I think you could probably draw an org chart based on build vs. not build 😄

from arcade.

@livarcocc ?

from arcade.

Our product construction V2 plans essentially mandate that there is a folder in every repository that core-eng can use. This is necessary to hold flow information and other declarative files. In our docs I believe we tentatively call it "core-eng" just to have a name on paper.
The only reason I'd change this folder is as a part of pushing this down. Otherwise we're asking them to take directory changes twice.

Yes. We can change the name but only if that's a part of a broader change that's needed for some other reason than saving one keystroke.
(BTW, can we not call it core-eng but something that makes sense for none-core repos as well -- like tools or toolset e.g.? I would rather not end up with two directories containing build stuff)

I wish saving one keystroke was the biggest problem of our infrastructure.

from arcade.

Like @jaredpar stated, I would prefer to change this once and not twice. I have about 6 repos of the 19 described above, so a big chunk of this will fall on my team. Doing it twice would be pretty wasteful.

from arcade.

@tmat - I also wish that one keystroke was the worse of our worries.... grin

@mmitche - thoughts on how this fits into prodcon v2?

from arcade.

As @jaredpar noted, the directory name of the first prodcon v2 target (dependency structure) will likely be some other directory name, like 'eng'. I don't think in the near future prodcon v2 would be taking over things typically in the build/ folder, but when/if it did (e.g. more automated build generation), I think whatever data, scripts are needed would move into the 'eng', then the build folder would be eliminated. Making a change away from the build folder is pretty rough, since that has been baked into build definitions used for servicing and exists across a zillion branches.

I'd prefer that we change iteratively, as projects onboard into prodcon v2 if their build folders are no longer necessary. So maybe it makes sense to start with 'eng' in this repo now?

from arcade.

@mmitche - can you confirm that @jaredpar suggestion works for you too in regards to prodcon? (I see that @weshaggard thumbs up'd it)

If we're going to make a change - now's the time. Based on the conversation so far, it seems we should make a change.

@jaredpar has the best suggestion going so far. I'll check back in a bit, and hopefully we can move forward. My expectation is that these kinds of mini "squabbles" will be limited. As @tmat pointed out, we've got much bigger fish to fry.

from arcade.

Works for me.

from arcade.

ok - and the winner is.... @jaredpar last suggestion, namely:

eng
\core
\tools

@chcosta can make the update in Arcade. I'll leave it up to @jaredpar and @livarcocc when (or if) they want to change repotools to match.

from arcade.

I'll leave it up to @jaredpar and @livarcocc when (or if) they want to change repotools to match.

You all will rue the day that you let me win a naming debate.

from arcade.

LOL

from arcade.

I think the RepoToolset will need to be updated first, otherwise we'll be blocked on updating the version Arcade is using. If we update Arcade first we either won't be able to update the RepoToolset version or we'll live in this ugly hybrid world with an eng and a build folder (with duplicated code). RepoToolset has a couple (not many) of places where it explicitly looks in the build folder. It might be a good time to make that build folder path configurable as well.

Example - RepoLayout.props:

<VersionsPropsPath>$(RepoRoot)build\Versions.props</VersionsPropsPath>
<FixedVersionsPropsPath Condition="Exists('$(RepoRoot)build\FixedVersions.props')">$(RepoRoot)build\FixedVersions.props</FixedVersionsPropsPath>
<SignToolDataPath>$(RepoRoot)build\SignToolData.json</SignToolDataPath>

I've created an issue in Roslyn-Tools to change this.

I'm going to reopen this issue to track making that change once RepoToolset has updated.

from arcade.

We can't make this change only in one repo.

from arcade.

It might be a good time to make that build folder path configurable as well.

No, the folder should not be configurable. That would just add extra complexity.

from arcade.

I don't think this is blocking Arcade in any way at this point, is it?

from arcade.

No the folder should not be configurable

Even if it's not "configurable", it would still be nice if it was a single property that pointed to the folder. Just personal preference.

I don't think this is blocking Arcade in any way at this point, is it?

Only in the way I called out above. Arcade builds using the RepoToolset. RepoToolset looks for a few things in the build folder. Ideally, RepoToolset would look in the new folder structure instead and when Arcade updates to that version of RepoToolset, it updates its folder structure. If RepoToolset isn't going to move soon, then we'll have to make some temporary changes so that RepoToolset finds what it expects.

from arcade.

I can update the toolset to look at both locations for now.

from arcade.

@jaredpar Where would you expect build.ps1 be? In eng\core or eng\tools? I guess we might need to have two scripts: a standardized core\build.ps1 that's the same across all repos. If a repo needs some customization they would put one in eng\tools and use that one. Sounds reasonable?

from arcade.

That seems reasonable to me.

One item to think about is files which are naturally hierarchical: editorconfig, csproj, etc ... File formats which naturally look to parent directories in order to find other files to consider. This format means that for any such files the repo specific ones have the chance to override the ones in the common folder.

That seems like something we can guard against though. Just need to be dilligent about this for any files we put into common.

from arcade.

I actually don't think we should put any more files in common. There should just be the minimal amount necessary to restore the repo toolset. Everything else should be delivered by the toolset package.

from arcade.

I agree for now we don't. Trying to think forward to potential problems to watch out for in the future.

from arcade.

Makes sense. Though i wouldn't think we should add more files in the future either :)

from arcade.

Fixed with #83

from arcade.