New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up rakefiles #70

Closed
ccoupe opened this Issue Feb 18, 2015 · 20 comments

Comments

Projects
None yet
2 participants
@ccoupe
Contributor

ccoupe commented Feb 18, 2015

Fixing #50 provides a new set of APP['CONSTANTS'] that should be used in each env.rb and task.rb or at least create new constants from them with names that make sense. 12 sets!

This would also be a good time to clean up the comments, remove unused methods and other oddities. Move constants the user must set to the top of the file, ...

Since this is not critical or entertaining and a lengthy process, it's low priority.

@ccoupe ccoupe added the Low label Feb 18, 2015

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 18, 2015

Collaborator

This would be absolutely fantastic. Few suggestions on the top of my head:

  • The main Rakefile should be behave as a dispatcher
  • Each target should provide their own raketask.rb
  • YAML based target configuration (paths, etc.)
  • Refactoring duplicated codes in targets

You might remember when I submitted files for MINGW target build on Windows, it included raketask.rb. It is possible to require make/*/raketask.rb where each target specific tasks will be added and available (rake -T).

Collaborator

BackOrder commented Feb 18, 2015

This would be absolutely fantastic. Few suggestions on the top of my head:

  • The main Rakefile should be behave as a dispatcher
  • Each target should provide their own raketask.rb
  • YAML based target configuration (paths, etc.)
  • Refactoring duplicated codes in targets

You might remember when I submitted files for MINGW target build on Windows, it included raketask.rb. It is possible to require make/*/raketask.rb where each target specific tasks will be added and available (rake -T).

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 18, 2015

Contributor

Cross compiling make them ugly and difficult, particularly with extconf.rb which has to be forced to build for a different platform than the one running extconf/mkmf. Ruby makes it messy and Ruby changes a lot.

Now that I've poked around a bit with cleanup on my mind one big confuser is the use of ENV['VAR'] to pass values from Rakefile to env.rb and if the ENV['var'] doesn't exist (and it won't in Shoes 3.2) use what we really want. env.rb and tasks.rb are not subprocesses. ENV is only needed for the ext/gem builds because they are spawned subprocesses.

No doubt that was an attempt to have one env.rb and one tasks.rb for all platforms that would be externally driven. Probably for Hackety Hack to procreate it's self with a sh rake build. That stopped working years ago. With Shoes 3.2 we have a way to drive the packager externally for any platform, from any platform without rake or compilers needing to be invoked. The cowboy in me says all that can go in every env.rb I see,

Most of the new VERSION constants would apply to tasks.rb. Somehow, that seems wrong. But you gotta go with what you have.

Contributor

ccoupe commented Feb 18, 2015

Cross compiling make them ugly and difficult, particularly with extconf.rb which has to be forced to build for a different platform than the one running extconf/mkmf. Ruby makes it messy and Ruby changes a lot.

Now that I've poked around a bit with cleanup on my mind one big confuser is the use of ENV['VAR'] to pass values from Rakefile to env.rb and if the ENV['var'] doesn't exist (and it won't in Shoes 3.2) use what we really want. env.rb and tasks.rb are not subprocesses. ENV is only needed for the ext/gem builds because they are spawned subprocesses.

No doubt that was an attempt to have one env.rb and one tasks.rb for all platforms that would be externally driven. Probably for Hackety Hack to procreate it's self with a sh rake build. That stopped working years ago. With Shoes 3.2 we have a way to drive the packager externally for any platform, from any platform without rake or compilers needing to be invoked. The cowboy in me says all that can go in every env.rb I see,

Most of the new VERSION constants would apply to tasks.rb. Somehow, that seems wrong. But you gotta go with what you have.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 18, 2015

Collaborator

Wouldn't be a better approach to use YAML target configuration file and dynamically create ENV variables from the YAML when needed?

Collaborator

BackOrder commented Feb 18, 2015

Wouldn't be a better approach to use YAML target configuration file and dynamically create ENV variables from the YAML when needed?

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 19, 2015

Contributor

@BackOrder - I'm sure which point you're responding too. HH config or ext/gem building or ?

For ext/gem, one possibility would be to use multiple extconf.rb (macextconf, mingwextconf armhfextconf) and execute the one you want from instead of extconf.rb you can also pass commandline args for macextconf.rb if needed. That would make them a lot easier to deal with if you need to modify them. For sqlite3 that might the way to go. ftsearch - not so much. It will only get worse if we decide to include new gems with more dependencies.

Contributor

ccoupe commented Feb 19, 2015

@BackOrder - I'm sure which point you're responding too. HH config or ext/gem building or ?

For ext/gem, one possibility would be to use multiple extconf.rb (macextconf, mingwextconf armhfextconf) and execute the one you want from instead of extconf.rb you can also pass commandline args for macextconf.rb if needed. That would make them a lot easier to deal with if you need to modify them. For sqlite3 that might the way to go. ftsearch - not so much. It will only get worse if we decide to include new gems with more dependencies.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 19, 2015

Collaborator

@BackOrder - I'm sure which point you're responding too. HH config or ext/gem building or ?

You mentioned that only ext/gem building are using ENV variables. Those ENV variables should be generated at runtime in such way we can get rid of all ENV hardcoded references from Rakefile and target files.

Hardcoded matters should always be on top of a file or in a YAML configuration file.

By the way, devdll = "C:/Devkit/mingw/bin" in https://github.com/Shoes3/shoes3/blob/master/make/tightmingw/env.rb#L92 should be devdll = "#{ENV['RI_DEVKIT']}/mingw/bin" (from require 'devkit').

Collaborator

BackOrder commented Feb 19, 2015

@BackOrder - I'm sure which point you're responding too. HH config or ext/gem building or ?

You mentioned that only ext/gem building are using ENV variables. Those ENV variables should be generated at runtime in such way we can get rid of all ENV hardcoded references from Rakefile and target files.

Hardcoded matters should always be on top of a file or in a YAML configuration file.

By the way, devdll = "C:/Devkit/mingw/bin" in https://github.com/Shoes3/shoes3/blob/master/make/tightmingw/env.rb#L92 should be devdll = "#{ENV['RI_DEVKIT']}/mingw/bin" (from require 'devkit').

BackOrder added a commit that referenced this issue Feb 19, 2015

Clean up #70: Ruby style hash
Converted multiple SOLOCS to Ruby hash style declaration. Also, changed
devdll to use devkit's actual path.

ccoupe added a commit that referenced this issue Feb 20, 2015

ccoupe added a commit that referenced this issue Feb 20, 2015

ccoupe added a commit that referenced this issue Feb 20, 2015

BackOrder added a commit that referenced this issue Feb 20, 2015

Minor #70 cleaning for target xmingw32
Ruby hash style, needs testing.

ccoupe added a commit that referenced this issue Feb 20, 2015

BackOrder added a commit that referenced this issue Feb 21, 2015

Minor #70 cleaning user files
@ccoupe is leaving his traces everywhere, editor and desktop stuff. Does
Shoes.dekstop needs to be in .gitignore ?

BackOrder added a commit that referenced this issue Feb 21, 2015

Minor #70 cleaning: removed one bacon STRIP
STRIP constant was duplicated, removed the unused one.

ccoupe added a commit that referenced this issue Feb 21, 2015

ccoupe added a commit that referenced this issue Feb 21, 2015

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 21, 2015

Contributor

Status Report: Just a couple more to do (snow, mavericks, xsnow) and then delete some constants stuff in Rakefile that shouldn't be needed anywhere, snort. They still aren't pretty.

Unsure what to do with make/mingw and make/darwin - the loose/tight doesn't matter all that much to mingw or OSX.

Contributor

ccoupe commented Feb 21, 2015

Status Report: Just a couple more to do (snow, mavericks, xsnow) and then delete some constants stuff in Rakefile that shouldn't be needed anywhere, snort. They still aren't pretty.

Unsure what to do with make/mingw and make/darwin - the loose/tight doesn't matter all that much to mingw or OSX.

@ccoupe ccoupe added this to the 3.2.22 milestone Feb 21, 2015

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 21, 2015

Collaborator

Unsure what to do with make/mingw and make/darwin - the loose/tight doesn't matter all that much to mingw or OSX.

My feedback is that loose Shoes was actually useful to explore and fix things. Leaving legacy targets may be acceptable. Now the target names are unclear and it makes it difficult to know what those are for. Shall we reconsider renaming all targets in a coherent and comprehensive manner?

Collaborator

BackOrder commented Feb 21, 2015

Unsure what to do with make/mingw and make/darwin - the loose/tight doesn't matter all that much to mingw or OSX.

My feedback is that loose Shoes was actually useful to explore and fix things. Leaving legacy targets may be acceptable. Now the target names are unclear and it makes it difficult to know what those are for. Shall we reconsider renaming all targets in a coherent and comprehensive manner?

@ccoupe ccoupe self-assigned this Feb 21, 2015

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 21, 2015

Contributor

Loose Shoes on OSX could be useful for a Shoes developer, but its' a bit of a mess at the moment.

I'm listening if you have a proposal for target naming but remember that changes to packaged Shoes names/structures will bubble up, down and around the user packaging, the website, your projects and mine.

Contributor

ccoupe commented Feb 21, 2015

Loose Shoes on OSX could be useful for a Shoes developer, but its' a bit of a mess at the moment.

I'm listening if you have a proposal for target naming but remember that changes to packaged Shoes names/structures will bubble up, down and around the user packaging, the website, your projects and mine.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 21, 2015

Collaborator

The package names seem acceptable but the target names in make directory are confusing. For Windows alone: mingw, tightmingw, xmingw32, xmsw32. The name in rake -T are also confusing.

Perhaps changing the structure would make it easier to understand with a pattern to be repeated on each platform, such as, for make sub-directories:

  • windows-mingw-32
  • windows-mingw-cross-32
  • windows-mingw-tight-32
  • windows-native-32
Collaborator

BackOrder commented Feb 21, 2015

The package names seem acceptable but the target names in make directory are confusing. For Windows alone: mingw, tightmingw, xmingw32, xmsw32. The name in rake -T are also confusing.

Perhaps changing the structure would make it easier to understand with a pattern to be repeated on each platform, such as, for make sub-directories:

  • windows-mingw-32
  • windows-mingw-cross-32
  • windows-mingw-tight-32
  • windows-native-32

ccoupe added a commit that referenced this issue Feb 22, 2015

OSX cleanup (#70)
A lot of Rakefile has been devoted to OSX 'improvements'.  Moving them
to env.rb and task.rb is not complete.

ccoupe added a commit that referenced this issue Feb 23, 2015

ccoupe added a commit that referenced this issue Feb 23, 2015

ccoupe added a commit that referenced this issue Feb 24, 2015

More #70 OSX rake file changes
* they do work for targets mavericks and xsnow
* still not pretty in env.rb
* Commented out out, moved most OSX stuff from Rakefile to env.rb(s)
@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 24, 2015

Collaborator

12b34bc#diff-9b6f128e0f6d45004d090c8735ae4e50R159

cp "#{libn}", "#{TGT_DIR}/" unless File.exists? "#{TGT_DIR}/#{keyf}"

The problem with this approach is that it won't update a library if it is newer but instead only if it is not present. This is what Rake file and rule are for and would be Rakefile best practice. See link below.

http://madewithenvy.com/ecosystem/articles/2014/rake-rule-tasks/

But, in a gist, it means: the task will only be executed if the file does not exist, or unless it has a file task dependency with a newer timestamp than itself.

Collaborator

BackOrder commented Feb 24, 2015

12b34bc#diff-9b6f128e0f6d45004d090c8735ae4e50R159

cp "#{libn}", "#{TGT_DIR}/" unless File.exists? "#{TGT_DIR}/#{keyf}"

The problem with this approach is that it won't update a library if it is newer but instead only if it is not present. This is what Rake file and rule are for and would be Rakefile best practice. See link below.

http://madewithenvy.com/ecosystem/articles/2014/rake-rule-tasks/

But, in a gist, it means: the task will only be executed if the file does not exist, or unless it has a file task dependency with a newer timestamp than itself.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 24, 2015

Contributor

This part of the task.rb is a ruby method to find and copy the libraries that Ruby and Shoes need. In the case of xsnow, these are the 10.6 libraries and they do not want to be confused with newer 10.9 libraries nor can rake derive the the list with simple time stamps or file name parsing. Using cp_f might be more clear.

Contributor

ccoupe commented Feb 24, 2015

This part of the task.rb is a ruby method to find and copy the libraries that Ruby and Shoes need. In the case of xsnow, these are the 10.6 libraries and they do not want to be confused with newer 10.9 libraries nor can rake derive the the list with simple time stamps or file name parsing. Using cp_f might be more clear.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 24, 2015

Collaborator

It wouldn't confuse 10.6 libraries with 10.9 but you might be right about cp_f being more clear. The rakefiles were not exactly written in a Rakeish manner to start with. The cleaning seems to going well. Keep it up!

Collaborator

BackOrder commented Feb 24, 2015

It wouldn't confuse 10.6 libraries with 10.9 but you might be right about cp_f being more clear. The rakefiles were not exactly written in a Rakeish manner to start with. The cleaning seems to going well. Keep it up!

ccoupe added a commit that referenced this issue Feb 25, 2015

More #70 cleanup for snowleopard. Doesn't suck much.
* Attempt to remove all OSX specific constants from Rakefile
* snowleopard/env.rb CAIRO/PANGO flags/libs should be put into xsnow
* Info.plist had old constants.

ccoupe added a commit that referenced this issue Feb 25, 2015

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 25, 2015

Contributor

target snowleopard (10.6 on 10.60 doesn't compile the hpricot exts because of a bad -Wno-something directive in the extconf.rb . Loose Shoes on darwin (all osx versions) is probably incorrect. Loose Shoes on mingw is untested, probably not correct.

On the bright side, I can build the major platforms that are distributed with one shell script . I don't care that much about Loose Shoes on Windows or OSX. New developers can handle the tightshoes configuration just as easily as Loose Shoes (since the configure values are the same)

I'd rather tackle the extconf.rb mess. One platform at a time, sigh.

Contributor

ccoupe commented Feb 25, 2015

target snowleopard (10.6 on 10.60 doesn't compile the hpricot exts because of a bad -Wno-something directive in the extconf.rb . Loose Shoes on darwin (all osx versions) is probably incorrect. Loose Shoes on mingw is untested, probably not correct.

On the bright side, I can build the major platforms that are distributed with one shell script . I don't care that much about Loose Shoes on Windows or OSX. New developers can handle the tightshoes configuration just as easily as Loose Shoes (since the configure values are the same)

I'd rather tackle the extconf.rb mess. One platform at a time, sigh.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Feb 25, 2015

Collaborator

I'm liking the changes so far because things are getting orderly and a bit more clear. Still thinking it would be a good idea to move out the namespaces to their respective targets. What do you think?

https://github.com/Shoes3/shoes3/blob/master/Rakefile#L322

Collaborator

BackOrder commented Feb 25, 2015

I'm liking the changes so far because things are getting orderly and a bit more clear. Still thinking it would be a good idea to move out the namespaces to their respective targets. What do you think?

https://github.com/Shoes3/shoes3/blob/master/Rakefile#L322

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 25, 2015

Contributor

It's been a bottom up cleanup so far which was the original intent for this issue. OSX is the more complex platform and the source of much confusion and errors. Declining ROI on the 10.6 stuff doesn't help either.

Some of your suggestions suggest a top-down or rethink/re-architect of the whole mess which may be the only way to really get the warts off and should consider my ideas about making gems/exts more independent build-ables.

Contributor

ccoupe commented Feb 25, 2015

It's been a bottom up cleanup so far which was the original intent for this issue. OSX is the more complex platform and the source of much confusion and errors. Declining ROI on the 10.6 stuff doesn't help either.

Some of your suggestions suggest a top-down or rethink/re-architect of the whole mess which may be the only way to really get the warts off and should consider my ideas about making gems/exts more independent build-ables.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Feb 27, 2015

Contributor

I'm going to keep the namespace for a while but I am going to rename things to be a little more consistent. darwin, linux, mingw will be the Loose Shoes names - same as it ever was. Tight Shoes names will change slightly. They all need a name space (osx:|| win32:|| linux:) same as before.

There will be native builds and not native builds (cross compile or down shifting). The latter have an'x' in front of their name. xwin7, x86_64-linux, xi686-linux, xsnowleopard. It's easier to just do it than explain.

win32:setup:tightmingw will become win32:setup:win7 and linux:setup:mingw32 will be linux:setup:xwin7

Contributor

ccoupe commented Feb 27, 2015

I'm going to keep the namespace for a while but I am going to rename things to be a little more consistent. darwin, linux, mingw will be the Loose Shoes names - same as it ever was. Tight Shoes names will change slightly. They all need a name space (osx:|| win32:|| linux:) same as before.

There will be native builds and not native builds (cross compile or down shifting). The latter have an'x' in front of their name. xwin7, x86_64-linux, xi686-linux, xsnowleopard. It's easier to just do it than explain.

win32:setup:tightmingw will become win32:setup:win7 and linux:setup:mingw32 will be linux:setup:xwin7

ccoupe added a commit that referenced this issue Feb 27, 2015

rename raspberry pi build - more #70
Also fix win7 native build - comment ENV flags.
@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Mar 11, 2015

Contributor

For win7 and xwin7 targets, the rake build will look for win7-custom.yaml and xwin7-custom.yaml. (respectively). These files are not in github. They are local customizations that won't be overwritten with a github pull or fetch. For example my win7-custom.yaml might like this

Deps: E:/shoesdeps/mingw
Ruby: C:/Ruby21
MS-Theme: false
Debug: true

Debug is true/false - false if not specified at all. It sets/unsets the ENV['GDB'].
MS-Theme is true/false (false if not given). False is the old 3.2.21 appearance and true uses MS-Windows theme GTK2/Windows.

Contributor

ccoupe commented Mar 11, 2015

For win7 and xwin7 targets, the rake build will look for win7-custom.yaml and xwin7-custom.yaml. (respectively). These files are not in github. They are local customizations that won't be overwritten with a github pull or fetch. For example my win7-custom.yaml might like this

Deps: E:/shoesdeps/mingw
Ruby: C:/Ruby21
MS-Theme: false
Debug: true

Debug is true/false - false if not specified at all. It sets/unsets the ENV['GDB'].
MS-Theme is true/false (false if not given). False is the old 3.2.21 appearance and true uses MS-Windows theme GTK2/Windows.

ccoupe added a commit that referenced this issue Mar 11, 2015

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Mar 11, 2015

Collaborator

Excellent. Very promising. This should be documented somewhere since it is in .gitignore.

Collaborator

BackOrder commented Mar 11, 2015

Excellent. Very promising. This should be documented somewhere since it is in .gitignore.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Mar 20, 2015

Contributor

Added mavericks-custom.yaml. Mine looks like:

Deps: /usr/local
Zlib: /usr/local/opt/zlib/lib
Debug: true
CFLAGS: -DNEW_RADIO

Delete the CFLAGS line if you want the older behavior for bug #6

Contributor

ccoupe commented Mar 20, 2015

Added mavericks-custom.yaml. Mine looks like:

Deps: /usr/local
Zlib: /usr/local/opt/zlib/lib
Debug: true
CFLAGS: -DNEW_RADIO

Delete the CFLAGS line if you want the older behavior for bug #6

ccoupe added a commit that referenced this issue Mar 20, 2015

@ccoupe ccoupe modified the milestones: 3.2.23, 3.2.22 Mar 27, 2015

ccoupe added a commit that referenced this issue Mar 31, 2015

more rakefile cleanup (hah!) for #70 and prep for #72
* Use TGT_ARCH from Rakefile to pick a target specific extconf.rb
  if it exists. Other wise use the monster ugly extconf.rb
* Should have done this months ago.
* Still need to invent/discover a way to delete existing RbCongfig::
  constants and reload
* had to modify .gitignore more better.

ccoupe added a commit that referenced this issue Apr 1, 2015

for #70 - create xarmv6hf-extconf.rb
* building hpricot_scan still whines like a spoiled child.
  diminishing returns for that situation.

ccoupe added a commit that referenced this issue Apr 1, 2015

add extconf.rb for #70, x86_64_linux and xi686_linux
* i686 was never a whiner but consistent small minds are at work.
* yes x86_64 and i686 are identical.

ccoupe added a commit that referenced this issue Apr 2, 2015

ccoupe added a commit that referenced this issue Apr 3, 2015

add extconf.rb for win7 build. #70.
All tight Shoes can be built using just the special extconfs
except for the 10.6 on 10.6 (I don't care about that one since building 10.6 from 10.9 works well enough)

ccoupe added a commit that referenced this issue Apr 18, 2015

Separate gems and shoes building #72 and #70.
* still has backwards/default just in case code
* expect changes - doesn't suck much as is.

ccoupe added a commit that referenced this issue Apr 19, 2015

rakefile OSX for seperate gems location/compile #72, #70
* start the move of common_build from make/*/tasks to make/common-build.rb
* differentiate between '.bundle' and '.so' for exts/gems

ccoupe added a commit that referenced this issue Apr 19, 2015

@ccoupe ccoupe closed this Jun 13, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment