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

Error if AddPostAction() on node with no action (e.g., Alias nodes) #2281

Open
bdbaddog opened this issue Jan 2, 2018 · 4 comments · Fixed by #4674
Open

Error if AddPostAction() on node with no action (e.g., Alias nodes) #2281

bdbaddog opened this issue Jan 2, 2018 · 4 comments · Fixed by #4674

Comments

@bdbaddog
Copy link
Contributor

bdbaddog commented Jan 2, 2018

This issue was originally created at: 2008-12-21 07:18:51.
This issue was reported by: rhl.

rhl said at 2008-12-21 07:18:51

The following SConstruct file prints, "Hello"

show = Command("show",  "", "@echo Hello")
show = Alias("foo", show)
AddPostAction(show,  "@echo Goodbye")

I.e. the AddPostAction is accepted, but does nothing; if you comment the Alias line you get the expected "Hello" and "Goodbye".

IMHO, scons should either handle this case, or flag an error.

gregnoel said at 2008-12-26 15:25:19

What's happening is that the AddPostAction() is being applied to the alias Node, not the file Node, where it is ignored. I believe the only possible place that an action can be validly extended is on a file Node (with a non-Null Executor?), so it ought to be possible to detect when it is not and generate an error.

In the case here, the workaround is to use different variable names for the file Node and the alias Node and use AddPostAction() on the file Node.

stevenknight said at 2009-01-07 19:03:57

Bug party triage: Decided that SCons should issue an error (or perhaps a warning?) if you issue AddPostAction() on a Node that has no existing action.

gregnoel said at 2009-01-08 12:07:58

Should have been assigned to 2.1, not 2.0.

stevenknight said at 2009-11-10 18:00:19

stevenknight => issues@scons

gregnoel said at 2010-06-24 14:21:49

Change subject to reflect decision at bug party.

gregnoel said at 2010-07-21 17:10:33

Bug party triage.

@bdbaddog bdbaddog added this to the -research- milestone Jan 2, 2018
@mwichmann mwichmann added Alias and removed P4 labels Apr 7, 2021
@mwichmann
Copy link
Collaborator

Does this one have a straightforward way of moving forward. Seems like it was decided to have this situation cause error/warn, but it never happened.

@bdbaddog
Copy link
Contributor Author

Yes. I'll take a look.

@ilitzroth
Copy link

ilitzroth commented Dec 30, 2024

On Scons 4.8.1
This SConstruct file

output_file = File("output_file")
Command(output_file, [], Touch(output_file))
Alias("alias1", output_file, "@true")
AddPostAction("alias1", "@echo running post_action for $TARGET")

Alias("alias2", output_file)
AddPostAction("alias2", "@echo running post_action for $TARGET")

Default(None)

Gives this result

$ scons -Q alias1 && scons -Q -c alias1 && scons -Q alias2 && scons -Q -c alias2
Touch("output_file")
running post_action for alias1
Removed output_file
Touch("output_file")
Removed output_file

gives this output which seems to show that the postactions are only run when the
the aliases have an attached action. Just attaching the the action to the alias by calling
Alias(alias2", [], "@echo running post_action for $TARGET")
does run this action. I find this behaviour inconsistent. We are using scons to build an
extensible build system, and we would like to use AddPost, AddPre to allow the builds to
be extended by users in a simple way.

@mwichmann
Copy link
Collaborator

Aliases are a little funky in that they're their own node type, and AddPostAction doesn't distinguish - it just adds the new action to the chain for the node it's called on. That "works" for an alias node with an action because that is already a kind of post-action and the added one can chain onto that. The complication is that aliases don't have a sense of completion - you can keep adding to them via multiple calls, so you might call AddPostAction on an alias that isn't yet associated with the real target you wanted the post action associated with. In fact, many people use a kind of global alias, where everywhere in their build an exectable-type target is created, it is added to the alias so you can eventually "build everything" by giving that alias on the command line. Would an AddPostAction apply to all of those, or to just a single one? So much for internal details...

So yes, it's inconsistent from an expectation sense; the docs say nothing about a limitation. Looks like we were going to look into at least letting you know this wasn't going to do what you expect, but it still hasn't happened.

mwichmann added a commit to mwichmann/scons that referenced this issue Jan 3, 2025
AddPostAction and AddPreAction apply to the action of the alias,
which may not have been clear previously (see SCons#2281).

Expanded the example for AddPostAction using a multi-step build.

Signed-off-by: Mats Wichmann <[email protected]>
mwichmann added a commit to mwichmann/scons that referenced this issue Jan 3, 2025
AddPostAction and AddPreAction apply to the action of the alias,
which may not have been clear previously (see SCons#2281).

Expanded the example for AddPreAction using a multi-step build.

Signed-off-by: Mats Wichmann <[email protected]>
bdbaddog added a commit to bdbaddog/scons that referenced this issue Jan 20, 2025
…ded to an Alias() node, if there was no action specified when that node was created
mwichmann added a commit to mwichmann/scons that referenced this issue Jan 21, 2025
AddPostAction and AddPreAction apply to the action of the alias,
which may not have been clear previously (see SCons#2281).

Expanded the example for AddPreAction using a multi-step build.

Signed-off-by: Mats Wichmann <[email protected]>
@bdbaddog bdbaddog linked a pull request Jan 22, 2025 that will close this issue
3 tasks
mwichmann added a commit to mwichmann/scons that referenced this issue Mar 3, 2025
AddPostAction and AddPreAction apply to the action of the alias,
which may not have been clear previously (see SCons#2281).

Expanded the example for AddPreAction using a multi-step build.

Signed-off-by: Mats Wichmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants