Not sure about debugflavour

[TurkeyMan]

@redorav recently introduced a new api which I'm not so sure about.
I feel there's already comprehensive set of api's to give sufficient debugger configuration, and the new API feels poorly defined. I feel like it can be expressed in terms of the existing API's...

Perhaps we could get some comment about the reason for this approach?

[redorav]

@TurkeyMan I haven't added any new apis to premake (as far as I know!) I've looked to see who added it but I can't find "debugflavour" in the repo.

[tdesveauxPKFX]

Hey, I did.

And now that you mention it, I see other already existing APIs that I could have used, like debugger.
I don't know how I missed it. I will wait a little to see if someone find a reason I should not extend debugger and submit a PR fixing this if not.

[TurkeyMan]

Oh I'm sorry, I was browsing the recent commit log, and you both have a lot of commits near the top. Sorry for the confusion!

[TurkeyMan]

I see how I made the confusion... you have both merged master into your working branches a few times, and then produce PR's which have been accepted. Those merges are pulling each others patches into your working branches. The top of the revision graph is a mess! ;)

For future reference, can we please be sure to sanitise branches before creating PR's?
Rebase on master, eliminate all the merges into working branch from the timeline, squash incremental changes if it makes sense to do so, or massage into a small number of patches for convenient review; ie, one patch implements feature, next patch updates unit-tests, next refactors existing code to use the new feature. It's nice to present PR's as a series of patches that the reviewer should understand and digest in sequence, and it preserves much better understanding of history.

I'll try and keep my eye on it better in the future. Sorry I missed it!

[redorav]

I apologize for any mess I may have caused in the history. I'm starting to learn how git works and I'm sure I'm doing a few things wrong on the way. My first PR ever has been in this project if that serves as an indicator :) I'll be sure to follow your advice for the next.

[TurkeyMan]

Regarding the debugflavour thing. Options are Local, Remote, WebBrowser, WebService.
I think local/remote distinction can be implied by other existing options, like debugremotehost. Safe to say, if that's set, we're remote-debugging.
The the 2 web options can probably be debuggertype's? Does that feel right?

[tdesveauxPKFX]

@TurkeyMan Yeah, my last PRs are somewhat a mess with all the merges. I was not sure how github would react if I rebased and trashed the merges but it seems it will be fine? I will try it on my last open one. EDIT: or not, there is no merge in this one.

As for debuggerflavor, I will look into the existing debug APIs and get back to you.

[TurkeyMan]

@redorav No problem. I also learned git at one point! ;) ... it brings a lot of new concepts to source-control, especially when producing PR's for consumption by reviewers in OSS projects. You have more control over how your code is received and reviewed, and how it should appear in the history once it's merged.
If you're new to rebase, make sure to create a backup branch at the point before any rebases you perform, because you're almost certain to mess it up a few times!

@tdesveauxPKFX Github will accept whatever you do to your git repo. It doesn't keep state, it just takes the branch history as given. If you do a complex rebase and force-push, it'll just accept it as truth.

[samsinsane]

I think local/remote distinction can be implied by other existing options, like debugremotehost. Safe to say, if that's set, we're remote-debugging.

Not true, there are situations where you can run locally and remotely, where you heavily favour the former. This has been a requirement at work for the last 5+ years, due to the nature of those projects, remote debugging just isn't feasible 100% of the time.

As for flavor vs type, these are different. Flavour sets the default debugger, so next to the green triangle instead of "Local Windows Debugger" it would be "Web Browser Debugger", while Type sets the kind of local windows debugger so instead of "Native Only", you could have "Mixed" or "Script". FWIW, all four of the debugger flavours support a different set of types.

The top of the revision graph is a mess!

Welcome to the new and improved GitHub! Every PR now requires "administrator privileges" if it's behind master, and there is an "Update Branch" button to merge master into your branch. I understand why it exists, but it makes the commit history an absolute nightmare to navigate since the author and core devs can click the button.

[tdesveauxPKFX]

@TurkeyMan So I looked at the existing APIs and debuggerflavor should totally be in debugger.
However, that's as far as I would go after reading @samsinsane argument's.

Also, somewhat unrelated, there is some APIs such as debugtoolargs and debugtoolcommand that are not used anywhere. Should they be removed?

[samsinsane]

I'm not really "against" the merge of debuggerflavor and debugger, however, looking at the VSLinux extension and the Android support in VS2015+. It seems "weird" to merge them?

VSLinux can use GDB or GDB Server, and internally this uses remote debugger elements in the .vcxproj.user file.

Android uses GDB Server (I think - there's a precompiled binary for it, and no option to pick between local and server), and internally this uses one local debugger element in the .vcxproj.user file. It doesn't seem to say anywhere what debugger you're going to be using, and the "flavor" is AndroidDebugger.

Again, I'm not against this, it just seems weird to me. I'm really just looking to see what others have to say on the matter with the extra (mostly useless) information I've just provided.

[TurkeyMan]

Wow, that "Update Branch" button is the most hideous thing that's ever happened to Github!!

[TurkeyMan]

Regarding the API layout; I don't really see how the selection of GDB or LLDB can be in exclusion of Local/Remote. Surely those values are on another axis... since GDB/LLDB can be used either locally or remotely?

I don't understand Sam's argument; why can't the local/remote-ness be implied? When do you configure remote debug settings, and then NOT remote debug?

[samsinsane]

I don't understand Sam's argument; why can't the local/remote-ness be implied? When do you configure remote debug settings, and then NOT remote debug?

We do this all the time at work, we have two teams developing software for CAVEs, and another team developing content for them. This means the settings are configured but most of the software development team use their local machines as much as possible before moving to a CAVE. If you were to imply local/remote from the settings being set, you would assume wrong for our situation and we would have no API to force Premake to do the right thing.

I'm not suggesting it's an overly common situation but it's the situation that we're in and I can't imagine that we're the only ones. If it's decided to not support this situation, I don't mind, we'll work around it or disable the "flavour" option altogether locally.

As for your comments regarding GDB/LLDB, couldn't we just as easily change Local/Remote to be more accurate in what they are? WindowsLocal and WindowsRemote or VisualStudio instead of Windows? I do agree that it's weird, but we're dealing with a really bizarre set of APIs that mean vastly different things in different tools.

[tdesveauxPKFX]

I was finally able to take a look at this.

I would side with @samsinsane on this and not force DebuggerFlavor depending on other APIs.
But I can see something like:
Is debugger set?
- Yes, use it.
- No. Are any other APIs set? If yes, set a default value that make sense.

I also agree that just Local Remote are not clear anymore. VSLocal seems fine to me, VisualStudioLocal would be too much and Windows is not clear enough as you can use GDB as @TurkeyMan mentionned.

If this look good to you, I will set it up this week-end.