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

Rework in-docker tests so they handle comments in scripts #67

Merged
merged 2 commits into from
Nov 22, 2020

Conversation

kgugala
Copy link
Member

@kgugala kgugala commented Nov 22, 2020

fixes #65

@mithro
Copy link
Contributor

mithro commented Nov 22, 2020

@kgugala - I'm okay with this temporary fix, however the tuttest tool should understand how comments work?

@mgielda
Copy link
Member

mgielda commented Nov 22, 2020

Well, the original idea for tuttest was not to be language specific and not really provide some of the functionality that you had since requested (which are bash-specific). It's the --single-command option which introduced the bug, but to fix it properly we'd need to make tuttest understand comments in various languages. Bash as such is of course easy to fix, so I think we'll just make this fix in single-command since it's kinda bash-specific anyhow.

@mithro
Copy link
Contributor

mithro commented Nov 22, 2020

@mgielda -- Well, shouldn't bash understand that # is a comment and just execute nothing?

@mithro
Copy link
Contributor

mithro commented Nov 22, 2020

I have forgotten why we needed --single-command? I wonder if --single-command should actually be generating a script file which it then executes?

@pgielda
Copy link
Member

pgielda commented Nov 22, 2020

I actually think this is not acceptable to merge and we have to fix the underlying issue.

@pgielda pgielda changed the title Remove comment in xc7 guide [WIP] [DNM] Remove comment in xc7 guide Nov 22, 2020
@pgielda
Copy link
Member

pgielda commented Nov 22, 2020

@kgugala can you apply the approach #68 here instead?

@pgielda pgielda changed the title [WIP] [DNM] Remove comment in xc7 guide [WIP] Fix comment handling in docker Nov 22, 2020
@kgugala kgugala changed the title [WIP] Fix comment handling in docker Rework in-docker tests so they handle comments in scripts Nov 22, 2020
@kgugala
Copy link
Member Author

kgugala commented Nov 22, 2020

@kgugala can you apply the approach #68 here instead?

done

Signed-off-by: Karol Gugala <kgugala@antmicro.com>
This option is not supported in older distros we test.
We need to manually create the directory to which we're
going to unpack architectures definitions.

Signed-off-by: Karol Gugala <kgugala@antmicro.com>
@pgielda
Copy link
Member

pgielda commented Nov 22, 2020

Ok, now we have a real CI feedback. It seems we have two problems:

  1. on old OSes seems that LiteX fails
  2. we hit 50min timeout of Travis

I would suggest merging this and open separate issue for (1) -- probably in LiteX using ubuntu:trusty fail as an example.
As for (2) I would suggest temporarily not test the longest test in tuttest (LiteX? Linux/LiteX?). Long term we would have to migrate from Travis, but I know its not trivial because of storage limitations of Github Actions and we need a short term solution.

@pgielda pgielda self-requested a review November 22, 2020 23:14
@pgielda pgielda merged commit de551c7 into chipsalliance:master Nov 22, 2020
@mgielda mgielda deleted the fix-xc7-build branch November 26, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None of the XC7 tests are running ... easy fix
4 participants