constabulary / gb

gb, the project based build tool for Go
https://getgb.io/
MIT License
2.15k stars 147 forks source link

Compiler output should be relative to a fixed point #406

Open davecheney opened 9 years ago

davecheney commented 9 years ago

Having the compiler output a unique file path when compilation fails has been a long standing issue, see #57, #79, #99.

57 changed to make paths relative to $PROJECT, but this since regressed :(

#395 #377 addressed the regression, by making the output relative to the current working directory.

This is a tracking issue to come up with a better plan than the piecemeal solutions so far.

Requirements:

/cc @fatih, @inconshreveable, @tianon

inconshreveable commented 9 years ago

I think you mean #377 not #395?

377 is both unit tested and properly works with context.Workdir() on Windows. (It only modifies paths that were already relative. Any absolute paths are left unchanged.)

Unfortunately, #377's behavior with -R is suboptimal. Paths relative to $pwd are probably not the most helpful in that situation. Worse, if you use -R and specify a path on a different drive on Windows, gb will now break.

I would suggest one of two alternatives:

  1. Paths should always be relative to gb project root
  2. Paths should always be relative to $pwd or, if supplied, the argument to -R

Edit: I'm ambivalent between these options.

davecheney commented 9 years ago

@inconshreveable correct. To be clear, I am not saying your work on #377 is incorrect. I am grateful for it. This issue is to track the remaining work.

I would suggest one of two alternatives:

  1. Paths should always be relative to gb project root
  2. Paths should always be relative to $pwd or, if supplied, the argument to -R

Sadly I think the solution is going to be more complicated. gb will have to handle both relative and absolute paths. We should be using relative paths internally whereever possible, and probably relative to the project root. But I fear there will be cases where absolute paths are required when dealing with generated code from tests or cgo.

More generally, I am concerned there is too much path munging going on inside build.go and gc.go, which is #99. I think paths should be generated once, and transformed a little as possible after that.

inconshreveable commented 9 years ago

Yep, I understand! I just wanted to summarize where #377 leaves the current state of the world for everyone.

You make a good point about the cgo/temp files. Perhaps it makes more sense to say:

Does that seem more reasonable? I agree that the less path munging the better, especially if they can be created with the proper form.

davecheney commented 9 years ago

@inconshreveable yes, I like that, paths outside the project should be absolute. I think as per #57 paths should be relative to the project root. Specifically the $cwd should never be used for path computation within go (cmd/gb is the exception as it has to do a lot of skutt work to unpack package globs and things like that). I think this will resolve any problems with -R.

The decision of what is inside or outside a project should not be based on the $cwd of the user. It should be part of the logic that generates the paths, things like build.go:objfile and build.go:pkgfile, and friends. There is probably a lot to be done there.

inconshreveable commented 9 years ago

+1 that all sounds good to me

tianon commented 9 years ago

Yeah, big +1 from me for paths outside being absolute and inside relative to project. Clean and simple IMO.

davecheney commented 9 years ago

Excellent. I plan to work on this, but probably not til after Gotham Go. Feel free to jump in if you like.

To give some background about my suggestion for how this should be tested, I view testing the output of commands by looking at stdout/stderr with about the same level of disdain as I view using selenium for doing front end testing -- it works, but it has a low cost to benefit ratio.

I'd like to try to do this another way, generate all the paths we need via helper functions, then unit test those, assuming the results from those functions remains unmolested this should give stable results without the need to use end to end testing.

dradtke commented 9 years ago

Ah, I wasn't aware of this issue when I created my PR. When I have some time I'll look through the proposal here to see if I can help out.

davecheney commented 9 years ago

@dradtke thanks. I'm sorry about your previous PR. The goal of #406 is to try to always involve the compiler from at fixed point in the directory tree, so that the output from the compiler naturally falls into the correct place relative to $PROJECT.

ssmccoy commented 8 years ago

I have submitted what I believe to be a very simple fix for this problem. I am using it in our project and it appears to solve the problem. (PR #652)

My project does not make comprehensive use of all gb features, so please let me know if there's anything this may have broken. But the tests still pass and this looks like it works.

> make bin/kamke
gb build
# kamke/calculator
src/kamke/calculator/calculator.go:113: undefined: BROKEN
FATAL: command "build" failed: exit status 2
Makefile:39: recipe for target 'bin/kamke' failed
make: *** [bin/kamke] Error 1

This makes vim's compiler integration much happier.