Skip to content
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

function naming should be tweaked according to community consensus #99

Closed
boegel opened this issue Aug 17, 2012 · 14 comments
Closed

function naming should be tweaked according to community consensus #99

boegel opened this issue Aug 17, 2012 · 14 comments

Comments

@boegel
Copy link
Member

boegel commented Aug 17, 2012

Some of the functions (mainly the ones being called in the build function of build.py) could be named better.

E.g., make_install should be changed to install, make to build, etc.

@fgeorgatos will study if there's a consensus on a naming scheme by looking at porting systems (e.g., MacPorts).

@ghost ghost assigned fgeorgatos Aug 17, 2012
@fgeorgatos
Copy link
Collaborator

Hi there,

according to this collection of bits
https://github.com/fgeorgatos/easybuild.experimental/wiki/Build-ports-naming-conventions

the pkgsrc seems to be the most generic convention (albeit more verbose than needed in eb case).

Check it out, these are Makefile targets really:

  • depends: to build and install dependencies
  • fetch: to fetch distribution file(s)
  • checksum: to fetch and check distribution file(s)
  • extract: to look at unmodified source
  • patch: to look at initial source
  • configure: to stop after configure stage
  • build: to stop after build stage
  • stage-install: to install under stage directory
  • test: to run package's self-tests, if any exist and supported
  • package: to create binary package before installing it
  • replace: to change (upgrade, downgrade, or just replace) installed package in-place
  • deinstall: to deinstall previous package
  • package-install: to install package and build binary package
  • install: to install package
  • bin-install: to attempt to skip building from source and use pre-built binary package

@fgeorgatos
Copy link
Collaborator

Hi there,

I recommend we make this a top priority issue for coming week, since it affects too many things:

Adopting the style has one obvious advantage: when people will need to make extensions to easybuild for the various steps (esp. as regards pre- & post- functions), there is established knowledge about what works well and how; and also what has been tried elsewhere without great results and should rather be avoided...

@boegel
Copy link
Member Author

boegel commented Aug 18, 2012

I broke these down, you're welcome to add remarks on this.

Note: work-in-progress

  • depends: to build and install dependencies
    • I guess this comes closest to what we do with --robot. Renaming --robot to --depends is a no-go imho though, because it's not exactly the same.
  • fetch: to fetch distribution file(s)
    • In EasyBuild, this is called file_locate now. I guess renaming this to fetch makes sense.
  • checksum: to fetch and check distribution file(s)
    • nothing like this there yet, but there will be soon (see [1st-EasyBuild-hackathon---meeting-minutes-day-1]), so we'll use checksum for that
  • extract: to look at unmodified source
    • this is called unpack now, changing to extract seems fine
  • patch: to look at initial source
    • this is called apply_patch now, ok to change to patch
  • configure: to stop after configure stage
  • build: to stop after build stage
    • this is called make now, and should be changed to build (which will clash with another function named build that will have to be renamed)
  • stage-install: to install under stage directory
    • EasyBuild doesn't have something like this imho, it just installs straight into the final install dir (should we have something like that?)
  • test: to run package's self-tests, if any exist and supported
    • test is there already, so this one is covered
  • package: to create binary package before installing it
    • EasyBuild doesn't do this
  • replace: to change (upgrade, downgrade, or just replace) installed package in-place
    • I guess this is close to what force does, but there's no function for it, and force is more than just replacing existing installs
  • deinstall: to deinstall previous package
    • EasyBuild doesn't do this (why would we?)
  • package-install: to install package and build binary package
  • is this for packages like numpy/scipy, or to create a package for the software that was built? If the latter, EasyBuild doesn't do this yet, but might in the future (e.g. when we add support for generating RPMs)
  • install: to install package
    • this is called make_install now, and should be changed to install
  • bin-install: to attempt to skip building from source and use pre-built binary package
    • we have a seperate easyblock for that, so this doesn't apply imho

@fgeorgatos
Copy link
Collaborator

Note: work-in-progress; FIXME

  • depends: to build and install dependencies
    • KH: I guess this comes closest to what we do with --robot. Renaming --robot to --depends is a no-go imho though, because it's not exactly the same.
    • FG: I think --robot is calling multiple packages each with multiple depends so it's clear what's going on now, no?
  • fetch: to fetch distribution file(s)
    • KH: In EasyBuild, this is called file_locate now. I guess renaming this to fetch makes sense.
    • FG: Agreed: renaming this to fetch makes sense.
  • checksum: to fetch and check distribution file(s)
    • KH: nothing like this there yet, but there will be soon (see [1st-EasyBuild-hackathon---meeting-minutes-day-1]), so we'll use checksum for that
    • FG: this does not have to be ready before v1.0 but we should put a placeholder just to make it clear we have it in mind; one way to prepare well for it would be, to state sha1 checksums inside easybuild_ebfiles_repo auto-produced files; that would allow for an easy massive regression test with the pkg2eb provided package list (>600 successful targets) and if that works fine, it's green light.
  • extract: to look at unmodified source
    • KH: this is called unpack now, changing to extract seems fine
    • FG: Agreed
  • patch: to look at initial source
    • KH: this is called apply_patch now, ok to change to patch
    • FG: Agreed
  • configure: to stop after configure stage
  • KH: ok
  • FG: as says
  • build: to stop after build stage
  • KH: this is called make now, and should be changed to build (which will clash with another function named build that will have to be renamed)
  • FG: Agreed
  • stage-install: to install under stage directory
    • KH: EasyBuild doesn't have something like this imho, it just installs straight into the final install dir (should we have something like that?)
    • FG: Agreed; this would only be needed for software that requires it (I don't have an example, but some software stages via an intermediate code generator or something). We need to investigate but, it doesn't look like urgent.
  • test: to run package's self-tests, if any exist and supported
    • KH: test is there already, so this one is covered
    • FG: Agreed
  • package: to create binary package before installing it
    • KH: EasyBuild doesn't do this
    • FG: I have the impression this is in effect the rpm generation business we discussed (wire it out to OSB API anyone? ;-)
  • replace: to change (upgrade, downgrade, or just replace) installed package in-place
    • KH: I guess this is close to what force does, but there's no function for it, and force is more than just replacing existing installs
    • FG: I'm not sure about the semantic of this one; it might mean to simply deinstall a previous modulefile and make the new one default; I think we should leave force as-is though
  • deinstall: to deinstall previous package
    • KH: EasyBuild doesn't do this (why would we?)
    • FG: A simply rm on the modulefile should do fine with this one; I recommend we don't jump in implementation of these items before we have use-cases (then we'll have better understanding on the what and how)
  • package-install: to install package and build binary package
    • KH: is this for packages like numpy/scipy, or to create a package for the software that was built? If the latter, EasyBuild doesn't do this yet, but might in the future (e.g. when we add support for generating RPMs)
    • FG: I think this is a combination of "package" & something like rpm -Uvh (ie. verify the package style of install)
  • install: to install package
    • KH: this is called make_install now, and should be changed to install
    • FG: Agreed: this is called make_install now, and should be changed to install
  • bin-install: to attempt to skip building from source and use pre-built binary package
    • KH: we have a seperate easyblock for that, so this doesn't apply imho
    • FG: Not sure how to handle this but it certainly corresponds to EB_Binary() method now

@fgeorgatos
Copy link
Collaborator

related discussion on possible function prefix

btw. what if we had to prepend these functions with something to make them a family?

so, here is another lingo-related finding:

macports: phases
portage: functions
pkgsrc: targets
ports: targets
rpmspec: stage

So, for me the prefix should be either Stage_ or Phase_ or '' ... what are your preferences?

@boegel
Copy link
Member Author

boegel commented Aug 19, 2012

Now, we're using 'step', i.e. we run every step of the build/install process using a runstep function.

There's clearly no consensus on that (5 tools, 4 different lingos), so why change it?

@boegel
Copy link
Member Author

boegel commented Aug 22, 2012

@stdweird, @JensTimmerman: Can you guys check the proposal made in easybuilders/easybuild#99 (comment), and let us know whether you have any objections to this?

If not, I'd like to make a pull request for these changes this week.

@stdweird
Copy link
Contributor

a few remarks
reserve all functions in Application class, even if we don't implement them yet (just add a description that they are reserved pending future implemntation)

depend vs robot: robot is a runtime option, but it sort of does what depend suggests. depend is like the auto-robot (ie just build all deps always)

i think at some point we should have a deinstall (some people might want to cleanup software, eg because they run out of space): check is tool/version is a dependency of something else; if not, remove module file and install dir

@boegel
Copy link
Member Author

boegel commented Aug 22, 2012

@stdweird: see #144 for the deinstall option

I will take the others remarks into account while making the pull request.

@ghost ghost assigned boegel Aug 22, 2012
@JensTimmerman
Copy link
Contributor

I agree with Stijn, adding all these function to easyblock, and calling them in runstep now will allow for greater flexibility when needed in the future.

@fgeorgatos
Copy link
Collaborator

Hi,

On Thu, Aug 23, 2012 at 11:41 AM, Jens Timmerman
notifications@github.comwrote:

I agree with Stijn, adding all these function to easyblock, and calling
them in runstep now will allow for greater flexibility when needed in the
future.

+1

In fact, doing so and homogenizing their pre/post/during calling
conventions
(preconfigopts, configopts, premakeopts, makeopts, installopts,
unpackOptions)
would make the whole thing feel more robust and extendible.

It would be useful to allow for extensions across all functions like:
build.before="cp Makefile.template Makefile" # ok, the equivalent in python
build.after="cp timestamp ./build"

This gives freedom to other developers to override behavior
(but, then again, you may bring the argument of doing these in the
easyblock way).

@fgeorgatos
Copy link
Collaborator

btw.
there has to be some definition about where depends (--robots) sits in the list of steps:

  • depends -> fetch -> checksum -> extract

Some implementations, like ports, seem to put it after checksum;
presumably to avoid legthy builds, whenever there is an issue with the original package.

OTOH, doing so masks dependency issues; what is your stance?
I have no particular recommendation but it would be good to probe you
for any strong technical argument as regards the order of "steps".

@boegel
Copy link
Member Author

boegel commented Aug 24, 2012

I worked out the function renaming scheme for this issue on the whiteboard at our lab, together with the required changes for # 136 and #90.

http://users.ugent.be/~kehoste/function_renaming_scheme.jpg

Don't worry if it doesn't make sense to you, it does to me. I'll be implementing it during the next couple of days in https://github.com/boegel/easybuild/tree/99_function_renaming, and I'll issue a pull request when it's done.

@JensTimmerman
Copy link
Contributor

For folow up on this, see #272

boegel added a commit that referenced this issue Oct 14, 2012
implement function renaming scheme (see issue #99)
fgeorgatos added a commit to fgeorgatos/easybuild-framework that referenced this issue Feb 6, 2013
…as per issue easyblocks/easybuilders#99

Signed-off-by: Fotis Georgatos <fotis.georgatos@uni.lu>
boegel added a commit that referenced this issue Feb 11, 2013
add quotes around setenv definitions in modulefiles & define authors as per issue easyblocks/#99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants