Is it still possible to convert:

@AutotoolsPackage.run_after('install')

to:

@run_after('install')

?

@adamjstewart It's another thing on my todo list, but not done here. I wouldn't mind implementing it in this PR if we don't want this to be an intermediate step.

Any reason to keep the old names?

Also, +1 on converting. I'm still thinking post_ and pre_ methods would be nice for users, but that can wait.

Any reason to keep the old names?

@tgamblin If they are really sanity checks or precondition checks? But that's really a super minor point, so I wouldn't mind if we decide to delete them.

I don't mind keeping the old ones. Sometimes they really are for preconditions and sanity checks. But I also wouldn't be opposed to removing them. KISS principle always wins. I'll let someone else decide.

I have an inconsistent argument. I think we should get rid of the old names so there's one way to do this. I also think we should add pre_ and post_ methods so we have one more (easier) way to do it.

Or, we could just use pre_ and post_ methods so that user-contributed packages are clearer... Homebrew has pre/post methods FWIW. I worry that things will get hard to read when we start stacking @when decorators on top of the before and after ones, and more confusing with multiple when/before/after combinations. If you restrict to a single pre or post method, the when combinations won't get quite so out of hand.

I'm not particularly passionate about this, though so if you just want to convert and leave one way to do it for now that's fine with me.

I agree with @tgamblin on adding pre_ and post_ methods. Decorators are kind of an advanced topic in Python. We don't want the learning curve to be too high. But I can see why also having the decorators would still be useful, so that you can have multiple pre and post methods, some defined in AutotoolsPackage that you don't want the user to override, like the check for the installation directory.

KISS. Use the new ones.

I worry that things will get hard to read when we start stacking @when decorators on top of the before and after ones, and more confusing with multiple when/before/after combinations.

On the other hand with a decorator you can separate different actions into different functions and execute them after or before the same phase. This helps maintaining the code clean.

It'll also help new people read packages and contribute-- one less thing to learn.

Decorators are kind of an advanced topic in Python

Maybe writing them (and not in this specific case). Using them should be a piece of cake if they are written correctly 😄

Ok, so I'll try to make them "free-standing" decorators and remove the old names. I'll ping people when done.

I know you would have liked to keep the old names, but I think:

@sanity_check('install')
def sanity_check_prefix:

is just as explanatory as:

@run_after('install')
def sanity_check_prefix:

You can always make it sound like a sanity check in the function name.

Right, so I think for now let's go with one name for simple decorators and plan to add pre and post. Sound good?

@tgamblin How do you feel about:

1. doing what we planned for decorators
2. implement also a naming mechanism based on appended _pre_<phase> or _post_<phase>

What I have in mind is that:

@run_after('install')
def foo(self):
    ...

should be equivalent to:

def foo_post_install(self):
    ...

I don't think it requires a lot more code to do that.

I was thinking just:

• post_<phase>
• pre_<phase>

For simplicity. Only challenge: maintain execution order == declaration order if these are mixed with decorators.

Only challenge: maintain execution order == declaration order if these are mixed with decorators.

That could be difficult without the __prepare__ hook to grant we use an ordered dict. Do we really need to ensure this ?

The contract I was thinking was weaker: no relative order ensured for functions in the same pool (e.g. the ones that execute before a given phase). After all if you need to execute foo strictly before bar maybe they should be only one function...

Any reason to keep the old names?

to have less things to remember, i would remove old names completely. Even if it's a sanity check, it still reads fine with run_after(install), package developers can always name the method properly or add a quick documentation string to explain that it's a sanity check.

p.s. apparently, the same point is already raised by @adamjstewart above #2860 (comment) 😄

Fair enough. We do guarantee that whens are evaluated in order, though that hasn't really been a problem or come up at all. I suspect most packages will use one or the other of these. If they want order they can use decorators.

I also think we should add pre_ and post_ methods so we have one more (easier) way to do it.

from a python noob's perspective, i don't know how decorators work, but it's enough to see a single example to adapt it to ones needs. Personally, i would not bother with pre_ and post_ to simplify package developer's life.

@davydden You came late to the party 😆

Old names already removed, and decorators are not tied to base classes anymore. So:

@run_before('install')
def foo(self):
    ...

is the new

@AutotoolsPackage.precondition('install')
def foo(self):
    ...

@tgamblin @adamjstewart Let me know if you want me to proceed further here or in another PR. I am not in a rush to get this in, so any solution is fine with me.

BTW, does the metaclass code look cleaner? I tried to make it less complicated while I was at it.

@adamjstewart you may want to have a look at 6b64146. That's what I intended yesterday when I said in #2869 that I wanted to push the driver function for tests up in the hierarchy

ping

@alalazo: thanks! If you want to continue in another PR that is fine, but I don't think there is a rush.

@tgamblin I'll try to fix #2386 first then.